Skip to content

Repositories now allows lookup of parent repositories for sub-types. #2406

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

Closed
wants to merge 1 commit into from

Conversation

odrotbohm
Copy link
Member

When inheritance is used for aggregates, lookups of the repository for a Child aggregate instance have so far failed to return a repository registered for the Parent. Client could had to consider that scenario explicitly.

This commit introduces an additional lookup step in case we cannot find a repository for an aggregate type immediately. In this case, we then check for assignability of any of the known aggregate types we have registered repositories for and the type requested. I.e. for a request for the repository of Child, a repository, explicitly registered to manage Child instances would still be used. In the sole presence of a repository managing Parent instances, that would be returned for the request for Child, too.

@odrotbohm odrotbohm added in: repository Repositories abstraction type: enhancement A general enhancement labels Jul 7, 2021
odrotbohm added a commit that referenced this pull request Jul 7, 2021
When inheritance is used for aggregates, lookups of the repository for a Child aggregate instance have so far failed to return a repository registered for the Parent. Client could had to consider that scenario explicitly.

This commit introduces an additional lookup step in case we cannot find a repository for an aggregate type immediately. In this case, we then check for assignability of any of the known aggregate types we have registered repositories for and the type requested. I.e. for a request for the repository of Child, a repository, explicitly registered to manage Child instances would still be used. In the sole presence of a repository managing Parent instances, that would be returned for the request for Child, too.

Fixes GH-2406.
@odrotbohm odrotbohm force-pushed the feature/repository-subtypes branch from a487b5b to 4079810 Compare July 7, 2021 12:08
When inheritance is used for aggregates, lookups of the repository for a Child aggregate instance have so far failed to return a repository registered for the Parent. Client could had to consider that scenario explicitly.

This commit introduces an additional lookup step in case we cannot find a repository for an aggregate type immediately. In this case, we then check for assignability of any of the known aggregate types we have registered repositories for and the type requested. I.e. for a request for the repository of Child, a repository, explicitly registered to manage Child instances would still be used. In the sole presence of a repository managing Parent instances, that would be returned for the request for Child, too.

Fixes GH-2406.
@odrotbohm odrotbohm force-pushed the feature/repository-subtypes branch from 4079810 to 9b76c38 Compare July 7, 2021 12:09
@odrotbohm odrotbohm changed the title Repositories now allows lookup of parent repositories for sub-types. Repositories now allows lookup of parent repositories for sub-types. Jul 7, 2021
@odrotbohm
Copy link
Member Author

I think it's safe to back-port to 2021.0. Nothing particular preventing it from even going into 2020.0, but would understand if you'd like to minimize the risk for that branch.

@mp911de mp911de added this to the 2.4.11 (2020.0.11) milestone Jul 7, 2021
mp911de pushed a commit that referenced this pull request Jul 7, 2021
When inheritance is used for aggregates, lookups of the repository for a Child aggregate instance have so far failed to return a repository registered for the Parent. Client code had to consider that scenario explicitly.

This commit introduces an additional lookup step in case we cannot find a repository for an aggregate type immediately. In this case, we then check for assignability of any of the known aggregate types we have registered repositories for and the type requested. I.e. for a request for the repository of Child, a repository, explicitly registered to manage Child instances would still be used. In the sole presence of a repository managing Parent instances, that would be returned for the request for Child, too.

Original pull request: #2406.
mp911de added a commit that referenced this pull request Jul 7, 2021
Fix Javadoc formatting. Consistently mention proxy class unwrapping.

Original pull request: #2406.
mp911de pushed a commit that referenced this pull request Jul 7, 2021
When inheritance is used for aggregates, lookups of the repository for a Child aggregate instance have so far failed to return a repository registered for the Parent. Client code had to consider that scenario explicitly.

This commit introduces an additional lookup step in case we cannot find a repository for an aggregate type immediately. In this case, we then check for assignability of any of the known aggregate types we have registered repositories for and the type requested. I.e. for a request for the repository of Child, a repository, explicitly registered to manage Child instances would still be used. In the sole presence of a repository managing Parent instances, that would be returned for the request for Child, too.

Original pull request: #2406.
mp911de added a commit that referenced this pull request Jul 7, 2021
Fix Javadoc formatting. Consistently mention proxy class unwrapping.

Original pull request: #2406.
mp911de pushed a commit that referenced this pull request Jul 7, 2021
When inheritance is used for aggregates, lookups of the repository for a Child aggregate instance have so far failed to return a repository registered for the Parent. Client code had to consider that scenario explicitly.

This commit introduces an additional lookup step in case we cannot find a repository for an aggregate type immediately. In this case, we then check for assignability of any of the known aggregate types we have registered repositories for and the type requested. I.e. for a request for the repository of Child, a repository, explicitly registered to manage Child instances would still be used. In the sole presence of a repository managing Parent instances, that would be returned for the request for Child, too.

Original pull request: #2406.
mp911de added a commit that referenced this pull request Jul 7, 2021
Fix Javadoc formatting. Consistently mention proxy class unwrapping.

Original pull request: #2406.
@mp911de
Copy link
Member

mp911de commented Jul 7, 2021

That's merged, polished, and backported now.

@mp911de mp911de closed this Jul 7, 2021
@mp911de mp911de deleted the feature/repository-subtypes branch July 7, 2021 12:54
odrotbohm added a commit to spring-projects/spring-data-rest that referenced this pull request Jul 7, 2021
…itory mappings.

We now consider all registered PersistentEntity instances and try to detect repository metadata for them. A case we didn't cover before was that a repository was declared for an aggregate super type but the actual child aggregate class didn't have a dedicated repository declared. While this is a perfectly valid scenario, the mapping information was broken as it fell back on the plain domain type information and produced paths and relation names derived from that, even if there's a super type repository available.

Thus, solely traversing the aggregate types we have repositories registered for is not enough. We now traverse all known PersistentEntity types and also register repository metadata for all types that are assignable to a known domain type. The latter is actually implemented in Spring Data Commons' Repositories via spring-projects/spring-data-commons#2406.
odrotbohm added a commit to spring-projects/spring-data-rest that referenced this pull request Jul 7, 2021
…itory mappings.

We now consider all registered PersistentEntity instances and try to detect repository metadata for them. A case we didn't cover before was that a repository was declared for an aggregate super type but the actual child aggregate class didn't have a dedicated repository declared. While this is a perfectly valid scenario, the mapping information was broken as it fell back on the plain domain type information and produced paths and relation names derived from that, even if there's a super type repository available.

Thus, solely traversing the aggregate types we have repositories registered for is not enough. We now traverse all known PersistentEntity types and also register repository metadata for all types that are assignable to a known domain type. The latter is actually implemented in Spring Data Commons' Repositories via spring-projects/spring-data-commons#2406.
odrotbohm added a commit to spring-projects/spring-data-rest that referenced this pull request Jul 7, 2021
…itory mappings.

We now consider all registered PersistentEntity instances and try to detect repository metadata for them. A case we didn't cover before was that a repository was declared for an aggregate super type but the actual child aggregate class didn't have a dedicated repository declared. While this is a perfectly valid scenario, the mapping information was broken as it fell back on the plain domain type information and produced paths and relation names derived from that, even if there's a super type repository available.

Thus, solely traversing the aggregate types we have repositories registered for is not enough. We now traverse all known PersistentEntity types and also register repository metadata for all types that are assignable to a known domain type. The latter is actually implemented in Spring Data Commons' Repositories via spring-projects/spring-data-commons#2406.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: repository Repositories abstraction type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants