#781: Clone properties to avoid sharing mutable values in multiple el…

…ements
eclipse · Sep 28, 2021 · 25c2db3 · 25c2db3
1 parent 26f62dd
commit 25c2db3
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 9 deletions.
diff --git a/plugins/org.eclipse.elk.core/src/org/eclipse/elk/core/LayoutConfigurator.java b/plugins/org.eclipse.elk.core/src/org/eclipse/elk/core/LayoutConfigurator.java
@@ -27,6 +27,7 @@
 import org.eclipse.elk.graph.properties.IPropertyHolder;
 import org.eclipse.elk.graph.properties.MapPropertyHolder;
 import org.eclipse.elk.graph.properties.Property;
+import org.eclipse.elk.graph.util.ElkReflect;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
@@ -218,16 +219,20 @@ public void visit(final ElkGraphElement element) {
     @SuppressWarnings("unchecked")
     protected void applyProperties(final ElkGraphElement element, final IPropertyHolder properties) {
         if (properties != null) {
-            if (!optionFilters.isEmpty()) {
-                for (Map.Entry<IProperty<?>, Object> entry : properties.getAllProperties().entrySet()) {
-                    boolean accept = optionFilters.stream()
-                            .allMatch(filter -> filter.accept(element, entry.getKey()));
-                    if (accept) {
-                        element.setProperty((IProperty<Object>) entry.getKey(), entry.getValue());
+            List<IOptionFilter> filters = getFilters();
+            for (Map.Entry<IProperty<?>, Object> entry : properties.getAllProperties().entrySet()) {
+                boolean accept = filters.stream()
+                        .allMatch(filter -> filter.accept(element, entry.getKey()));
+                if (accept) {
+                    Object value = entry.getValue();
+                    if (value instanceof Cloneable) {
+                        Object clone = ElkReflect.clone(value);
+                        if (clone != null) {
+                            value = clone;
+                        }
                     }
+                    element.setProperty((IProperty<Object>) entry.getKey(), value);
                 }
-            } else {
-                element.copyProperties(properties);
             }
         }
     }

diff --git a/plugins/org.eclipse.elk.core/src/org/eclipse/elk/core/util/ElkUtil.java b/plugins/org.eclipse.elk.core/src/org/eclipse/elk/core/util/ElkUtil.java
@@ -347,7 +347,7 @@ public static KVector effectiveMinSizeConstraintFor(final ElkNode node) {
 
         if (sizeConstraint.contains(SizeConstraint.MINIMUM_SIZE)) {
             Set<SizeOptions> sizeOptions = node.getProperty(CoreOptions.NODE_SIZE_OPTIONS);
-            KVector minSize = node.getProperty(CoreOptions.NODE_SIZE_MINIMUM);
+            KVector minSize = new KVector(node.getProperty(CoreOptions.NODE_SIZE_MINIMUM));
 
             // If minimum width or height are not set, maybe default to default values
             if (sizeOptions.contains(SizeOptions.DEFAULT_MINIMUM_SIZE)) {