From 1ee8e403b9c9eb9e8596b5e89cc5ed47676eb71c Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 19 Oct 2020 15:39:51 +0200 Subject: [PATCH] Expose property name when checking key components Experience with null enforcement of key components indicates we should carry the name of the property in the exception message -- otherwise the string is not really helpful. This patch re-organizes the checking so that we do that, via newly-introduced CodeHelpers.requireKeyProp() method. Since we are in the area, also codegen methods' handling of arrays by creating an explicit 'cloneCall()' utility method. JIRA: MDSAL-599 Change-Id: If9a87b4976ecdcd09e1fdd83ddfaf03ab4d09a85 Signed-off-by: Robert Varga --- .../java/api/generator/BaseTemplate.xtend | 22 ++++++------ .../java/api/generator/ClassTemplate.xtend | 8 ++--- .../java/api/generator/JavaFileTemplate.java | 10 ++++++ .../java/api/generator/ListKeyTemplate.xtend | 35 +++++-------------- .../yangtools/yang/binding/CodeHelpers.java | 17 ++++++++- 5 files changed, 47 insertions(+), 45 deletions(-) diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BaseTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BaseTemplate.xtend index e6a0a9be30..ed6bcea2c1 100644 --- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BaseTemplate.xtend +++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BaseTemplate.xtend @@ -124,18 +124,16 @@ abstract class BaseTemplate extends JavaFileTemplate { * generated property with data about field which is generated as the getter method * @return string with the getter method source code in JAVA format */ - protected def getterMethod(GeneratedProperty field) { - ''' - public «field.returnType.importedName» «field.getterMethodName»() { - «val fieldName = field.fieldName» - «IF field.returnType.name.endsWith("[]")» - return «fieldName» == null ? null : «fieldName».clone(); - «ELSE» - return «fieldName»; - «ENDIF» - } - ''' - } + protected def getterMethod(GeneratedProperty field) ''' + public «field.returnType.importedName» «field.getterMethodName»() { + «val fieldName = field.fieldName» + «IF field.returnType.name.endsWith("[]")» + return «fieldName» == null ? null : «fieldName».clone(); + «ELSE» + return «fieldName»; + «ENDIF» + } + ''' final protected def getterMethodName(GeneratedProperty field) { val prefix = if(field.returnType.equals(Types.BOOLEAN)) "is" else "get" diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ClassTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ClassTemplate.xtend index 4ee8d603f9..71368811f2 100644 --- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ClassTemplate.xtend +++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ClassTemplate.xtend @@ -214,7 +214,7 @@ class ClassTemplate extends BaseTemplate { def private scalarTypeObjectValue(GeneratedProperty field) ''' @«OVERRIDE.importedName» public «field.returnType.importedName» «BindingMapping.SCALAR_TYPE_OBJECT_GET_VALUE_NAME»() { - return «field.fieldName»«IF field.returnType.name.endsWith("[]")».clone()«ENDIF»; + return «field.fieldName»«field.cloneCall»; } ''' @@ -306,11 +306,7 @@ class ClassTemplate extends BaseTemplate { «FOR p : properties» «val fieldName = p.fieldName» - «IF p.returnType.name.endsWith("[]")» - this.«fieldName» = «fieldName».clone(); - «ELSE» - this.«fieldName» = «fieldName»; - «ENDIF» + this.«fieldName» = «fieldName»«p.cloneCall»; «ENDFOR» } ''' diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/JavaFileTemplate.java b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/JavaFileTemplate.java index cec3079586..ec891f2625 100644 --- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/JavaFileTemplate.java +++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/JavaFileTemplate.java @@ -241,6 +241,16 @@ class JavaFileTemplate { return null; } + /** + * Generate a call to {@link Object#clone()} if target field represents an array. Returns an empty string otherwise. + * + * @param property Generated property + * @return The string used to clone the property, or an empty string + */ + static final String cloneCall(final GeneratedProperty property) { + return property.getReturnType().getName().endsWith("[]") ? ".clone()" : ""; + } + /** * Returns set of method signature instances which contains all the methods of the genType * and all the methods of the implemented interfaces. diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ListKeyTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ListKeyTemplate.xtend index 55f099e95d..0087e95aeb 100644 --- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ListKeyTemplate.xtend +++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ListKeyTemplate.xtend @@ -13,8 +13,7 @@ import org.opendaylight.mdsal.binding.model.api.GeneratedTransferObject /** * Template for generating JAVA class. */ -class ListKeyTemplate extends ClassTemplate { - +final class ListKeyTemplate extends ClassTemplate { /** * Creates instance of this class with concrete genType. * @@ -24,37 +23,21 @@ class ListKeyTemplate extends ClassTemplate { super(genType) } - - override final allValuesConstructor() ''' + override allValuesConstructor() ''' public «type.name»(«allProperties.asNonNullArgumentsDeclaration») { «FOR p : allProperties» - «CODEHELPERS.importedName».requireValue(«p.fieldName»); + «val fieldName = p.fieldName» + this.«fieldName» = «CODEHELPERS.importedName».requireKeyProp(«fieldName», "«p.name»")«p.cloneCall»; «ENDFOR» «FOR p : properties» «generateRestrictions(type, p.fieldName, p.returnType)» «ENDFOR» - - «FOR p : allProperties» - «val fieldName = p.fieldName» - «IF p.returnType.name.endsWith("[]")» - this.«fieldName» = «fieldName».clone(); - «ELSE» - this.«fieldName» = «fieldName»; - «ENDIF» - «ENDFOR» } ''' - override final getterMethod(GeneratedProperty field) { - ''' - public «field.returnType.importedNonNull» «field.getterMethodName»() { - «val fieldName = field.fieldName» - «IF field.returnType.name.endsWith("[]")» - return «fieldName».clone(); - «ELSE» - return «fieldName»; - «ENDIF» - } - ''' - } + override getterMethod(GeneratedProperty field) ''' + public «field.returnType.importedNonNull» «field.getterMethodName»() { + return «field.fieldName»«field.cloneCall»; + } + ''' } diff --git a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java index 5d224d300a..1952e10cf5 100644 --- a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java +++ b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java @@ -53,13 +53,28 @@ public final class CodeHelpers { checkArgument(expression, "expected one of: %s \n%but was: %s", options, value); } + /** + * A shortcut for {@code Preconditions.checkNotNull(value, "Key component \"%s\" must not be null", name)}. + * + * @param value Value itself + * @param name Name of the value + * @return Non-null value + * @throws NullPointerException if value is null + */ + public static @NonNull T requireKeyProp(final @Nullable T value, final @NonNull String name) { + if (value == null) { + throw new NullPointerException("Key component \"" + name + "\" may not be null"); + } + return value; + } + /** * A shortcut for {@code Objects.requireNonNull(value, "Supplied value may not be null")}. * * @param value Value itself * @throws NullPointerException if value is null */ - public static void requireValue(@Nullable final Object value) { + public static void requireValue(final @Nullable Object value) { requireNonNull(value, "Supplied value may not be null"); } -- 2.36.6