stefan-egli commented on code in PR #1474:
URL: https://github.com/apache/jackrabbit-oak/pull/1474#discussion_r1624467573


##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/VersionEditor.java:
##########
@@ -152,20 +153,37 @@ public void propertyChanged(PropertyState before, 
PropertyState after)
                 vMgr.checkin(node);
             }
         } else if (propName.equals(JCR_BASEVERSION)) {
-            String baseVersion = after.getValue(Type.REFERENCE);
-            if (baseVersion.startsWith(RESTORE_PREFIX)) {
-                baseVersion = baseVersion.substring(RESTORE_PREFIX.length());
-                node.setProperty(JCR_BASEVERSION, baseVersion, Type.REFERENCE);
+            // OAK-8848: skip restore if this is an overwrite of a node and a 
restore operation is not in progress
+            if (!nodeWasMovedOrCopied() || 
after.getValue(Type.REFERENCE).startsWith(RESTORE_PREFIX)) {

Review Comment:
   I'm a bit lost at this condition here and wonder if it needs to be at this 
complexity. I'm also wondering if all permutations of this are covered in unit 
tests. Let's say nodeWasMovedOrCopied is false, why is the second part of the 
condition needed then? Can you please elaborate a bit on this?



-- 
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