findLastChangeRevision doesn't return last revision if two revisions with exactly the same timestamp exist

[flauschtrud]

While running automated tests I ran into an issue with RevisionRepository.findLastChangeRevision.

If two revisions with exactly the same timestamp exist findLastChangeRevision does not necessarily return the revision with the highest revision number (which is what I would expect).

If my revinfo table has the following content findLastChangeRevision for the associated audited entity actually returns the revision with id 1 and not the one with id 2.

id,timestamp
1,2024-08-13 08:49:48.501000
2,2024-08-13 08:49:48.501000

In the implementation the revisions are ordered based on revisionProperty("timestamp") and the ID is not taken into account.

spring-data-jpa/spring-data-envers/src/main/java/org/springframework/data/envers/repository/support/EnversRevisionRepositoryImpl.java

Line 96 in a44bca8

.addOrder(AuditEntity.revisionProperty("timestamp").desc()) //

I am aware that this is an edge case and that it's way more likely to run into it in artificial scenarios but it's also not impossible to cause issues for production code.

[mp911de]

Actually, shouldn't the retrieval sort by AuditEntity.revisionNumber().desc()? Identifiers could be non-incrementable types such as UUID and sorting by these wouldn't work.

[schauder]

Hibernates documentation isn't overly clear of the semantics of the two properties.
But in the javadoc to @RevisionNumber I found Values of this property should form a strictly-increasing sequence of numbers.
And there is also this old discussion in the Hibernate forum.
https://discourse.hibernate.org/t/order-of-revision-number-in-envers/2399

So it does indeed seem that the revisionNumber should be used.

I'll fix that.

[wacker]

We have a regression here, as revision numbers are in fact not always sequential (at least not in a multi-node environment), because of the pre-allocation of Hibernate sequence numbers. The 'order by' should probably be composed of timestamp AND rev in order to handle the issue of exactly the same timestamps.