Skip to content

Partially populated PersistentEntity instances not properly removed from cache (1st nesting level edge case) #2329

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

Closed
tomspendier opened this issue Mar 17, 2021 · 0 comments
Assignees
Labels
type: bug A general bug

Comments

@tomspendier
Copy link

This bug relates to #2000 where it has been overlooked to also cover the following edge case within the fix (@odrotbohm FYI):

The bug was supposed to get (entirely) fixed in #2000 by cleaning up the cache (AbstractMappingContext#persistentEntities) on any kind of RuntimeException (instead of originally only MappingException, see the already fixed version:
https://github.com/spring-projects/spring-data-commons/blob/2.2.0.RC3/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java#L383

However, this doesn't cover the edge case when an Exception is thrown outside of the inner try block in AbstractMappingContext#addPersistentEntity().
As it turned out we ran into a TypeNotPresentException when invoking BeanUtils.getPropertyDescriptors(type)

As AbstractMappingContext#addPersistentEntity() is invoked recursively to traverse the whole object tree of the persistent entity, the fix applied in #2000 doesn't clean up the cache when an Exception is thrown where the traversal is still on the first nesting level of the persistent entity.

How to fix it:
To also cover this case the outer try block should get catched by a RuntimeException as well (currently only BeansException is catched) - so to cover also a TypeNotPresentException. In addition this catch block needs to also clear the cache with
persistentEntities.remove(typeInformation) before rethrowing.

I have created a reproduction covering this edge case (where documents get wrongfully inserted to MongoDB - in the same way corrupted as documented in #2000). Applying the before mentioned fix solved the issue. Unfortunately our reproduction includes many dependencies and the fix can also not get property unit tested without performing a refactoring of AbstractMappingContext.

Applying this fix essentially makes the inner catch block in AbstractMappingContext#addPersistentEntity() obsolete.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 17, 2021
gregturn added a commit that referenced this issue May 3, 2021
gregturn added a commit that referenced this issue May 3, 2021
When an error happens inside the AbstractMappingContext, the caching sometimes gets corrupted. That's because some exceptions are caught, others are not. Instead, the error handling that clears out the cache needs to be shifted up one level, resulting in a simpler code block.

See #2329.
@mp911de mp911de added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 20, 2021
mp911de pushed a commit that referenced this issue Jul 20, 2021
…tialization errors.

When an error happens inside the AbstractMappingContext, the caching sometimes gets corrupted. That's because some exceptions are caught, others are not. Instead, the error handling that clears out the cache needs to be shifted up one level, resulting in a simpler code block.

Closes #2329
Original pull request: #2367.
mp911de added a commit that referenced this issue Jul 20, 2021
Extract adding the actual entity to the MappingContext into its own method along with error handling spanning the entire entity initialization process.

See #2329
Original pull request: #2367.
mp911de added a commit that referenced this issue Jul 20, 2021
Extract adding the actual entity to the MappingContext into its own method along with error handling spanning the entire entity initialization process.

See #2329
Original pull request: #2367.
@mp911de mp911de added this to the 2.5.4 (2021.0.4) milestone Jul 20, 2021
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
4 participants