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

toString id clashes with parameters of type Long (SDN 7.1.3/7.1.4) #2872

Open
sobczynskipio opened this issue Mar 13, 2024 · 11 comments · May be fixed by #2874
Open

toString id clashes with parameters of type Long (SDN 7.1.3/7.1.4) #2872

sobczynskipio opened this issue Mar 13, 2024 · 11 comments · May be fixed by #2874
Assignees
Labels
status: waiting-for-triage An issue we've not yet triaged

Comments

@sobczynskipio
Copy link

sobczynskipio commented Mar 13, 2024

Hi, seems like I've found one bug which really blocks stepping of SDN in my project (springboot 3.1.3 -> 3.1.4):

There's been a change between SDN 7.1.3 and 7.1.4 which made method name derived queries return empty results for entities marked as containsPossibleCircles in Neo4jTemplate.

In Neo4jTemplate class, there's a TemplateSupport.convertToLongIdOrStringElementId(nodeIds) invocation which makes query params of type Long always NOT match with entity nodeIds. See screenshot attached. SDN logic somehow get's there only for entities for which containsPossibleCircles is set to true in Neo4jTemplate. My entities don't really contain circles in relationships but this probably does not matter. Repository methods like findAll or findById always give empty results for those cause SDN compares stringified ids with Long params. It works in SDN 7.1.3 and is broken in 7.1.4 and later

Regards !

SDN-ids-issue

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 13, 2024
@michael-simons michael-simons self-assigned this Mar 14, 2024
@michael-simons
Copy link
Collaborator

Hej, thanks for the detailed report.
Are you able to share the relevant entities that cause this? Thank you in advance.

@sobczynskipio
Copy link
Author

sobczynskipio commented Mar 14, 2024

Of course, going through the SDN code I can see you check for those possibleCircles, that's why in my code the issue applies only to a subset of entities. I can share one of the simpler ones. Please see the screenshots attached. This entity is part of a linked list cause it has a relationship to previous of the same type. My logic prevents pointing at the very same object (node), it's just a linked list of objects of the same type so we keep a relationship to the previous one.

Screenshot 2024-03-14 at 10 55 41

Feels like it's ok to have an entity with possibleCircles. The reason is why only in this case we compare stringified ids with Long params for methods like findAll, findById and so on.

@sobczynskipio
Copy link
Author

sobczynskipio commented Mar 14, 2024

In that case above, with repository defined as here (see screenshot), querying userChangesRepository.findAll() gives out an empty result even though there are many nodes of a kind in the DB. No Exceptions are thrown.
Screenshot 2024-03-14 at 11 08 17

@michael-simons
Copy link
Collaborator

Can I see your Entity definition, pls?

@sobczynskipio
Copy link
Author

sobczynskipio commented Mar 14, 2024

Sure.
Screenshot 2024-03-14 at 11 13 14
Screenshot 2024-03-14 at 11 13 28

I may be having a redundant getNodeId method here, I know. That should not matter though, many Entities extending Entity work fine, except those that have relationships which end up being possibleCircles

michael-simons added a commit that referenced this issue Mar 14, 2024
@michael-simons michael-simons linked a pull request Mar 14, 2024 that will close this issue
@michael-simons
Copy link
Collaborator

I cannot reproduce this, see linked PR.

Make sure you configure the Cypher-DSL dialect, pls

https://docs.spring.io/spring-data/neo4j/reference/getting-started.html#configure-cypher-dsl-dialect

I tried both the main branch and the current 7.1.x branch.

@sobczynskipio
Copy link
Author

Thank you for your effort, let me see if I can provide simpliest (not)working runnable example. Let's leave this ticket hanging for a moment more please.

@michael-simons
Copy link
Collaborator

Ofc, feel free to take my branch as a working base.

@sobczynskipio
Copy link
Author

Hey, apparently what solved my issue was downgrading cypherdsl to 2023.2.0 That's what is used in your branch Michael. There's no toString methods performed on entity nodeIds there. I think we can close this issue. Still I'll need to keep my eyes out next time when I need to step versions and cypherdsl. Maybe worth looking at from your side at one point ?

Regards

@michael-simons
Copy link
Collaborator

Hey, thanks for your detailed feedback so far.
Now I would like to keep this open for further investigation (I'm just juggling a bit with time atm).

Thank!!

@sobczynskipio
Copy link
Author

No worries, feel free to contact me for more details if needed. Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants