You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by "Dave Brosius (JIRA)" <ji...@apache.org> on 2009/02/05 05:59:59 UTC

[jira] Created: (JCR-1967) Impossible comparison in NodeTypeImpl

Impossible comparison in NodeTypeImpl
-------------------------------------

                 Key: JCR-1967
                 URL: https://issues.apache.org/jira/browse/JCR-1967
             Project: Jackrabbit Content Repository
          Issue Type: Bug
          Components: jackrabbit-jcr2spi
    Affects Versions: 1.6.0
            Reporter: Dave Brosius
            Priority: Minor
             Fix For: 1.6.0


org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does

    public boolean isNodeType(Name nodeTypeName) {
        return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
    }


as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant

    public boolean isNodeType(Name nodeTypeName) {
        return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
    }



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


[jira] Updated: (JCR-1967) Impossible comparison in NodeTypeImpl

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

Michael Dürig updated JCR-1967:
-------------------------------

    Affects Version/s: 1.5.0

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Improvement
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.5.0, 1.6.0
>            Reporter: Dave Brosius
>            Assignee: Michael Dürig
>            Priority: Trivial
>             Fix For: 1.6.0
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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


[jira] Updated: (JCR-1967) Impossible comparison in NodeTypeImpl

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

angela updated JCR-1967:
------------------------

    Priority: Trivial  (was: Minor)

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Bug
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.6.0
>            Reporter: Dave Brosius
>            Assignee: angela
>            Priority: Trivial
>             Fix For: 1.6.0
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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


[jira] Resolved: (JCR-1967) Impossible comparison in NodeTypeImpl

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

angela resolved JCR-1967.
-------------------------

    Resolution: Fixed
      Assignee: angela

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Bug
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.6.0
>            Reporter: Dave Brosius
>            Assignee: angela
>            Priority: Minor
>             Fix For: 1.6.0
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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


[jira] Commented: (JCR-1967) Impossible comparison in NodeTypeImpl

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

Marcel Reutegger commented on JCR-1967:
---------------------------------------

I think it should rather be:

Index: src/main/java/org/apache/jackrabbit/jcr2spi/nodetype/NodeTypeImpl.java
===================================================================
--- src/main/java/org/apache/jackrabbit/jcr2spi/nodetype/NodeTypeImpl.java	(revision 741110)
+++ src/main/java/org/apache/jackrabbit/jcr2spi/nodetype/NodeTypeImpl.java	(working copy)
@@ -132,7 +132,7 @@
      * from the specified node type, otherwise false.
      */
     public boolean isNodeType(Name nodeTypeName) {
-        return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
+        return nodeTypeName.equals(ntd.getName()) ||  ent.includesNodeType(nodeTypeName);
     }
 
     /**

note, that I reversed the equals because ntd.getName() may return null.

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Bug
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.6.0
>            Reporter: Dave Brosius
>            Priority: Minor
>             Fix For: 1.6.0
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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


[jira] Commented: (JCR-1967) Impossible comparison in NodeTypeImpl

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

angela commented on JCR-1967:
-----------------------------

both in core and in jcr2spi EffectiveNodeType#includesNodeType() just does a lookup in a set. i don't see any object creation there.

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Bug
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.6.0
>            Reporter: Dave Brosius
>            Assignee: angela
>            Priority: Trivial
>             Fix For: 1.6.0
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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


[jira] Updated: (JCR-1967) Impossible comparison in NodeTypeImpl

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

Jukka Zitting updated JCR-1967:
-------------------------------

    Affects Version/s:     (was: 1.6.0)
        Fix Version/s:     (was: 1.6.0)

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Improvement
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.5.0
>            Reporter: Dave Brosius
>            Assignee: Michael Dürig
>            Priority: Trivial
>             Fix For: 1.5.5
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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


[jira] Commented: (JCR-1967) Impossible comparison in NodeTypeImpl

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

Stefan Guggisberg commented on JCR-1967:
----------------------------------------

FWIW: 

i guess the code in question is based on the node type implementation in core.

i remember that i added the redundant call to name.equalsI() as an optimization.
some extensive profiling sessions (a long time ago) showed that 
EffectiveNodeType#includesNodeType() is somewhat expensive as it creates a 
number of objetcs, thus keeping the gc busy.  

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Bug
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.6.0
>            Reporter: Dave Brosius
>            Assignee: angela
>            Priority: Trivial
>             Fix For: 1.6.0
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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


[jira] Reopened: (JCR-1967) Impossible comparison in NodeTypeImpl

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

Michael Dürig reopened JCR-1967:
--------------------------------

      Assignee: Michael Dürig  (was: angela)

Reopening for 1.5.0

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Improvement
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.5.0, 1.6.0
>            Reporter: Dave Brosius
>            Assignee: Michael Dürig
>            Priority: Trivial
>             Fix For: 1.6.0
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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


[jira] Commented: (JCR-1967) Impossible comparison in NodeTypeImpl

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

Marcel Reutegger commented on JCR-1967:
---------------------------------------

I think you are right. The first check doesn't buy us too much, I guess. And it simplifies the code.

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Bug
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.6.0
>            Reporter: Dave Brosius
>            Priority: Minor
>             Fix For: 1.6.0
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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


[jira] Updated: (JCR-1967) Impossible comparison in NodeTypeImpl

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

Jukka Zitting updated JCR-1967:
-------------------------------

    Issue Type: Improvement  (was: Bug)

Classifying this as an improvement, since even though the code wasn't correct, AFAUI it didn't cause anything to break.

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Improvement
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.6.0
>            Reporter: Dave Brosius
>            Assignee: angela
>            Priority: Trivial
>             Fix For: 1.6.0
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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


[jira] Commented: (JCR-1967) Impossible comparison in NodeTypeImpl

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

angela commented on JCR-1967:
-----------------------------

the problem was never appearing because of the ||... therefore i would say that we should get rid of the initial equality comparison altogether:

     public boolean isNodeType(Name nodeTypeName) {
- return getName().equals(nodeTypeName) || ent.includesNodeType(nodeTypeName);
+ return ent.includesNodeType(nodeTypeName);
     } 

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Bug
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.6.0
>            Reporter: Dave Brosius
>            Priority: Minor
>             Fix For: 1.6.0
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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


[jira] Commented: (JCR-1967) Impossible comparison in NodeTypeImpl

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

Stefan Guggisberg commented on JCR-1967:
----------------------------------------

> both in core and in jcr2spi EffectiveNodeType#includesNodeType() just does a lookup in a set. i don't see any object creation there.

you're right. so the optimization is obsolete anyway.

i believe it used to check EffectiveNodeTypeCache at some point,  creating WeightedKey objects. 

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Bug
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.6.0
>            Reporter: Dave Brosius
>            Assignee: angela
>            Priority: Trivial
>             Fix For: 1.6.0
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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


[jira] Commented: (JCR-1967) Impossible comparison in NodeTypeImpl

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

angela commented on JCR-1967:
-----------------------------

> Classifying this as an improvement

agreed... i was already tempted to make that change :)

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Improvement
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.6.0
>            Reporter: Dave Brosius
>            Assignee: angela
>            Priority: Trivial
>             Fix For: 1.6.0
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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


[jira] Resolved: (JCR-1967) Impossible comparison in NodeTypeImpl

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

Michael Dürig resolved JCR-1967.
--------------------------------

       Resolution: Fixed
    Fix Version/s: 1.5.5

Fixed at revision 766747  

> Impossible comparison in NodeTypeImpl
> -------------------------------------
>
>                 Key: JCR-1967
>                 URL: https://issues.apache.org/jira/browse/JCR-1967
>             Project: Jackrabbit Content Repository
>          Issue Type: Improvement
>          Components: jackrabbit-jcr2spi
>    Affects Versions: 1.5.0, 1.6.0
>            Reporter: Dave Brosius
>            Assignee: Michael Dürig
>            Priority: Trivial
>             Fix For: 1.5.5, 1.6.0
>
>
> org.apache.jackrabbit.jcr2spi.nodetype.NodeTypeImpl does
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName) ||  ent.includesNodeType(nodeTypeName);
>     }
> as getName() is a string and nodeTypeName is a Name this will always be false. Perhaps you meant
>     public boolean isNodeType(Name nodeTypeName) {
>         return getName().equals(nodeTypeName.getLocalName()) ||  ent.includesNodeType(nodeTypeName);
>     }

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