OpenJPA
  1. OpenJPA
  2. OPENJPA-1707

A warning message should be logged when a down level enhanced Entity is encountered.

    Details

    • Type: Improvement Improvement
    • Status: Reopened
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.0.0, 2.1.0
    • Fix Version/s: 2.1.0
    • Component/s: kernel
    • Labels:
      None

      Description

      Currently when PCEnhancer is called to enhance an Entity that was previous enhanced it will simply noop and move on. It could be smarter and use the PersistenceCapable.pcGetEnhancementContractVersion to detect and when an Entity has been enhanced with an older version of the enhancer and log a warning message. A similar change would also need to be made to PCClassFileTransformer.needsEnhance to perform the same sort of logic at runtime.

      This would be particularly helpful in the case where an application was compiled/enhanced/packaged and a bug was fixed in the enhancer but the pre-packaged app still has the bad bytecode.

        Issue Links

          Activity

          Hide
          Rick Curtis added a comment -

          The change that I committed to trunk utilizes the PCEnhancer.checkEnhancementLevel() method which was added in OPENJPA-151 but as far as I can tell was never used. I believe the intent of this field was to be used for changes to enhanced bytecode that may break serialization. Since this field was never utilized, I took the liberty to repurpose the method.

          I added some magic to the build which now will write the revision of the PCEnhancer to META-INF/org.apache.openjpa.revision.properties. This value will be read into PCEnhancer in a static initializer which will be used for validation, and will be written to enhanced bytecode. This way we won't have to manually update the PCEnhancer.ENHANCER_VERSION every time a change is made to that file.

          Currently I have it so that when trace IS NOT enabled, we will only log one message when a down level Entity is encountered. If trace is enabled we will log a message for each down level Entity that is encountered.

          I flipped back and forth on whether or not this should be an INFO or WARN message. I ended up going with an INFO message because for the time being any Entity that was enhanced prior to this commit will result in a message being logged.

          Show
          Rick Curtis added a comment - The change that I committed to trunk utilizes the PCEnhancer.checkEnhancementLevel() method which was added in OPENJPA-151 but as far as I can tell was never used. I believe the intent of this field was to be used for changes to enhanced bytecode that may break serialization. Since this field was never utilized, I took the liberty to repurpose the method. I added some magic to the build which now will write the revision of the PCEnhancer to META-INF/org.apache.openjpa.revision.properties. This value will be read into PCEnhancer in a static initializer which will be used for validation, and will be written to enhanced bytecode. This way we won't have to manually update the PCEnhancer.ENHANCER_VERSION every time a change is made to that file. Currently I have it so that when trace IS NOT enabled, we will only log one message when a down level Entity is encountered. If trace is enabled we will log a message for each down level Entity that is encountered. I flipped back and forth on whether or not this should be an INFO or WARN message. I ended up going with an INFO message because for the time being any Entity that was enhanced prior to this commit will result in a message being logged.
          Hide
          Michael Dick added a comment -

          Closing issues which have been resolved for some time. If the problem persists, please reopen.

          Show
          Michael Dick added a comment - Closing issues which have been resolved for some time. If the problem persists, please reopen.
          Hide
          Pinaki Poddar added a comment -

          > Since this field was never utilized, I took the liberty to repurpose the method.
          Such liberty is not recommended for several reasons
          a) the purpose of the field is lost by your change.
          b) The purpose was non-trivial and now replaced with a rather trivial semantics. What code reversion was used to enhance a class is not as important as to know whether the serialization data for an enhanced entity has been modified by us which is critical information in a version upgrade.
          With this change one can add a new log statement in PCEnhancer and this change will start logging warning messages for no good reason.

          If the concern is why this information is not used so far, then change that by adding some code that validates that enhancement is compatible.
          If the concern is to carry that information on code revision then that has to be done using a separate field, and upgrading the value of ENHANCEMENT_CONTRACT to its next integral level.

          Show
          Pinaki Poddar added a comment - > Since this field was never utilized, I took the liberty to repurpose the method. Such liberty is not recommended for several reasons a) the purpose of the field is lost by your change. b) The purpose was non-trivial and now replaced with a rather trivial semantics. What code reversion was used to enhance a class is not as important as to know whether the serialization data for an enhanced entity has been modified by us which is critical information in a version upgrade. With this change one can add a new log statement in PCEnhancer and this change will start logging warning messages for no good reason. If the concern is why this information is not used so far, then change that by adding some code that validates that enhancement is compatible. If the concern is to carry that information on code revision then that has to be done using a separate field, and upgrading the value of ENHANCEMENT_CONTRACT to its next integral level.
          Hide
          Pinaki Poddar added a comment -

          Please see my comment

          Show
          Pinaki Poddar added a comment - Please see my comment

            People

            • Assignee:
              Rick Curtis
              Reporter:
              Rick Curtis
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:

                Development