-
Notifications
You must be signed in to change notification settings - Fork 358
DATAJDBC-437 - In strict mode we only claim repositories for domain types with @Table
annotation.
#177
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.
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
Conversation
…ypes with @table annotation. Before this change Spring Data JDBC didn't specify any identifying annotation and therefore would claim all or no repository depending on the the version of Spring Data Commons. Also added the RepositoryFactorySupport to spring.factory in order to support detection of multiple RepositoryFactorySupport implementations on the classpath. See also: https://jira.spring.io/browse/DATACMNS-1596.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments mostly to reduce the need for touching unrelated tests.
@@ -56,7 +58,8 @@ | |||
|
|||
@Configuration | |||
@Import(TestConfiguration.class) | |||
@EnableJdbcRepositories(considerNestedRepositories = true) | |||
@EnableJdbcRepositories(includeFilters = @Filter(type = FilterType.REGEX, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should improve handling for non-existent entity types in JdbcRepositoryFactory.getTargetRepository(…)
(e.g. calling getRequiredPersistentEntity(…)
instead of getPersistentEntity(…)
).
java.lang.IllegalArgumentException: entity is marked non-null but is null
at org.springframework.data.jdbc.repository.support.SimpleJdbcRepository.<init>(SimpleJdbcRepository.java:36)
is hard to debug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✓
* Unit tests for {@link JdbcRepositoryConfigExtension}. | ||
* | ||
* @author Jens Schauder | ||
* @since 1.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: No need for @since
in test classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
interface SampleRepository extends Repository<Sample, Long> {} | ||
|
||
interface UnannotatedRepository extends Repository<Object, Long> {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, we don't use Object
here to avoid the need to touch other, unrelated tests. Let's use a different type here.
The repository for an unannotated entity now can be constructed, avoiding the need to exclude it in other tests. Improved error message for failure to create a repository by using `getRequiredPersistentEntity`. Removed superfluous `@since` annotation.
I implemented the suggested improvements. |
…ypes with @table annotation. Before this change Spring Data JDBC didn't specify any identifying annotation and therefore would claim all or no repository depending on the the version of Spring Data Commons. Also added the RepositoryFactorySupport to spring.factory in order to support detection of multiple RepositoryFactorySupport implementations on the classpath. Related ticket: DATACMNS-1596. Original pull request: #177.
Remove unused imports. Original pull request: #177.
…ypes with @table annotation. Before this change Spring Data JDBC didn't specify any identifying annotation and therefore would claim all or no repository depending on the the version of Spring Data Commons. Also added the RepositoryFactorySupport to spring.factory in order to support detection of multiple RepositoryFactorySupport implementations on the classpath. Related ticket: DATACMNS-1596. Original pull request: #177.
Remove unused imports. Original pull request: #177.
That's merged, polished, and backported now. |
…ypes with @table annotation. Before this change Spring Data JDBC didn't specify any identifying annotation and therefore would claim all or no repository depending on the the version of Spring Data Commons. Also added the RepositoryFactorySupport to spring.factory in order to support detection of multiple RepositoryFactorySupport implementations on the classpath. Related ticket: DATACMNS-1596. Original pull request: #177.
Remove unused imports. Original pull request: #177.
Before this change Spring Data JDBC didn't specify any identifying annotation and therefore would claim all or no repository depending on the the version of Spring Data Commons.
Also added the RepositoryFactorySupport to spring.factory in order to support detection of multiple RepositoryFactorySupport implementations on the classpath.
See also: https://jira.spring.io/browse/DATACMNS-1596.
Original issue: https://jira.spring.io/browse/DATAJDBC-437.