Merge pull request #259 from klum-dsl/feature/36-deep-clone

Deep clones for templates and copyFrom

CHANGES.md

@@ -3,6 +3,7 @@
     - Compatibility with Groovy 3. KlumAST is currently still built with Groovy 2.4 (for compatitibility with Jenkins).
     - Replace basic jackson transformation with a dedicated (beta) JacksonModule (see [Jackson Integration](https://github.com/klum-dsl/klum-ast/wiki/Migration))).
 - Improvements
+  - CopyFrom now creates deep clones (see [#36](https://github.com/klum-dsl/klum-ast/issues/36))
   - `boolean` fields are never validated (makes no sense), `Boolean` fields are evaluated against not null, not against Groovy Truth (i.e. the field must have an explicit value assigned) (see [#223](https://github.com/klum-dsl/klum-ast/issues/223))
   - Provide `@Required` as an alternative to an empty `@Validate` annotation (see [#221](https://github.com/klum-dsl/klum-ast/issues/221))
   - `EnumSet` fields are no supported. Note that for enum sets a copy of the underlying set is returned as opposed to a readonly instance. (see [#249](https://github.com/klum-dsl/klum-ast/issues/249))

klum-ast-runtime/src/main/java/com/blackbuild/klum/ast/util/KlumInstanceProxy.java

@@ -194,10 +194,18 @@ private void applyNamedParameters(Object rw, Map<String, Object> values) {
     public void copyFrom(Object template) {
         DslHelper.getDslHierarchyOf(instance.getClass()).forEach(it -> copyFromLayer(it, template));
     }
+    public Object cloneInstance() {
+        Object key = isKeyed(instance.getClass()) ? getKey() : null;
+        Object result = FactoryHelper.createInstance(instance.getClass(), (String) key);
+        getProxyFor(result).copyFrom(instance);
+        return result;
+    }
 
     private void copyFromLayer(Class<?> layer, Object template) {
         if (layer.isInstance(template))
-            Arrays.stream(layer.getDeclaredFields()).filter(this::isNotIgnored).forEach(field -> copyFromField(field, template));
+            Arrays.stream(layer.getDeclaredFields())
+                    .filter(this::isNotIgnored)
+                    .forEach(field -> copyFromField(field, template));
     }
 
     private boolean isIgnored(Field field) {
@@ -224,21 +232,45 @@ private void copyFromField(Field field, Object template) {
         else if (templateValue instanceof Map)
             copyFromMapField((Map<?,?>) templateValue, fieldName);
         else
-            setInstanceAttribute(fieldName, templateValue);
+            setInstanceAttribute(fieldName, getCopiedValue(templateValue));
+    }
+
+    private <T> T getCopiedValue(T templateValue) {
+        if (isDslType(templateValue.getClass()))
+            return (T) getProxyFor(templateValue).cloneInstance();
+        else if (templateValue instanceof Collection)
+            return (T) createCopyOfCollection((Collection) templateValue);
+        else if (templateValue instanceof Map)
+            return (T) createCopyOfMap((Map) templateValue);
+        else
+            return templateValue;
+    }
+
+    private <T> Collection<T> createCopyOfCollection(Collection<T> templateValue) {
+        Collection<T> result = (Collection<T>) InvokerHelper.invokeConstructorOf(templateValue.getClass(), null);
+        templateValue.stream().map(this::getCopiedValue).forEach(result::add);
+        return result;
+    }
+
+    private <T> Map<String, T> createCopyOfMap(Map<String, T> templateValue) {
+        Map<String, T> result = (Map<String, T>) InvokerHelper.invokeConstructorOf(templateValue.getClass(), null);
+        templateValue.forEach((key, value) -> result.put(key, getCopiedValue(value)));
+        return result;
     }
 
     private <K,V> void copyFromMapField(Map<K,V> templateValue, String fieldName) {
         if (templateValue.isEmpty()) return;
-        Map<K,V> instanceField = (Map<K,V>) getInstanceAttribute(fieldName);
+        Map<K,V> instanceField = getInstanceAttribute(fieldName);
         instanceField.clear();
-        instanceField.putAll(templateValue);
+        templateValue.forEach((k, v) -> instanceField.put(k, getCopiedValue(v)));
     }
 
     private <T> void copyFromCollectionField(Collection<T> templateValue, String fieldName) {
         if (templateValue.isEmpty()) return;
-        Collection<T> instanceField = (Collection<T>) getInstanceAttribute(fieldName);
+        Collection<T> instanceField = getInstanceAttribute(fieldName);
         instanceField.clear();
-        instanceField.addAll(templateValue);
+
+        templateValue.stream().map(this::getCopiedValue).forEach(instanceField::add);
     }
 
     /**

klum-ast-runtime/src/test/groovy/com/blackbuild/klum/ast/util/KlumInstanceProxyTest.groovy

@@ -23,6 +23,7 @@
  */
 package com.blackbuild.klum.ast.util
 
+import spock.lang.Issue
 import spock.lang.Subject
 
 class KlumInstanceProxyTest extends AbstractRuntimeTest {
@@ -175,4 +176,157 @@ class KlumInstanceProxyTest extends AbstractRuntimeTest {
         !proxy.resolveKeyForFieldFromAnnotation("noFieldAnnotation", proxy.getField("noFieldAnnotation")).isPresent()
     }
 
+    @Issue("36")
+    def "copy from creates copies of nested DSL objects"() {
+        given:
+        createClass('''
+            package pk
+
+import com.blackbuild.groovy.configdsl.transform.DSL
+
+            @SuppressWarnings('UnnecessaryQualifiedReference')
+            @DSL
+            class Outer {
+                KlumInstanceProxy $proxy = new KlumInstanceProxy(this)
+                String name
+                
+                Inner inner
+            }
+            
+            @DSL
+            class Inner {
+                KlumInstanceProxy $proxy = new KlumInstanceProxy(this)
+                String value
+            } 
+         ''')
+
+        def inner = newInstanceOf("pk.Inner")
+        def outer = newInstanceOf("pk.Outer")
+        inner.value = "bla"
+        outer.inner = inner
+        outer.name = "bli"
+
+        when:
+        def copy = newInstanceOf("pk.Outer")
+        proxy = KlumInstanceProxy.getProxyFor(copy)
+        proxy.copyFrom(outer)
+
+        then:
+        copy.name == "bli"
+        copy.inner.value == "bla"
+        !copy.inner.is(inner)
+    }
+
+    @Issue("36")
+    def "copy from creates copies of nested DSL object collections and maps"() {
+        given:
+        createClass('''
+            package pk
+
+import com.blackbuild.groovy.configdsl.transform.DSL
+
+            @SuppressWarnings('UnnecessaryQualifiedReference')
+            @DSL
+            class Outer {
+                KlumInstanceProxy $proxy = new KlumInstanceProxy(this)
+                String name
+                
+                List<Inner> inners = []
+                Map<String, Inner> mappedInners = [:]
+            }
+            
+            @DSL
+            class Inner {
+                KlumInstanceProxy $proxy = new KlumInstanceProxy(this)
+                String value
+            } 
+         ''')
+
+        def inner = newInstanceOf("pk.Inner")
+        def inner2 = newInstanceOf("pk.Inner")
+        def minner = newInstanceOf("pk.Inner")
+        def minner2 = newInstanceOf("pk.Inner")
+        def outer = newInstanceOf("pk.Outer")
+        inner.value = "bla"
+        inner2.value = "blu"
+        minner.value = "mbla"
+        minner2.value = "mblu"
+        outer.inners.add inner
+        outer.inners.add inner2
+
+        outer.mappedInners.putAll(one: minner, two: minner2)
+        outer.name = "bli"
+
+        when:
+        def copy = newInstanceOf("pk.Outer")
+        proxy = KlumInstanceProxy.getProxyFor(copy)
+        proxy.copyFrom(outer)
+
+        then:
+        copy.name == "bli"
+        !copy.inners.is(outer.inners)
+        copy.inners.size() == 2
+
+        and:
+        copy.inners[0].value == "bla"
+        copy.inners[1].value == "blu"
+        !copy.inners[0].is(inner)
+        !copy.inners[1].is(inner2)
+
+        and:
+        copy.mappedInners.one.value == "mbla"
+        copy.mappedInners.two.value == "mblu"
+        !copy.mappedInners.one.is(minner)
+        !copy.mappedInners.two.is(minner2)
+    }
+
+    @Issue("36")
+    def "copy from creates copies of Maps of Lists"() {
+        given:
+        createClass('''
+            package pk
+
+            import com.blackbuild.groovy.configdsl.transform.DSL
+
+            @SuppressWarnings('UnnecessaryQualifiedReference')
+            @DSL
+            class Outer {
+                KlumInstanceProxy $proxy = new KlumInstanceProxy(this)
+                String name
+                
+                Map<String, List<String>> inners = [:]
+                List<List<String>> innerLists = []
+            }
+         ''')
+
+        def outer = newInstanceOf("pk.Outer")
+        outer.name = "bli"
+        outer.inners.put "a", ["a1", "a2"]
+        outer.inners.put "b", ["b1", "b2"]
+        outer.innerLists.add(["a1", "a2"])
+        outer.innerLists.add(["b1", "b2"])
+
+        when:
+        def copy = newInstanceOf("pk.Outer")
+        proxy = KlumInstanceProxy.getProxyFor(copy)
+        proxy.copyFrom(outer)
+
+        then:
+        copy.name == "bli"
+        copy.inners == outer.inners
+        !copy.inners.is(outer.inners)
+        copy.innerLists == outer.innerLists
+        !copy.innerLists.is(outer.innerLists)
+
+        and:
+        copy.inners.a == outer.inners.a
+        !copy.inners.a.is(outer.inners.a)
+
+        and:
+        copy.innerLists[0] == outer.innerLists[0]
+        !copy.innerLists[0].is(outer.innerLists[0])
+
+    }
+
+    // TODO: List of Lists, mixed dsl / non dsl elements
 }

wiki/Templates.md

@@ -3,7 +3,7 @@
 The system includes a simple mechanism for configuring default values (as part of the instance creation, not in the classes):
 
 Templates are regular instances of DSL objects, which will usually be assigned to a local variable. Applying a template means
- that all non-null / non-empty fields in the template are copied over from template. For Lists and Maps, shallow copies 
+ that all non-null / non-empty fields in the template are copied over from template. For Lists and Maps, deep copies 
  will be created. 
 
  Ignorable fields of the template (key, owner, transient or marked as `@Ignore`) are never copied over. To make creating