3 Replies Latest reply on May 20, 2011 9:35 AM by Adam Warski

    ValidityAuditStrategy pk re-insertion issue

    nbeag Newbie

      Hi,

      I'm having a problem with the validity audit strategy in the following scenario:

      On an audited entity whose id column is not auto-generated (i.e. no @Generate annotation), if you delete an entity and persist another with the same id, and subsequently attempt to update the new entity, it fails with the following error:

       

      javax.persistence.RollbackException: Error while committing the transaction

                at org.hibernate.ejb.TransactionImpl.commit(TransactionImpl.java:93)

                at enversauditing.tests.EnversPKAuditTests.updateRow2(EnversPKAuditTests.java:49)

                at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

                at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)

                at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)

                at java.lang.reflect.Method.invoke(Method.java:597)

                at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)

                at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)

                at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)

                at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)

                at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)

                at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:31)

                at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:76)

                at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)

                at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)

                at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)

                at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)

                at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)

                at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)

                at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)

                at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:31)

                at org.junit.runners.ParentRunner.run(ParentRunner.java:236)

                at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:49)

                at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)

                at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)

                at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)

                at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)

                at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)

      Caused by: org.hibernate.AssertionFailure: Unable to perform beforeTransactionCompletion callback

                at org.hibernate.engine.ActionQueue$BeforeTransactionCompletionProcessQueue.beforeTransactionCompletion(ActionQueue.java:549)

                at org.hibernate.engine.ActionQueue.beforeTransactionCompletion(ActionQueue.java:216)

                at org.hibernate.impl.SessionImpl.beforeTransactionCompletion(SessionImpl.java:571)

                at org.hibernate.jdbc.JDBCContext.beforeTransactionCompletion(JDBCContext.java:250)

                at org.hibernate.transaction.JDBCTransaction.commit(JDBCTransaction.java:138)

                at org.hibernate.ejb.TransactionImpl.commit(TransactionImpl.java:76)

                ... 27 more

      Caused by: java.lang.RuntimeException: Cannot find previous revision for entity enversauditing.tests.entity.TestEntity_AUD and id 2

                at org.hibernate.envers.strategy.ValidityAuditStrategy.updateLastRevision(ValidityAuditStrategy.java:175)

                at org.hibernate.envers.strategy.ValidityAuditStrategy.perform(ValidityAuditStrategy.java:66)

                at org.hibernate.envers.synchronization.work.AbstractAuditWorkUnit.perform(AbstractAuditWorkUnit.java:74)

                at org.hibernate.envers.synchronization.AuditProcess.executeInSession(AuditProcess.java:114)

                at org.hibernate.envers.synchronization.AuditProcess.doBeforeTransactionCompletion(AuditProcess.java:152)

                at org.hibernate.engine.ActionQueue$BeforeTransactionCompletionProcessQueue.beforeTransactionCompletion(ActionQueue.java:543)

                ... 32 more

       

       

      This is happening because ValidityAuditStrategy tries to update the previous row by finding the row for the matching id whose REVEND is null, but there are two rows with a null REVEND, the original deleted row and the new inserted one.

       

      Is this an issue with the validity audit strategy or is it designed only to work with entities whose id is autogenerated?

       

      If it is a bug I have a modification to the audit strategy that I would like to suggest

        • 1. ValidityAuditStrategy pk re-insertion issue
          Adam Warski Master

          You're right, right now the VAS assumes that each entity has a unique id (not necessarily auto-generated, but unique). I think in a system where Envers is used it makes sense to have such a requirement. After all, if you delete an entity and add a new one with the same id, is this the same entity? It will show up together when looking at the entity's history.

           

          But maybe sometimes this is the case. So maybe there should be a config option to enable duplicate ids, how do you think?

           

          Maybe this: http://opensource.atlassian.com/projects/hibernate/browse/HHH-5967 is the same problem?

           

          Adam

          • 2. Re: ValidityAuditStrategy pk re-insertion issue
            nbeag Newbie

            Thanks for the reply Adam.

             

            Yes I think it is similar to that problem, and also this one:

            http://opensource.atlassian.com/projects/hibernate/browse/HHH-6209?actionOrder=desc#issue-tabs

             

            I'm trying to keep track of changes to an indexed collection. The 'id' in the relationship table is a composite of the parent entity id and the string key.

            I think it makes sense that you would be able to remove an item from a collection and subsequently add a different item with the same key, and use Envers to get a snapshot of the collection at a given date.

            There are checks in the ValidityAuditStrategy class that conditionally update the previous row if the revtype is not an add

            e.g.

            if (getRevisionType(auditCfg, data) != RevisionType.ADD) 
            

            If these checks were removed, and the updateLastRevision() method updated to handle zero-previous-rev scenarios, it would allow for this scenario and I think not conflict with existing functionality. (I'm open to correction on this! )

             

            If you agree that such a change is beneficial, I can create a patch with my proposal for you to take a look at. (Or If there are test cases etc that I can run first to eliminate potential flaws then let me know. )

             

            Further advice on how to proceed much appreciated.

            • 3. Re: ValidityAuditStrategy pk re-insertion issue
              Adam Warski Master

              Hello,

               

              envers has quite a lot of tests - just clone the hibernate git repo and run gradle test in hibernate-envers.

               

              As for the indexed collection example, you are right that this should work as you describe.

               

              If you would like to create a pull request with the fix and a test case, it would be great!

               

              Adam