Bug 1443: Added support for misplaced augmentations. 01/10301/5
authorTony Tkacik <ttkacik@cisco.com>
Tue, 26 Aug 2014 11:35:22 +0000 (13:35 +0200)
committerRobert Varga <rovarga@cisco.com>
Tue, 26 Aug 2014 21:45:13 +0000 (21:45 +0000)
Binding Specification reuses elements inside groupings
as part of normal data tree. Augmentations and augmented
cases could be copied by user using copy builders
and may be used at incorrect place, where augmentation
with same data is present.

Binding Data Codec will treat this augmentations as
same, if augmentation definining same data subtree
is present at real location.

Change-Id: Ie232ef9b5e5bbe84f5562bfce612d8ea03b9a9a5
Signed-off-by: Tony Tkacik <ttkacik@cisco.com>
code-generator/binding-data-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/ChoiceNodeCodecContext.java
code-generator/binding-data-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/DataObjectCodecContext.java
code-generator/binding-data-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/AugmentationSubstitutionTest.java [new file with mode: 0644]
code-generator/binding-data-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/CaseSubstitutionTest.java [new file with mode: 0644]
yang/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/BindingReflections.java

index 700476f86bae3d79c3ed8c8d4d6e4315a7bac807..bac768703c24cab5f00e1b13014143f9b5fcf218 100644 (file)
@@ -11,8 +11,13 @@ import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
+
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.util.BindingReflections;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
@@ -36,20 +41,50 @@ final class ChoiceNodeCodecContext extends DataContainerCodecContext<ChoiceNode>
         Map<YangInstanceIdentifier.PathArgument, DataContainerCodecPrototype<?>> byYangCaseChildBuilder = new HashMap<>();
         Map<Class<?>, DataContainerCodecPrototype<?>> byClassBuilder = new HashMap<>();
         Map<Class<?>, DataContainerCodecPrototype<?>> byCaseChildClassBuilder = new HashMap<>();
-
+        Set<Class<?>> potentialSubstitutions = new HashSet<>();
+        // Walks all cases for supplied choice in current runtime context
         for (Class<?> caze : factory().getRuntimeContext().getCases(bindingClass())) {
+            // We try to load case using exact match thus name
+            // and original schema must equals
             DataContainerCodecPrototype<ChoiceCaseNode> cazeDef = loadCase(caze);
+            // If we have case definition, this case is instantiated
+            // at current location and thus is valid in context of parent choice
             if (cazeDef != null) {
                 byClassBuilder.put(cazeDef.getBindingClass(), cazeDef);
+                // Updates collection of case children
                 for (Class<? extends DataObject> cazeChild : BindingReflections.getChildrenClasses((Class) caze)) {
                     byCaseChildClassBuilder.put(cazeChild, cazeDef);
                 }
+                // Updates collection of YANG instance identifier to case
                 for (DataSchemaNode cazeChild : cazeDef.getSchema().getChildNodes()) {
                     byYangCaseChildBuilder.put(new NodeIdentifier(cazeChild.getQName()), cazeDef);
                 }
+            } else {
+                /*
+                 * If case definition is not available, we store it for
+                 * later check if it could be used as substitution of existing one.
+                 */
+                potentialSubstitutions.add(caze);
             }
         }
 
+        Map<Class<?>, DataContainerCodecPrototype<?>> bySubstitutionBuilder = new HashMap<>();
+        /*
+         * Walks all cases which are not directly instantiated and
+         * tries to match them to instantiated cases - represent same data as instantiated case,
+         * only case name or schema path is different. This is required due property of
+         * binding specification, that if choice is in grouping schema path location is lost,
+         * and users may use incorrect case class using copy builders.
+         */
+        for(Class<?> substitution : potentialSubstitutions) {
+            search: for(Entry<Class<?>, DataContainerCodecPrototype<?>> real : byClassBuilder.entrySet()) {
+                if(BindingReflections.isSubstitutionFor(substitution, real.getKey())) {
+                    bySubstitutionBuilder.put(substitution, real.getValue());
+                    break search;
+                }
+            }
+        }
+        byClassBuilder.putAll(bySubstitutionBuilder);
         byYangCaseChild = ImmutableMap.copyOf(byYangCaseChildBuilder);
         byClass = ImmutableMap.copyOf(byClassBuilder);
         byCaseChildClass = ImmutableMap.copyOf(byCaseChildClassBuilder);
index 32fdf6d80ea597da1c4ec000bcecb880c9564564..19a9600f62705ee7a3e0878c5386a582192f4266 100644 (file)
@@ -107,6 +107,29 @@ abstract class DataObjectCodecContext<T extends DataNodeContainer> extends DataC
     @Override
     protected DataContainerCodecContext<?> getStreamChild(final Class<?> childClass) {
         DataContainerCodecPrototype<?> childProto = byStreamClass.get(childClass);
+        if (childProto != null) {
+            return childProto.get();
+        }
+
+        if (Augmentation.class.isAssignableFrom(childClass))  {
+            /*
+             * It is potentially mismatched valid augmentation - we look up equivalent augmentation
+             * using reflection and walk all stream child and compare augmenations classes
+             * if they are equivalent.
+             *
+             * FIXME: Cache mapping of mismatched augmentation to real one, to speed up lookup.
+             */
+            Class<?> augTarget = BindingReflections.findAugmentationTarget((Class) childClass);
+            if ((bindingClass().equals(augTarget))) {
+                for (DataContainerCodecPrototype<?> realChild : byStreamClass.values()) {
+                    if (Augmentation.class.isAssignableFrom(realChild.getBindingClass())
+                            && BindingReflections.isSubstitutionFor(childClass,realChild.getBindingClass())) {
+                        childProto = realChild;
+                        break;
+                    }
+                }
+            }
+        }
         Preconditions.checkArgument(childProto != null, " Child %s is not valid child.",childClass);
         return childProto.get();
     }
diff --git a/code-generator/binding-data-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/AugmentationSubstitutionTest.java b/code-generator/binding-data-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/AugmentationSubstitutionTest.java
new file mode 100644 (file)
index 0000000..3438253
--- /dev/null
@@ -0,0 +1,86 @@
+/*
+ * Copyright (c) 2014 Cisco Systems, Inc. and others.  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.binding.data.codec.test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Collections;
+import java.util.Map.Entry;
+
+import javassist.ClassPool;
+
+import org.junit.Test;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.RpcComplexUsesAugment;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.RpcComplexUsesAugmentBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.TreeComplexUsesAugment;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.TreeComplexUsesAugmentBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.TreeLeafOnlyAugment;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.complex.from.grouping.ContainerWithUsesBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.complex.from.grouping.ListViaUses;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.Top;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.two.level.list.TopLevelList;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.two.level.list.TopLevelListBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.two.level.list.TopLevelListKey;
+import org.opendaylight.yangtools.binding.data.codec.gen.impl.StreamWriterGenerator;
+import org.opendaylight.yangtools.binding.data.codec.impl.BindingNormalizedNodeCodecRegistry;
+import org.opendaylight.yangtools.sal.binding.generator.util.JavassistUtils;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.AugmentationIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
+import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
+
+
+public class AugmentationSubstitutionTest extends AbstractBindingRuntimeTest {
+
+    private static final TopLevelListKey TOP_FOO_KEY = new TopLevelListKey("foo");
+    private static final InstanceIdentifier<TopLevelList> BA_TOP_LEVEL_LIST = InstanceIdentifier.builder(Top.class)
+            .child(TopLevelList.class, TOP_FOO_KEY).toInstance();
+    private static final InstanceIdentifier<TreeLeafOnlyAugment> BA_TREE_LEAF_ONLY = BA_TOP_LEVEL_LIST
+            .augmentation(TreeLeafOnlyAugment.class);
+    private static final InstanceIdentifier<TreeComplexUsesAugment> BA_TREE_COMPLEX_USES = BA_TOP_LEVEL_LIST
+            .augmentation(TreeComplexUsesAugment.class);
+    private static final QName SIMPLE_VALUE_QNAME = QName.create(TreeComplexUsesAugment.QNAME, "simple-value");
+
+    private BindingNormalizedNodeCodecRegistry registry;
+
+    @Override
+    public void setup() {
+        super.setup();
+        JavassistUtils utils = JavassistUtils.forClassPool(ClassPool.getDefault());
+        registry = new BindingNormalizedNodeCodecRegistry(StreamWriterGenerator.create(utils));
+        registry.onBindingRuntimeContextUpdated(getRuntimeContext());
+    }
+
+    @Test
+    public void augmentationInGroupingSubstituted() {
+        TopLevelList baRpc = new TopLevelListBuilder()
+            .setKey(TOP_FOO_KEY)
+            .addAugmentation(RpcComplexUsesAugment.class, new RpcComplexUsesAugmentBuilder(createComplexData()).build())
+            .build();
+        TopLevelList baTree = new TopLevelListBuilder()
+            .setKey(TOP_FOO_KEY)
+            .addAugmentation(TreeComplexUsesAugment.class, new TreeComplexUsesAugmentBuilder(createComplexData()).build())
+            .build();
+        NormalizedNode<?, ?> domTreeEntry = registry.toNormalizedNode(BA_TOP_LEVEL_LIST, baTree).getValue();
+        NormalizedNode<?, ?> domRpcEntry = registry.toNormalizedNode(BA_TOP_LEVEL_LIST, baRpc).getValue();
+        assertEquals(domTreeEntry, domRpcEntry);
+    }
+
+    private RpcComplexUsesAugment createComplexData() {
+        return new RpcComplexUsesAugmentBuilder()
+        .setContainerWithUses(new ContainerWithUsesBuilder()
+            .setLeafFromGrouping("foo")
+        .build())
+        .setListViaUses(Collections.<ListViaUses>emptyList())
+        .build();
+    }
+
+}
diff --git a/code-generator/binding-data-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/CaseSubstitutionTest.java b/code-generator/binding-data-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/CaseSubstitutionTest.java
new file mode 100644 (file)
index 0000000..2d5a0bd
--- /dev/null
@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2014 Cisco Systems, Inc. and others.  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.binding.data.codec.test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Collections;
+import java.util.Map.Entry;
+
+import javassist.ClassPool;
+
+import org.junit.Test;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.RpcComplexUsesAugment;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.RpcComplexUsesAugmentBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.TreeComplexUsesAugment;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.TreeComplexUsesAugmentBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.TreeLeafOnlyAugment;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.complex.from.grouping.ContainerWithUsesBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.complex.from.grouping.ListViaUses;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.put.top.input.top.level.list.choice.in.list.ComplexViaUsesWithDifferentNameBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.augment.rev140709.top.top.level.list.choice.in.list.ComplexViaUsesBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.Top;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.two.level.list.TopLevelList;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.two.level.list.TopLevelListBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.two.level.list.TopLevelListKey;
+import org.opendaylight.yangtools.binding.data.codec.gen.impl.StreamWriterGenerator;
+import org.opendaylight.yangtools.binding.data.codec.impl.BindingNormalizedNodeCodecRegistry;
+import org.opendaylight.yangtools.sal.binding.generator.util.JavassistUtils;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.AugmentationIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
+import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
+
+
+public class CaseSubstitutionTest extends AbstractBindingRuntimeTest {
+
+    private static final TopLevelListKey TOP_FOO_KEY = new TopLevelListKey("foo");
+    private static final InstanceIdentifier<TopLevelList> BA_TOP_LEVEL_LIST = InstanceIdentifier.builder(Top.class)
+            .child(TopLevelList.class, TOP_FOO_KEY).toInstance();
+    private static final InstanceIdentifier<TreeLeafOnlyAugment> BA_TREE_LEAF_ONLY = BA_TOP_LEVEL_LIST
+            .augmentation(TreeLeafOnlyAugment.class);
+    private static final InstanceIdentifier<TreeComplexUsesAugment> BA_TREE_COMPLEX_USES = BA_TOP_LEVEL_LIST
+            .augmentation(TreeComplexUsesAugment.class);
+    private static final QName SIMPLE_VALUE_QNAME = QName.create(TreeComplexUsesAugment.QNAME, "simple-value");
+
+    private BindingNormalizedNodeCodecRegistry registry;
+
+    @Override
+    public void setup() {
+        super.setup();
+        JavassistUtils utils = JavassistUtils.forClassPool(ClassPool.getDefault());
+        registry = new BindingNormalizedNodeCodecRegistry(StreamWriterGenerator.create(utils));
+        registry.onBindingRuntimeContextUpdated(getRuntimeContext());
+    }
+
+    @Test
+    public void choiceInGroupingSubstituted() {
+        TopLevelList baRpc = new TopLevelListBuilder()
+            .setKey(TOP_FOO_KEY)
+            .setChoiceInList(new ComplexViaUsesWithDifferentNameBuilder(createComplexData()).build())
+            .build();
+        TopLevelList baTree = new TopLevelListBuilder()
+            .setKey(TOP_FOO_KEY)
+            .setChoiceInList(new ComplexViaUsesBuilder(createComplexData()).build())
+            .build();
+        NormalizedNode<?, ?> domTreeEntry = registry.toNormalizedNode(BA_TOP_LEVEL_LIST, baTree).getValue();
+        NormalizedNode<?, ?> domRpcEntry = registry.toNormalizedNode(BA_TOP_LEVEL_LIST, baRpc).getValue();
+        assertEquals(domTreeEntry, domRpcEntry);
+    }
+
+    private RpcComplexUsesAugment createComplexData() {
+        return new RpcComplexUsesAugmentBuilder()
+        .setContainerWithUses(new ContainerWithUsesBuilder()
+            .setLeafFromGrouping("foo")
+        .build())
+        .setListViaUses(Collections.<ListViaUses>emptyList())
+        .build();
+    }
+
+}
index f9f4c5c7b547774e3e4567a326d2192541d7fbc1..bc7474a9f2cb765796063f07e8c9cd65dc2b5e96 100644 (file)
@@ -16,12 +16,15 @@ import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSet.Builder;
+import com.google.common.collect.Sets;
 
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Type;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -581,4 +584,51 @@ public class BindingReflections {
     public static Map<Class<? extends Augmentation<?>>, Augmentation<?>> getAugmentations(final Augmentable<?> input) {
         return AugmentationFieldGetter.getGetter(input.getClass()).getAugmentations(input);
     }
+
+    /**
+     *
+     * Determines if two augmentation classes or case classes represents same data.
+     * <p>
+     * Two augmentations or cases could be substituted only if and if:
+     * <ul>
+     * <li>Both implements same interfaces</li>
+     * <li>Both have same children</li>
+     * <li>If augmentations: Both have same augmentation target class. Target class was generated for data node in grouping.</li>
+     * <li>If cases: Both are from same choice. Choice class was generated for data node in grouping.</li>
+     * </ul>
+     * <p>
+     * <b>Explanation:</b> Binding Specification reuses classes generated for groupings as part of normal data tree,
+     * this classes from grouping could be used at various locations and user may not be aware of it
+     * and may use incorrect case or augmentation in particular subtree (via copy constructors, etc).
+     *
+     * @param potential Class which is potential substition
+     * @param target Class which should be used at particular subtree
+     * @return true if and only if classes represents same data.
+     */
+    @SuppressWarnings({"rawtypes","unchecked"})
+    public static boolean isSubstitutionFor(Class potential, Class target) {
+        HashSet<Class> subImplemented = Sets.newHashSet(potential.getInterfaces());
+        HashSet<Class> targetImplemented = Sets.newHashSet(target.getInterfaces());
+        if(!subImplemented.equals(targetImplemented)) {
+            return false;
+        }
+        if(Augmentation.class.isAssignableFrom(potential)
+                && !BindingReflections.findAugmentationTarget(potential).equals(BindingReflections.findAugmentationTarget(target))) {
+                return false;
+        }
+        for(Method potentialMethod : potential.getMethods()) {
+            try {
+                Method targetMethod = target.getMethod(potentialMethod.getName(), potentialMethod.getParameterTypes());
+                if(!potentialMethod.getReturnType().equals(targetMethod.getReturnType())) {
+                    return false;
+                }
+            } catch (NoSuchMethodException e) {
+                // Counterpart method is missing, so classes could not be substituted.
+                return false;
+            } catch (SecurityException e) {
+                throw new IllegalStateException("Could not compare methods",e);
+            }
+        }
+        return true;
+    }
 }