Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: current
    • Fix Version/s: milestone 1
    • Component/s: observation
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      461

      Description

      The spec currently says:

      "Event.getMethod will return the method event type corresponding to the method
      that caused this event (see XX below) and Event.getMethodInfo will return a Map
      holding name value pairs of the parameters passed on that method call. The name
      of each pair is the name of the parameter used in the Javadoc. The value of each
      pair is the value of the parameter passed. If the passed value is a
      java.lang.Object then it is held in the Map unchanged. If it is an array or a
      primitive type then it is converted to an Object of the appropriate class (an
      int becomes a java.lang.Integer, etc.)."

      There are two issues with this paragraph:

      1) At the last F2F we decided that name and path parameters are returned with
      the local namespace mapping.

      2) Returning all Objects as is does not work for Items (Node, Property, Version,
      etc. ). Those belong to the session that called the method and cannot just be
      passed to another session. For some methods the Item may not even exist anymore
      (because it had been removed). I therefore suggest to return the path of the
      Item that was used as a parameter.

        Issue Links

          Activity

          Hide
          stefan_guggisberg added a comment -

          add dependency to #477

          Show
          stefan_guggisberg added a comment - add dependency to #477
          Hide
          Peeter Piegaze added a comment -

          Proposal: Change the description of Event.getMethodInfo() to the following:

          Map Event.getMethodInfo() throws RepositoryException

          If this Event is a state-change event, getMethodInfo() returns null.

          If this Event is a method event, getMethodInfo() returns a Map holding the parameters and return value
          of the method call that caused this Event.

          For each parameter, an entry is placed in the Map where the key, a String, is the name of the parameter
          as specified in the Javadoc and the value is as follows:

          • If the parameter is null then the value is null.
          • If the parameter is a Java primitive then the value is that parameter converted to the corresponding
            Java class (for example, an int becomes an Integer).
          • If the parameter is a Node or Property then the value is the String form of the absolute path of the
            Node or Property under the local namespace mapping of the current Session.
          • If the parameter is another class of object then the value is either that object or null. An
            implementation should only use null in cases where keeping the object reference in the Event is
            impractical for performance or other resource-related reasons. For example, an implementation may
            choose to use null for objects of type Binary.

          In addition to the entries described above:

          • For each Node parameter, a second entry is created where the key is the name of the parameter as
            specified in the Javadoc, prefixed with a hash character('#') and the value is the identifier of the Node (a
            String). For example, a Node parameter called "foo" would produce two Map entries, one with key "foo"
            and the other with key "#foo". Note that since no Java identifier can begin with the character '#', this
            entry will never collide with another parameter.
          • Two entries are created for the return value using the same pattern as for parameters except that
            keys are "return" and "#return" and:

          – If the method does not return anything (its return type is void) or if this particular call returned null
          then the values of both entries are null.

          – If the object returned is not a Node then the "#return" entry is null.

          • Note that since "return" is a Java keyword it will never collide with a parameter name.
          Show
          Peeter Piegaze added a comment - Proposal: Change the description of Event.getMethodInfo() to the following: Map Event.getMethodInfo() throws RepositoryException If this Event is a state-change event, getMethodInfo() returns null. If this Event is a method event, getMethodInfo() returns a Map holding the parameters and return value of the method call that caused this Event. For each parameter, an entry is placed in the Map where the key, a String, is the name of the parameter as specified in the Javadoc and the value is as follows: If the parameter is null then the value is null. If the parameter is a Java primitive then the value is that parameter converted to the corresponding Java class (for example, an int becomes an Integer). If the parameter is a Node or Property then the value is the String form of the absolute path of the Node or Property under the local namespace mapping of the current Session. If the parameter is another class of object then the value is either that object or null. An implementation should only use null in cases where keeping the object reference in the Event is impractical for performance or other resource-related reasons. For example, an implementation may choose to use null for objects of type Binary. In addition to the entries described above: For each Node parameter, a second entry is created where the key is the name of the parameter as specified in the Javadoc, prefixed with a hash character('#') and the value is the identifier of the Node (a String). For example, a Node parameter called "foo" would produce two Map entries, one with key "foo" and the other with key "#foo". Note that since no Java identifier can begin with the character '#', this entry will never collide with another parameter. Two entries are created for the return value using the same pattern as for parameters except that keys are "return" and "#return" and: – If the method does not return anything (its return type is void) or if this particular call returned null then the values of both entries are null. – If the object returned is not a Node then the "#return" entry is null. Note that since "return" is a Java keyword it will never collide with a parameter name.
          Hide
          Peeter Piegaze added a comment -

          Since we are sticking all this information into the getMethodInfo Map perhaps we should also put the
          method name in there. This would allow us to get rid of the long list of method event type constants and
          so make for a more compact spec and Javadoc.

          Show
          Peeter Piegaze added a comment - Since we are sticking all this information into the getMethodInfo Map perhaps we should also put the method name in there. This would allow us to get rid of the long list of method event type constants and so make for a more compact spec and Javadoc.
          Hide
          Peeter Piegaze added a comment -

          Actually, I guess it would make more sense to use getMethod to return the method name.

          In other words for Node.addNode the Event.getMethod would return "javax.jcr.Node.addNode" instead
          of the current "javax.jcr.observation.Event.ADD_NODE".

          Though we could still supply string constants for each JCR method, it would have a few advantages:

          1) It's self-documenting. We don't have to define and explain a new String and how it maps to its
          method.
          2) It let's us get rid of awkward method type names like SESSION_MOVE and COPY_EXTERNAL
          3) The method Event no longer has a "name" per se, so the motivation to convert the names to JCR
          names disappears.
          4) It allows a clear extension point; Implementation-specific method events would fit cleanly into the
          system.

          Any objections?

          Show
          Peeter Piegaze added a comment - Actually, I guess it would make more sense to use getMethod to return the method name. In other words for Node.addNode the Event.getMethod would return "javax.jcr.Node.addNode" instead of the current "javax.jcr.observation.Event.ADD_NODE". Though we could still supply string constants for each JCR method, it would have a few advantages: 1) It's self-documenting. We don't have to define and explain a new String and how it maps to its method. 2) It let's us get rid of awkward method type names like SESSION_MOVE and COPY_EXTERNAL 3) The method Event no longer has a "name" per se, so the motivation to convert the names to JCR names disappears. 4) It allows a clear extension point; Implementation-specific method events would fit cleanly into the system. Any objections?
          Hide
          stefan_guggisberg added a comment -

          > 1) It's self-documenting. We don't have to define and explain a new String and how it maps to its
          > method.

          we would probably still have to list all the method events covered, otherwise it would IMO be useless
          for an application.

          > 2) It let's us get rid of awkward method type names like SESSION_MOVE and COPY_EXTERNAL

          how would you e.g. distinguish the 2 copy methods on Workspace?

          > 3) The method Event no longer has a "name" per se, so the motivation to convert the names to JCR
          > names disappears.
          > 4) It allows a clear extension point; Implementation-specific method events would fit cleanly into the
          > system.
          >
          > Any objections?

          yes maybe i am the only one having these concerns but the proposal still feels somehow awkward
          and unnatural. seems like we're trying to solve this issue with a hammer. and before you ask: no, i don't
          have a better proposal right now (apart from getting rid of method events altogether).

          Show
          stefan_guggisberg added a comment - > 1) It's self-documenting. We don't have to define and explain a new String and how it maps to its > method. we would probably still have to list all the method events covered, otherwise it would IMO be useless for an application. > 2) It let's us get rid of awkward method type names like SESSION_MOVE and COPY_EXTERNAL how would you e.g. distinguish the 2 copy methods on Workspace? > 3) The method Event no longer has a "name" per se, so the motivation to convert the names to JCR > names disappears. > 4) It allows a clear extension point; Implementation-specific method events would fit cleanly into the > system. > > Any objections? yes maybe i am the only one having these concerns but the proposal still feels somehow awkward and unnatural. seems like we're trying to solve this issue with a hammer. and before you ask: no, i don't have a better proposal right now (apart from getting rid of method events altogether).
          Hide
          Peeter Piegaze added a comment -

          >> 1) It's self-documenting. We don't have to define and explain a new String and how it maps to its
          >> method.
          >
          > we would probably still have to list all the method events covered, otherwise it would IMO be useless
          > for an application.

          Yes, the spec would still define which JCR methods must (or may) cause a method event.

          >> 2) It let's us get rid of awkward method type names like SESSION_MOVE and COPY_EXTERNAL
          >
          > how would you e.g. distinguish the 2 copy methods on Workspace?

          The Map returned by Event.getMethodInfo() lists the parameters, so the full signature is recoverable.

          The motivation is that since the Map already uses strings derived from Javadoc (the parameter names)
          to provide info about the method, it would be consistent to extend that to the actual name of the
          method, instead of introducing an arbitrary intermediate name for the method (either a string like
          "javax.jcr.observation.Event.ADD_NODE" or a JCR name like "jcr:addNode").

          Show
          Peeter Piegaze added a comment - >> 1) It's self-documenting. We don't have to define and explain a new String and how it maps to its >> method. > > we would probably still have to list all the method events covered, otherwise it would IMO be useless > for an application. Yes, the spec would still define which JCR methods must (or may) cause a method event. >> 2) It let's us get rid of awkward method type names like SESSION_MOVE and COPY_EXTERNAL > > how would you e.g. distinguish the 2 copy methods on Workspace? The Map returned by Event.getMethodInfo() lists the parameters, so the full signature is recoverable. The motivation is that since the Map already uses strings derived from Javadoc (the parameter names) to provide info about the method, it would be consistent to extend that to the actual name of the method, instead of introducing an arbitrary intermediate name for the method (either a string like "javax.jcr.observation.Event.ADD_NODE" or a JCR name like "jcr:addNode").
          Hide
          fguillaume added a comment -

          Returning the method name seems wrong to me. We don't want to express what method was precisely called,
          we want to express the semantics of what that call did.
          I haven't looked in detail, but there may be several methods with different names which have the exact same
          semantics from an event's point of view. For instance I want node.setProperty and property.setValue to be
          logged the same way, there's no need to distinguish them. Etc.

          Similarly, getMethodInfo returning the exact parameter names to the call seems wrong too. We want to take a
          step back and generate events with synthesized (although precise) information in them.

          So just turning method events into a kind of audit of all the API calls will not be a simplification for a
          programmer needing to treat these events. He might as well wrap the JCR session in a reflection-based logger
          that logs all that. What's needed for a programmer that uses these events is a precise list of the low-level
          semantic changes that can occur in the database (at the level of a method call). There will be some form of
          redundancy with a full listing of the API, but this will be much smaller and manageable and comprehensible
          by the programmer.

          Show
          fguillaume added a comment - Returning the method name seems wrong to me. We don't want to express what method was precisely called, we want to express the semantics of what that call did. I haven't looked in detail, but there may be several methods with different names which have the exact same semantics from an event's point of view. For instance I want node.setProperty and property.setValue to be logged the same way, there's no need to distinguish them. Etc. Similarly, getMethodInfo returning the exact parameter names to the call seems wrong too. We want to take a step back and generate events with synthesized (although precise) information in them. So just turning method events into a kind of audit of all the API calls will not be a simplification for a programmer needing to treat these events. He might as well wrap the JCR session in a reflection-based logger that logs all that. What's needed for a programmer that uses these events is a precise list of the low-level semantic changes that can occur in the database (at the level of a method call). There will be some form of redundancy with a full listing of the API, but this will be much smaller and manageable and comprehensible by the programmer.
          Hide
          Peeter Piegaze added a comment -

          Florent's point is right. I have transfered his comment over to 477, where we can debate exactly how to
          spec the method events.

          Show
          Peeter Piegaze added a comment - Florent's point is right. I have transfered his comment over to 477, where we can debate exactly how to spec the method events.

            People

            • Assignee:
              jsr-283-issues
              Reporter:
              mreutegg
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: