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

Preserve existing persistence context property value during AOT processing #29796

Conversation

christophstrobl
Copy link
Member

The generated CodeBlock that takes care of injecting the PersistenceContext may override an existing property value. This PR checks existing values and potentially omits creating the code block.

Related to: spring-projects/spring-data-jpa#2753 & spring-projects/spring-data-commons#2760

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 10, 2023
@sbrannen sbrannen added in: data Issues in data modules (jdbc, orm, oxm, tx) theme: aot An issue related to Ahead-of-time processing labels Jan 11, 2023
@sdeleuze sdeleuze self-requested a review February 9, 2023 09:06
@sdeleuze sdeleuze requested a review from snicoll April 28, 2023 06:33
@sdeleuze
Copy link
Contributor

Worth to review with @snicoll when he is back from PTO.

@christophstrobl Is this PR ready for review for potential merge or do you plan further refinement?

@christophstrobl
Copy link
Member Author

@sdeleuze you can review it already. Happy to modify if necessary.

@christophstrobl christophstrobl marked this pull request as ready for review May 10, 2023 12:24
@snicoll
Copy link
Member

snicoll commented May 10, 2023

So it looks like something has post-processed the persistence context already and the core goes in the way. I think we need to fix this differently.

If I understand correctly, Spring Data has knowledge of something that we don't. Why isn't that reflected in the BeanDefinition. If it is, why is the skip necessary?

@christophstrobl
Copy link
Member Author

There's an EntityManagerBeanDefinitionRegistrarPostProcessor in data-jpa that post processes EntityManagerFactory. @odrotbohm can you please help us find the right point for the changes.

@mp911de
Copy link
Member

mp911de commented May 11, 2023

Spring Data is setting the property value for entityManager within the BeanDefinition. In JVM mode, the container skips @PersistenceContext post-processing for the property while AOT mode works differently.

@snicoll
Copy link
Member

snicoll commented May 11, 2023

InjectionMetadata#inject has a checkPropertySkipping that is meant to determine if injection should take place. I missed that while building the AOT contribution for the persistence context. It should attempt to run the same code so that no contribution takes place if it should be skipped.

@snicoll
Copy link
Member

snicoll commented May 11, 2023

Thanks for the PR @christophstrobl but I think we should tackle this one differently. I've created #30476

@snicoll snicoll closed this May 11, 2023
@snicoll snicoll added status: superseded An issue that has been superseded by another and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) status: superseded An issue that has been superseded by another theme: aot An issue related to Ahead-of-time processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants