Skip to content

DATAJDBC-234 - Support properties file based query declaration #180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

DATAJDBC-234 - Support properties file based query declaration #180

wants to merge 8 commits into from

Conversation

mcisnerosb57
Copy link
Contributor

JDBC-234 with a integration test.

@schauder
Copy link
Contributor

This supersedes #134.

@schauder schauder changed the title JDBC-234 DATAJDBC-234 - Support properties file based query declaration Nov 19, 2019
@schauder schauder mentioned this pull request Nov 19, 2019
Closed
@mcisnerosb57
Copy link
Contributor Author

This supersedes #134.

Good Morning, i have this error but not everytime.

[ERROR] Failures:
[ERROR] EnableJdbcAuditingHsqlIntegrationTests.auditForAnnotatedEntity:70->lambda$5:221->lambda$0:95
The following 2 assertions failed:

  1. [created date set]
    Expecting:
    <2019-11-19T16:38:08.271>
    to be strictly after:
    <2019-11-19T16:38:08.271>
    at EnableJdbcAuditingHsqlIntegrationTests.lambda$0(EnableJdbcAuditingHsqlIntegrationTests.java:79)
  2. [modified date after instance creation]
    Expecting:
    <2019-11-19T16:38:08.271>
    to be strictly after:
    <2019-11-19T16:38:08.271>
    at EnableJdbcAuditingHsqlIntegrationTests.lambda$0(EnableJdbcAuditingHsqlIntegrationTests.java:83)

@schauder
Copy link
Contributor

Good Morning, i have this error but not everytime.

Interesting, kind of strange that I haven't seen that one.
Sounds like an issue that might only occure when the submillisecond of a time stamp is all zero.

Please create a new issue for that one and please include the JDK version you are using.
I think there was a change in precision somewhere.
This might explain why we didn't see it so far.

schauder
schauder requested changes Nov 20, 2019
View reviewed changes
Copy link
Contributor

@schauder schauder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two major things we need before I can proceed with merging this:

  1. We need a way to configure a property file to use including a decent default value. Compare https://github.com/spring-projects/spring-data-jpa/blob/master/src/main/java/org/springframework/data/jpa/repository/config/EnableJpaRepositories.java#L97

  2. The method name should be used as the default name for the lookup.

I also left some more detailed comments.

...ata-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryMethod.java Outdated
String annotatedName = getMergedAnnotationAttribute("name");
return (StringUtils.hasText(annotatedName) && this.namedQueries.hasQuery(annotatedName))
? this.namedQueries.getQuery(annotatedName)
: super.getName();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't return the name of the method as a query, we have to use it as the query name to look up a query.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is looking up the query?, in the properties file?.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly.

@schauder
Copy link
Contributor

Good Morning,
the build no works, because Java instalation is failed i guess, only version 8 and 9, can you fix it?.

Don't worry about the Travis build a fix for that is in the working.

The important part is that a local mvn clean install -Pall-dbs works.

@mcisnerosb57
Copy link
Contributor Author

mcisnerosb57 commented Nov 21, 2019 •
edited
Loading

There are two major things we need before I can proceed with merging this:

  1. We need a way to configure a property file to use including a decent default value. Compare https://github.com/spring-projects/spring-data-jpa/blob/master/src/main/java/org/springframework/data/jpa/repository/config/EnableJpaRepositories.java#L97
  2. The method name should be used as the default name for the lookup.

I also left some more detailed comments.

not understand what you mean . the default file is jdbc-named-query.properties. And it change if indicat the property file .
/**
* Configures the location of where to find the Spring Data named queries properties file. Will default to
* {@code META-INF/jdbc-named-queries.properties}.
*/
String namedQueriesLocation() default "";

@mcisnerosb57
Copy link
Contributor Author

There are two major things we need before I can proceed with merging this:

  1. We need a way to configure a property file to use including a decent default value. Compare https://github.com/spring-projects/spring-data-jpa/blob/master/src/main/java/org/springframework/data/jpa/repository/config/EnableJpaRepositories.java#L97
  2. The method name should be used as the default name for the lookup.

I also left some more detailed comments.

Do you need any thing more? . thanks

schauder pushed a commit that referenced this pull request Jan 8, 2020
@mcisnerosb57 @schauder
DATAJDBC-234 - Adds support for named queries stored in property files.
93272c8 
Queries are expected in a properties file on the classpath `META-INF/jdbc-named-queries.properties`.
The name of the query is by default `<simple class name of the domain type>.<methodName>`.
Alternatively it can be set using the `@Query` annotations `name` attribute.

Original pull request: #180.
schauder added a commit that referenced this pull request Jan 8, 2020
@schauder
DATAJDBC-234 - Fixes the default query naming and polishing.
09fd0fc 
The default name now does include the name of the domain class.
The default name now is no longer used when no query with the name specified in the `@Query` annotation is found.

Simplified the unit tests by extracting common functionality and improving naming.
Simplified test configuration.

Original pull request: #180.
schauder added a commit that referenced this pull request Jan 8, 2020
@schauder
DATAJDBC-234 - Adds documentation.
8420a56 
Original pull request: #180.
@schauder
Copy link
Contributor

schauder commented Jan 8, 2020

Thanks, I moved some things around and merged it into master.

@schauder schauder closed this Jan 8, 2020
@spring-projects-issues spring-projects-issues mentioned this pull request Dec 31, 2020
Closed
mp911de added a commit that referenced this pull request Feb 21, 2022
@mp911de
#180 - Remove jcenter repository from pom.
761995b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schauder schauder Awaiting requested review from schauder

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mcisnerosb57 @schauder