Small follow up Will:

Your fix, however I agree with it, seem to break some unit tests. I've
only given it a short look, but it seems some unit tests depend on
this 'bug' : which might imply that there are also customers relying
on this 'bug'.

Either way, for a new 2.x released version, it is imo acceptable to do
a backwards incompatible bug fix (meaning I'll fix the faulty unit
tests). Either way, when jira is back up, I'll create a jira issue for
it : We can discuss it further in that issue if needed

Regards Ard

On Thu, May 24, 2012 at 11:50 AM, Ard Schrijvers
<[email protected]> wrote:
> Hello Will,
>
> Thanks a lot! I'll include your fix (and commit it once apache jira is
> up again but it seems to be down the entire morning already). Afaics,
> a similar improvement should be done in that class for
> #updateManagableCollection :
>
> When the collection node gets removed and recreated, it should be
> recreated with the same type again. Thus instead of
>
> if (!elementClassDescriptor.hasIdField() &&
> !elementClassDescriptor.hasUUIdField()) {
>            collectionNode.remove();
>            collectionNode = parentNode.addNode(jcrName);
>        }
>
> it should be
>
> if (!elementClassDescriptor.hasIdField() &&
> !elementClassDescriptor.hasUUIdField()) {
>            String nodeTypeName = 
> collectionNode.getPrimaryNodeType().getName();
>            collectionNode.remove();
>            collectionNode = parentNode.addNode(jcrName, nodeTypeName);
> }
>
> Either way, thanks for your heads up and I'll include your fix
>
> Regards Ard
>
> On Thu, May 24, 2012 at 11:34 AM, Will Scheidegger
> <[email protected]> wrote:
>> Hi Ard
>>
>> This is only very vaguely related to what you are doing,… but since you seem 
>> to have commit rights on OCM and you will be working on it:
>>
>> A long time ago I discussed a bug in the DefaultCollectionConverterImpl 
>> class [1]. It seems like this bug still exists today [2]. I'm not sure, but 
>> I think I provided a patch back then. In any case, the patch would be:
>>
>> --- Base (BASE)
>> +++ Locally Modified (Based On LOCAL)
>> @@ -111,7 +111,7 @@
>>                     + collectionDescriptor.getFieldName() + " for the 
>> classdescriptor : " + 
>> collectionDescriptor.getClassDescriptor().getClassName());
>>         }
>>
>> -        Node collectionNode = parentNode.addNode(jcrName);
>> +        Node collectionNode = parentNode.addNode(jcrName, 
>> collectionDescriptor.getJcrType());
>>
>>         ClassDescriptor elementClassDescriptor = 
>> mapper.getClassDescriptorByClass( 
>> ReflectionUtils.forName(collectionDescriptor.getElementClassName()));
>>
>>
>>
>> Best regards,
>> will
>>
>>
>> [1] 
>> http://www.apacheserver.net/OCM-1-6-CollectionDescriptor-getJcrType-at196070.htm
>> [2] 
>> https://svn.apache.org/repos/asf/jackrabbit/commons/ocm/trunk/src/main/java/org/apache/jackrabbit/ocm/manager/collectionconverter/impl/DefaultCollectionConverterImpl.java
>>
>>
>>
>> On 23.05.2012, at 17:34, Ard Schrijvers wrote:
>>
>>> Hello,
>>>
>>> I will be working on Jackrabbit ocm [1] the coming weeks. Currently,
>>> there are no branches nor tags since it has been moved to jackrabbit
>>> commons. I also noticed that it does compile against jackrabbit core
>>> 2.4.1, however it has some test failures. All tests run fine until jr
>>> core 2.2.9
>>>
>>> I'll fix the code / unit tests to align it with 2.4.1, but will also
>>> pick up other work/tasks (like trying to avoid slow queries) : We are
>>> planning to integrate it into rave [1] as an optional persistence
>>> layer to store for example page definitions
>>>
>>> If there are developers would like me to first branch the current
>>> trunk against jackrabbit core 2.2.9, please let me know. Any other
>>> feedback is also more than welcome.
>>>
>>> Regards Ard
>>>
>>> [1] https://svn.apache.org/repos/asf/jackrabbit/commons/ocm/trunk/
>>> [2] http://rave.apache.org/
>>>
>>> --
>>> Amsterdam - Oosteinde 11, 1017 WT Amsterdam
>>> Boston - 1 Broadway, Cambridge, MA 02142
>>>
>>> US +1 877 414 4776 (toll free)
>>> Europe +31(0)20 522 4466
>>> www.onehippo.com
>>
>
>
>
> --
> Amsterdam - Oosteinde 11, 1017 WT Amsterdam
> Boston - 1 Broadway, Cambridge, MA 02142
>
> US +1 877 414 4776 (toll free)
> Europe +31(0)20 522 4466
> www.onehippo.com



-- 
Amsterdam - Oosteinde 11, 1017 WT Amsterdam
Boston - 1 Broadway, Cambridge, MA 02142

US +1 877 414 4776 (toll free)
Europe +31(0)20 522 4466
www.onehippo.com

Reply via email to