From 057b0247a0e33f963555c6879ad92e1995748357 Mon Sep 17 00:00:00 2001 From: Stephan Pauxberger Date: Sun, 5 Jun 2022 18:03:33 +0200 Subject: [PATCH 1/4] Copy single dsl child --- .../klum/ast/util/KlumInstanceProxy.java | 20 ++++++++- .../ast/util/KlumInstanceProxyTest.groovy | 41 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/klum-ast-runtime/src/main/java/com/blackbuild/klum/ast/util/KlumInstanceProxy.java b/klum-ast-runtime/src/main/java/com/blackbuild/klum/ast/util/KlumInstanceProxy.java index 53507e29..d7ec07d7 100644 --- a/klum-ast-runtime/src/main/java/com/blackbuild/klum/ast/util/KlumInstanceProxy.java +++ b/klum-ast-runtime/src/main/java/com/blackbuild/klum/ast/util/KlumInstanceProxy.java @@ -194,6 +194,12 @@ private void applyNamedParameters(Object rw, Map 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)) @@ -223,20 +229,30 @@ private void copyFromField(Field field, Object template) { copyFromCollectionField((Collection) templateValue, fieldName); else if (templateValue instanceof Map) copyFromMapField((Map) templateValue, fieldName); + else if (isDslType(templateValue.getClass())) + setInstanceAttribute(fieldName, getProxyFor(templateValue).cloneInstance()); else setInstanceAttribute(fieldName, templateValue); } + private T getCopiedValue(T templateValue) { + if (isDslType(templateValue.getClass())) + return (T) getProxyFor(templateValue).cloneInstance(); + else + return templateValue; + + } + private void copyFromMapField(Map templateValue, String fieldName) { if (templateValue.isEmpty()) return; - Map instanceField = (Map) getInstanceAttribute(fieldName); + Map instanceField = getInstanceAttribute(fieldName); instanceField.clear(); instanceField.putAll(templateValue); } private void copyFromCollectionField(Collection templateValue, String fieldName) { if (templateValue.isEmpty()) return; - Collection instanceField = (Collection) getInstanceAttribute(fieldName); + Collection instanceField = getInstanceAttribute(fieldName); instanceField.clear(); instanceField.addAll(templateValue); } diff --git a/klum-ast-runtime/src/test/groovy/com/blackbuild/klum/ast/util/KlumInstanceProxyTest.groovy b/klum-ast-runtime/src/test/groovy/com/blackbuild/klum/ast/util/KlumInstanceProxyTest.groovy index b231528c..3e144e67 100644 --- a/klum-ast-runtime/src/test/groovy/com/blackbuild/klum/ast/util/KlumInstanceProxyTest.groovy +++ b/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,44 @@ 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) + } } From caf8ef7ad0067c7727dd7643ade46b9422fb57ce Mon Sep 17 00:00:00 2001 From: Stephan Pauxberger Date: Wed, 8 Jun 2022 09:53:51 +0200 Subject: [PATCH 2/4] Copy lists and maps of dsl objects --- .../klum/ast/util/KlumInstanceProxy.java | 10 ++- .../ast/util/KlumInstanceProxyTest.groovy | 64 +++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/klum-ast-runtime/src/main/java/com/blackbuild/klum/ast/util/KlumInstanceProxy.java b/klum-ast-runtime/src/main/java/com/blackbuild/klum/ast/util/KlumInstanceProxy.java index d7ec07d7..e5ffbf57 100644 --- a/klum-ast-runtime/src/main/java/com/blackbuild/klum/ast/util/KlumInstanceProxy.java +++ b/klum-ast-runtime/src/main/java/com/blackbuild/klum/ast/util/KlumInstanceProxy.java @@ -229,10 +229,8 @@ private void copyFromField(Field field, Object template) { copyFromCollectionField((Collection) templateValue, fieldName); else if (templateValue instanceof Map) copyFromMapField((Map) templateValue, fieldName); - else if (isDslType(templateValue.getClass())) - setInstanceAttribute(fieldName, getProxyFor(templateValue).cloneInstance()); else - setInstanceAttribute(fieldName, templateValue); + setInstanceAttribute(fieldName, getCopiedValue(templateValue)); } private T getCopiedValue(T templateValue) { @@ -240,21 +238,21 @@ private T getCopiedValue(T templateValue) { return (T) getProxyFor(templateValue).cloneInstance(); else return templateValue; - } private void copyFromMapField(Map templateValue, String fieldName) { if (templateValue.isEmpty()) return; Map instanceField = getInstanceAttribute(fieldName); instanceField.clear(); - instanceField.putAll(templateValue); + templateValue.forEach((k, v) -> instanceField.put(k, getCopiedValue(v))); } private void copyFromCollectionField(Collection templateValue, String fieldName) { if (templateValue.isEmpty()) return; Collection instanceField = getInstanceAttribute(fieldName); instanceField.clear(); - instanceField.addAll(templateValue); + + templateValue.stream().map(this::getCopiedValue).forEach(instanceField::add); } /** diff --git a/klum-ast-runtime/src/test/groovy/com/blackbuild/klum/ast/util/KlumInstanceProxyTest.groovy b/klum-ast-runtime/src/test/groovy/com/blackbuild/klum/ast/util/KlumInstanceProxyTest.groovy index 3e144e67..46aee168 100644 --- a/klum-ast-runtime/src/test/groovy/com/blackbuild/klum/ast/util/KlumInstanceProxyTest.groovy +++ b/klum-ast-runtime/src/test/groovy/com/blackbuild/klum/ast/util/KlumInstanceProxyTest.groovy @@ -216,4 +216,68 @@ import com.blackbuild.groovy.configdsl.transform.DSL copy.inner.value == "bla" !copy.inner.is(inner) } + @Issue("36") + def "copy from creates copies of nested DSL object collection"() { + given: + createClass(''' + package pk + +import com.blackbuild.groovy.configdsl.transform.DSL + + @SuppressWarnings('UnnecessaryQualifiedReference') + @DSL + class Outer { + KlumInstanceProxy $proxy = new KlumInstanceProxy(this) + String name + + List inners = [] + Map 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) + } + + // TODO: List of Lists, Maps of Lists, mixed dsl / non dsl elements } From 4a558df8f3a4f9c3d39497b998f6e56f4d921833 Mon Sep 17 00:00:00 2001 From: Stephan Pauxberger Date: Wed, 22 Jun 2022 22:34:28 +0200 Subject: [PATCH 3/4] Copy nested collections and maps --- .../klum/ast/util/KlumInstanceProxy.java | 20 ++++++- .../ast/util/KlumInstanceProxyTest.groovy | 53 ++++++++++++++++++- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/klum-ast-runtime/src/main/java/com/blackbuild/klum/ast/util/KlumInstanceProxy.java b/klum-ast-runtime/src/main/java/com/blackbuild/klum/ast/util/KlumInstanceProxy.java index e5ffbf57..6c1ad05b 100644 --- a/klum-ast-runtime/src/main/java/com/blackbuild/klum/ast/util/KlumInstanceProxy.java +++ b/klum-ast-runtime/src/main/java/com/blackbuild/klum/ast/util/KlumInstanceProxy.java @@ -203,7 +203,9 @@ public Object cloneInstance() { 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) { @@ -236,10 +238,26 @@ else if (templateValue instanceof Map) private 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 Collection createCopyOfCollection(Collection templateValue) { + Collection result = (Collection) InvokerHelper.invokeConstructorOf(templateValue.getClass(), null); + templateValue.stream().map(this::getCopiedValue).forEach(result::add); + return result; + } + + private Map createCopyOfMap(Map templateValue) { + Map result = (Map) InvokerHelper.invokeConstructorOf(templateValue.getClass(), null); + templateValue.forEach((key, value) -> result.put(key, getCopiedValue(value))); + return result; + } + private void copyFromMapField(Map templateValue, String fieldName) { if (templateValue.isEmpty()) return; Map instanceField = getInstanceAttribute(fieldName); diff --git a/klum-ast-runtime/src/test/groovy/com/blackbuild/klum/ast/util/KlumInstanceProxyTest.groovy b/klum-ast-runtime/src/test/groovy/com/blackbuild/klum/ast/util/KlumInstanceProxyTest.groovy index 46aee168..5bc70ffe 100644 --- a/klum-ast-runtime/src/test/groovy/com/blackbuild/klum/ast/util/KlumInstanceProxyTest.groovy +++ b/klum-ast-runtime/src/test/groovy/com/blackbuild/klum/ast/util/KlumInstanceProxyTest.groovy @@ -216,8 +216,9 @@ import com.blackbuild.groovy.configdsl.transform.DSL copy.inner.value == "bla" !copy.inner.is(inner) } + @Issue("36") - def "copy from creates copies of nested DSL object collection"() { + def "copy from creates copies of nested DSL object collections and maps"() { given: createClass(''' package pk @@ -279,5 +280,53 @@ import com.blackbuild.groovy.configdsl.transform.DSL !copy.mappedInners.two.is(minner2) } - // TODO: List of Lists, Maps of Lists, mixed dsl / non dsl elements + @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> inners = [:] + List> 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 } From c220f152f125b853e253b8d4a0c7f18267b491fd Mon Sep 17 00:00:00 2001 From: Stephan Pauxberger Date: Wed, 22 Jun 2022 22:39:47 +0200 Subject: [PATCH 4/4] Documentation --- CHANGES.md | 1 + wiki/Templates.md | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 791b8fa4..2f94048b 100644 --- a/CHANGES.md +++ b/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)) diff --git a/wiki/Templates.md b/wiki/Templates.md index 47e54563..25967d20 100644 --- a/wiki/Templates.md +++ b/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