[ 
https://issues.apache.org/jira/browse/JCR-3579?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13640389#comment-13640389
 ] 

Sergiy Shyrkov commented on JCR-3579:
-------------------------------------

I've extended org.apache.jackrabbit.core.version.RemoveVersionTest with a new 
test for version removal and checking if jcr:successors / jcr:predecessors are 
properly updated: 
https://issues.apache.org/jira/secure/attachment/12580276/RemoveVersionTest.patch

The test case uses two workspaces (default and test) and creates a couple of 
version, merges two branches and removes two versions: 
https://issues.apache.org/jira/secure/attachment/12580275/test-case-version-graph.jpg

After all it checks the jcr:successors / jcr:predecessors and the test fails as 
in this case the 1.0 version 1.1.0 as a successor twice and 1.1.0 has 1.0 twice 
as predecessor.

The attached patch 
https://issues.apache.org/jira/secure/attachment/12580277/InternalVersionImpl.patch
 solves the issue by using LinkedHashSet instead of a list to prevent adding 
same values.
Additionally (cosmetics) it avoids creating temporary list (Arrays.asList()) 
and also in the storeXCessors() it uses non-recursive node.store(false); as 
there is no need to update recursively after updating  jcr:successors / 
jcr:predecessors properties.

Thank you in advance for verifying the patches!
                
> InternalVersionImpl.internalDetachPredecessor() and internalDetachSuccessor() 
> do not check for duplicates when updating jcr:successors / jcr:predecessors
> ---------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: JCR-3579
>                 URL: https://issues.apache.org/jira/browse/JCR-3579
>             Project: Jackrabbit Content Repository
>          Issue Type: Bug
>          Components: versioning
>    Affects Versions: 2.2.13, 2.4.3, 2.6
>            Reporter: Sergiy Shyrkov
>            Priority: Minor
>         Attachments: InternalVersionImpl.patch, RemoveVersionTest.patch, 
> screenshot-1.jpg, test-case-version-graph.jpg
>
>
> We are using Jackrabbit 2.2.x branch (although the issue seems the same with 
> 2.4.x and 2.6 so far I see).
> We have a maintenance job that runs periodically and purges the old unused 
> versions of nodes to keep the versioning store and index in boundaries.
> The Job is using Jackrabbit Versioning API to remove versions.
> The 
> org.apache.jackrabbit.core.version.InternalVersionHistoryImpl.removeVersion(Name)
>  has the call to detaching the removed version from the version graph:
>         // detach from the version graph
>         v.internalDetach();
> which in turn calls InternalVersionImpl.internalDetachPredecessor() and 
> internalDetachSuccessor() methods to update predecessors and successor.
> Those two methods are using List to calculate new values and store them in 
> corresponding jcr:successors or jcr:predecessors properties.
> The only thing is there is no check for duplicates in those lists, so with 
> the time we see that jcr:successors and jcr:predecessors have duplicates (and 
> they are growing with each version removal :-)). See attached screenshot.
> From my feeling the internalDetachPredecessor() and internalDetachSuccessor() 
> should rather use HashSet (or if the order is important a LinkedHashSet) 
> instead of List to prevent storing duplicates.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to