OpenJPA
  1. OpenJPA
  2. OPENJPA-1018

@PreUpdate raised for new entities annotated with @EntityListeners

    Details

      Description

      Given the following entity:

      @Entity
      @EntityListeners(

      {Auditing.class}

      )
      @Table(...)
      public class A

      { ... }

      and the following Auditing class:

      public class Auditing {
      @PreUpdate
      public void preUpdate(Object entity)

      { // the provided object is supposed to be a PersistenceCapable ... }

      }

      When using runtime enhancement, the PreUpdate event is raised and preUpdate(Object) is called when persisting a new entity: the call is not expected as the entity is not yet persisted (moreover, the entity passed in parameter is not an instance of PersistenceCapable).

      This is due to StateManagerImpl.preFlush() lifecycle event firing conditions:

      // BEFORE_PERSIST is handled during Broker.persist and Broker.attach
      if (isDeleted())
      fireLifecycleEvent(LifecycleEvent.BEFORE_DELETE);
      else if (!(isNew() && !isFlushed())
      && (ImplHelper.getUpdateFields(this) != null))
      fireLifecycleEvent(LifecycleEvent.BEFORE_UPDATE);

      When processing a PNewState, the condition for BEFORE_UPDATE event becomes simply: isFlushed(), i.e. the BEFORE_UPDATE event is raised for a new Entity! (stuff below is supposed to be a boolean table, sorry for the loosy presentation):

      isNew
      true.....false
      isFlushed.......true......fire.......fire
      false......X.........fire

      where X means "do nothing" and fire means "fire the BEFORE_UPDATE event".

      The correct full condition would include a condition to prevent raising BEFORE_UPDATE for new entities:

      isNew
      true.....false
      isFlushed.......true........X.........fire
      false......X.........fire

      where X means "do nothing" and fire means "fire the BEFORE_UPDATE event", which finally gives:

      else if (!isNew() && (ImplHelper.getUpdateFields(this) != null))

        Issue Links

          Activity

          Hide
          Julien Kronegg added a comment -

          tried to make the boolean table more readable

          Show
          Julien Kronegg added a comment - tried to make the boolean table more readable
          Hide
          B.J. Reed added a comment -

          I made the suggested code change, but that has broken (at least) 2 of our JUnit test cases.

          Failed tests:
          testPreUpdateExceptionDuringFlushWithNewFlushedInstance(org.apache.openjpa.persistence.callbacks.TestExceptionsFromCallbacks)
          testPreUpdateExceptionDuringCommitWithNewFlushedInstance(org.apache.openjpa.persistence.callbacks.TestExceptionsFromCallbacks)

          Show
          B.J. Reed added a comment - I made the suggested code change, but that has broken (at least) 2 of our JUnit test cases. Failed tests: testPreUpdateExceptionDuringFlushWithNewFlushedInstance(org.apache.openjpa.persistence.callbacks.TestExceptionsFromCallbacks) testPreUpdateExceptionDuringCommitWithNewFlushedInstance(org.apache.openjpa.persistence.callbacks.TestExceptionsFromCallbacks)
          Hide
          Michael Dick added a comment -

          The testcases in question mutate a new entity before committing the transaction. This is allowed behavior per section 3.5.2 of the JPA specification :
          "The PreUpdate and PostUpdate callbacks occur before and after the database update operations to
          entity data respectively. These database operations may occur at the time the entity state is updated or
          they may occur at the time state is flushed to the database (which may be at the end of the transaction).

          Note that it is implementation-dependent as to whether PreUpdate and PostUpdate callbacks
          occur when an entity is persisted and subsequently modified in a single transaction or
          when an entity is modified and subsequently removed within a single transaction. Portable
          applications should not rely on such behavior."

          Julien, in the original scenario does the application make any changes to the entity after calling persist? If so then the Pre/PostUpdate callback methods may be invoked. If not then we need to modify TestExceptionsFromCallbacks to cover this scenario.

          Show
          Michael Dick added a comment - The testcases in question mutate a new entity before committing the transaction. This is allowed behavior per section 3.5.2 of the JPA specification : "The PreUpdate and PostUpdate callbacks occur before and after the database update operations to entity data respectively. These database operations may occur at the time the entity state is updated or they may occur at the time state is flushed to the database (which may be at the end of the transaction). Note that it is implementation-dependent as to whether PreUpdate and PostUpdate callbacks occur when an entity is persisted and subsequently modified in a single transaction or when an entity is modified and subsequently removed within a single transaction. Portable applications should not rely on such behavior." Julien, in the original scenario does the application make any changes to the entity after calling persist? If so then the Pre/PostUpdate callback methods may be invoked. If not then we need to modify TestExceptionsFromCallbacks to cover this scenario.
          Hide
          Michael Dick added a comment -

          Some background info. The testcases in question were originally added [1] to address exception handling, but a little tweaking makes it look like the call to preUpdate only occurs if the entity has been mutated after being persisted.

          A testcase that shows the behavior would be very helpful with this issue.

          [1] http://markmail.org/thread/vzpvoruahwqmvsnx

          Show
          Michael Dick added a comment - Some background info. The testcases in question were originally added [1] to address exception handling, but a little tweaking makes it look like the call to preUpdate only occurs if the entity has been mutated after being persisted. A testcase that shows the behavior would be very helpful with this issue. [1] http://markmail.org/thread/vzpvoruahwqmvsnx
          Hide
          Julien Kronegg added a comment -

          I tested using the following code:

          EntityManager em;
          // ... create the EntityManager
          MyEntity entity = new MyEntity(); // entity annotated with @PrePersist and @PreUpdate
          // ... fill the entity fields
          em.getTransaction().begin();
          em.persist(entity);
          em.getTransaction().commit();

          My persistence.xml file contains the <class>myPackage.MyEntity</class>.

          When running the example above using runtime enhancement, both PrePersist and PreUpdate events are raised (expected: only PrePersist should be raised).
          But when running the example above using the "-javaagent:lib/openjpa.jar" JVM option, only the PrePersist event is raised (this is the expected result).

          So it seems this is a problem with the runtime enhancement process.

          Show
          Julien Kronegg added a comment - I tested using the following code: EntityManager em; // ... create the EntityManager MyEntity entity = new MyEntity(); // entity annotated with @PrePersist and @PreUpdate // ... fill the entity fields em.getTransaction().begin(); em.persist(entity); em.getTransaction().commit(); My persistence.xml file contains the <class>myPackage.MyEntity</class>. When running the example above using runtime enhancement, both PrePersist and PreUpdate events are raised (expected: only PrePersist should be raised). But when running the example above using the "-javaagent:lib/openjpa.jar" JVM option, only the PrePersist event is raised (this is the expected result). So it seems this is a problem with the runtime enhancement process.
          Hide
          Julien Kronegg added a comment -

          About a testcase: TestEntityListener tests [1] should fail when the EntityListenerEntity or GlobalListenerEntity are enhanced at runtime.

          [1] http://fisheye6.atlassian.com/browse/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/callbacks/TestEntityListeners.java?r=757278

          Show
          Julien Kronegg added a comment - About a testcase: TestEntityListener tests [1] should fail when the EntityListenerEntity or GlobalListenerEntity are enhanced at runtime. [1] http://fisheye6.atlassian.com/browse/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/callbacks/TestEntityListeners.java?r=757278
          Hide
          Michael Dick added a comment -

          I think I see the problem now. When you said runtime enhancement I thought you were using the javaagent [1]. Did you mean runtimeUnenhancedSubclasses [2] instead?

          [1] http://openjpa.apache.org/builds/latest/docs/manual/manual.html#ref_guide_pc_enhance_runtime_ex
          [2] http://openjpa.apache.org/builds/latest/docs/manual/manual.html#ref_guide_pc_enhance_unenhanced_types

          Show
          Michael Dick added a comment - I think I see the problem now. When you said runtime enhancement I thought you were using the javaagent [1] . Did you mean runtimeUnenhancedSubclasses [2] instead? [1] http://openjpa.apache.org/builds/latest/docs/manual/manual.html#ref_guide_pc_enhance_runtime_ex [2] http://openjpa.apache.org/builds/latest/docs/manual/manual.html#ref_guide_pc_enhance_unenhanced_types
          Hide
          Julien Kronegg added a comment -

          Yes, I mean Runtime Unenhanced Subclasses. So:

          • when using "Runtime Unenhanced Classes" method with property access: PrePersist and PreUpdate are raised
          • when using "Java 5 class redefinition/javaagent" method with property access: only PrePersist is raised (expected behavior)
          Show
          Julien Kronegg added a comment - Yes, I mean Runtime Unenhanced Subclasses. So: when using "Runtime Unenhanced Classes" method with property access: PrePersist and PreUpdate are raised when using "Java 5 class redefinition/javaagent" method with property access: only PrePersist is raised (expected behavior)

            People

            • Assignee:
              Unassigned
              Reporter:
              Julien Kronegg
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:

                Time Tracking

                Estimated:
                Original Estimate - 4h
                4h
                Remaining:
                Remaining Estimate - 4h
                4h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development