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


##########
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:
   Thx for the very detailed description!
   
   > Regarding the after.getValue(Type.REFERENCE).startsWith(RESTORE_PREFIX)) 
condition:
   
   As the code is now `restore()` is only called if this condition is true. 
Before this change however, it was also called in other cases. Now if those 
other cases were wrong, then that's a bug. I would however prefer to look into 
that and have a test cases that proofs that is a bug. I'm suspecting that with 
this change we'll introduce a regression...



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