From a4138ed82da194dfffdec266944f5f71b523f0e2 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Tue, 21 Apr 2020 15:11:41 +0200 Subject: [PATCH] Squash empty lists/maps Dealing with lists requires us to interfere with what the user is giving us in builders, effectively creating new mechanics. This patch adds the model metadata to carry these around, exposing just how badly broken some binding.model.api construcs are misused. We do not fix the misuse, but make it slightly worse, but also mark the mess for future cleanup. JIRA: MDSAL-451 Change-Id: I43b3ddccff69bec9a4e98b3c8e73aac4679c1d1f Signed-off-by: Robert Varga --- .../binding/model/api/GeneratedProperty.java | 28 +++++++++++++------ .../binding/model/api/MethodSignature.java | 23 +++++++++++++++ .../builder/GeneratedPropertyBuilder.java | 4 +++ .../type/builder/MethodSignatureBuilder.java | 5 ++++ .../generator/impl/AbstractTypeGenerator.java | 3 +- .../builder/GeneratedPropertyBuilderImpl.java | 10 ++++++- .../type/builder/GeneratedPropertyImpl.java | 9 +++++- .../builder/MethodSignatureBuilderImpl.java | 13 ++++++++- .../type/builder/MethodSignatureImpl.java | 15 ++++++++-- .../type/builder/GeneratedPropertyTest.java | 2 +- .../generator/AbstractBuilderTemplate.xtend | 8 ++---- .../java/api/generator/BuilderGenerator.java | 10 ++++++- .../api/generator/BuilderImplTemplate.xtend | 10 +++++++ .../java/api/generator/BuilderTemplate.xtend | 7 +++++ .../api/generator/BuilderGeneratorTest.java | 2 ++ 15 files changed, 126 insertions(+), 23 deletions(-) diff --git a/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/GeneratedProperty.java b/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/GeneratedProperty.java index 4a9ddb30bd..011e7ad612 100644 --- a/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/GeneratedProperty.java +++ b/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/GeneratedProperty.java @@ -8,23 +8,33 @@ package org.opendaylight.mdsal.binding.model.api; /** - * Generated Property extends interface {@link MethodSignature} interface.
- * The Generated Property interface is designed to store information of fields - * (or members) declared in Java Transfer Objects (or any java classes) and - * their access counterparts (getters and setters). + * Generated Property extends {@link TypeMember} interface with additional information about fields (and other members) + * declared in Java Transfer Objects (or any java classes) and their access counterparts (getters and setters). * - * @see MethodSignature + * @see TypeMember */ +// FIXME: 7.0.0: this interface (and others) need to be refactored: +// - getValue() is pretty much unused and its semantics are undefined +// - isReadOnly() is not related to getValue() and is not used together +// - nullifyEmpty() is applicable only to collection types and implies non-read-only and without value +// - this is misused by Builder spec :( public interface GeneratedProperty extends TypeMember { String getValue(); /** - * Returns true if the property is declared as read-only.
- * If the property has flag isReadOnly == true the property - * SHOULD be generated as getter only. + * Returns true if the property is declared as read-only. If this {@code true} the property should be + * generated with only a getter. * - * @return true if the property is declared as read-only. + * @return {@code true<} if the property is declared as read-only. */ boolean isReadOnly(); + + /** + * Returns indication whether the value should be squashed from empty collection to a null. This property is valid + * only if {@link #getReturnType()} results in a well-known collection type: List or Map. + * + * @return True if empty collections should be turned to nulls + */ + boolean nullifyEmpty(); } diff --git a/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/MethodSignature.java b/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/MethodSignature.java index b6cb410b36..dafcfe4e3f 100644 --- a/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/MethodSignature.java +++ b/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/MethodSignature.java @@ -47,6 +47,13 @@ public interface MethodSignature extends TypeMember { */ List getParameters(); + /** + * Return the mechanics associated with this method. + * + * @return Associated mechanics + */ + ValueMechanics getMechanics(); + /** * The Parameter interface is designed to hold the information of method * Parameter(s). The parameter is defined by his Name which MUST be unique @@ -69,4 +76,20 @@ public interface MethodSignature extends TypeMember { */ Type getType(); } + + /** + * Method return type mechanics. This is a bit of an escape hatch for various behaviors which are supported by + * code generation. + */ + enum ValueMechanics { + /** + * Usual mechanics, nothing special is going on. + */ + NORMAL, + /** + * Mechanics signaling that the method should not be returning empty collections, but rather squash tham + * to null. + */ + NULLIFY_EMPTY, + } } diff --git a/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/type/builder/GeneratedPropertyBuilder.java b/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/type/builder/GeneratedPropertyBuilder.java index 114e37b651..79bedba9b3 100644 --- a/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/type/builder/GeneratedPropertyBuilder.java +++ b/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/type/builder/GeneratedPropertyBuilder.java @@ -7,6 +7,7 @@ */ package org.opendaylight.mdsal.binding.model.api.type.builder; +import com.google.common.annotations.Beta; import org.opendaylight.mdsal.binding.model.api.GeneratedProperty; import org.opendaylight.mdsal.binding.model.api.Type; @@ -27,6 +28,9 @@ public interface GeneratedPropertyBuilder extends TypeMemberBuildernew immutable instance of Generated Property.
* The definingType param cannot be null. The diff --git a/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/type/builder/MethodSignatureBuilder.java b/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/type/builder/MethodSignatureBuilder.java index 34c7b3861d..5e69bef48b 100644 --- a/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/type/builder/MethodSignatureBuilder.java +++ b/binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/type/builder/MethodSignatureBuilder.java @@ -7,7 +7,9 @@ */ package org.opendaylight.mdsal.binding.model.api.type.builder; +import com.google.common.annotations.Beta; import org.opendaylight.mdsal.binding.model.api.MethodSignature; +import org.opendaylight.mdsal.binding.model.api.MethodSignature.ValueMechanics; import org.opendaylight.mdsal.binding.model.api.Type; /** @@ -42,6 +44,9 @@ public interface MethodSignatureBuilder extends TypeMemberBuilder annotations = toAnnotationTypes(); return new GeneratedPropertyImpl(definingType, getName(), annotations, getComment(), getAccessModifier(), - getReturnType(), isFinal(), isStatic(), this.readOnly, this.value); + getReturnType(), isFinal(), isStatic(), this.readOnly, this.nullifyEmpty, this.value); } @Override diff --git a/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/GeneratedPropertyImpl.java b/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/GeneratedPropertyImpl.java index c83aefb7a6..24f81399f1 100644 --- a/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/GeneratedPropertyImpl.java +++ b/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/GeneratedPropertyImpl.java @@ -16,13 +16,15 @@ import org.opendaylight.mdsal.binding.model.api.Type; final class GeneratedPropertyImpl extends AbstractTypeMember implements GeneratedProperty { private final String value; private final boolean readOnly; + private final boolean nullifyEmpty; GeneratedPropertyImpl(final Type definingType, final String name, final List annotations, final String comment, final AccessModifier accessModifier, final Type returnType, final boolean isFinal, - final boolean isStatic, final boolean isReadOnly, final String value) { + final boolean isStatic, final boolean isReadOnly, final boolean nullifyEmpty, final String value) { super(definingType, name, annotations, comment, accessModifier, returnType, isFinal, isStatic); this.value = value; this.readOnly = isReadOnly; + this.nullifyEmpty = nullifyEmpty; } @Override @@ -35,6 +37,11 @@ final class GeneratedPropertyImpl extends AbstractTypeMember implements Generate return this.readOnly; } + @Override + public boolean nullifyEmpty() { + return this.nullifyEmpty; + } + @Override public String toString() { final StringBuilder builder = new StringBuilder(); diff --git a/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/MethodSignatureBuilderImpl.java b/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/MethodSignatureBuilderImpl.java index f82bbfa9d7..1eff581953 100644 --- a/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/MethodSignatureBuilderImpl.java +++ b/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/MethodSignatureBuilderImpl.java @@ -7,11 +7,14 @@ */ package org.opendaylight.mdsal.binding.model.util.generated.type.builder; +import static java.util.Objects.requireNonNull; + import java.util.Collections; import java.util.List; import java.util.Objects; import org.opendaylight.mdsal.binding.model.api.AnnotationType; import org.opendaylight.mdsal.binding.model.api.MethodSignature; +import org.opendaylight.mdsal.binding.model.api.MethodSignature.ValueMechanics; import org.opendaylight.mdsal.binding.model.api.Type; import org.opendaylight.mdsal.binding.model.api.type.builder.MethodSignatureBuilder; import org.opendaylight.yangtools.util.LazyCollections; @@ -21,6 +24,7 @@ final class MethodSignatureBuilderImpl extends AbstractTypeMemberBuilder parameters = Collections.emptyList(); private List unmodifiableParams = Collections.emptyList(); + private ValueMechanics mechanics = ValueMechanics.NORMAL; private boolean isAbstract; private boolean isDefault; @@ -40,6 +44,13 @@ final class MethodSignatureBuilderImpl extends AbstractTypeMemberBuilder annotations = toAnnotationTypes(); return new MethodSignatureImpl(definingType, getName(), annotations, getComment(), getAccessModifier(), - getReturnType(), this.unmodifiableParams, isFinal(), this.isAbstract, isStatic(), isDefault); + getReturnType(), this.unmodifiableParams, isFinal(), this.isAbstract, isStatic(), isDefault, mechanics); } @Override diff --git a/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/MethodSignatureImpl.java b/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/MethodSignatureImpl.java index 867e905b8e..5a211b9d41 100644 --- a/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/MethodSignatureImpl.java +++ b/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/MethodSignatureImpl.java @@ -7,6 +7,9 @@ */ package org.opendaylight.mdsal.binding.model.util.generated.type.builder; +import static java.util.Objects.requireNonNull; + +import com.google.common.annotations.VisibleForTesting; import java.util.List; import java.util.Objects; import org.opendaylight.mdsal.binding.model.api.AccessModifier; @@ -16,24 +19,27 @@ import org.opendaylight.mdsal.binding.model.api.Type; class MethodSignatureImpl extends AbstractTypeMember implements MethodSignature { private final List params; + private final ValueMechanics mechanics; private final boolean isAbstract; private final boolean isDefault; + @VisibleForTesting MethodSignatureImpl(final Type definingType, final String name, final List annotations, final String comment, final AccessModifier accessModifier, final Type returnType, final List params, final boolean isFinal, final boolean isAbstract, final boolean isStatic) { this(definingType, name, annotations, comment, accessModifier, returnType, params, isFinal, isAbstract, - isStatic, false); + isStatic, false, ValueMechanics.NORMAL); } MethodSignatureImpl(final Type definingType, final String name, final List annotations, final String comment, final AccessModifier accessModifier, final Type returnType, final List params, final boolean isFinal, final boolean isAbstract, final boolean isStatic, - final boolean isDefault) { + final boolean isDefault, final ValueMechanics mechanics) { super(definingType, name, annotations, comment, accessModifier, returnType, isFinal, isStatic); this.params = params; this.isAbstract = isAbstract; this.isDefault = isDefault; + this.mechanics = requireNonNull(mechanics); } @Override @@ -51,6 +57,11 @@ class MethodSignatureImpl extends AbstractTypeMember implements MethodSignature return this.params; } + @Override + public ValueMechanics getMechanics() { + return mechanics; + } + @Override public int hashCode() { final int prime = 31; diff --git a/binding/mdsal-binding-generator-util/src/test/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/GeneratedPropertyTest.java b/binding/mdsal-binding-generator-util/src/test/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/GeneratedPropertyTest.java index 5e3ef4e37f..eba0b9b297 100644 --- a/binding/mdsal-binding-generator-util/src/test/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/GeneratedPropertyTest.java +++ b/binding/mdsal-binding-generator-util/src/test/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/GeneratedPropertyTest.java @@ -33,7 +33,7 @@ public class GeneratedPropertyTest { @Test public void testMethodsForGeneratedPropertyImpl() { final GeneratedPropertyImpl propertyImpl = new GeneratedPropertyImpl(null, "Test", null, "test property", - AccessModifier.PRIVATE, null, true, true, true, "test value"); + AccessModifier.PRIVATE, null, true, true, true, true, "test value"); assertEquals("test value", propertyImpl.getValue()); assertTrue(propertyImpl.isReadOnly()); diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/AbstractBuilderTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/AbstractBuilderTemplate.xtend index 010450485b..11e8629f95 100644 --- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/AbstractBuilderTemplate.xtend +++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/AbstractBuilderTemplate.xtend @@ -152,14 +152,10 @@ abstract class AbstractBuilderTemplate extends BaseTemplate { return generateDeprecatedAnnotation(found) } - def protected final CharSequence generateCopyNonKeys(Collection props) ''' - «FOR field : props» - this.«field.fieldName» = base.«field.getterMethodName»(); - «ENDFOR» - ''' - def protected abstract CharSequence generateCopyKeys(List keyProps) + def protected abstract CharSequence generateCopyNonKeys(Collection props) + def protected abstract CharSequence generateCopyAugmentation(Type implType) def protected abstract CharSequence generateDeprecatedAnnotation(AnnotationType ann) diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGenerator.java b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGenerator.java index a632394a75..b7ac63f7b2 100644 --- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGenerator.java +++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGenerator.java @@ -29,6 +29,7 @@ import org.opendaylight.mdsal.binding.model.api.JavaTypeName; import org.opendaylight.mdsal.binding.model.api.MethodSignature; import org.opendaylight.mdsal.binding.model.api.ParameterizedType; import org.opendaylight.mdsal.binding.model.api.Type; +import org.opendaylight.mdsal.binding.model.api.type.builder.GeneratedPropertyBuilder; import org.opendaylight.mdsal.binding.model.api.type.builder.GeneratedTOBuilder; import org.opendaylight.mdsal.binding.model.api.type.builder.GeneratedTypeBuilder; import org.opendaylight.mdsal.binding.model.util.Types; @@ -215,7 +216,14 @@ public final class BuilderGenerator implements CodeGenerator { final String fieldName = StringExtensions.toFirstLower(method.getName().substring(prefix.length())); final GeneratedTOBuilder tmpGenTO = new CodegenGeneratedTOBuilder(JavaTypeName.create("foo", "foo")); - tmpGenTO.addProperty(fieldName).setReturnType(method.getReturnType()); + final GeneratedPropertyBuilder builder = tmpGenTO.addProperty(fieldName).setReturnType(method.getReturnType()); + switch (method.getMechanics()) { + case NULLIFY_EMPTY: + builder.setNullifyEmpty(true); + break; + default: + break; + } return tmpGenTO.build().getProperties().get(0); } } diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderImplTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderImplTemplate.xtend index 5a37e75809..53bffa4f18 100644 --- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderImplTemplate.xtend +++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderImplTemplate.xtend @@ -147,6 +147,16 @@ class BuilderImplTemplate extends AbstractBuilderTemplate { «ENDFOR» ''' + override protected CharSequence generateCopyNonKeys(Collection props) ''' + «FOR field : props» + «IF field.nullifyEmpty» + this.«field.fieldName» = «CODEHELPERS.importedName».emptyToNull(base.«field.getterMethodName»()); + «ELSE» + this.«field.fieldName» = base.«field.getterMethodName»(); + «ENDIF» + «ENDFOR» + ''' + override protected generateCopyAugmentation(Type implType) ''' super(base.«AUGMENTATION_FIELD»); ''' diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderTemplate.xtend index d095d726be..e4e093ae06 100644 --- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderTemplate.xtend +++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderTemplate.xtend @@ -432,6 +432,13 @@ class BuilderTemplate extends AbstractBuilderTemplate { «generateCopyNonKeys(keyProps)» ''' + + override protected CharSequence generateCopyNonKeys(Collection props) ''' + «FOR field : props» + this.«field.fieldName» = base.«field.getterMethodName»(); + «ENDFOR» + ''' + override protected generateCopyAugmentation(Type implType) { val augmentationHolderRef = AugmentationHolder.importedName val typeRef = targetType.importedName diff --git a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGeneratorTest.java b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGeneratorTest.java index bdcda6be6a..6602747408 100644 --- a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGeneratorTest.java +++ b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGeneratorTest.java @@ -18,6 +18,7 @@ import org.junit.Test; import org.opendaylight.mdsal.binding.model.api.GeneratedType; import org.opendaylight.mdsal.binding.model.api.JavaTypeName; import org.opendaylight.mdsal.binding.model.api.MethodSignature; +import org.opendaylight.mdsal.binding.model.api.MethodSignature.ValueMechanics; import org.opendaylight.mdsal.binding.model.api.Type; public class BuilderGeneratorTest { @@ -148,6 +149,7 @@ public class BuilderGeneratorTest { final Type methType = mock(Type.class); doReturn(TYPE_NAME).when(methType).getIdentifier(); doReturn(methType).when(methSign).getReturnType(); + doReturn(ValueMechanics.NORMAL).when(methSign).getMechanics(); return methSign; } } -- 2.36.6