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 2008/02/27 07:34:51 UTC
[jira] Created: (JCR-1423) [PATCH] fix clone implementation
[PATCH] fix clone implementation
--------------------------------
Key: JCR-1423
URL: https://issues.apache.org/jira/browse/JCR-1423
Project: Jackrabbit
Issue Type: Improvement
Components: jackrabbit-core
Affects Versions: core 1.4.1
Reporter: Dave Brosius
Priority: Trivial
Fix For: core 1.4.2
Attachments: fix_clone_impl.patch
Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JCR-1423) [PATCH] fix clone implementation
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jukka Zitting updated JCR-1423:
-------------------------------
Fix Version/s: (was: 1.5.0)
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit
> Issue Type: Improvement
> Components: jackrabbit-core, jackrabbit-jcr2spi
> Reporter: Dave Brosius
> Priority: Trivial
> Attachments: fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JCR-1423) [PATCH] fix clone implementation
Posted by "Dave Brosius (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dave Brosius updated JCR-1423:
------------------------------
Attachment: fix_clone_impl.patch
i should say in NodeTypeDef 'dependencies' is NOT copied, and so i kept that behaviour. Not sure if that was intended...
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit
> Issue Type: Improvement
> Components: jackrabbit-core, jackrabbit-jcr2spi
> Reporter: Dave Brosius
> Priority: Trivial
> Fix For: 1.5
>
> Attachments: fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JCR-1423) [PATCH] fix clone implementation
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jukka Zitting updated JCR-1423:
-------------------------------
Status: Patch Available (was: Reopened)
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit Content Repository
> Issue Type: Improvement
> Components: jackrabbit-core, jackrabbit-jcr2spi
> Reporter: Dave Brosius
> Priority: Trivial
> Attachments: fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JCR-1423) [PATCH] fix clone implementation
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jukka Zitting updated JCR-1423:
-------------------------------
Affects Version/s: (was: core 1.4.1)
Fix Version/s: (was: core 1.4.2)
1.5
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit
> Issue Type: Improvement
> Components: jackrabbit-core, jackrabbit-jcr2spi
> Reporter: Dave Brosius
> Priority: Trivial
> Fix For: 1.5
>
> Attachments: fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1423) [PATCH] fix clone implementation
Posted by "Alexander Klimetschek (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12572951#action_12572951 ]
Alexander Klimetschek commented on JCR-1423:
--------------------------------------------
clone() will copy the object at hand with the correct type - the caller doesn't have to know the exact subclass of the object. With a copy constructor, the client has to know the exact type.
Both can be combined; a typical implementation calls the (correct) copy constructor from the clone() method.
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit
> Issue Type: Improvement
> Components: jackrabbit-core, jackrabbit-jcr2spi
> Reporter: Dave Brosius
> Priority: Trivial
> Fix For: 1.5
>
> Attachments: fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (JCR-1423) [PATCH] fix clone implementation
Posted by "Stefan Guggisberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Stefan Guggisberg resolved JCR-1423.
------------------------------------
Resolution: Fixed
applied patch with 2 minor modifications:
- changed formatting (avoid tabs)
- patched
jackrabbit-core/..../nodetype/EffectiveNodeType.java
rather than
jackrabbit-jcr2spi/..../nodetype/EffectiveNodeTypeImpl.java
fixed in svn r631547
thanks!
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit
> Issue Type: Improvement
> Components: jackrabbit-core
> Affects Versions: core 1.4.1
> Reporter: Dave Brosius
> Priority: Trivial
> Fix For: core 1.4.2
>
> Attachments: fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JCR-1423) [PATCH] fix clone implementation
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jukka Zitting updated JCR-1423:
-------------------------------
Status: Resolved (was: Patch Available)
Resolution: Won't Fix
Resolving as Won't Fix since I don't see much benefit in this change and since the patch no longer applies cleanly. Apologies for not reviewing this in a more timely manner.
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit Content Repository
> Issue Type: Improvement
> Components: jackrabbit-core, jackrabbit-jcr2spi
> Reporter: Dave Brosius
> Priority: Trivial
> Attachments: fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Reopened: (JCR-1423) [PATCH] fix clone implementation
Posted by "angela (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
angela reopened JCR-1423:
-------------------------
stefan, can change jcr2spi/..../EffectiveNodeTypeImpl as well (as suggested by the patch)? thanks in advance
angela
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit
> Issue Type: Improvement
> Components: jackrabbit-core, jackrabbit-jcr2spi
> Reporter: Dave Brosius
> Priority: Trivial
> Fix For: 1.5
>
> Attachments: fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1423) [PATCH] fix clone implementation
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12572942#action_12572942 ]
Jukka Zitting commented on JCR-1423:
------------------------------------
This change breaks test cases.
I suspect the reason to be that "new ThisClass()" is not the same as "(ThisClass) super.clone()". The latter copies all field values as-is, so for example all reference variables in the cloned object point back to components of the original object instead of being freshly initialized.
Why do we even need clone()? IMHO a copying constructor is much better solution for cloning objects.
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit
> Issue Type: Improvement
> Components: jackrabbit-core, jackrabbit-jcr2spi
> Reporter: Dave Brosius
> Priority: Trivial
> Fix For: 1.5
>
> Attachments: fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JCR-1423) [PATCH] fix clone implementation
Posted by "angela (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
angela updated JCR-1423:
------------------------
Component/s: jackrabbit-jcr2spi
adjust affected components
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit
> Issue Type: Improvement
> Components: jackrabbit-core, jackrabbit-jcr2spi
> Reporter: Dave Brosius
> Priority: Trivial
> Fix For: 1.5
>
> Attachments: fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1423) [PATCH] fix clone implementation
Posted by "Dave Brosius (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573030#action_12573030 ]
Dave Brosius commented on JCR-1423:
-----------------------------------
That was really stupid on my part, my apologies. I will fix it and resubmit.
As an aside, i normally get all kinds of junit test failures, so i guess i didn't notice this. Am i doing something wrong that these failures are happening?
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit
> Issue Type: Improvement
> Components: jackrabbit-core, jackrabbit-jcr2spi
> Reporter: Dave Brosius
> Priority: Trivial
> Fix For: 1.5
>
> Attachments: fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JCR-1423) [PATCH] fix clone implementation
Posted by "Dave Brosius (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dave Brosius updated JCR-1423:
------------------------------
Attachment: (was: fix_clone_impl.patch)
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit
> Issue Type: Improvement
> Components: jackrabbit-core, jackrabbit-jcr2spi
> Reporter: Dave Brosius
> Priority: Trivial
> Fix For: 1.5
>
> Attachments: fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JCR-1423) [PATCH] fix clone implementation
Posted by "Dave Brosius (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dave Brosius updated JCR-1423:
------------------------------
Attachment: fix_clone_impl.patch
Fix the boneheaded previous implementation of clone.
One point of note is that NodeTypeDef.clone does a shallow copy of member 'dependencies', and so i left it that way. Was that intended?
Also note that the two clone's in core were incorrect as well.
Again my apologies
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit
> Issue Type: Improvement
> Components: jackrabbit-core, jackrabbit-jcr2spi
> Reporter: Dave Brosius
> Priority: Trivial
> Fix For: 1.5
>
> Attachments: fix_clone_impl.patch, fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JCR-1423) [PATCH] fix clone implementation
Posted by "Dave Brosius (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dave Brosius updated JCR-1423:
------------------------------
Attachment: fix_clone_impl.patch
> [PATCH] fix clone implementation
> --------------------------------
>
> Key: JCR-1423
> URL: https://issues.apache.org/jira/browse/JCR-1423
> Project: Jackrabbit
> Issue Type: Improvement
> Components: jackrabbit-core
> Affects Versions: core 1.4.1
> Reporter: Dave Brosius
> Priority: Trivial
> Fix For: core 1.4.2
>
> Attachments: fix_clone_impl.patch
>
>
> Several classes implement clone, by doing new XXXX() to create the object. As these classes aren't final, this will fail if these classes are ever derived from (which they are not, right now). The correct implementation is to call super.clone(). This patch fixes this.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.