OpenJPA
  1. OpenJPA
  2. OPENJPA-956 OpenJPA 2.0 iteration 5 primary task
  3. OPENJPA-990

setHint should return IllegalArgumentException for invalid query/lock timeout values

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-M2
    • Fix Version/s: 2.0.0-M2
    • Component/s: query
    • Labels:
      None

      Description

      JPA2 public draft spec section 3.6.1 Query Interface notes that Query.setHint() should throw a IllegalArgumentException if a supplied hint value is not valid for a given implementation.
      For lock/query timeout, any user supplied value < -1 is invalid and should throw an exception, which is not occurring.

      1. OPENJPA-990_20090326.patch
        13 kB
        Donald Woods
      2. OPENJPA-990.patch
        11 kB
        Donald Woods

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        3d 22h 7m 1 Donald Woods 23/Mar/09 17:56
        In Progress In Progress Resolved Resolved
        6d 23h 53m 1 Donald Woods 30/Mar/09 18:49
        Resolved Resolved Closed Closed
        344d 41m 1 Donald Woods 09/Mar/10 18:31
        Donald Woods made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Donald Woods made changes -
        Description JPA2 spec notes that Query.setHint() should throw a IllegalArgumentException if a supplied hint value is not valid for a given implementation.
        For lock/query timeout, any user supplied value < -1 is invalid and should throw an exception, which is not occurring.
        JPA2 public draft spec section 3.6.1 Query Interface notes that Query.setHint() should throw a IllegalArgumentException if a supplied hint value is not valid for a given implementation.
        For lock/query timeout, any user supplied value < -1 is invalid and should throw an exception, which is not occurring.
        Donald Woods made changes -
        Status In Progress [ 3 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Jeremy Bauer made changes -
        Assignee Jeremy Bauer [ techhusky ] Donald Woods [ drwoods ]
        Hide
        Jeremy Bauer added a comment -

        Committed OPENJPA-990_20090326.patch for Donald under revision 760056.

        Show
        Jeremy Bauer added a comment - Committed OPENJPA-990 _20090326.patch for Donald under revision 760056.
        Donald Woods made changes -
        Assignee Donald Woods [ drwoods ] Jeremy Bauer [ techhusky ]
        Hide
        Donald Woods added a comment -

        Jeremy, updated patch that addresses the localizer problem.

        Show
        Donald Woods added a comment - Jeremy, updated patch that addresses the localizer problem.
        Donald Woods made changes -
        Attachment OPENJPA-990_20090326.patch [ 12403733 ]
        Hide
        Donald Woods added a comment -

        Updated patch that
        1) fixes the localizer.properties bug mentioned by Jeremy.
        2) No changes to the exception type being created in FetchConfigurationImpl, as openjpa-kernel code does not have access to the ArgumentException class in openjpa-persistence and the exception is being coverted to a ArgumentException type by the HintHandler -
        107 test TRACE [main] Tests - testQueryTimeout5() - Caught expected Exception = <openjpa-2.0.0-SNAPSHOT-r422266:758490M nonfatal user error> org.apache.openjpa.persistence.ArgumentException: Invalid value specified for query hint "javax.persistence.query.timeout": -2,000

        Show
        Donald Woods added a comment - Updated patch that 1) fixes the localizer.properties bug mentioned by Jeremy. 2) No changes to the exception type being created in FetchConfigurationImpl, as openjpa-kernel code does not have access to the ArgumentException class in openjpa-persistence and the exception is being coverted to a ArgumentException type by the HintHandler - 107 test TRACE [main] Tests - testQueryTimeout5() - Caught expected Exception = <openjpa-2.0.0-SNAPSHOT-r422266:758490M nonfatal user error> org.apache.openjpa.persistence.ArgumentException: Invalid value specified for query hint "javax.persistence.query.timeout": -2,000
        Hide
        Donald Woods added a comment -

        ArgumentException cannot be used from openjpa-kernel code like FetchConfigurationImpl, as that exception class resides in openjpa-persistence.
        Also, there is existing code in openjpa-kernel and openjpa-lib that throws IllegalArugmentException instances, so this is not a new behavior.

        Show
        Donald Woods added a comment - ArgumentException cannot be used from openjpa-kernel code like FetchConfigurationImpl, as that exception class resides in openjpa-persistence. Also, there is existing code in openjpa-kernel and openjpa-lib that throws IllegalArugmentException instances, so this is not a new behavior.
        Hide
        Donald Woods added a comment -

        Thanks for the review. I'll post an update patch today that addresses both points.

        Show
        Donald Woods added a comment - Thanks for the review. I'll post an update patch today that addresses both points.
        Donald Woods made changes -
        Assignee Jeremy Bauer [ techhusky ] Donald Woods [ drwoods ]
        Hide
        Pinaki Poddar added a comment -

        should throw openjpa.ArgumentException from FetchConfigurationImpl rather than IllegalArgumentException – for consistency.

        Show
        Pinaki Poddar added a comment - should throw openjpa.ArgumentException from FetchConfigurationImpl rather than IllegalArgumentException – for consistency.
        Hide
        Jeremy Bauer added a comment -

        There is a minor problem with the patch. The Localizer will try to load the "invalid-timeout" message from the resource package of FetchConfigurationImpl (org.apache.openjpa.kernel). The message is in "org/apache/openjpa/util/localizer.properties". The message must also be added to org/apache/openjpa/kernel/localizer.properties in order to be loaded. I set some breakpoints in the code to verify that the message does not get loaded in FetchConfigurationImpl.

        Also, the message contains this sentence: Expected a value that is greater than or equal to zero. Should this read "greater than or equal to -1 ", since -1 is also valid?

        Show
        Jeremy Bauer added a comment - There is a minor problem with the patch. The Localizer will try to load the "invalid-timeout" message from the resource package of FetchConfigurationImpl (org.apache.openjpa.kernel). The message is in "org/apache/openjpa/util/localizer.properties". The message must also be added to org/apache/openjpa/kernel/localizer.properties in order to be loaded. I set some breakpoints in the code to verify that the message does not get loaded in FetchConfigurationImpl. Also, the message contains this sentence: Expected a value that is greater than or equal to zero. Should this read "greater than or equal to -1 ", since -1 is also valid?
        Donald Woods made changes -
        Assignee Donald Woods [ drwoods ] Jeremy Bauer [ techhusky ]
        Hide
        Donald Woods added a comment -

        Jeremy, can you review the patch and commit if all looks well? Thanks.

        Show
        Donald Woods added a comment - Jeremy, can you review the patch and commit if all looks well? Thanks.
        Donald Woods made changes -
        Attachment OPENJPA-990.patch [ 12403454 ]
        Hide
        Donald Woods added a comment -

        Patch that will cause setHint() for javax.persistence.lock/query.timeout to return a OpenJPA ArgumentException (which extends the spec required IllegalArgumentException) if the provided timeout is invalid (timeout < -1), as -1 is allowed by OpenJPA to denote no timeout (the same as providing 0).
        Three junit tests were added to test for the new IllegalArgumentException behavior.

        Show
        Donald Woods added a comment - Patch that will cause setHint() for javax.persistence.lock/query.timeout to return a OpenJPA ArgumentException (which extends the spec required IllegalArgumentException) if the provided timeout is invalid (timeout < -1), as -1 is allowed by OpenJPA to denote no timeout (the same as providing 0). Three junit tests were added to test for the new IllegalArgumentException behavior.
        Donald Woods made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Donald Woods made changes -
        Field Original Value New Value
        Fix Version/s 2.0.0 [ 12313483 ]
        Affects Version/s 2.0.0 [ 12313483 ]
        Description JPA2 spec notes that Query.setHint() should throw a IllegalArgumentException if a supplied hint value is not valid for a given implementation.
        For lock/query timeout, any user supplied value < -1 is invalid and should throw an exception, which is not occurring.
        Component/s query [ 12311309 ]
        Donald Woods created issue -

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development