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