You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by GitBox <gi...@apache.org> on 2022/09/05 12:48:34 UTC

[GitHub] [jackrabbit-oak] ArunOnCloud opened a new pull request, #688: Removal (purge) of version of a node does not remove associated labels

ArunOnCloud opened a new pull request, #688:
URL: https://github.com/apache/jackrabbit-oak/pull/688

   pull request for issue : OAK-9891
   mail thread link: https://lists.apache.org/thread/b1g2f3cxxfpfnqxzr3oz20lyp8676tnm


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [jackrabbit-oak] ArunOnCloud commented on pull request #688: Removal (purge) of version of a node does not remove associated labels

Posted by GitBox <gi...@apache.org>.
ArunOnCloud commented on PR #688:
URL: https://github.com/apache/jackrabbit-oak/pull/688#issuecomment-1240237803

   @mreutegg can you please review it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [jackrabbit-oak] mreutegg merged pull request #688: OAK-9891: Removal (purge) of version of a node does not remove associated labels

Posted by GitBox <gi...@apache.org>.
mreutegg merged PR #688:
URL: https://github.com/apache/jackrabbit-oak/pull/688


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #688: Removal (purge) of version of a node does not remove associated labels

Posted by GitBox <gi...@apache.org>.
mreutegg commented on code in PR #688:
URL: https://github.com/apache/jackrabbit-oak/pull/688#discussion_r970721813


##########
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionHistoryTest.java:
##########
@@ -159,4 +162,48 @@ public void testRemoveVHR() throws RepositoryException {
 
         assertFalse("VersionHistory node should have disappeared", superuser.itemExists(vhrpath));
     }
+
+    public void testRemoveVersionLabelWithRemovalOfVersion() throws RepositoryException{
+        int createVersions = 3;
+        Node n = testRootNode.addNode(nodeName1, testNodeType);
+        n.addMixin(mixVersionable);
+        superuser.save();
+
+        VersionManager vm = superuser.getWorkspace().getVersionManager();
+        for (int i = 0; i < createVersions; i++) {
+            vm.checkout(n.getPath());
+            vm.checkin(n.getPath());
+        }
+        superuser.save();

Review Comment:
   This is unnecessary. Checkin and checkout are workspace operations and do not need to be saved.



##########
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionHistoryTest.java:
##########
@@ -159,4 +162,48 @@ public void testRemoveVHR() throws RepositoryException {
 
         assertFalse("VersionHistory node should have disappeared", superuser.itemExists(vhrpath));
     }
+
+    public void testRemoveVersionLabelWithRemovalOfVersion() throws RepositoryException{
+        int createVersions = 3;
+        Node n = testRootNode.addNode(nodeName1, testNodeType);
+        n.addMixin(mixVersionable);
+        superuser.save();
+
+        VersionManager vm = superuser.getWorkspace().getVersionManager();

Review Comment:
   There is already a `VersionManager` available as an instance variable `versionManager`.



##########
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionHistoryTest.java:
##########
@@ -159,4 +162,48 @@ public void testRemoveVHR() throws RepositoryException {
 
         assertFalse("VersionHistory node should have disappeared", superuser.itemExists(vhrpath));
     }
+
+    public void testRemoveVersionLabelWithRemovalOfVersion() throws RepositoryException{
+        int createVersions = 3;
+        Node n = testRootNode.addNode(nodeName1, testNodeType);
+        n.addMixin(mixVersionable);
+        superuser.save();
+
+        VersionManager vm = superuser.getWorkspace().getVersionManager();
+        for (int i = 0; i < createVersions; i++) {
+            vm.checkout(n.getPath());
+            vm.checkin(n.getPath());
+        }
+        superuser.save();
+
+        VersionHistory vhr = vm.getVersionHistory(n.getPath());
+        // initialize versionName
+        String versionName = "";
+        VersionIterator allversions = vhr.getAllVersions();
+        int count = 0;
+        while (allversions.hasNext()) {
+            Version version = allversions.nextVersion();
+            if(count == 1) {
+                versionName = version.getName();
+            }
+            count++;
+        }
+        int versonLabelCount = 3;
+        List<String> versionLabels = new ArrayList<>();
+        for(int i = 0; i < versonLabelCount; i++) {
+            String labelName = "Label_" + versionName + "_" + i;
+            vhr.addVersionLabel(versionName, labelName,false);
+            versionLabels.add(labelName);
+        }
+
+        superuser.save();
+        vhr.removeVersion(versionName);
+        superuser.save();
+
+        for(String label : versionLabels) {
+            assertEquals("version label should not exist", false, vhr.hasVersionLabel(label));

Review Comment:
   This assertion would be easier to read if it was using `assertFalse()`.



##########
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionHistoryTest.java:
##########
@@ -159,4 +162,48 @@ public void testRemoveVHR() throws RepositoryException {
 
         assertFalse("VersionHistory node should have disappeared", superuser.itemExists(vhrpath));
     }
+
+    public void testRemoveVersionLabelWithRemovalOfVersion() throws RepositoryException{
+        int createVersions = 3;
+        Node n = testRootNode.addNode(nodeName1, testNodeType);
+        n.addMixin(mixVersionable);
+        superuser.save();
+
+        VersionManager vm = superuser.getWorkspace().getVersionManager();
+        for (int i = 0; i < createVersions; i++) {
+            vm.checkout(n.getPath());
+            vm.checkin(n.getPath());
+        }
+        superuser.save();
+
+        VersionHistory vhr = vm.getVersionHistory(n.getPath());
+        // initialize versionName
+        String versionName = "";
+        VersionIterator allversions = vhr.getAllVersions();
+        int count = 0;
+        while (allversions.hasNext()) {
+            Version version = allversions.nextVersion();
+            if(count == 1) {
+                versionName = version.getName();
+            }
+            count++;
+        }
+        int versonLabelCount = 3;
+        List<String> versionLabels = new ArrayList<>();
+        for(int i = 0; i < versonLabelCount; i++) {
+            String labelName = "Label_" + versionName + "_" + i;
+            vhr.addVersionLabel(versionName, labelName,false);
+            versionLabels.add(labelName);
+        }
+
+        superuser.save();
+        vhr.removeVersion(versionName);
+        superuser.save();

Review Comment:
   Similar as above, `removeVersion()` is again a workspace operation.



##########
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionHistoryTest.java:
##########
@@ -159,4 +162,48 @@ public void testRemoveVHR() throws RepositoryException {
 
         assertFalse("VersionHistory node should have disappeared", superuser.itemExists(vhrpath));
     }
+
+    public void testRemoveVersionLabelWithRemovalOfVersion() throws RepositoryException{
+        int createVersions = 3;
+        Node n = testRootNode.addNode(nodeName1, testNodeType);
+        n.addMixin(mixVersionable);
+        superuser.save();
+
+        VersionManager vm = superuser.getWorkspace().getVersionManager();
+        for (int i = 0; i < createVersions; i++) {
+            vm.checkout(n.getPath());
+            vm.checkin(n.getPath());
+        }
+        superuser.save();
+
+        VersionHistory vhr = vm.getVersionHistory(n.getPath());
+        // initialize versionName
+        String versionName = "";
+        VersionIterator allversions = vhr.getAllVersions();
+        int count = 0;
+        while (allversions.hasNext()) {
+            Version version = allversions.nextVersion();
+            if(count == 1) {
+                versionName = version.getName();
+            }
+            count++;
+        }
+        int versonLabelCount = 3;
+        List<String> versionLabels = new ArrayList<>();
+        for(int i = 0; i < versonLabelCount; i++) {
+            String labelName = "Label_" + versionName + "_" + i;
+            vhr.addVersionLabel(versionName, labelName,false);
+            versionLabels.add(labelName);
+        }
+
+        superuser.save();

Review Comment:
   Again, this save() call is unnecessary. `VersionHistory.addVersionLabel()` is a workspace operation.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org