[GitHub] [jackrabbit-oak] amit-jain commented on a diff in pull request #880: OAK-10162: Fix Version copier with preserveOnTarget to ignore diverge…

["amit-jain (via GitHub)" <gi...@apache.org>]

amit-jain commented on code in PR #880:
URL: https://github.com/apache/jackrabbit-oak/pull/880#discussion_r1155470241


##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/migration/NodeStateCopier.java:
##########
@@ -145,29 +150,73 @@ public static boolean copyNodeStore(@NotNull final NodeStore source, @NotNull fi
      *
      * @param source The NodeState to copy from.
      * @param target The NodeBuilder to copy to.
+     * @param preserveOnTarget boolean to indicate no changes on target except additions
+     * @param path current path
      * @return Whether changes were made or not.
      */
-    public static boolean copyProperties(NodeState source, NodeBuilder target) {
+    public static boolean copyProperties(NodeState source, NodeBuilder target, boolean preserveOnTarget, String path) {
         boolean hasChanges = false;
 
         // remove removed properties
-        for (final PropertyState property : target.getProperties()) {
-            final String name = property.getName();
-            if (!source.hasProperty(name)) {
-                target.removeProperty(name);
-                hasChanges = true;
+        if (!preserveOnTarget) {
+            for (final PropertyState property : target.getProperties()) {
+                final String name = property.getName();
+                if (!source.hasProperty(name)) {
+                    target.removeProperty(name);
+                    hasChanges = true;
+                }
             }
         }
 
         // add new properties and change changed properties
         for (PropertyState property : source.getProperties()) {
             if (!property.equals(target.getProperty(property.getName()))) {
-                target.setProperty(property);
-                hasChanges = true;
+                if (!isVersionPropertyEmpty(source, property, preserveOnTarget, path)) {
+                    target.setProperty(property);
+                    hasChanges = true;
+                }
             }
         }
         return hasChanges;
     }
+    
+    private static Set<String> getValues(NodeState nodeState, String prop) {
+        if (nodeState.hasProperty(prop)) {
+            Iterable<String> values = nodeState.getProperty(prop).getValue(Type.STRINGS);
+            return StreamSupport.stream(values.spliterator(), false).filter(s -> !s.isEmpty()).collect(Collectors.toSet());
+        }
+        return new HashSet<>();

Review Comment:
   I had left it to leave room for furture in case needed but not a strong opinion here.



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

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