Skip to content
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

Persistent and RelationshipProperties-annotated types are not scanned with Spring Data Neo4j #24239

Closed

Conversation

@meistermeier
Copy link
Contributor

@meistermeier meistermeier commented Nov 24, 2020

To align with the other store modules, I added the Persistent type to the initial scan list. We already added this in SDN itself as a supported type.
Also the RelationshipProperties type got added to this list, because the loading of those entities would fail if we are running in Spring Data's strict mode (e.g. multi store modules) and they would get requested during runtime. In contrast to the startup phase when this method gets called.

Copy link
Member

@snicoll snicoll left a comment

@meistermeier thanks for the PR. While I can see adding Persistent to be generally useful and consistent, I think that adding RelationshipProperties calls for adding a test that showcases why that is necessary. We may need to rephrase the issue so that the user-facing change is clearer.

@meistermeier
Copy link
Contributor Author

@meistermeier meistermeier commented Nov 24, 2020

Thanks for the quick response @snicoll .
I will have to check now first other changes that will come into SDN that might encapsulate this behaviour. So the additional tests will be placed in SDN and the auto configuration will just call its methods.
(Also I have noticed that I missed to add the changes for the reactive configuration to this commit)

@meistermeier meistermeier force-pushed the meistermeier:sdn6_alignement branch from effc76d to ac1c445 Dec 7, 2020
@meistermeier meistermeier changed the title Accept additional type annotations in SDN's initial entity scan. More precise filtering of initial entities. Dec 7, 2020
@meistermeier meistermeier marked this pull request as ready for review Dec 7, 2020
@meistermeier
Copy link
Contributor Author

@meistermeier meistermeier commented Dec 7, 2020

Unfortunately how I thought the change could be done better was not successful. So I stick now to the initial solution and added the tests to ensure that only those classes with annotations that mark explicitly entities (@Node and @Persistent) or are some kind of helper entity (@RelationshipProperties) will be part of the initial entity set.

@snicoll
Copy link
Member

@snicoll snicoll commented Dec 21, 2020

Thanks for the follow-up.

Also the RelationshipProperties type got added to this list, because the loading of those entities would fail if we are running in Spring Data's strict mode (e.g. multi store modules)

I am not sure I got that. Does adding this type fixes a bug? And if so, can we please have a test that showcases that? The tests that were added merely checked that scanning works as expect but that's not really explaining why adding that type is necessary.

@meistermeier
Copy link
Contributor Author

@meistermeier meistermeier commented Dec 21, 2020

In a scenario that enforces the Spring Data infrastructure to go into strict mode, implicit registration of entities are not possible (throws exception on later registration). Besides being not a prominent entity in the broader API sense, technically a RelationshipProperties annotated class is used as one internally.
As a result classes with this annotation also needs to be one of the accepted types for the set of initial entities.

@snicoll snicoll changed the title More precise filtering of initial entities. Persistent and RelationshipProperties-annotated types are not scanned with Spring Data Neo4j Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants