Subquery of DefaultAuditStrategy results in a wrong revision

Description

Hello everyone,
In our project we use the EnversRevisionRepository of spring-data-envers to request our revisions. Right now we just implemented a REST API to get the information to other systems.
Now it happened that our test engineer found an unexpected behaviour when requesting a wrong and not existing combination of an ID of the entity and the wanted REV.

id

REV

REVTYPE

NAME

DESCRIPTION

8

9

0

first entry

first entry description

8

19

1

first entry update

first entry extended description

17

18

0

second entry

second entry description

17

20

1

second entry udpate

second entry extended description

So if you request a special combination of ID and REV like ID = 8 and REV = 18 , which is obviously wrong but technically possible, you receive the a mix of the revision meta data information and a wrong entity revision.
The reason seems to be a subquery which is generated by the call of
'org.hibernate.envers.strategy.DefaultAuditStrategy.addEntityAtRevisionRestriction'
the subquery is this one
```
select max(entity1_ .REV)
from schema.SomeEntity_AUD entity1_
where entity1_.REV <=18 and entity1_.id = 8
```
In this case the result of the subquery is the REV 9, which is correctly an existing entry belonging to the entity with the ID = 8 and it has an REV <= 18.

id

REV

REVTYPE

NAME

DESCRIPTION

8

9

0

first entry

first entry description

But we would have expected a result like a zero size list or an exception, because of the invalid combination of ID and REV.
The result of such a wrong combination is not predictable and it returns the first revison found which has a REV smaller then the wanted one.
If the REV in the query would be 21 instead of 18 you would get the updated revision of the entity ID = 8

id

REV

REVTYPE

NAME

DESCRIPTION

8

19

1

first entry update

first entry extended description

I know that usually the process would be requesting all possible revisions to an entity and then select the wanted revision from this list. But with the REST API of us you just dont know at the moment whether it is a valid combination or just the one the subselect found.

Maybe we have a wrong understanding of the use here, but to us it seems to be a bug at the moment.

What is your opinion about this behaviour ?

Environment

hibernate-core , hibernate-envers 5.2.12.Final
spring-data-envers: 1.0.3.RELEASE
JVM 1.8.0_65,
DB: MSSQL

Activity

Show:
Christian Gratzel
July 18, 2019, 10:00 AM
Edited

Hi Chris,

Thank you for the very quick response.

This would be the complete the query

 

The values differ a little at this point .

This would be the actual table of values

ID

REV

REVTYPE

name

description

reference

1

5

0

name 1

description 1

reference 1

1

16

1

name 1 updated

description 1 updated

reference 1 updated

6

10

0

name 2

description 2

reference 2

6

17

1

name 2 updated

description 2 updated

reference 2updated

11

15

0

name 3

description 3

reference 3

11

18

1

name 3 updated

description 3 udpated

reference 3 updated

and the result

ID

REV

REVTYPE

name

description

reference

1

16

1

name 1 udpated

description 1 updated

reference 1 updated

 

 

 

 

 

 

So the subquery evaluated to the next smaller revision then 18 of ID 1.

For a REV = 10 it would evaluate to REV = 5;

The result comes from this call in the

 

 

from the org.hibernate.envers.internal.reader.AuditReaderImpl;

Chris Cranford
July 18, 2019, 3:00 PM

Hi Christian, are you able to use the query as I outlined in my prior comment rather than the {{#find}} method? That would at least work as a workaround.

Thinking about this a bit more over-night, I am inclined not to change the current {{#find}} implementation as I’m afraid there may be users that are expecting the behavior that you found to be problematic although let me confer with the team and decide on a consensus. If nothing else, I do think the {{#find}} method’s javadoc would need to be updated if the current implementation is kept none-the-less.

If we do decide to keep the existing functionality as-is, I can look to add a {{#findExact}} helper method that basically constructs the query with the additional predicate to get the same result as my prior comment’s query would return you.

Christian Gratzel
July 19, 2019, 7:55 AM

Right now we are just using the implementation of the spring-data-envers project and this seems not to be very extendable. In a prior project we used the Envers AuditReader directly and created a api very similiar to the spring-data-envers project. But we decided to use this one on our actual project.

 

I am inclined not to change the current {{#find}} implementation as I’m afraid there may be users that are expecting the behavior that you found to be problematic..

Of course not. We spoke about override or intergrate a own strategy or something like that, but at this point there is no way to have an overview about possible side effects if we change this subquery. So we droped the idea instantly.

 

Your example could be a possible solution, but then we ll have change the whole repository. We could create some kind of validator before calling the find method to ensure that there is definetly the exact combination of entity and revision or raise an error. I guess well wait what you and your team decide on this one and then think about our options.

Thanks for your effort.

 

Chris Cranford
July 22, 2019, 5:45 PM

Hi Christian,

After further discussion I ultimately think that per the javadoc the method’s return value likely should not be a fuzzy hit for that specific method where you supply the primary-key and the revision number. What I think may work for the short-term would be to introduce a configuration option that forces the method to act in an exact match form without changing the API. This would allow your spring-data-envers solution to work as you desire and would maintain backward compatibility which is critical for us.

I’m currently working on some release tasks for another project today but once I get that completed I’ll try to jump on this and submit a PR for the next 5.4.x release. You should be able to snag the latest master & merge the fix locally if you want to test it out for me and let me know if you run into any problems.

I’ll update this issue once I have done that so you can test it if you want.

 

Chris Cranford
July 22, 2019, 8:36 PM

I’ve added PR https://github.com/hibernate/hibernate-orm/pull/2947 if you want to take a look Christian and let me know if that satisfies your requirements.

Assignee

Chris Cranford

Reporter

Christian Gratzel

Fix versions

Labels

None

backPortable

None

Suitable for new contributors

None

Requires Release Note

None

backportDecision

None

Components

Affects versions

Priority

Major
Configure