Bug 1573: Fixed deserialization of not representable Instance Identifier

Not all YangInstanceIdentifier are representable in Binding form,
codecs were designed to handle this and omit such cases,
but one ommision case was missing in deserializing Instance Identifier
without data and this resulted into returning incorrectly
constructed InstanceIdentifier.

Updated test which was testing incorrect behaviour to test correct
behaviour.

Change-Id: I97215f674f7ff164bc2c87d9eef4a254510b0280
Signed-off-by: Tony Tkacik <ttkacik@cisco.com>

diff --git a/code-generator/binding-data-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/BindingCodecContext.java b/code-generator/binding-data-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/BindingCodecContext.java
index 0ecb622367bd7e29ddee09742725e279d673a76a..414c553578960f0973175869f01813091748fe3a 100644 (file)
--- a/code-generator/binding-data-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/BindingCodecContext.java
+++ b/code-generator/binding-data-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/BindingCodecContext.java
@@ -9,6 +9,7 @@ package org.opendaylight.yangtools.binding.data.codec.impl;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -90,9 +91,9 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
 
     public Entry<YangInstanceIdentifier, BindingStreamEventWriter> newWriter(final InstanceIdentifier<?> path,
             final NormalizedNodeStreamWriter domWriter) {
-        LinkedList<YangInstanceIdentifier.PathArgument> yangArgs = new LinkedList<>();
-        DataContainerCodecContext<?> codecContext = getCodecContextNode(path, yangArgs);
-        BindingStreamEventWriter writer = new BindingToNormalizedStreamWriter(codecContext, domWriter);
+        final LinkedList<YangInstanceIdentifier.PathArgument> yangArgs = new LinkedList<>();
+        final DataContainerCodecContext<?> codecContext = getCodecContextNode(path, yangArgs);
+        final BindingStreamEventWriter writer = new BindingToNormalizedStreamWriter(codecContext, domWriter);
         return new SimpleEntry<>(YangInstanceIdentifier.create(yangArgs), writer);
     }
 
@@ -104,7 +105,7 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
     public DataContainerCodecContext<?> getCodecContextNode(final InstanceIdentifier<?> binding,
             final List<YangInstanceIdentifier.PathArgument> builder) {
         DataContainerCodecContext<?> currentNode = root;
-        for (InstanceIdentifier.PathArgument bindingArg : binding.getPathArguments()) {
+        for (final InstanceIdentifier.PathArgument bindingArg : binding.getPathArguments()) {
             currentNode = currentNode.getIdentifierChild(bindingArg, builder);
         }
         return currentNode;
@@ -126,7 +127,7 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
         NodeCodecContext currentNode = root;
         ListNodeCodecContext currentList = null;
 
-        for (YangInstanceIdentifier.PathArgument domArg : dom.getPathArguments()) {
+        for (final YangInstanceIdentifier.PathArgument domArg : dom.getPathArguments()) {
             Preconditions.checkArgument(currentNode instanceof DataContainerCodecContext<?>, "Unexpected child of non-container node %s", currentNode);
             final DataContainerCodecContext<?> previous = (DataContainerCodecContext<?>) currentNode;
             final NodeCodecContext nextNode = previous.getYangIdentifierChild(domArg);
@@ -185,8 +186,8 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
     @Override
     public ImmutableMap<String, LeafNodeCodecContext> getLeafNodes(final Class<?> parentClass,
             final DataNodeContainer childSchema) {
-        HashMap<String, DataSchemaNode> getterToLeafSchema = new HashMap<>();
-        for (DataSchemaNode leaf : childSchema.getChildNodes()) {
+        final HashMap<String, DataSchemaNode> getterToLeafSchema = new HashMap<>();
+        for (final DataSchemaNode leaf : childSchema.getChildNodes()) {
             final TypeDefinition<?> typeDef;
             if (leaf instanceof LeafSchemaNode) {
                 typeDef = ((LeafSchemaNode) leaf).getType();
@@ -196,14 +197,14 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
                 continue;
             }
 
-            String getterName = getGetterName(leaf.getQName(), typeDef);
+            final String getterName = getGetterName(leaf.getQName(), typeDef);
             getterToLeafSchema.put(getterName, leaf);
         }
         return getLeafNodesUsingReflection(parentClass, getterToLeafSchema);
     }
 
     private String getGetterName(final QName qName, TypeDefinition<?> typeDef) {
-        String suffix = BindingMapping.getClassName(qName);
+        final String suffix = BindingMapping.getClassName(qName);
 
         while (typeDef.getBaseType() != null) {
             typeDef = typeDef.getBaseType();
@@ -216,15 +217,15 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
 
     private ImmutableMap<String, LeafNodeCodecContext> getLeafNodesUsingReflection(final Class<?> parentClass,
             final Map<String, DataSchemaNode> getterToLeafSchema) {
-        Map<String, LeafNodeCodecContext> leaves = new HashMap<>();
-        for (Method method : parentClass.getMethods()) {
+        final Map<String, LeafNodeCodecContext> leaves = new HashMap<>();
+        for (final Method method : parentClass.getMethods()) {
             if (method.getParameterTypes().length == 0) {
-                DataSchemaNode schema = getterToLeafSchema.get(method.getName());
+                final DataSchemaNode schema = getterToLeafSchema.get(method.getName());
                 final Class<?> valueType;
                 if (schema instanceof LeafSchemaNode) {
                     valueType = method.getReturnType();
                 } else if (schema instanceof LeafListSchemaNode) {
-                    Type genericType = ClassLoaderUtils.getFirstGenericParameter(method.getGenericReturnType());
+                    final Type genericType = ClassLoaderUtils.getFirstGenericParameter(method.getGenericReturnType());
 
                     if (genericType instanceof Class<?>) {
                         valueType = (Class<?>) genericType;
@@ -236,7 +237,7 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
                 } else {
                     continue; // We do not have schema for leaf, so we will ignore it (eg. getClass, getImplementedInterface).
                 }
-                Codec<Object, Object> codec = getCodec(valueType, schema);
+                final Codec<Object, Object> codec = getCodec(valueType, schema);
                 final LeafNodeCodecContext leafNode = new LeafNodeCodecContext(schema, codec, method);
                 leaves.put(schema.getQName().getLocalName(), leafNode);
             }
@@ -284,15 +285,15 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
         } else if (rootType instanceof InstanceIdentifierTypeDefinition) {
             return ValueTypeCodec.encapsulatedValueCodecFor(valueType, instanceIdentifierCodec);
         } else if (rootType instanceof UnionTypeDefinition) {
-            Callable<UnionTypeCodec> loader = UnionTypeCodec.loader(valueType, (UnionTypeDefinition) rootType);
+            final Callable<UnionTypeCodec> loader = UnionTypeCodec.loader(valueType, (UnionTypeDefinition) rootType);
             try {
                 return loader.call();
-            } catch (Exception e) {
+            } catch (final Exception e) {
                 throw new IllegalStateException("Unable to load codec for " + valueType, e);
             }
         } else if(rootType instanceof LeafrefTypeDefinition) {
-            Entry<GeneratedType, Object> typeWithSchema = context.getTypeWithSchema(valueType);
-            Object schema = typeWithSchema.getValue();
+            final Entry<GeneratedType, Object> typeWithSchema = context.getTypeWithSchema(valueType);
+            final Object schema = typeWithSchema.getValue();
             Preconditions.checkState(schema instanceof TypeDefinition<?>);
             return getCodec(valueType, (TypeDefinition<?>) schema);
         }
@@ -303,7 +304,7 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
 
         @Override
         public YangInstanceIdentifier serialize(final InstanceIdentifier<?> input) {
-            List<YangInstanceIdentifier.PathArgument> domArgs = new ArrayList<>();
+            final List<YangInstanceIdentifier.PathArgument> domArgs = new ArrayList<>();
             getCodecContextNode(input, domArgs);
             return YangInstanceIdentifier.create(domArgs);
         }
@@ -312,7 +313,15 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
         public InstanceIdentifier<?> deserialize(final YangInstanceIdentifier input) {
             final List<InstanceIdentifier.PathArgument> builder = new ArrayList<>();
             final NodeCodecContext codec = getCodecContextNode(input, builder);
-            return codec == null ? null : InstanceIdentifier.create(builder);
+            if (codec == null) {
+                return null;
+            }
+            if (codec instanceof ListNodeCodecContext && Iterables.getLast(builder) instanceof InstanceIdentifier.Item) {
+                // We ended up in list, but without key, which means it represent list as a whole,
+                // which is not binding representable.
+                return null;
+            }
+            return InstanceIdentifier.create(builder);
         }
     }
 
@@ -354,7 +363,7 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
 
         public Object getAndSerialize(final Object obj) {
             try {
-                Object value = getter.invoke(obj);
+                final Object value = getter.invoke(obj);
                 Preconditions.checkArgument(value != null,
                         "All keys must be specified for %s. Missing key is %s. Supplied key is %s",
                         getter.getDeclaringClass(), getter.getName(), obj);
@@ -388,7 +397,7 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
              * they are defined.
              */
             final Map<QName, ValueContext> keys = new LinkedHashMap<>();
-            for (QName qname : schema.getKeyDefinition()) {
+            for (final QName qname : schema.getKeyDefinition()) {
                 keys.put(qname, keyValueContexts.get(qname));
             }
             this.keyValueContexts = ImmutableMap.copyOf(keys);
@@ -398,8 +407,8 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
         public IdentifiableItem<?, ?> deserialize(final NodeIdentifierWithPredicates input) {
             final Collection<QName> keys = schema.getKeyDefinition();
             final ArrayList<Object> bindingValues = new ArrayList<>(keys.size());
-            for (QName key : keys) {
-                Object yangValue = input.getKeyValues().get(key);
+            for (final QName key : keys) {
+                final Object yangValue = input.getKeyValues().get(key);
                 bindingValues.add(keyValueContexts.get(key).deserialize(yangValue));
             }
 
@@ -417,10 +426,10 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
 
         @Override
         public NodeIdentifierWithPredicates serialize(final IdentifiableItem<?, ?> input) {
-            Object value = input.getKey();
+            final Object value = input.getKey();
 
-            Map<QName, Object> values = new LinkedHashMap<>();
-            for (Entry<QName, ValueContext> valueCtx : keyValueContexts.entrySet()) {
+            final Map<QName, Object> values = new LinkedHashMap<>();
+            for (final Entry<QName, ValueContext> valueCtx : keyValueContexts.entrySet()) {
                 values.put(valueCtx.getKey(), valueCtx.getValue().getAndSerialize(value));
             }
             return new NodeIdentifierWithPredicates(schema.getQName(), values);
@@ -429,8 +438,8 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
 
     @SuppressWarnings("unchecked")
     private static Constructor<? extends Identifier<?>> getConstructor(final Class<? extends Identifier<?>> clazz) {
-        for (@SuppressWarnings("rawtypes") Constructor constr : clazz.getConstructors()) {
-            Class<?>[] parameters = constr.getParameterTypes();
+        for (@SuppressWarnings("rawtypes") final Constructor constr : clazz.getConstructors()) {
+            final Class<?>[] parameters = constr.getParameterTypes();
             if (!clazz.equals(parameters[0])) {
                 // It is not copy constructor;
                 return constr;
@@ -442,11 +451,11 @@ final class BindingCodecContext implements CodecContextFactory, Immutable {
     @Override
     public Codec<NodeIdentifierWithPredicates, IdentifiableItem<?, ?>> getPathArgumentCodec(final Class<?> listClz,
             final ListSchemaNode schema) {
-        Class<? extends Identifier<?>> identifier = ClassLoaderUtils.findFirstGenericArgument(listClz,
+        final Class<? extends Identifier<?>> identifier = ClassLoaderUtils.findFirstGenericArgument(listClz,
                 Identifiable.class);
-        Map<QName, ValueContext> valueCtx = new HashMap<>();
-        for (LeafNodeCodecContext leaf : getLeafNodes(identifier, schema).values()) {
-            QName name = leaf.getDomPathArgument().getNodeType();
+        final Map<QName, ValueContext> valueCtx = new HashMap<>();
+        for (final LeafNodeCodecContext leaf : getLeafNodes(identifier, schema).values()) {
+            final QName name = leaf.getDomPathArgument().getNodeType();
             valueCtx.put(name, new ValueContext(identifier, leaf));
         }
         return new IdentifiableItemCodec(schema, identifier, listClz, valueCtx);

diff --git a/code-generator/binding-data-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/InstanceIdentifierSerializeDeserializeTest.java b/code-generator/binding-data-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/InstanceIdentifierSerializeDeserializeTest.java
index 67b7d1f9037e6b727eb7b08e8b76d9692e48881d..b0fca82863c6a8ca3553755be5e18135af5fd0df 100644 (file)
--- a/code-generator/binding-data-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/InstanceIdentifierSerializeDeserializeTest.java
+++ b/code-generator/binding-data-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/InstanceIdentifierSerializeDeserializeTest.java
@@ -7,6 +7,11 @@
  */
 package org.opendaylight.yangtools.binding.data.codec.test;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
 import com.google.common.collect.Iterables;
 import javassist.ClassPool;
 import org.junit.Before;
@@ -24,10 +29,6 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
 public class InstanceIdentifierSerializeDeserializeTest extends AbstractBindingRuntimeTest{
     public static final String TOP_LEVEL_LIST_KEY_VALUE = "foo";
 
@@ -52,62 +53,62 @@ public class InstanceIdentifierSerializeDeserializeTest extends AbstractBindingR
 
     private BindingNormalizedNodeCodecRegistry registry;
 
+    @Override
     @Before
     public void setup() {
         super.setup();
-        JavassistUtils utils = JavassistUtils.forClassPool(ClassPool.getDefault());
+        final JavassistUtils utils = JavassistUtils.forClassPool(ClassPool.getDefault());
         registry = new BindingNormalizedNodeCodecRegistry(StreamWriterGenerator.create(utils));
         registry.onBindingRuntimeContextUpdated(getRuntimeContext());
     }
 
     @Test
     public void testYangIIToBindingAwareII() {
-        InstanceIdentifier<?> instanceIdentifier = registry.fromYangInstanceIdentifier(BI_TOP_PATH);
+        final InstanceIdentifier<?> instanceIdentifier = registry.fromYangInstanceIdentifier(BI_TOP_PATH);
         assertEquals(Top.class, instanceIdentifier.getTargetType());
     }
 
     @Test
     public void testYangIIToBindingAwareIIListWildcarded() {
-        InstanceIdentifier<?> instanceIdentifier = registry.fromYangInstanceIdentifier(BI_TOP_LEVEL_LIST_PATH);
-        assertEquals(TopLevelList.class, instanceIdentifier.getTargetType());
-        assertTrue(instanceIdentifier.isWildcarded());
+        final InstanceIdentifier<?> instanceIdentifier = registry.fromYangInstanceIdentifier(BI_TOP_LEVEL_LIST_PATH);
+        assertNull(instanceIdentifier);
     }
 
     @Test
     public void testYangIIToBindingAwareIIListWithKey() {
-        InstanceIdentifier<?> instanceIdentifier = registry.fromYangInstanceIdentifier(BI_TOP_LEVEL_LIST_1_PATH);
-        InstanceIdentifier.PathArgument last = Iterables.getLast(instanceIdentifier.getPathArguments());
+        final InstanceIdentifier<?> instanceIdentifier = registry.fromYangInstanceIdentifier(BI_TOP_LEVEL_LIST_1_PATH);
+        final InstanceIdentifier.PathArgument last = Iterables.getLast(instanceIdentifier.getPathArguments());
         assertEquals(TopLevelList.class, instanceIdentifier.getTargetType());
         assertFalse(instanceIdentifier.isWildcarded());
         assertTrue(last instanceof InstanceIdentifier.IdentifiableItem);
-        Identifier key = ((InstanceIdentifier.IdentifiableItem) last).getKey();
+        final Identifier key = ((InstanceIdentifier.IdentifiableItem) last).getKey();
         assertEquals(TopLevelListKey.class, key.getClass());
         assertEquals(TOP_LEVEL_LIST_KEY_VALUE, ((TopLevelListKey)key).getName());
     }
 
     @Test
     public void testBindingAwareIIToYangIContainer() {
-        YangInstanceIdentifier yangInstanceIdentifier = registry.toYangInstanceIdentifier(
+        final YangInstanceIdentifier yangInstanceIdentifier = registry.toYangInstanceIdentifier(
                 InstanceIdentifier.create(Top.class).child(TopLevelList.class));
-        YangInstanceIdentifier.PathArgument lastPathArgument = yangInstanceIdentifier.getLastPathArgument();
+        final YangInstanceIdentifier.PathArgument lastPathArgument = yangInstanceIdentifier.getLastPathArgument();
         assertTrue(lastPathArgument instanceof YangInstanceIdentifier.NodeIdentifier);
         assertEquals(TopLevelList.QNAME, lastPathArgument.getNodeType());
     }
 
     @Test
     public void testBindingAwareIIToYangIIWildcard() {
-        YangInstanceIdentifier yangInstanceIdentifier = registry.toYangInstanceIdentifier(
+        final YangInstanceIdentifier yangInstanceIdentifier = registry.toYangInstanceIdentifier(
                 InstanceIdentifier.create(Top.class).child(TopLevelList.class));
-        YangInstanceIdentifier.PathArgument lastPathArgument = yangInstanceIdentifier.getLastPathArgument();
+        final YangInstanceIdentifier.PathArgument lastPathArgument = yangInstanceIdentifier.getLastPathArgument();
         assertTrue(lastPathArgument instanceof YangInstanceIdentifier.NodeIdentifier);
         assertEquals(TopLevelList.QNAME, lastPathArgument.getNodeType());
     }
 
     @Test
     public void testBindingAwareIIToYangIIListWithKey() {
-        YangInstanceIdentifier yangInstanceIdentifier = registry.toYangInstanceIdentifier(
+        final YangInstanceIdentifier yangInstanceIdentifier = registry.toYangInstanceIdentifier(
                 InstanceIdentifier.create(Top.class).child(TopLevelList.class, TOP_FOO_KEY));
-        YangInstanceIdentifier.PathArgument lastPathArgument = yangInstanceIdentifier.getLastPathArgument();
+        final YangInstanceIdentifier.PathArgument lastPathArgument = yangInstanceIdentifier.getLastPathArgument();
         assertTrue(lastPathArgument instanceof YangInstanceIdentifier.NodeIdentifierWithPredicates);
         assertTrue(((YangInstanceIdentifier.NodeIdentifierWithPredicates) lastPathArgument).getKeyValues().containsValue(TOP_LEVEL_LIST_KEY_VALUE));
         assertEquals(TopLevelList.QNAME, lastPathArgument.getNodeType());
@@ -115,13 +116,13 @@ public class InstanceIdentifierSerializeDeserializeTest extends AbstractBindingR
 
     @Test
     public void testBindingAwareIIToYangIIAugmentation() {
-        YangInstanceIdentifier.PathArgument lastArg = registry.toYangInstanceIdentifier(BA_TREE_COMPLEX_USES).getLastPathArgument();
+        final YangInstanceIdentifier.PathArgument lastArg = registry.toYangInstanceIdentifier(BA_TREE_COMPLEX_USES).getLastPathArgument();
         assertTrue(lastArg instanceof YangInstanceIdentifier.AugmentationIdentifier);
     }
 
     @Test
     public void testBindingAwareIIToYangIILeafOnlyAugmentation() {
-        YangInstanceIdentifier.PathArgument leafOnlyLastArg = registry.toYangInstanceIdentifier(BA_TREE_LEAF_ONLY).getLastPathArgument();
+        final YangInstanceIdentifier.PathArgument leafOnlyLastArg = registry.toYangInstanceIdentifier(BA_TREE_LEAF_ONLY).getLastPathArgument();
         assertTrue(leafOnlyLastArg instanceof YangInstanceIdentifier.AugmentationIdentifier);
         assertTrue(((YangInstanceIdentifier.AugmentationIdentifier) leafOnlyLastArg).getPossibleChildNames().contains(SIMPLE_VALUE_QNAME));
     }