OpenJPA
  1. OpenJPA
  2. OPENJPA-1097

Detachment processing of our proxied mutable types (Date, Timestamp, etc) needs to be consistent

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.2, 1.3.0, 2.0.0-M1
    • Fix Version/s: 1.3.0, 2.0.0-beta3
    • Component/s: jpa
    • Labels:
      None

      Description

      Per the discussion on our forums [1], there seems to some consistent issues with how we perform detachment processing of the mutable types such as Date, Timestamp, etc. There seems to be a couple of issues (at least)...

      o Detachment processing is not consistent between the detach() method and the clear() method.
      o Tracking of changes on a detached entity, but still allow it to be serialized without requiring the openjpa jar file.

      [1] http://n2.nabble.com/Date-Problem-td2943310.html#a2943310

      1. Proxies.java
        7 kB
        Donald Woods
      2. OPENJPA-1097-trunk.diff
        18 kB
        Donald Woods
      3. OPENJPA-1097-tests.diff
        11 kB
        Donald Woods
      4. OPENJPA-1097-12x-tests.diff
        29 kB
        Donald Woods
      5. OPENJPA-1097.patch
        3 kB
        Rick Curtis
      6. OPENJPA-1097.2-trunk.patch
        4 kB
        Donald Woods
      7. OPENJPA-1097.1-trunk.patch
        11 kB
        Donald Woods

        Issue Links

          Activity

          Hide
          Donald Woods added a comment -

          Updated code and docs in 1.3.x and 2.0.0.
          Attached patch for 1.2.3 if we ever want to include it there.

          Show
          Donald Woods added a comment - Updated code and docs in 1.3.x and 2.0.0. Attached patch for 1.2.3 if we ever want to include it there.
          Hide
          Donald Woods added a comment -

          Mike, the OPENJPA-1097-12x-code.patch is for 1.2.3, which backports the code changes from 1.3.x/trunk and updates the TestDetachNoProxy.testClear() to verify that there is no $proxy usage after EM.clear() and the entity is serialized.

          Show
          Donald Woods added a comment - Mike, the OPENJPA-1097 -12x-code.patch is for 1.2.3, which backports the code changes from 1.3.x/trunk and updates the TestDetachNoProxy.testClear() to verify that there is no $proxy usage after EM.clear() and the entity is serialized.
          Hide
          Donald Woods added a comment -

          Long saga of this code change.... Capturing a version of the fully commented/debug code here in case we have to revisit this.... Not to commit as-is, but for historical purposes.

          Show
          Donald Woods added a comment - Long saga of this code change.... Capturing a version of the fully commented/debug code here in case we have to revisit this.... Not to commit as-is, but for historical purposes.
          Hide
          Donald Woods added a comment -

          Variation on previous patch, which I'm not planning on committing, but for historical purposes:
          1) when an entity is serialized after a find() but before a detach()/detachAll()/detachCopy()/clear(), then it will always contain the $proxy wrapper classes
          2) after detach/detachAll/detachCopy/clear, the $proxy classes will be removed at serialization

          Show
          Donald Woods added a comment - Variation on previous patch, which I'm not planning on committing, but for historical purposes: 1) when an entity is serialized after a find() but before a detach()/detachAll()/detachCopy()/clear(), then it will always contain the $proxy wrapper classes 2) after detach/detachAll/detachCopy/clear, the $proxy classes will be removed at serialization
          Hide
          Donald Woods added a comment -

          Tests showing how detach()/detachAll()/clear() are currently working in 1.2.x, where entities serialized after calling clear() still contain our OpenJPA $proxy class wrappers.

          Show
          Donald Woods added a comment - Tests showing how detach()/detachAll()/clear() are currently working in 1.2.x, where entities serialized after calling clear() still contain our OpenJPA $proxy class wrappers.
          Hide
          Donald Woods added a comment -

          Updated patch that reverts some earlier DetachManager changes that broke the TestProxyCollection junits and updates the Proxies class to always remove $proxy classes on serialization if the entity is detachable.

          Show
          Donald Woods added a comment - Updated patch that reverts some earlier DetachManager changes that broke the TestProxyCollection junits and updates the Proxies class to always remove $proxy classes on serialization if the entity is detachable.
          Hide
          Donald Woods added a comment -
          Show
          Donald Woods added a comment - See OPENJPA-1571
          Hide
          Donald Woods added a comment -

          Basically, the patch fixed the clear() behavior, in that JavaType.OBJECT fields were not being unproxied and added junits to verify the following behavior by using an Entity20 class that contains the following fields -
          @Temporal(TemporalType.DATE) java.sql.Date
          @Temporal(TemporalType.TIME) java.sql.Time
          @Temporal(TemporalType.TIMESTAMP) java.sql.Timestamp

          For trunk (when using the 2.0 default compatibility settings and the new JPA 2.0 Spec required in-place detach and no return values, which differs from the OpenJPA 1.x value-added detach()/detachAll() methods):
          1) find() - entity uses the proxied classes before and after serialization
          2) after in-place detach() - entity does NOT use proxied classes before or after serialization
          3) after in-place detachAll() - none of the entities use proxied classes before or after serialization
          4) detachCopy() - returns an entity that does NOT use proxied classes before or after serialization
          5) clear() - existing entity instances after EM.clear() do NOT use proxied classes before or after serialization

          For 1.3.x (same for proposed 1.2.x changes):
          1) find() - entity uses the proxied classes before and after serialization
          2) detach() returns a copy of the original entity that does NOT use proxied classes before or after serialization, but the original entity instance still uses proxies and returns FALSE for isDetached()
          3) detachAll() returns copies of the original entities that do NOT use proxied classes before or after serialization, but the original entity instances still use proxies and returns FALSE for isDetached()
          4) there is no tests for detachCopy() as this was added in 2.0.0 because JPA 2.0 Spec required in-place detach() and no return value
          5) clear() - existing entity instances after EM.clear() do NOT use proxied classes before or after serialization

          Show
          Donald Woods added a comment - Basically, the patch fixed the clear() behavior, in that JavaType.OBJECT fields were not being unproxied and added junits to verify the following behavior by using an Entity20 class that contains the following fields - @Temporal(TemporalType.DATE) java.sql.Date @Temporal(TemporalType.TIME) java.sql.Time @Temporal(TemporalType.TIMESTAMP) java.sql.Timestamp For trunk (when using the 2.0 default compatibility settings and the new JPA 2.0 Spec required in-place detach and no return values, which differs from the OpenJPA 1.x value-added detach()/detachAll() methods): 1) find() - entity uses the proxied classes before and after serialization 2) after in-place detach() - entity does NOT use proxied classes before or after serialization 3) after in-place detachAll() - none of the entities use proxied classes before or after serialization 4) detachCopy() - returns an entity that does NOT use proxied classes before or after serialization 5) clear() - existing entity instances after EM.clear() do NOT use proxied classes before or after serialization For 1.3.x (same for proposed 1.2.x changes): 1) find() - entity uses the proxied classes before and after serialization 2) detach() returns a copy of the original entity that does NOT use proxied classes before or after serialization, but the original entity instance still uses proxies and returns FALSE for isDetached() 3) detachAll() returns copies of the original entities that do NOT use proxied classes before or after serialization, but the original entity instances still use proxies and returns FALSE for isDetached() 4) there is no tests for detachCopy() as this was added in 2.0.0 because JPA 2.0 Spec required in-place detach() and no return value 5) clear() - existing entity instances after EM.clear() do NOT use proxied classes before or after serialization
          Hide
          Donald Woods added a comment -

          1.3.x - the 1.0b TCK passed with the integrated patches
          trunk - the 1.0b and 2.0 TCK passed with the integrated patches

          Will leave it to Mike to determine if the 1.2.x patch should be applied to the 1.2.3 release.

          Show
          Donald Woods added a comment - 1.3.x - the 1.0b TCK passed with the integrated patches trunk - the 1.0b and 2.0 TCK passed with the integrated patches Will leave it to Mike to determine if the 1.2.x patch should be applied to the 1.2.3 release.
          Hide
          Donald Woods added a comment -

          Patch for 1.2.x which includes the new junits.

          Show
          Donald Woods added a comment - Patch for 1.2.x which includes the new junits.
          Hide
          Donald Woods added a comment -

          Initial patch for 1.2.x to fix clear() not unproxying JavaTypes.OBJECT fields.

          Show
          Donald Woods added a comment - Initial patch for 1.2.x to fix clear() not unproxying JavaTypes.OBJECT fields.
          Hide
          Donald Woods added a comment -

          Updated trunk patch

          Show
          Donald Woods added a comment - Updated trunk patch
          Hide
          Donald Woods added a comment -

          Latest patch for trunk based on DetachManager changes from Rick Curtis.

          Show
          Donald Woods added a comment - Latest patch for trunk based on DetachManager changes from Rick Curtis.
          Hide
          Donald Woods added a comment -

          Updated patch that includes fixes for some failing proxy tests.

          Show
          Donald Woods added a comment - Updated patch that includes fixes for some failing proxy tests.
          Hide
          Donald Woods added a comment -

          Minor patch updates for 13x.

          Show
          Donald Woods added a comment - Minor patch updates for 13x.
          Hide
          Donald Woods added a comment -

          New tests and DetachManager updates from Rick Curtis back ported to 1.3.x.

          Show
          Donald Woods added a comment - New tests and DetachManager updates from Rick Curtis back ported to 1.3.x.
          Hide
          Rick Curtis added a comment -

          Sorry I didn't have much time this morning to look through the entire UT, but I think this should fix the problem.

          Show
          Rick Curtis added a comment - Sorry I didn't have much time this morning to look through the entire UT, but I think this should fix the problem.
          Hide
          Donald Woods added a comment -

          968 contains some important details, like the change in method signature/behavior of detach() in 2.0 (as required in the JPA 2.0 Spec) vs what was shipping in OpenJPA 1.x (value-add and not Spec defined.)
          We do have some questionable behavior when detachAll() or clear() are called, in that in DetachManager.java the _copy is forced to false when _full is being set.
          Also, calling detach(Entity) and then detachCopy(Entity) does not return an unproxied Entity as expected (if detachCopy is called first it works), but the same in-place entity from the earlier detach() call, as it was located in the _detached Map.

          Show
          Donald Woods added a comment - 968 contains some important details, like the change in method signature/behavior of detach() in 2.0 (as required in the JPA 2.0 Spec) vs what was shipping in OpenJPA 1.x (value-add and not Spec defined.) We do have some questionable behavior when detachAll() or clear() are called, in that in DetachManager.java the _copy is forced to false when _full is being set. Also, calling detach(Entity) and then detachCopy(Entity) does not return an unproxied Entity as expected (if detachCopy is called first it works), but the same in-place entity from the earlier detach() call, as it was located in the _detached Map.
          Hide
          Donald Woods added a comment -

          some tests showing how detach() and clear() do not unproxy the entities -

          160 test INFO [main] Tests - e20.sqlDate = org.apache.openjpa.util.java$sql$Date$proxy
          160 test INFO [main] Tests - e20.sqlTime = org.apache.openjpa.util.java$sql$Time$proxy
          160 test INFO [main] Tests - e20.sqlTimestamp = org.apache.openjpa.util.java$sql$Timestamp$proxy

          but detachCopy() does -

          161 test INFO [main] Tests - after detach e20copy
          161 test INFO [main] Tests - e20.sqlDate = java.sql.Date
          161 test INFO [main] Tests - e20.sqlTime = java.sql.Time
          161 test INFO [main] Tests - e20.sqlTimestamp = java.sql.Timestamp

          Show
          Donald Woods added a comment - some tests showing how detach() and clear() do not unproxy the entities - 160 test INFO [main] Tests - e20.sqlDate = org.apache.openjpa.util.java$sql$Date$proxy 160 test INFO [main] Tests - e20.sqlTime = org.apache.openjpa.util.java$sql$Time$proxy 160 test INFO [main] Tests - e20.sqlTimestamp = org.apache.openjpa.util.java$sql$Timestamp$proxy but detachCopy() does - 161 test INFO [main] Tests - after detach e20copy 161 test INFO [main] Tests - e20.sqlDate = java.sql.Date 161 test INFO [main] Tests - e20.sqlTime = java.sql.Time 161 test INFO [main] Tests - e20.sqlTimestamp = java.sql.Timestamp

            People

            • Assignee:
              Donald Woods
              Reporter:
              Kevin Sutter
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development