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