Squash empty lists/maps 36/89236/2
authorRobert Varga <robert.varga@pantheon.tech>
Tue, 21 Apr 2020 13:11:41 +0000 (15:11 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Tue, 21 Apr 2020 19:59:01 +0000 (21:59 +0200)
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 <robert.varga@pantheon.tech>
15 files changed:
binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/GeneratedProperty.java
binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/MethodSignature.java
binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/type/builder/GeneratedPropertyBuilder.java
binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/type/builder/MethodSignatureBuilder.java
binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java
binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/GeneratedPropertyBuilderImpl.java
binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/GeneratedPropertyImpl.java
binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/MethodSignatureBuilderImpl.java
binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/MethodSignatureImpl.java
binding/mdsal-binding-generator-util/src/test/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/GeneratedPropertyTest.java
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/AbstractBuilderTemplate.xtend
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGenerator.java
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderImplTemplate.xtend
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderTemplate.xtend
binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGeneratorTest.java

index 4a9ddb30bd2170fb1c200a44b9fea8411b788b39..011e7ad6121a1c29a0e6aa9dd78c2bcf8750f195 100644 (file)
@@ -8,23 +8,33 @@
 package org.opendaylight.mdsal.binding.model.api;
 
 /**
- * Generated Property extends interface {@link MethodSignature} interface. <br>
- * 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 <code>true</code> if the property is declared as read-only. <br>
-     * If the property has flag <code>isReadOnly == true</code> the property
-     * SHOULD be generated as getter only.
+     * Returns <code>true</code> if the property is declared as read-only. If this {@code true} the property should be
+     * generated with only a getter.
      *
-     * @return <code>true</code> 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();
 }
index b6cb410b36a967ee398d8dd15306148a70705787..dafcfe4e3f3fe89a72725e4437f175f340af1ae4 100644 (file)
@@ -47,6 +47,13 @@ public interface MethodSignature extends TypeMember {
      */
     List<Parameter> 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,
+    }
 }
index 114e37b65157cb381075cc1a9529178509b12ec6..79bedba9b35db5d4fecf1e044fd60fba95be59ea 100644 (file)
@@ -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 TypeMemberBuilder<GeneratedPro
      */
     GeneratedPropertyBuilder setReadOnly(boolean isReadOnly);
 
+    @Beta
+    GeneratedPropertyBuilder setNullifyEmpty(boolean flag);
+
     /**
      * Returns <code>new</code> <i>immutable</i> instance of Generated Property. <br>
      * The <code>definingType</code> param cannot be <code>null</code>. The
index 34c7b3861d7ada88072689697c32332584573259..5e69bef48bdf826a36e376c90fef8f60da516465 100644 (file)
@@ -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<MethodSignatur
      */
     MethodSignatureBuilder setDefault(boolean isDefault);
 
+    @Beta
+    MethodSignatureBuilder setMechanics(ValueMechanics mechanics);
+
     /**
      * Adds Parameter into the List of method parameters. Neither the Name or Type of parameter can be {@code null}.
      *
index 571fbd8a30c13e1d9e01cf8f0ea40c44a7af6ef3..90d6ccf3969845798d77863b7f35ed16d3bde1c2 100644 (file)
@@ -70,6 +70,7 @@ import org.opendaylight.mdsal.binding.model.api.DefaultType;
 import org.opendaylight.mdsal.binding.model.api.GeneratedTransferObject;
 import org.opendaylight.mdsal.binding.model.api.GeneratedType;
 import org.opendaylight.mdsal.binding.model.api.JavaTypeName;
+import org.opendaylight.mdsal.binding.model.api.MethodSignature.ValueMechanics;
 import org.opendaylight.mdsal.binding.model.api.ParameterizedType;
 import org.opendaylight.mdsal.binding.model.api.Restrictions;
 import org.opendaylight.mdsal.binding.model.api.Type;
@@ -335,7 +336,7 @@ abstract class AbstractTypeGenerator {
                 listType = listTypeFor(genType);
             }
 
-            constructGetter(parent, listType, node);
+            constructGetter(parent, listType, node).setMechanics(ValueMechanics.NULLIFY_EMPTY);
             constructNonnull(parent, listType, node);
 
             actionsToGenType(context, genType, node, keyTypeBuilder, inGrouping);
index 07e5492297d211a5ddf3448461a49444f1af6d93..5c9665e086a7269f4abb38a34b058d340c6e3c2c 100644 (file)
@@ -17,10 +17,12 @@ public final class GeneratedPropertyBuilderImpl extends AbstractTypeMemberBuilde
         implements GeneratedPropertyBuilder {
     private String value;
     private boolean readOnly;
+    private boolean nullifyEmpty;
 
     public GeneratedPropertyBuilderImpl(final String name) {
         super(name);
         this.readOnly = true;
+        this.nullifyEmpty = false;
     }
 
     @Override
@@ -35,6 +37,12 @@ public final class GeneratedPropertyBuilderImpl extends AbstractTypeMemberBuilde
         return this;
     }
 
+    @Override
+    public GeneratedPropertyBuilderImpl setNullifyEmpty(final boolean flag) {
+        this.nullifyEmpty = flag;
+        return this;
+    }
+
     @Override
     protected GeneratedPropertyBuilderImpl thisInstance() {
         return this;
@@ -44,7 +52,7 @@ public final class GeneratedPropertyBuilderImpl extends AbstractTypeMemberBuilde
     public GeneratedProperty toInstance(final Type definingType) {
         final List<AnnotationType> 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
index c83aefb7a6f5c9f2ef5871170fa3260b91ae68b2..24f81399f1dcbaaedeaa5de474f7a6615bddd42f 100644 (file)
@@ -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<AnnotationType> 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();
index f82bbfa9d74458fabd1230cb111be2bc9af8b779..1eff5819533bbe4fd1f68229f9814545ad7d7bb2 100644 (file)
@@ -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<MethodS
 
     private List<MethodSignature.Parameter> parameters = Collections.emptyList();
     private List<MethodSignature.Parameter> unmodifiableParams = Collections.emptyList();
+    private ValueMechanics mechanics = ValueMechanics.NORMAL;
     private boolean isAbstract;
     private boolean isDefault;
 
@@ -40,6 +44,13 @@ final class MethodSignatureBuilderImpl extends AbstractTypeMemberBuilder<MethodS
         return this;
     }
 
+
+    @Override
+    public MethodSignatureBuilder setMechanics(final ValueMechanics newMechanics) {
+        this.mechanics = requireNonNull(newMechanics);
+        return this;
+    }
+
     @Override
     public MethodSignatureBuilder addParameter(final Type type, final String name) {
         this.parameters = LazyCollections.lazyAdd(this.parameters, new MethodParameterImpl(name, type));
@@ -56,7 +67,7 @@ final class MethodSignatureBuilderImpl extends AbstractTypeMemberBuilder<MethodS
     public MethodSignature toInstance(final Type definingType) {
         final List<AnnotationType> 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
index 867e905b8ee7c21ae29fb32267b9cbf6ae77256e..5a211b9d415333ed69b914b3c11aad70f63179b6 100644 (file)
@@ -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<Parameter> params;
+    private final ValueMechanics mechanics;
     private final boolean isAbstract;
     private final boolean isDefault;
 
+    @VisibleForTesting
     MethodSignatureImpl(final Type definingType, final String name, final List<AnnotationType> annotations,
         final String comment, final AccessModifier accessModifier, final Type returnType,
         final List<Parameter> 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<AnnotationType> annotations,
             final String comment, final AccessModifier accessModifier, final Type returnType,
             final List<Parameter> 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;
index 5e3ef4e37f58fb07549aab8ef57e91a82b3a9344..eba0b9b297108345c2802d5fddd27a217e8ca347 100644 (file)
@@ -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());
index 010450485b20ce354bd291c6aabd318d09b5d82f..11e8629f95cd05a78fbddea37921cc4708b891ba 100644 (file)
@@ -152,14 +152,10 @@ abstract class AbstractBuilderTemplate extends BaseTemplate {
         return generateDeprecatedAnnotation(found)
     }
 
-    def protected final CharSequence generateCopyNonKeys(Collection<GeneratedProperty> props) '''
-        «FOR field : props»
-            this.«field.fieldName» = base.«field.getterMethodName»();
-        «ENDFOR»
-    '''
-
     def protected abstract CharSequence generateCopyKeys(List<GeneratedProperty> keyProps)
 
+    def protected abstract CharSequence generateCopyNonKeys(Collection<GeneratedProperty> props)
+
     def protected abstract CharSequence generateCopyAugmentation(Type implType)
 
     def protected abstract CharSequence generateDeprecatedAnnotation(AnnotationType ann)
index a632394a7569f3fa8a7f782dacd6f2efaeb9ca3d..b7ac63f7b2450432f5c7f040bb441eb07e075234 100644 (file)
@@ -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);
     }
 }
index 5a37e75809aafbc86c4da39b0ad3ec0a4a9d1af6..53bffa4f18f504f946880282d73b985f1fe3f9d6 100644 (file)
@@ -147,6 +147,16 @@ class BuilderImplTemplate extends AbstractBuilderTemplate {
         «ENDFOR»
     '''
 
+    override protected  CharSequence generateCopyNonKeys(Collection<GeneratedProperty> 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»);
     '''
index d095d726be2b9dca6061c7272ad2f656e5e69847..e4e093ae06554308394910dd888e6973647429fb 100644 (file)
@@ -432,6 +432,13 @@ class BuilderTemplate extends AbstractBuilderTemplate {
         «generateCopyNonKeys(keyProps)»
     '''
 
+
+    override protected  CharSequence generateCopyNonKeys(Collection<GeneratedProperty> props) '''
+        «FOR field : props»
+            this.«field.fieldName» = base.«field.getterMethodName»();
+        «ENDFOR»
+    '''
+
     override protected generateCopyAugmentation(Type implType) {
         val augmentationHolderRef = AugmentationHolder.importedName
         val typeRef = targetType.importedName
index bdcda6be6ab7a44cef778c76609e98a15878464a..6602747408725f7ca202481991e8738b57b170cb 100644 (file)
@@ -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;
     }
 }