Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.6
    • Fix Version/s: 0.7
    • Component/s: core, jcr
    • Labels:
      None

      Description

      The fix for OAK-702 introduced a regression not caught by our tests but is visible with Apache Sling on top of an Oak repository. The JSP compiler in Sling creates an nt:resource node with a jcr:lastModified property using a long value. As of svn revision 1461058 this results in a ConstraintViolationException:

      javax.jcr.nodetype.ConstraintViolationException: No matching property definition found for jcr:lastModified
      	at org.apache.jackrabbit.oak.plugins.nodetype.EffectiveNodeType.getPropertyDefinition(EffectiveNodeType.java:354)
      	at org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager.getDefinition(ReadOnlyNodeTypeManager.java:435)
      	at org.apache.jackrabbit.oak.jcr.NodeImpl$32.perform(NodeImpl.java:1610)
      	at org.apache.jackrabbit.oak.jcr.NodeImpl$32.perform(NodeImpl.java:1599)
      	at org.apache.jackrabbit.oak.jcr.delegate.SessionDelegate.perform(SessionDelegate.java:101)
      	at org.apache.jackrabbit.oak.jcr.ItemImpl.perform(ItemImpl.java:60)
      	at org.apache.jackrabbit.oak.jcr.NodeImpl.internalSetProperty(NodeImpl.java:1599)
      	at org.apache.jackrabbit.oak.jcr.NodeImpl.setProperty(NodeImpl.java:519)
      

        Activity

        Hide
        Jukka Zitting added a comment -

        I'd say all setProperty() methods without type parameter should call internalSetProperty() with exactType = false.

        Sounds reasonable.

        I'm wondering why we enforced exact type before in those methods

        Not sure about that. I wondered the same thing when recently refactoring the setProperty() methods, but didn't want to change the default exactType setting on the assumption that those type-specific method signatures should perhaps be treated as if the equivalent setProperty(String, Value, int) method was called instead of just setProperty(String, Value).

        how this worked in the past

        Probably because the type validator was less strict than it now is after my recent changes. Or perhaps because the order in which matching item definitions is looked up is now slightly different than before.

        Show
        Jukka Zitting added a comment - I'd say all setProperty() methods without type parameter should call internalSetProperty() with exactType = false. Sounds reasonable. I'm wondering why we enforced exact type before in those methods Not sure about that. I wondered the same thing when recently refactoring the setProperty() methods, but didn't want to change the default exactType setting on the assumption that those type-specific method signatures should perhaps be treated as if the equivalent setProperty(String, Value, int) method was called instead of just setProperty(String, Value) . how this worked in the past Probably because the type validator was less strict than it now is after my recent changes. Or perhaps because the order in which matching item definitions is looked up is now slightly different than before.
        Hide
        Marcel Reutegger added a comment -

        Fixed in revision: 1461600

        Show
        Marcel Reutegger added a comment - Fixed in revision: 1461600
        Hide
        Marcel Reutegger added a comment - - edited

        Hmm, I'd say all setProperty() methods without type parameter should call internalSetProperty() with exactType = false. E.g. the JavaDoc of setProperty(String, long) says:

        The behavior of this method is identical to that of setProperty(String name, Value value) except that the value is specified as a long and, if possible, the type assigned to the property is LONG, otherwise a best-effort conversion is attempted.

        And in NodeImpl.setProperty(String, Value) we pass exactType = false.

        All tests pass with this modification.

        I'm wondering why we enforced exact type before in those methods and how this worked in the past...

        Show
        Marcel Reutegger added a comment - - edited Hmm, I'd say all setProperty() methods without type parameter should call internalSetProperty() with exactType = false . E.g. the JavaDoc of setProperty(String, long) says: The behavior of this method is identical to that of setProperty(String name, Value value) except that the value is specified as a long and, if possible, the type assigned to the property is LONG, otherwise a best-effort conversion is attempted. And in NodeImpl.setProperty(String, Value) we pass exactType = false . All tests pass with this modification. I'm wondering why we enforced exact type before in those methods and how this worked in the past...
        Hide
        Marcel Reutegger added a comment -

        Added test case in revision 1461560.

        Show
        Marcel Reutegger added a comment - Added test case in revision 1461560.

          People

          • Assignee:
            Marcel Reutegger
            Reporter:
            Marcel Reutegger
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development