HSEARCH-5517 Adjust index plan updates caused by "association" for deleted entities #4887
Conversation
marko-bekhta
force-pushed
the
fix/HSEARCH-5517-Document-is-updated-instead-of-deleted-on-cascading-delete-in-OneToOne-relations
branch
from
a7668c3 to
2e1f0c0
Compare
| // If the current entity state is "removed" and we are trying to add/update something because of | ||
| // the association, then we should ignore that action, since the actual value is ... removed?! | ||
| if ( !EntityStatus.ABSENT.equals( state.currentStatus ) ) { | ||
| state.addOrUpdate( entitySupplier, dirtyPaths, false, false ); | ||
| } |
There was a problem hiding this comment.
Hey @yrodiere
soooo this change feels a bit too naive ...
but depending on the order of events in the queue we can get a delete event which will trigger updateBecauseOfContainedAssociation and we will add things for indexing, even though the entitiy is actually removed ....
at first I thought about adding some alternative to updateBecauseOfContainedAssociation, e.g. updateBecauseOfContainedRemoved 🤪 ... but then maybe this ^ is actually enough?
There was a problem hiding this comment.
I think it makes sense, though you're certainly trying to handle an edge case and I wonder how many more there might be. If other reports around cascade come your way, you may want to consider how much you want to support exactly.
By the way, this makes me wonder... Can't you reproduce this without cascades? Just be removing both the parent and child explicitly?
| @JoinColumn(name = "PARENT_ID_CUSTOM_NAME") | ||
| @OneToOne | ||
| @IndexedEmbedded | ||
| @AssociationInverseSide(inversePath = @ObjectPath(@PropertyValue(propertyName = "cascadableChildMapsId"))) |
There was a problem hiding this comment.
This shouldn't be necessary, there's already a mappedBy on the other side?
| CascadableChildMapsId child = session.find( CascadableChildMapsId.class, 1L ); | ||
| child.setName( "to-be-deleted" ); | ||
| session.persist( child ); |
There was a problem hiding this comment.
This call to persist is strange, as the entity is already persisted?
There was a problem hiding this comment.
oh yeah, missed it when cleaning up the reproducer, 👍🏻
| CascadableChildMapsId child = session.find( CascadableChildMapsId.class, 1L ); | ||
| child.setName( "to-be-deleted" ); | ||
| session.persist( child ); | ||
| session.remove( parent ); |
There was a problem hiding this comment.
Does the problem still get reproduced if you call child.setParent(null) before the end of the transaction?
Because we expect people to in general: https://docs.hibernate.org/search/7.2/reference/en-US/html_single/#mapper-orm-indexing-automatic-concepts-session-consistency
Now I'll grant you, cascades are a bit more specific.
| // If the current entity state is "removed" and we are trying to add/update something because of | ||
| // the association, then we should ignore that action, since the actual value is ... removed?! | ||
| if ( !EntityStatus.ABSENT.equals( state.currentStatus ) ) { | ||
| state.addOrUpdate( entitySupplier, dirtyPaths, false, false ); | ||
| } |
There was a problem hiding this comment.
I think it makes sense, though you're certainly trying to handle an edge case and I wonder how many more there might be. If other reports around cascade come your way, you may want to consider how much you want to support exactly.
By the way, this makes me wonder... Can't you reproduce this without cascades? Just be removing both the parent and child explicitly?
I don't think so ... if we start removing things explicitly without cascading, then ORM would force us to set nulls or we'll get Hibernate ORM exceptions about those dangling objects ... (I think we already have tests for that scenario in place)...
yeah... I only could get it to "fail" with this mapping, removing any other part of what's in was leading to a different oreder of remove events and all was "ok" in a sense that update was planned first but then the remove event from ORM came in and the final plan was correct ... 😖 |
marko-bekhta
force-pushed
the
fix/HSEARCH-5517-Document-is-updated-instead-of-deleted-on-cascading-delete-in-OneToOne-relations
branch
from
2e1f0c0 to
4ebe234
Compare
|
2 participants
https://hibernate.atlassian.net/browse/HSEARCH-5517
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.