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-unsubscr...@jackrabbit.apache.org

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

Reply via email to