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

Cyclic references resolve #2491

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

m1c1b
Copy link

@m1c1b m1c1b commented Jan 18, 2023

  • You have read the Spring Data contribution guidelines.
  • You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.
  • You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

Hello! First of all I want to describe the problem that I have fixed.

Imagine a couple of @RedisHash entities that references one to anoter like A <->B. In that case StackOverflow will be thrown because of infinite calls stack for resolving @Reference fields on any kind of reading.

So I have made a clone of MappingRedisConverter with name ReferenceMappingRedisConverter but with one difference. ReferenceMappingRedisConverter has context of already resolved field branches, ReferenceRedisAdapter control the context of converter on reading operations.

I'm not really confident should I edit already existing classes or create new, this is why I have created new ones. I didn't know what version should I write in @since javadoc annotation, so I just didn't write it.

And of course I am ready to write documentation for my code and a sample configuration in spring boot, but I need a little bit explanation of how to do it.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 18, 2023
@mp911de mp911de self-assigned this Apr 24, 2023
@mp911de
Copy link
Member

mp911de commented Jul 4, 2023

Thanks for your pull request and sorry for the long silence. It makes indeed sense to have cyclic guards in place. With the current state, it is impossible to review the changes because the converter is fully duplicated. Instead, it would make sense to carry around a contextual state object that holds references to resolved objects from hashes and have that as starting point.

@mp911de mp911de added the status: waiting-for-feedback We need additional information before we can continue label Jul 4, 2023
@jxblum jxblum force-pushed the main branch 2 times, most recently from d43adc9 to a71f042 Compare October 18, 2023 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants