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

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.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 24, 2020
Copy link
Member

@snicoll snicoll left a comment

Choose a reason for hiding this comment

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

@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.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Nov 24, 2020
@meistermeier
Copy link
Contributor Author

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 marked this pull request as draft November 24, 2020 22:58
@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 24, 2020
@snicoll snicoll added status: blocked An issue that's blocked on an external project change status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Nov 25, 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 December 7, 2020 18:55
@meistermeier
Copy link
Contributor Author

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.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 7, 2020
@snicoll
Copy link
Member

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.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: blocked An issue that's blocked on an external project change status: feedback-provided Feedback has been provided labels Dec 21, 2020
@meistermeier
Copy link
Contributor Author

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.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 21, 2020
@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
@snicoll snicoll added type: bug A general bug and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants