jsr-283
  1. jsr-283
  2. JSR_283-772

QueryObjectModelConstants should not be JCR names

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: current
    • Fix Version/s: milestone 1
    • Component/s: sql
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      772

      Description

      Currently the QueryObjectModelConstants are defined as strings that are expanded-form JCR names.
      For example:

      public static final String JCR_OPERATOR_LESS_THAN = "

      {http://www.jcp.org/jcr/1.0}

      operatorLessThan"

      The constants are clearly meant to be used for comparison with the strings returned from methods
      such as

      PropertyDefinition.getAvailableQueryOperators()

      Such methods, therefore, must return JCR names in expanded form. However, this violates the rule
      stated in 3.2.6 Use of Qualified and Expanded Name, which states:

      When a JCR name is passed as an argument to a JCR method it may be in either expanded or qualified
      form. When a repository returns a JCR name it must be in qualified form. The qualified form of a name
      depends upon the prevailing local namespace mapping of the current session (see §3.5 Namespace
      Mapping).

      So, the possible solutions are:

      1) Allow methods to return expanded form names

      2) Require methods like that mentioned above to return qualified form and provide something like
      Session.compareNames (or comparePaths) that can properly judge the equality of JCR names, and
      indicate that the returned values be compared to the constants using this method.

      3) Make the QueryObjectModelConstants plain strings.

      The same issue arises with the constants defined in Privileges.java.

      As I recall the motivation for namespacing the privileges was that one might introduce ones own
      privileges, and this seems like a good argument.

      I suppose this argument might also apply to the query operators but not to the join types and orderings
      (unless we want to introduce JCR_ORDER_SIDEWAYS

        Activity

        Hide
        mreutegg added a comment -

        > I propose that we change the QueryObjectModelConstants [...] to plain strings
        > using the java namespacing convention (javax.jcr etc.)

        +1 for changing the constants in QueryObjectModelConstants

        that still allows for proprietary extensions within an implementation owned
        package space.

        Don't know about Privileges and what implications that would have.

        Show
        mreutegg added a comment - > I propose that we change the QueryObjectModelConstants [...] to plain strings > using the java namespacing convention (javax.jcr etc.) +1 for changing the constants in QueryObjectModelConstants that still allows for proprietary extensions within an implementation owned package space. Don't know about Privileges and what implications that would have.
        Hide
        anchela added a comment -

        > I think for the privileges we do want extensibility, and potentially
        > compatibility with RFC3744 (WebDAV) privilege names.

        that's why we had huge discussions in the past and decided to change them to JCR name
        instead of java namespacing convention...

        the main argument for not having the java convention was:

        • if a Privilege has a name it MUST be a JCR name
        • it doesn't make sense to define different type of names within the scope of JCR.

        > Also, we should be VERY careful with changes at this point; I'm not sure they
        > are getting as much review as they should.

        definitely.
        the QueryObjectModelConstants issue was only detected because we are having to JCR
        impls in jackrabbit and i interpreted the constant in the expanded-name format
        different that stefan did...

        regarding changing the QueryObjectModelConstants only:

        • it feels somehow awkward to me to have different 'name' constant types in the API and Javadoc
          and introducing the java-packaging stuff in a single place only.
        • i get the impression that we are back defining different type of names.
        • if i was an API user i would expect it very short... or as short as possible.
          and i would find the packaging name alsmost as cumbersome as some expanded name.
        • finally, if extensibility of the constants is a criteria, then i don't really see, what was
          the problem with the constants being of type NAME instead of STRING.

        having said that, my preferred options would be:

        • either make them simple strings without any time of prefix/namespace (see also the action
          constants on Session)
        • or define them as expanded JCR names and avoid inventing something new at this state of the spec.
        Show
        anchela added a comment - > I think for the privileges we do want extensibility, and potentially > compatibility with RFC3744 (WebDAV) privilege names. that's why we had huge discussions in the past and decided to change them to JCR name instead of java namespacing convention... the main argument for not having the java convention was: if a Privilege has a name it MUST be a JCR name it doesn't make sense to define different type of names within the scope of JCR. > Also, we should be VERY careful with changes at this point; I'm not sure they > are getting as much review as they should. definitely. the QueryObjectModelConstants issue was only detected because we are having to JCR impls in jackrabbit and i interpreted the constant in the expanded-name format different that stefan did... regarding changing the QueryObjectModelConstants only: it feels somehow awkward to me to have different 'name' constant types in the API and Javadoc and introducing the java-packaging stuff in a single place only. i get the impression that we are back defining different type of names. if i was an API user i would expect it very short... or as short as possible. and i would find the packaging name alsmost as cumbersome as some expanded name. finally, if extensibility of the constants is a criteria, then i don't really see, what was the problem with the constants being of type NAME instead of STRING. having said that, my preferred options would be: either make them simple strings without any time of prefix/namespace (see also the action constants on Session) or define them as expanded JCR names and avoid inventing something new at this state of the spec.
        Hide
        Peeter Piegaze added a comment -

        Ok, what about this:

        1) Privileges are left as is. We simply clarify that the method Privilege.getName() must return the privilege
        name in qualified form and that AccessControlManager.privilegeFromName(String) takes a privilege name
        in either qualified or expanded form. This aligns with the already established rule that the API accepts
        either form but returns only qualified form.

        2) QueryObjectModelConstants are changed to simple strings (not Java namesapced, just strings, let's say
        we make them identical to their constant identifers). This removes the problem of violating the only-
        return-qualified-form rule.

        Show
        Peeter Piegaze added a comment - Ok, what about this: 1) Privileges are left as is. We simply clarify that the method Privilege.getName() must return the privilege name in qualified form and that AccessControlManager.privilegeFromName(String) takes a privilege name in either qualified or expanded form. This aligns with the already established rule that the API accepts either form but returns only qualified form. 2) QueryObjectModelConstants are changed to simple strings (not Java namesapced, just strings, let's say we make them identical to their constant identifers). This removes the problem of violating the only- return-qualified-form rule.
        Hide
        reschke added a comment -

        works for me.

        Show
        reschke added a comment - works for me.
        Hide
        Peeter Piegaze added a comment -

        Fixed:

        1) Privileges are left as is. We simply clarify that the method Privilege.getName() must return the
        privilege
        name in qualified form and that AccessControlManager.privilegeFromName(String) takes a privilege
        name
        in either qualified or expanded form. This aligns with the already established rule that the API accepts
        either form but returns only qualified form.

        2) QueryObjectModelConstants are changed to simple strings (using same convention as repo
        descriptors: identifier is lowercased and underscores are changed to periods). This removes the
        problem of violating the only-
        return-qualified-form rule.

        Show
        Peeter Piegaze added a comment - Fixed: 1) Privileges are left as is. We simply clarify that the method Privilege.getName() must return the privilege name in qualified form and that AccessControlManager.privilegeFromName(String) takes a privilege name in either qualified or expanded form. This aligns with the already established rule that the API accepts either form but returns only qualified form. 2) QueryObjectModelConstants are changed to simple strings (using same convention as repo descriptors: identifier is lowercased and underscores are changed to periods). This removes the problem of violating the only- return-qualified-form rule.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: