You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by "Julian Reschke (JIRA)" <ji...@apache.org> on 2007/03/13 18:21:09 UTC

[jira] Created: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

PathElement.equals doesn't handle INDEX_UNDEFINED
-------------------------------------------------

                 Key: JCR-789
                 URL: https://issues.apache.org/jira/browse/JCR-789
             Project: Jackrabbit
          Issue Type: Bug
          Components: core
            Reporter: Julian Reschke
            Priority: Minor


PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Marcel Reutegger (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12485448 ] 

Marcel Reutegger commented on JCR-789:
--------------------------------------

Julian Reschke (JIRA) wrote:
> Unless it's used somewhere where relative paths are matched (such as in
> Node.getnNodes(relpath)).

relative paths have the same restrictions and rules like absolute paths. The method you mention does not take a relative path argument but a name pattern and those never have an index.

Tobias wrote:
> - use a 0-based index (when storing as int) throughout the code (also in NodeState)

What's the benefit of using a 0-based index when the spec says they are 1-based? Wouldn't that make the implementation more complicated to understand?

IMO the INDEX_UNDEFINED only makes sense in context of an XPath query. If a path element does not have an index in XPath then all same named nodes are selected regardless of what index they have. Whereas in the JCR API Node.getNode("foo") and Node.getNode("foo[1]") is the same.

I think the crucial question is: does the spec says anywhere an 1-index needs to preserved in a path?

> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>            Priority: Minor
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Stefan Guggisberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12485453 ] 

Stefan Guggisberg commented on JCR-789:
---------------------------------------

Marcel wrote:
> IMO the INDEX_UNDEFINED only makes sense in context of an XPath query. If a path element does not have an index in XPath then all same named nodes are selected regardless of what index they have. Whereas in the JCR API Node.getNode("foo") and Node.getNode("foo[1]") is the same.
> 
> I think the crucial question is: does the spec says anywhere an 1-index needs to preserved in a path?

you're right, it doesn't. 

the original idea behind INDEX_UNDEFINED was to distinguish an implicit "[1]" subscript from an explicit "[1]".
i.e.  to preserve e.g. the following string representation: "/foo[1]/bar". it's not required by the spec and whether 
that's useful feature or not is another question. 

personally i am not particularly attached to INDEX_UNDEFINED, i wouldn't be opposed to removing it.

> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>            Priority: Minor
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12485340 ] 

Jukka Zitting commented on JCR-789:
-----------------------------------

Sounds like a correct change to me.

BTW, is there even a need for the INDEX_UNDEFINED case? In other words, could we always normalize path components to internally always have an index that is >= 1? If the index is 1, then the "[n]" part would be skipped during string serialization.

Another note about this issue, does the comparison failure cause a regression against documented behaviour? If not, then I'd categorize this as an improvement rather than a bug.

> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>            Priority: Minor
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Julian Reschke (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Julian Reschke reassigned JCR-789:
----------------------------------

    Assignee: Julian Reschke

> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>         Assigned To: Julian Reschke
>            Priority: Minor
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Stefan Guggisberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12485440 ] 

Stefan Guggisberg commented on JCR-789:
---------------------------------------

> - use a 0-based index (when storing as int) throughout the code (also in NodeState)

-1,  the scope of the required code change is IMO too big (with the risk of causing new issues)
whereas the benefit is IMO relatively small.

> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>            Priority: Minor
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Julian Reschke (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12489175 ] 

Julian Reschke commented on JCR-789:
------------------------------------

Revision 529332.



> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>         Assigned To: Julian Reschke
>            Priority: Minor
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Julian Reschke (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12485342 ] 

Julian Reschke commented on JCR-789:
------------------------------------

> BTW, is there even a need for the INDEX_UNDEFINED case? In other words, could we always normalize path components to internally always have an index that is >= 1? If the index is 1, then the "[n]" part would be skipped during string serialization. 

Unless it's used somewhere where relative paths are matched (such as in Node.getnNodes(relpath)).

> Another note about this issue, does the comparison failure cause a regression against documented behaviour? If not, then I'd categorize this as an improvement rather than a bug.

I think it *did* cause a failure for me in JCR2SPI, until I adjusted my SPI implementation so that Path objects were built exactly the way expected by JCR2SPI (because that one is using Path.equals to compare paths).

To summarize: we either should change .equals(), or, if we can't, provide an additional, weaker comparison method.

> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>            Priority: Minor
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "angela (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12485209 ] 

angela commented on JCR-789:
----------------------------

i just happen to read that jsr170 makes an explicit statement regarding this:

"However, as opposed to the semantics of XPath, a name in a content repository path that does not explicitly specify an index implies an index of 1. For example, /a/b/c is equivalent to /a[1]/b[1]/c[1]."



> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>            Priority: Minor
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Julian Reschke (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Julian Reschke updated JCR-789:
-------------------------------

    Attachment: jcr789.diff.txt

Proposed patch.

> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>            Priority: Minor
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Julian Reschke (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12489173 ] 

Julian Reschke commented on JCR-789:
------------------------------------

OK, I just realized that I've been using the updated version of Path.java in my SPI implementation without noticing. Reverting back to the trunk version triggers the problem again.

Thus, I'll commit my minor change (treating 0 and 1 in equals and hashCode as the same). We can then still discuss whether a bigger change makes more sense.


> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>            Priority: Minor
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Julian Reschke (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12511107 ] 

Julian Reschke commented on JCR-789:
------------------------------------

...as we are planning to refactor the (Q)Name/Path implementations anyway, should this issue be closed now?


> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>            Assignee: Julian Reschke
>            Priority: Minor
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jukka Zitting resolved JCR-789.
-------------------------------

       Resolution: Fixed
    Fix Version/s: 1.3

Agreed. I'm resolving this as fixed already for 1.3. Let's use separate issues for any followup.

> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>            Assignee: Julian Reschke
>            Priority: Minor
>             Fix For: 1.3
>
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Tobias Bocanegra (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12485435 ] 

Tobias Bocanegra commented on JCR-789:
--------------------------------------

i'm in favor of normalizing the index and also make it 0-based throughout the code. there are several places where confusion exists about if the index is 0- or 1-based, with statements like this: if (index == 0) { index = 1}

suggest to:
- use a 0-based index (when storing as int) throughout the code (also in NodeState)
- drop INDEX_UNDEFINED in PathElement
- during serialization, index == 0 will not be translated into ...[1].



> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>            Priority: Minor
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Tobias Bocanegra (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12485449 ] 

Tobias Bocanegra commented on JCR-789:
--------------------------------------

> What's the benefit of using a 0-based index when the spec says they are 1-based? 
> Wouldn't that make the implementation more complicated to understand? 
first, computer science is 0-based :-)
i remember a lot of bugs that were caused because the code was not very clear about if the index is 0 or 1 based and what happens if the index is 0 (eg. the node state throws an exception if access with a index < 0).

however, "/a".equals("/a[1]") should return true (and compute the same hashcode).


> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>            Priority: Minor
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Closed: (JCR-789) PathElement.equals doesn't handle INDEX_UNDEFINED

Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/JCR-789?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jukka Zitting closed JCR-789.
-----------------------------


> PathElement.equals doesn't handle INDEX_UNDEFINED
> -------------------------------------------------
>
>                 Key: JCR-789
>                 URL: https://issues.apache.org/jira/browse/JCR-789
>             Project: Jackrabbit
>          Issue Type: Bug
>          Components: core
>            Reporter: Julian Reschke
>            Assignee: Julian Reschke
>            Priority: Minor
>             Fix For: 1.3
>
>         Attachments: jcr789.diff.txt
>
>
> PathElement (and therefore Path) comparisons fail when INDEX_UNDEFINED is used (it's treated differently from INDEX_DEFAULT).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.