Instance with wrong modification counter returned by TablemanagementImpl

[jomora]

Observation: When saving an existing Table via the REST api the returned instance's modification counter has not been incremented. When trying to save the same instance (with possibly altered values) again on a javax.persistence.OptimisticLockException is thrown.

Cause: After inspecting the code I found the following: TablemanagementImpl@130

The Javadoc of GenericDao.save states the following:

Saves a given entity. Use the returned instance for further operations as the save operation might have changed the entity instance completely.

Obviously, line 130 of TablemanagementImpl does not adhere to this contract.

Solution: Change to TablemanagementImpl@130 tableEntity = getTableDao().save(tableEntity);.

Next steps: we have to check if this issue occurrs on the other components (Staff/Offer/Salesmanagement) as well.

[jomora]

I located usages of GenericDao#save:

• 84@UcManageOrderImpl: seems OK
• 80@UcManageOrderPositionImpl: seems to be faulty... must be checked
• 110@UcManageOrderPositionImpl: seems OK (be aware of two different calls in this class)
• 137-138@StaffmanagementImpl: seems OK
• 230@OffermanagementImpl: seems OK
• 367@OffermanagementImpl: seems OK
• 47@UcManageBinaryObjectImpl: seems faulty
• 134@UcManageBillImpl: seems faulty

The following picture shows all usages of GenericDao#save as shown by Eclipse. I used this list to identify potentially erroneous usages.

The mentioned files must be reviewed!

[hohwille]

@jomora Your observation is completely right. Also all the places that you stated as "faulty" have to be fixed analogue. Thanks for figuring this out. IMHO the problem ist that the sample is just used occasionally and not maintained like a real productive customer project. Hence we have such issues there for a longer time. This is a big pity as the sample should actually act as a reference where to learn how to do it right. Currently this is unfortunately not the case. So with such issues we are getting closer to the actual goal.

[jomora]

@hohwille, thanks for the feedback. AFAIK @SimonHuber is already working on this. Please, assign him to this issue.

[maybeec]

@hohwille due to our local observations, the current solution does not seem to work deterministically.

In the GenericDao implementation, the entity to be saved is merged into the database. However, as the instruction is not executed immediately, the resulting updated entity does not have the increased modificationCounter. Therefore, we have to flush the entityManager instruction cache.

What I do not understand is, why this has not been observed so far. Any suggestions? Do I oversee anything?

[hohwille]

This issue is actually invalid. Hibernate only updates the modification counter only on flush or when the transaction is comitted. Hence, we created a solution for this problem already a long time ago but failed to update and close this issue. When an entity is converted to an ETO we connect that ETO internally with the Entity and implicitly update the modification counter during serialization. Works like a charm.

The "faulty" code indeed is not perfectly using the JPA contract but IMHO nearly all projects are writing code in this way as with common JPA vendor implementations like hibernate it does not matter at all. We could of course improve the code but actually the restaurant is a dead project and will be deleted. If someone cares for this kind of perfection review MTS (https://github.com/oasp/my-thai-star) and raise an issue there if needed.