From d80486297d4a94043f8058207353081320274df1 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Wed, 28 Feb 2018 23:51:02 +0100 Subject: [PATCH] Export declared regexes to generated code This patch exports declared pattern regular expressions to binding generators, thus allowing them to report violations using the declared string rather than the mangled Pattern string we use to implement enforcement. As it turns out there is place for improvement with generated code size, as decoding pattern negation requires a bit of repetition as well as needing to capture additional strings. Since we can take advantage of common code library, do not encapsulate single expressions in an array. This patch battles those ill effects by exposing CodeHelpers class from yang-binding, which hosts common code blocks for reuse by generated code. This means classes generated by this and newer codegen plugins require a matching or newer version of yang-binding at runtime. JIRA: MDSAL-313 Change-Id: I8abb33660c5182703f2db5faf0eee7e08ce15f06 Signed-off-by: Robert Varga --- binding/mdsal-binding-generator-impl/pom.xml | 4 - .../binding/yang/types/TypeProviderImpl.java | 166 ++++++++---------- .../impl/GeneratedTypesStringTest.java | 8 +- .../binding/model/util/TypeConstants.java | 5 +- .../mdsal-binding-java-api-generator/pom.xml | 20 +-- .../java/api/generator/BuilderTemplate.xtend | 15 +- .../java/api/generator/ClassTemplate.xtend | 59 +++---- .../binding/java/api/generator/Constants.java | 11 +- .../java/api/generator/GeneratorUtil.java | 4 +- .../generator/test/CompilationTestUtils.java | 9 +- .../test/TypedefCompilationTest.java | 2 +- .../yangtools/yang/binding/CodeHelpers.java | 121 +++++++++++++ 12 files changed, 258 insertions(+), 166 deletions(-) create mode 100644 binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java diff --git a/binding/mdsal-binding-generator-impl/pom.xml b/binding/mdsal-binding-generator-impl/pom.xml index 43ffb739a2..52bd73f261 100644 --- a/binding/mdsal-binding-generator-impl/pom.xml +++ b/binding/mdsal-binding-generator-impl/pom.xml @@ -74,10 +74,6 @@ test - - org.apache.commons - commons-text - com.google.guava guava diff --git a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/yang/types/TypeProviderImpl.java b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/yang/types/TypeProviderImpl.java index b3f6061656..1f49f16943 100644 --- a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/yang/types/TypeProviderImpl.java +++ b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/yang/types/TypeProviderImpl.java @@ -14,7 +14,8 @@ import static org.opendaylight.yangtools.yang.model.util.SchemaContextUtil.findP import com.google.common.base.Preconditions; import com.google.common.base.Strings; -import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.common.io.BaseEncoding; import java.io.Serializable; @@ -33,7 +34,6 @@ import java.util.Set; import java.util.TreeMap; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.apache.commons.text.StringEscapeUtils; import org.opendaylight.mdsal.binding.generator.spi.TypeProvider; import org.opendaylight.mdsal.binding.model.api.AccessModifier; import org.opendaylight.mdsal.binding.model.api.ConcreteType; @@ -193,7 +193,8 @@ public final class TypeProviderImpl implements TypeProvider { * */ @Override - public Type javaTypeForSchemaDefinitionType(final TypeDefinition typeDefinition, final SchemaNode parentNode, final Restrictions r) { + public Type javaTypeForSchemaDefinitionType(final TypeDefinition typeDefinition, final SchemaNode parentNode, + final Restrictions r) { Preconditions.checkArgument(typeDefinition != null, "Type Definition cannot be NULL!"); Preconditions.checkArgument(typeDefinition.getQName() != null, "Type Definition cannot have non specified QName (QName cannot be NULL!)"); @@ -206,7 +207,8 @@ public final class TypeProviderImpl implements TypeProvider { // and generated an enclosing ExtendedType to hold any range constraints. The new parser instantiates // a base type which holds these constraints. if (typeDefinition instanceof DecimalTypeDefinition) { - final Type ret = BaseYangTypes.BASE_YANG_TYPES_PROVIDER.javaTypeForSchemaDefinitionType(typeDefinition, parentNode, r); + final Type ret = BaseYangTypes.BASE_YANG_TYPES_PROVIDER.javaTypeForSchemaDefinitionType(typeDefinition, + parentNode, r); if (ret != null) { return ret; } @@ -232,7 +234,8 @@ public final class TypeProviderImpl implements TypeProvider { final GeneratedTransferObject gto = (GeneratedTransferObject) returnType; final Module module = findParentModule(schemaContext, parentNode); final String basePackageName = BindingMapping.getRootPackageName(module.getQNameModule()); - final String packageName = BindingGeneratorUtil.packageNameForGeneratedType(basePackageName, typeDefinition.getPath()); + final String packageName = BindingGeneratorUtil.packageNameForGeneratedType(basePackageName, + typeDefinition.getPath()); final String genTOName = BindingMapping.getClassName(typedefName); final String name = packageName + "." + genTOName; if (!returnType.getFullyQualifiedName().equals(name)) { @@ -242,14 +245,15 @@ public final class TypeProviderImpl implements TypeProvider { return returnType; } - private static GeneratedTransferObject shadedTOWithRestrictions(final GeneratedTransferObject gto, final Restrictions r) { + private static GeneratedTransferObject shadedTOWithRestrictions(final GeneratedTransferObject gto, + final Restrictions r) { final GeneratedTOBuilder gtob = new GeneratedTOBuilderImpl(gto.getPackageName(), gto.getName()); final GeneratedTransferObject parent = gto.getSuperType(); if (parent != null) { gtob.setExtendsType(parent); } gtob.setRestrictions(r); - for (final GeneratedProperty gp : gto.getProperties()) { + for (GeneratedProperty gp : gto.getProperties()) { final GeneratedPropertyBuilder gpb = gtob.addProperty(gp.getName()); gpb.setValue(gp.getValue()); gpb.setReadOnly(gp.isReadOnly()); @@ -346,7 +350,8 @@ public final class TypeProviderImpl implements TypeProvider { final Module module = findParentModule(schemaContext, typeDefinition); final Restrictions r = BindingGeneratorUtil.getRestrictions(typeDefinition); if (module != null) { - final Map, Map> modulesByDate = genTypeDefsContextMap.get(module.getName()); + final Map, Map> modulesByDate = genTypeDefsContextMap.get( + module.getName()); final Map genTOs = modulesByDate.get(module.getRevision()); if (genTOs != null) { returnType = genTOs.get(typedefName); @@ -385,7 +390,7 @@ public final class TypeProviderImpl implements TypeProvider { final QName baseIdQName = identities.iterator().next().getQName(); final Module module = schemaContext.findModule(baseIdQName.getModule()).orElse(null); IdentitySchemaNode identity = null; - for (final IdentitySchemaNode id : module.getIdentities()) { + for (IdentitySchemaNode id : module.getIdentities()) { if (id.getQName().equals(baseIdQName)) { identity = id; } @@ -393,7 +398,8 @@ public final class TypeProviderImpl implements TypeProvider { Preconditions.checkArgument(identity != null, "Target identity '" + baseIdQName + "' do not exists"); final String basePackageName = BindingMapping.getRootPackageName(module.getQNameModule()); - final String packageName = BindingGeneratorUtil.packageNameForGeneratedType(basePackageName, identity.getPath()); + final String packageName = BindingGeneratorUtil.packageNameForGeneratedType(basePackageName, + identity.getPath()); final String genTypeName = BindingMapping.getClassName(identity.getQName()); final Type baseType = Types.typeForClass(Class.class); @@ -481,7 +487,8 @@ public final class TypeProviderImpl implements TypeProvider { final Module module = findParentModule(schemaContext, parentNode); if (module != null) { - final Map, Map> modulesByDate = genTypeDefsContextMap.get(module.getName()); + final Map, Map> modulesByDate = genTypeDefsContextMap.get( + module.getName()); final Map genTOs = modulesByDate.get(module.getRevision()); if (genTOs != null) { return genTOs.get(typeDefinition.getQName().getLocalName()); @@ -634,7 +641,8 @@ public final class TypeProviderImpl implements TypeProvider { *
  • if name of enumTypeDef equal null
    • * */ - private Enumeration provideTypeForEnum(final EnumTypeDefinition enumTypeDef, final String enumName, final SchemaNode parentNode) { + private Enumeration provideTypeForEnum(final EnumTypeDefinition enumTypeDef, final String enumName, + final SchemaNode parentNode) { Preconditions.checkArgument(enumTypeDef != null, "EnumTypeDefinition reference cannot be NULL!"); Preconditions.checkArgument(enumTypeDef.getValues() != null, "EnumTypeDefinition MUST contain at least ONE value definition!"); @@ -680,7 +688,8 @@ public final class TypeProviderImpl implements TypeProvider { * * */ - private static Enumeration addInnerEnumerationToTypeBuilder(final EnumTypeDefinition enumTypeDef, final String enumName, final GeneratedTypeBuilderBase typeBuilder) { + private static Enumeration addInnerEnumerationToTypeBuilder(final EnumTypeDefinition enumTypeDef, + final String enumName, final GeneratedTypeBuilderBase typeBuilder) { Preconditions.checkArgument(enumTypeDef != null, "EnumTypeDefinition reference cannot be NULL!"); Preconditions.checkArgument(enumTypeDef.getValues() != null, "EnumTypeDefinition MUST contain at least ONE value definition!"); @@ -739,7 +748,7 @@ public final class TypeProviderImpl implements TypeProvider { Preconditions.checkArgument(modules != null, "Set of Modules cannot be NULL!"); final List modulesSortedByDependency = ModuleDependencySort.sort(modules); - for (final Module module : modulesSortedByDependency) { + for (Module module : modulesSortedByDependency) { Map, Map> dateTypeMap = genTypeDefsContextMap.get(module.getName()); if (dateTypeMap == null) { dateTypeMap = new HashMap<>(); @@ -748,16 +757,13 @@ public final class TypeProviderImpl implements TypeProvider { genTypeDefsContextMap.put(module.getName(), dateTypeMap); } - for (final Module module : modulesSortedByDependency) { + for (Module module : modulesSortedByDependency) { if (module != null) { final String basePackageName = BindingMapping.getRootPackageName(module.getQNameModule()); if (basePackageName != null) { final List> typeDefinitions = TypedefResolver.getAllTypedefs(module); - final List> listTypeDefinitions = sortTypeDefinitionAccordingDepth(typeDefinitions); - if (listTypeDefinitions != null) { - for (final TypeDefinition typedef : listTypeDefinitions) { - typedefToGeneratedType(basePackageName, module, typedef); - } + for (TypeDefinition typedef : sortTypeDefinitionAccordingDepth(typeDefinitions)) { + typedefToGeneratedType(basePackageName, module, typedef); } } } @@ -906,9 +912,7 @@ public final class TypeProviderImpl implements TypeProvider { Preconditions.checkState(!builders.isEmpty(), "No GeneratedTOBuilder objects generated from union %s", typedef); final GeneratedTOBuilder resultTOBuilder = builders.remove(0); - for (final GeneratedTOBuilder genTOBuilder : builders) { - resultTOBuilder.addEnclosingTransferObject(genTOBuilder); - } + builders.forEach(resultTOBuilder::addEnclosingTransferObject); resultTOBuilder.addProperty("value").setReturnType(Types.CHAR_ARRAY); return resultTOBuilder; @@ -959,14 +963,17 @@ public final class TypeProviderImpl implements TypeProvider { generatedTOBuilders.add(unionGenTOBuilder); unionGenTOBuilder.setIsUnion(true); - final List regularExpressions = new ArrayList<>(); - for (final TypeDefinition unionType : unionTypes) { + + // Pattern string is the key, XSD regex is the value. The reason for this choice is that the pattern carries + // also negation information and hence guarantees uniqueness. + final Map expressions = new HashMap<>(); + for (TypeDefinition unionType : unionTypes) { final String unionTypeName = unionType.getQName().getLocalName(); // If we have a base type we should follow the type definition backwards, except for identityrefs, as those // do not follow type encapsulation -- we use the general case for that. if (unionType.getBaseType() != null && !(unionType instanceof IdentityrefTypeDefinition)) { - resolveExtendedSubtypeAsUnion(unionGenTOBuilder, unionType, regularExpressions, parentNode); + resolveExtendedSubtypeAsUnion(unionGenTOBuilder, unionType, expressions, parentNode); } else if (unionType instanceof UnionTypeDefinition) { generatedTOBuilders.addAll(resolveUnionSubtypeAsUnion(unionGenTOBuilder, (UnionTypeDefinition) unionType, basePackageName, parentNode)); @@ -979,9 +986,7 @@ public final class TypeProviderImpl implements TypeProvider { updateUnionTypeAsProperty(unionGenTOBuilder, javaType, unionTypeName); } } - if (!regularExpressions.isEmpty()) { - addStringRegExAsConstant(unionGenTOBuilder, regularExpressions); - } + addStringRegExAsConstant(unionGenTOBuilder, expressions); storeGenTO(typedef, unionGenTOBuilder, parentNode); @@ -1039,46 +1044,46 @@ public final class TypeProviderImpl implements TypeProvider { * @param unionSubtype * type definition of the ExtendedType type which * represents union subtype - * @param regularExpressions + * @param expressions * list of strings with the regular expressions * @param parentNode * parent Schema Node for Extended Subtype * */ private void resolveExtendedSubtypeAsUnion(final GeneratedTOBuilder parentUnionGenTOBuilder, - final TypeDefinition unionSubtype, final List regularExpressions, final SchemaNode parentNode) { + final TypeDefinition unionSubtype, final Map expressions, final SchemaNode parentNode) { final String unionTypeName = unionSubtype.getQName().getLocalName(); final Type genTO = findGenTO(unionTypeName, unionSubtype); if (genTO != null) { updateUnionTypeAsProperty(parentUnionGenTOBuilder, genTO, genTO.getName()); - } else { - final TypeDefinition baseType = baseTypeDefForExtendedType(unionSubtype); - if (unionTypeName.equals(baseType.getQName().getLocalName())) { - final Type javaType = BaseYangTypes.BASE_YANG_TYPES_PROVIDER.javaTypeForSchemaDefinitionType(baseType, - parentNode); - if (javaType != null) { - updateUnionTypeAsProperty(parentUnionGenTOBuilder, javaType, unionTypeName); - } - } else if (baseType instanceof LeafrefTypeDefinition) { - final Type javaType = javaTypeForSchemaDefinitionType(baseType, parentNode); - boolean typeExist = false; - for (final GeneratedPropertyBuilder generatedPropertyBuilder : parentUnionGenTOBuilder - .getProperties()) { - final Type origType = ((GeneratedPropertyBuilderImpl) generatedPropertyBuilder).getReturnType(); - if (origType != null && javaType != null && javaType == origType) { - typeExist = true; - break; - } - } - if (!typeExist && javaType != null) { - updateUnionTypeAsProperty(parentUnionGenTOBuilder, javaType, new StringBuilder(javaType.getName()) - .append(parentUnionGenTOBuilder.getName()).append("Value").toString()); + return; + } + + final TypeDefinition baseType = baseTypeDefForExtendedType(unionSubtype); + if (unionTypeName.equals(baseType.getQName().getLocalName())) { + final Type javaType = BaseYangTypes.BASE_YANG_TYPES_PROVIDER.javaTypeForSchemaDefinitionType(baseType, + parentNode); + if (javaType != null) { + updateUnionTypeAsProperty(parentUnionGenTOBuilder, javaType, unionTypeName); + } + } else if (baseType instanceof LeafrefTypeDefinition) { + final Type javaType = javaTypeForSchemaDefinitionType(baseType, parentNode); + boolean typeExist = false; + for (GeneratedPropertyBuilder generatedPropertyBuilder : parentUnionGenTOBuilder.getProperties()) { + final Type origType = ((GeneratedPropertyBuilderImpl) generatedPropertyBuilder).getReturnType(); + if (origType != null && javaType != null && javaType == origType) { + typeExist = true; + break; } } - if (baseType instanceof StringTypeDefinition) { - regularExpressions.addAll(resolveRegExpressionsFromTypedef(unionSubtype)); + if (!typeExist && javaType != null) { + updateUnionTypeAsProperty(parentUnionGenTOBuilder, javaType, + javaType.getName() + parentUnionGenTOBuilder.getName() + "Value"); } } + if (baseType instanceof StringTypeDefinition) { + expressions.putAll(resolveRegExpressionsFromTypedef(unionSubtype)); + } } /** @@ -1220,7 +1225,7 @@ public final class TypeProviderImpl implements TypeProvider { final List bitList = bitsTypeDefinition.getBits(); GeneratedPropertyBuilder genPropertyBuilder; - for (final Bit bit : bitList) { + for (Bit bit : bitList) { final String name = bit.getName(); genPropertyBuilder = genTOBuilder.addProperty(BindingMapping.getPropertyName(name)); genPropertyBuilder.setReadOnly(true); @@ -1247,19 +1252,15 @@ public final class TypeProviderImpl implements TypeProvider { * if typedef equals null * */ - private static List resolveRegExpressionsFromTypedef(final TypeDefinition typedef) { - Preconditions.checkArgument(typedef != null, "typedef can't be null"); - - final List patternConstraints; - if (typedef instanceof StringTypeDefinition) { - // FIXME: run diff against base - patternConstraints = ((StringTypeDefinition) typedef).getPatternConstraints(); - } else { - patternConstraints = ImmutableList.of(); + private static Map resolveRegExpressionsFromTypedef(final TypeDefinition typedef) { + if (!(typedef instanceof StringTypeDefinition)) { + return ImmutableMap.of(); } - final List regExps = new ArrayList<>(patternConstraints.size()); - for (final PatternConstraint patternConstraint : patternConstraints) { + // TODO: run diff against base ? + final List patternConstraints = ((StringTypeDefinition) typedef).getPatternConstraints(); + final Map regExps = Maps.newHashMapWithExpectedSize(patternConstraints.size()); + for (PatternConstraint patternConstraint : patternConstraints) { String regEx = patternConstraint.getJavaPatternString(); // The pattern can be inverted @@ -1268,8 +1269,7 @@ public final class TypeProviderImpl implements TypeProvider { regEx = applyModifier(optModifier.get(), regEx); } - final String modifiedRegEx = StringEscapeUtils.escapeJava(regEx); - regExps.add(modifiedRegEx); + regExps.put(regEx, patternConstraint.getRegularExpressionString()); } return regExps; @@ -1293,24 +1293,14 @@ public final class TypeProviderImpl implements TypeProvider { * @param genTOBuilder * generated TO builder to which are * regular expressions added - * @param regularExpressions + * @param expressions * list of string which represent regular expressions - * @throws IllegalArgumentException - *
      - *
    • if genTOBuilder equals null
      • - *
    • if regularExpressions equals null
      • - *
    */ - private static void addStringRegExAsConstant(final GeneratedTOBuilder genTOBuilder, final List regularExpressions) { - if (genTOBuilder == null) { - throw new IllegalArgumentException("Generated transfer object builder can't be null"); - } - if (regularExpressions == null) { - throw new IllegalArgumentException("List of regular expressions can't be null"); - } - if (!regularExpressions.isEmpty()) { + private static void addStringRegExAsConstant(final GeneratedTOBuilder genTOBuilder, + final Map expressions) { + if (!expressions.isEmpty()) { genTOBuilder.addConstant(Types.listTypeFor(BaseYangTypes.STRING_TYPE), TypeConstants.PATTERN_CONSTANT_NAME, - ImmutableList.copyOf(regularExpressions)); + ImmutableMap.copyOf(expressions)); } } @@ -1419,7 +1409,7 @@ public final class TypeProviderImpl implements TypeProvider { final List> sortedTypeDefinition = new ArrayList<>(); final Map>> typeDefinitionsDepths = new TreeMap<>(); - for (final TypeDefinition unsortedTypeDefinition : unsortedTypeDefinitions) { + for (TypeDefinition unsortedTypeDefinition : unsortedTypeDefinitions) { final Integer depth = getTypeDefinitionDepth(unsortedTypeDefinition); List> typeDefinitionsConcreteDepth = typeDefinitionsDepths.get(depth); if (typeDefinitionsConcreteDepth == null) { @@ -1430,7 +1420,7 @@ public final class TypeProviderImpl implements TypeProvider { } // SortedMap guarantees order corresponding to keys in ascending order - for (final List> v : typeDefinitionsDepths.values()) { + for (List> v : typeDefinitionsDepths.values()) { sortedTypeDefinition.addAll(v); } @@ -1463,7 +1453,7 @@ public final class TypeProviderImpl implements TypeProvider { final List> childTypeDefinitions = ((UnionTypeDefinition) baseType).getTypes(); int maxChildDepth = 0; int childDepth = 1; - for (final TypeDefinition childTypeDefinition : childTypeDefinitions) { + for (TypeDefinition childTypeDefinition : childTypeDefinitions) { childDepth = childDepth + getTypeDefinitionDepth(childTypeDefinition); if (childDepth > maxChildDepth) { maxChildDepth = childDepth; @@ -1721,7 +1711,7 @@ public final class TypeProviderImpl implements TypeProvider { Module module = null; final Set modules = schemaContext.findModules(typeQName.getNamespace()); if (modules.size() > 1) { - for (final Module m : modules) { + for (Module m : modules) { if (m.getRevision().equals(typeQName.getRevision())) { module = m; break; diff --git a/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/GeneratedTypesStringTest.java b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/GeneratedTypesStringTest.java index fb129d3654..c2069720e6 100644 --- a/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/GeneratedTypesStringTest.java +++ b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/GeneratedTypesStringTest.java @@ -11,6 +11,8 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import java.util.List; +import java.util.Map; +import java.util.Map.Entry; import org.junit.Test; import org.opendaylight.mdsal.binding.generator.api.BindingGenerator; import org.opendaylight.mdsal.binding.model.api.Constant; @@ -78,14 +80,14 @@ public class GeneratedTypesStringTest { break; } - if (con.getValue() instanceof List) { + if (con.getValue() instanceof Map) { constantRegExListValueOK = true; } else { break; } - for (Object obj : (List) con.getValue()) { - if (!(obj instanceof String)) { + for (Entry e : ((Map) con.getValue()).entrySet()) { + if (!(e.getKey() instanceof String) || !(e.getKey() instanceof String)) { noStringInReqExListFound = true; break; } diff --git a/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/TypeConstants.java b/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/TypeConstants.java index c7f94ac02d..9cd5ec3a27 100644 --- a/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/TypeConstants.java +++ b/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/TypeConstants.java @@ -8,14 +8,13 @@ package org.opendaylight.mdsal.binding.model.util; /** - * * Contains constants used in relations with Type. */ public final class TypeConstants { /** - * Name of the class constant which hold list of the regular expression - * strings. + * Name of the class constant which holds the map of regular expressions that need to be enforced on the string + * value. The map is keyed by Pattern-compatible string and values are XSD-compatible strings. */ public static final String PATTERN_CONSTANT_NAME = "PATTERN_CONSTANTS"; diff --git a/binding/mdsal-binding-java-api-generator/pom.xml b/binding/mdsal-binding-java-api-generator/pom.xml index cbd393cf27..8323ac604d 100644 --- a/binding/mdsal-binding-java-api-generator/pom.xml +++ b/binding/mdsal-binding-java-api-generator/pom.xml @@ -42,20 +42,10 @@ - - org.opendaylight.mdsal - mdsal-binding-generator-impl - test - org.opendaylight.mdsal mdsal-binding-generator-util - - junit - junit - test - org.eclipse.xtend org.eclipse.xtend.lib @@ -64,6 +54,10 @@ com.google.guava guava + + org.apache.commons + commons-text + org.sonatype.plexus plexus-build-api @@ -82,6 +76,7 @@ + org.mockito mockito-core @@ -90,6 +85,11 @@ org.opendaylight.yangtools yang-test-util + + org.opendaylight.mdsal + mdsal-binding-generator-impl + test + 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 4f5049915c..16cf52c024 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 @@ -34,6 +34,7 @@ import org.opendaylight.yangtools.yang.binding.Augmentable import org.opendaylight.yangtools.yang.binding.AugmentationHolder import org.opendaylight.yangtools.yang.binding.DataObject import org.opendaylight.yangtools.yang.binding.Identifiable +import org.opendaylight.yangtools.yang.binding.CodeHelpers /** * Template for generating JAVA builder classes. @@ -322,12 +323,7 @@ class BuilderTemplate extends BaseTemplate { «FOR impl : ifc.getAllIfcs» «generateIfCheck(impl, done)» «ENDFOR» - if (!isValidArg) { - throw new IllegalArgumentException( - "expected one of: «ifc.getAllIfcs.toListOfNames» \n" + - "but was: " + arg - ); - } + «CodeHelpers.importedName».validValue(isValidArg, arg, "«ifc.getAllIfcs.toListOfNames»"); } «ENDIF» «ENDIF» @@ -335,7 +331,7 @@ class BuilderTemplate extends BaseTemplate { def private generateMethodFieldsFromComment(GeneratedType type) ''' /** - *Set fields from given grouping argument. Valid argument is instance of one of following types: + * Set fields from given grouping argument. Valid argument is instance of one of following types: *
      «FOR impl : type.getAllIfcs» *
    • «impl.fullyQualifiedName»
      • @@ -604,10 +600,7 @@ class BuilderTemplate extends BaseTemplate { @SuppressWarnings("unchecked") «IF addOverride»@Override«ENDIF» public E get«augmentField.name.toFirstUpper»(«Class.importedName» augmentationType) { - if (augmentationType == null) { - throw new IllegalArgumentException("Augmentation Type reference cannot be NULL!"); - } - return (E) «augmentField.name».get(augmentationType); + return (E) «augmentField.name».get(«CodeHelpers.importedName».nonNullValue(augmentationType, "augmentationType")); } «ENDIF» ''' 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 54e2149f6f..3810644051 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 @@ -7,7 +7,9 @@ */ package org.opendaylight.mdsal.binding.java.api.generator -import com.google.common.base.Preconditions +import static java.util.Objects.requireNonNull +import static extension org.apache.commons.text.StringEscapeUtils.escapeJava; + import com.google.common.collect.ImmutableList import com.google.common.collect.Lists import com.google.common.io.BaseEncoding @@ -16,6 +18,7 @@ import java.util.ArrayList import java.util.Arrays import java.util.Collections import java.util.List +import java.util.Map import java.util.Objects import java.util.regex.Pattern import org.opendaylight.mdsal.binding.model.api.ConcreteType @@ -27,6 +30,7 @@ import org.opendaylight.mdsal.binding.model.api.GeneratedType import org.opendaylight.mdsal.binding.model.api.Restrictions import org.opendaylight.mdsal.binding.model.api.Type import org.opendaylight.mdsal.binding.model.util.TypeConstants +import org.opendaylight.yangtools.yang.binding.CodeHelpers import org.opendaylight.yangtools.yang.model.api.type.BitsTypeDefinition /** @@ -85,8 +89,7 @@ class ClassTemplate extends BaseTemplate { this.enclosedGeneratedTypes = genType.enclosedTypes if (restrictions !== null && restrictions.rangeConstraint.present) { - rangeGenerator = AbstractRangeGenerator.forType(findProperty(genType, "value").returnType) - Preconditions.checkNotNull(rangeGenerator) + rangeGenerator = requireNonNull(AbstractRangeGenerator.forType(findProperty(genType, "value").returnType)) } else { rangeGenerator = null } @@ -219,14 +222,10 @@ class ClassTemplate extends BaseTemplate { * consequence of how this code is structured. */ IF genTO.typedef && !allProperties.empty && allProperties.size == 1 && allProperties.get(0).name.equals("value")» - - «Preconditions.importedName».checkNotNull(_value, "Supplied value may not be null"); - + «Objects.importedName».requireNonNull(_value, "Supplied value may not be null"); «FOR c : consts» - «IF c.name == TypeConstants.PATTERN_CONSTANT_NAME && c.value instanceof List» - for (Pattern p : «Constants.MEMBER_PATTERN_LIST») { - «Preconditions.importedName».checkArgument(p.matcher(_value).matches(), "Supplied value \"%s\" does not match required pattern \"%s\"", _value, p); - } + «IF c.name == TypeConstants.PATTERN_CONSTANT_NAME» + «CodeHelpers.importedName».checkPattern(_value, «Constants.MEMBER_PATTERN_LIST», «Constants.MEMBER_REGEX_LIST»); «ENDIF» «ENDFOR» «ENDIF» @@ -291,7 +290,7 @@ class ClassTemplate extends BaseTemplate { «IF restrictions.rangeConstraint.present» «rangeGenerator.generateRangeCheckerCall(paramName, paramValue(returnType, paramName))» «ENDIF» - } + } «ENDIF» «ENDIF» ''' @@ -435,17 +434,16 @@ class ClassTemplate extends BaseTemplate { «IF !consts.empty» «FOR c : consts» «IF c.name == TypeConstants.PATTERN_CONSTANT_NAME» - «val cValue = c.value» - «IF cValue instanceof List» - private static final «Pattern.importedName»[] «Constants.MEMBER_PATTERN_LIST»; - public static final «List.importedName» «TypeConstants.PATTERN_CONSTANT_NAME» = «ImmutableList.importedName».of(« - FOR v : cValue SEPARATOR ", "»« - IF v instanceof String»"« - v»"« - ENDIF»« - ENDFOR»); - - «generateStaticInitializationBlock» + «val cValue = c.value as Map» + public static final «List.importedName» «TypeConstants.PATTERN_CONSTANT_NAME» = «ImmutableList.importedName».of(« + FOR v : cValue.keySet SEPARATOR ", "»"«v.escapeJava»"«ENDFOR»); + «IF cValue.size == 1» + private static final «Pattern.importedName» «Constants.MEMBER_PATTERN_LIST» = «Pattern.importedName».compile(«TypeConstants.PATTERN_CONSTANT_NAME».get(0)); + private static final String «Constants.MEMBER_REGEX_LIST» = "«cValue.values.get(0).escapeJava»"; + «ELSE» + private static final «Pattern.importedName»[] «Constants.MEMBER_PATTERN_LIST» = «CodeHelpers.importedName».compilePatterns(«TypeConstants.PATTERN_CONSTANT_NAME»); + private static final String[] «Constants.MEMBER_REGEX_LIST» = { « + FOR v : cValue.values SEPARATOR ", "»"«v.escapeJava»"«ENDFOR» }; «ENDIF» «ELSE» «emitConstant(c)» @@ -454,23 +452,6 @@ class ClassTemplate extends BaseTemplate { «ENDIF» ''' - /** - * Template method which generates JAVA static initialization block. - * - * @return string with static initialization block in JAVA format - */ - def protected generateStaticInitializationBlock() ''' - static { - final «Pattern.importedName» a[] = new «Pattern.importedName»[«TypeConstants.PATTERN_CONSTANT_NAME».size()]; - int i = 0; - for (String regEx : «TypeConstants.PATTERN_CONSTANT_NAME») { - a[i++] = Pattern.compile(regEx); - } - - «Constants.MEMBER_PATTERN_LIST» = a; - } - ''' - /** * Template method which generates JAVA class attributes. * diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/Constants.java b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/Constants.java index 9418d60866..0e00bcdb59 100644 --- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/Constants.java +++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/Constants.java @@ -18,11 +18,18 @@ final class Constants { public static final String DOT = "."; /** - * Name of the class constant which contains list of Pattern - * instances. + * Name of the class constant which contains list of Pattern instances. The type of this constant + * is Pattern[] for more than one pattern, or Pattern if there is only a single one. */ public static final String MEMBER_PATTERN_LIST = "patterns"; + /** + * Name of the class constant which contains a list of XSD regular expression strings. The type of this constant + * is String[] (or String for single strings) and it corresponds to {@link #MEMBER_PATTERN_LIST} in both size and + * ordering. + */ + public static final String MEMBER_REGEX_LIST = "regexes"; + /** * It doesn't have the sense to create the instances of this class. */ diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/GeneratorUtil.java b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/GeneratorUtil.java index 0b067740a1..fcd5217722 100644 --- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/GeneratorUtil.java +++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/GeneratorUtil.java @@ -64,8 +64,6 @@ public final class GeneratorUtil { if (genType instanceof GeneratedTransferObject && isConstantInTO(TypeConstants.PATTERN_CONSTANT_NAME, (GeneratedTransferObject) genType)) { putTypeIntoImports(genType, Types.typeForClass(java.util.regex.Pattern.class), imports); - putTypeIntoImports(genType, Types.typeForClass(java.util.Arrays.class), imports); - putTypeIntoImports(genType, Types.typeForClass(java.util.ArrayList.class), imports); } final List methods = genType.getMethodDefinitions(); @@ -324,7 +322,7 @@ public final class GeneratorUtil { final Type t = pTypes[i]; String separator = COMMA; - if (i == (pTypes.length - 1)) { + if (i == pTypes.length - 1) { separator = ""; } diff --git a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTestUtils.java b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTestUtils.java index ad6e250319..5765caa983 100644 --- a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTestUtils.java +++ b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTestUtils.java @@ -25,6 +25,7 @@ import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import javax.tools.Diagnostic; import javax.tools.JavaCompiler; import javax.tools.JavaFileObject; import javax.tools.StandardJavaFileManager; @@ -351,8 +352,12 @@ public class CompilationTestUtils { List filesList = getJavaFiles(sourcesOutputDir); Iterable compilationUnits = fileManager.getJavaFileObjectsFromFiles(filesList); Iterable options = Arrays.asList("-d", compiledOutputDir.getAbsolutePath()); - boolean compiled = compiler.getTask(null, null, null, options, null, compilationUnits).call(); - assertTrue("Compilation failed", compiled); + + List> diags = new ArrayList<>(); + boolean compiled = compiler.getTask(null, null, diags::add, options, null, compilationUnits).call(); + if (!compiled) { + fail("Compilation failed with " + diags); + } } /** diff --git a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/TypedefCompilationTest.java b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/TypedefCompilationTest.java index 3b0c0fe38e..faefe0627b 100644 --- a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/TypedefCompilationTest.java +++ b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/TypedefCompilationTest.java @@ -166,7 +166,7 @@ public class TypedefCompilationTest extends BaseCompilationTest { // typedef string-ext1 assertFalse(stringExt1Class.isInterface()); CompilationTestUtils.assertContainsField(stringExt1Class, VAL, String.class); - CompilationTestUtils.assertContainsField(stringExt1Class, "patterns", Pattern[].class); + CompilationTestUtils.assertContainsField(stringExt1Class, "patterns", Pattern.class); CompilationTestUtils.assertContainsField(stringExt1Class, "PATTERN_CONSTANTS", List.class); CompilationTestUtils.assertContainsFieldWithValue(stringExt1Class, "serialVersionUID", Long.TYPE, 6943827552297110991L, String.class); // assertEquals(5, stringExt1Class.getDeclaredFields().length); 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 new file mode 100644 index 0000000000..fd96883182 --- /dev/null +++ b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java @@ -0,0 +1,121 @@ +/* + * Copyright (c) 2018 Pantheon Technologies, s.r.o. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.yangtools.yang.binding; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Verify.verify; +import static java.util.Objects.requireNonNull; + +import com.google.common.base.VerifyException; +import java.util.List; +import java.util.regex.Pattern; +import org.eclipse.jdt.annotation.NonNull; +import org.eclipse.jdt.annotation.Nullable; + +/** + * Helper methods for generated binding code. This class concentrates useful primitives generated code may call + * to perform specific shared functions. This allows for generated classes to be leaner. Methods in this class follows + * general API stability requirements of the Binding Specification. + * + * @author Robert Varga + */ +public final class CodeHelpers { + private CodeHelpers() { + // Hidden + } + + /** + * Require that an a value-related expression is true. + * + * @param expression Expression to evaluate + * @param value Value being validated + * @param options Valid value options checked + * @throws IllegalArgumentException if expression is false + */ + public static void validValue(final boolean expression, final Object value, final String options) { + checkArgument(expression, "expected one of: %s \n%but was: %s", options, value); + } + + /** + * Require an argument being received. This is similar to {@link java.util.Objects#requireNonNull(Object)}, but + * throws an IllegalArgumentException. + * + *

      + * Implementation note: we expect argName to be a string literal or a constant, so that it's non-nullness can be + * quickly discovered for a call site (where we are going to be inlined). + * + * @param value Value itself + * @param name Symbolic name + * @return non-null value + * @throws IllegalArgumentException if value is null + * @throws NullPointerException if name is null + */ + // FIXME: another advantage is that it is JDT-annotated, but we could live without that. At some point we should + // schedule a big ISE-to-NPE conversion and just use Objects.requireNonNull() instead. + public static @NonNull T nonNullValue(@Nullable final T value, final @NonNull String name) { + requireNonNull(name); + checkArgument(value != null, "%s must not be null", name); + return value; + } + + /** + * Compile a list of pattern regular expressions and return them as an array. The list must hold at least two + * expressions. + * + * @param patterns Patterns to compile + * @return Compiled patterns in an array + * @throws NullPointerException if the list or any of its elements is null + * @throws VerifyException if the list has fewer than two elements + */ + public static @NonNull Pattern[] compilePatterns(final @NonNull List patterns) { + final int size = patterns.size(); + verify(size > 1, "Patterns has to have at least 2 elements"); + final @NonNull Pattern[] result = new Pattern[size]; + for (int i = 0; i < size; ++i) { + result[i] = Pattern.compile(patterns.get(i)); + } + return result; + } + + /** + * Check whether a specified string value matches a specified pattern. This method handles the distinction between + * modeled XSD expression and enforcement {@link Pattern} which may reflect negation. + * + * @param value Value to be checked. + * @param pattern Enforcement pattern + * @param regex Source regular expression, as defined in YANG model + * @throws IllegalArgumentException if the value does not match the pattern + * @throws NullPointerException if any of the arguments are null + */ + public static void checkPattern(final String value, final Pattern pattern, final String regex) { + if (!pattern.matcher(value).matches()) { + final String match = BindingMapping.isNegatedPattern(pattern) ? "matches forbidden" + : "does not match required"; + throw new IllegalArgumentException("Supplied value \"" + value + "\" " + match + " pattern \"" + + regex + "\""); + } + } + + /** + * Check whether a specified string value matches specified patterns. This method handles the distinction between + * modeled XSD expression and enforcement {@link Pattern} which may reflect negation. + * + * @param value Value to be checked. + * @param patterns Enforcement patterns + * @param regexes Source regular expression, as defined in YANG model. Size and order must match patterns. + * @throws IllegalArgumentException if the value does not match the pattern + * @throws NullPointerException if any of the arguments are null + * @throws VerifyException if the size of patterns and regexes does not match + */ + public static void checkPattern(final String value, final Pattern[] patterns, final String[] regexes) { + verify(patterns.length == regexes.length, "Patterns and regular expression lengths have to match"); + for (int i = 0; i < patterns.length; ++i) { + checkPattern(value, patterns[i], regexes[i]); + } + } +} -- 2.36.6