orm icon indicating copy to clipboard operation
orm copied to clipboard

DDC-3781: Fix Optimistic Locking Scenarios for Versioned Entities in Bidirectional Relationships

Open doctrinebot opened this issue 10 years ago • 7 comments

Jira issue originally created by user @zeroedin-bill:

As described in PR #1378, some scenarios involving a parent-child bidirectional relationship may incorrectly result in the parent's version not being incremented while the child's is.

An example: Versioned entity PurchaseOrder has a collection-valued association to versioned entity PurchaseOrderItem. Considering this as a business use-case, if the state of a PurchaseOrderItem changes during another transaction accessing the parent PurchaseOrder, the state of the PurchaseOrder can be considered to have changed as well. However, in the current ORM implementation, the PurchaseOrder is not considered to have changed, allowing transactions modifying it to proceed concurrently with transactions modifying its children, possibly resulting in inconsistent state.

The solution proposed in the referenced PR is unsuitable. Versioning should be transparent to userland code, and should be managed solely by the persistence layer - the EntityManager.

I think in this instance, we can look to the JPA spec as a good reference on how to handle this: {quote} The version attribute is updated by the persistence provider runtime when the object is written to the database. All non-relationship fields and properties and all relationships owned by the entity are included in version checks (35).

(35) This includes owned relationship maintained in join tables. {quote} JPA 2.1 spec

From what I understand of the implementation in Hibernate, version updates on the owning side of relationships propagate to the inverse side of bidirectional relationships if both sides are versioned entities.

So in the case of the example, if only a PurchaseOrderItem is changed and persisted, both the parent PurchaseOrder's version and the PurchaseOrderItem's version would be updated.

I am not certain what level of effort this would require, but I think changes would be required in UnitOfWork, BasicEntityPersister, and JoinedSubclassPersister.

As for performance effects, I think the performance impact will be minimal in most cases, as most scenarios will only add a single additional update statement. In cases where version updates propagate to multiple entities on inverse sides of relationship, each additional relation that meets the criteria will add an additional update statement.

If the persister is required to fetch the related entity before incrementing the version, it will add additional overhead to queries affecting related versioned entities. Therefore the documentation would need to discuss the potential performance impact, and recommend application of versioning and entity relationships with care.

A possible enhancement would be the addition of an @OptimisticLock(propagate=false) annotation for inverse-side properties, which would prevent propagation of version updates from related entities.

doctrinebot avatar Jun 18 '15 17:06 doctrinebot

Comment created by @doctrinebot:

A related Github Pull-Request [GH-1378] was closed: https://github.com/doctrine/doctrine2/pull/1378

doctrinebot avatar Jun 18 '15 17:06 doctrinebot

Comment created by dhager:

First, I'd like to toss another scenario on the pile: Consider an entity like, uhm, FavoritePurchaseOrders.

It refers to an owning User, references a set of PurchaseOrder items, and contains some statistics like "total price" or "earliest due-date".

This is different from the PurchaseOrder->PurchaseOrderItem relationship, because it probably uses a link-table and probably does not own/cascade-persist to PurchaseOrder items, since you generally favorite things which already exist. While it refers to a User, its version doesn't need to get updated if the owner is modified or reassigned, since it contains no user-specific information.

Secondly, should there be preFlush/postFlush events for entities whose versions are being altered, even if no other immediate values are changing? This probably relates to whether entities must be loaded in order to bump their versions.

doctrinebot avatar Jun 19 '15 01:06 doctrinebot

Comment created by dhager:

I've submitted DDC-3823 as a possible precursor for this ticket. It exposes a mechanism for UOW to record that an entity is scheduled to have its version increased for some reason other than direct alterations.

doctrinebot avatar Jul 14 '15 02:07 doctrinebot

@billschaller So is this whole initiative dead at this point?

DHager avatar Aug 31 '16 17:08 DHager

This use-case is essential with DDD Aggregate design, please consider supporting it.

f1amy avatar Dec 04 '23 17:12 f1amy

I still think it's an important use-case that maintainers keep dismissing, I just don't comment anymore because after 8 years I'm on a different tech-stack. :P

DHager avatar Jan 30 '24 05:01 DHager