From e704e6a6d1cc4db7ac1e1f53b54ec3bf51aaecc3 Mon Sep 17 00:00:00 2001 From: Peter Kajsa Date: Fri, 22 Jan 2016 11:39:37 +0100 Subject: [PATCH] Bug 4969: NPE in JSONCodecFactory by attempt to find codec for a leafref Attempt to find codec for a leafref results in NPE in JSONCodecFactory, because SchemaContextUtils cannot find baseType of derived leafref type. Change-Id: If5288210b4f4d200b18063cf58fdf873b1d12be1 Signed-off-by: Peter Kajsa (cherry picked from commit 9c98b9b0ea9d0f6ec26e64e8683cb3f1648d10d9) --- .../data/codec/gson/JSONCodecFactory.java | 2 + .../data/codec/gson/retest/Bug4969Test.java | 92 +++++++++++++++++++ .../src/test/resources/bug-4969/json/foo.json | 8 ++ .../src/test/resources/bug-4969/yang/bar.yang | 59 ++++++++++++ .../src/test/resources/bug-4969/yang/foo.yang | 27 ++++++ .../data/impl/codec/xml/XmlStreamUtils.java | 2 + .../yangtools/yang/model/util/BaseTypes.java | 62 ++++++++----- .../yang/model/util/SchemaContextUtil.java | 33 ++++++- 8 files changed, 261 insertions(+), 24 deletions(-) create mode 100644 yang/yang-data-codec-gson/src/test/java/org/opendaylight/yangtools/yang/data/codec/gson/retest/Bug4969Test.java create mode 100644 yang/yang-data-codec-gson/src/test/resources/bug-4969/json/foo.json create mode 100644 yang/yang-data-codec-gson/src/test/resources/bug-4969/yang/bar.yang create mode 100644 yang/yang-data-codec-gson/src/test/resources/bug-4969/yang/foo.yang diff --git a/yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/JSONCodecFactory.java b/yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/JSONCodecFactory.java index e58e1dfea4..1d6ffc8277 100644 --- a/yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/JSONCodecFactory.java +++ b/yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/JSONCodecFactory.java @@ -9,6 +9,7 @@ package org.opendaylight.yangtools.yang.data.codec.gson; import com.google.common.annotations.Beta; import com.google.common.base.Preconditions; +import com.google.common.base.Verify; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; @@ -111,6 +112,7 @@ public final class JSONCodecFactory { // FIXME: Verify if this does indeed support leafref of leafref final TypeDefinition referencedType = SchemaContextUtil.getBaseTypeForLeafRef(type, getSchemaContext(), schema); + Verify.verifyNotNull(referencedType, "Unable to find base type for leafref node '%s'.", schema.getPath()); return createCodec(schema, referencedType); } diff --git a/yang/yang-data-codec-gson/src/test/java/org/opendaylight/yangtools/yang/data/codec/gson/retest/Bug4969Test.java b/yang/yang-data-codec-gson/src/test/java/org/opendaylight/yangtools/yang/data/codec/gson/retest/Bug4969Test.java new file mode 100644 index 0000000000..d0e00d8bb4 --- /dev/null +++ b/yang/yang-data-codec-gson/src/test/java/org/opendaylight/yangtools/yang/data/codec/gson/retest/Bug4969Test.java @@ -0,0 +1,92 @@ +/* + * Copyright (c) 2015 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.yang.data.codec.gson.retest; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import com.google.common.base.Optional; +import com.google.gson.stream.JsonReader; +import java.io.File; +import java.io.IOException; +import java.io.StringReader; +import java.net.URISyntaxException; +import java.util.Set; +import org.junit.Test; +import org.opendaylight.yangtools.yang.common.QName; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; +import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; +import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter; +import org.opendaylight.yangtools.yang.data.codec.gson.JsonParserStream; +import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNormalizedNodeStreamWriter; +import org.opendaylight.yangtools.yang.data.impl.schema.NormalizedNodeResult; +import org.opendaylight.yangtools.yang.model.api.SchemaContext; +import org.opendaylight.yangtools.yang.parser.spi.meta.ReactorException; +import org.opendaylight.yangtools.yang.parser.spi.source.SourceException; + +public class Bug4969Test { + + @Test + public void test() throws SourceException, ReactorException, URISyntaxException, IOException { + File sourceDir = new File(Bug4969Test.class.getResource("/bug-4969/yang").toURI()); + SchemaContext context = RetestUtils.parseYangSources(sourceDir.listFiles()); + assertNotNull(context); + + final String inputJson = TestUtils.loadTextFile("/bug-4969/json/foo.json"); + final NormalizedNodeResult result = new NormalizedNodeResult(); + final NormalizedNodeStreamWriter streamWriter = ImmutableNormalizedNodeStreamWriter.from(result); + final JsonParserStream jsonParser = JsonParserStream.create(streamWriter, context); + jsonParser.parse(new JsonReader(new StringReader(inputJson))); + final NormalizedNode transformedInput = result.getResult(); + + assertTrue(transformedInput instanceof ContainerNode); + ContainerNode root = (ContainerNode) transformedInput; + Optional> ref1 = root.getChild(NodeIdentifier.create((QName + .create("foo", "2016-01-22", "ref1")))); + Optional> ref2 = root.getChild(NodeIdentifier.create((QName + .create("foo", "2016-01-22", "ref2")))); + Optional> ref3 = root.getChild(NodeIdentifier.create((QName + .create("foo", "2016-01-22", "ref3")))); + Optional> ref4 = root.getChild(NodeIdentifier.create((QName + .create("foo", "2016-01-22", "ref4")))); + + assertTrue(ref1.isPresent()); + assertTrue(ref2.isPresent()); + assertTrue(ref3.isPresent()); + assertTrue(ref4.isPresent()); + + Object value1 = ref1.get().getValue(); + Object value2 = ref2.get().getValue(); + Object value3 = ref3.get().getValue(); + Object value4 = ref4.get().getValue(); + + assertTrue(value1 instanceof Set); + assertTrue(value2 instanceof Set); + assertTrue(value3 instanceof Set); + assertTrue(value4 instanceof Set); + + Set set1 = (Set) value1; + Set set2 = (Set) value2; + Set set3 = (Set) value3; + Set set4 = (Set) value4; + + assertEquals(1, set1.size()); + assertEquals(2, set2.size()); + assertEquals(3, set3.size()); + assertEquals(4, set4.size()); + + assertTrue(set1.contains("a")); + assertTrue(set2.contains("a") && set2.contains("b")); + assertTrue(set3.contains("a") && set3.contains("b") && set3.contains("c")); + assertTrue(set4.contains("a") && set4.contains("b") && set4.contains("c") && set4.contains("d")); + } +} diff --git a/yang/yang-data-codec-gson/src/test/resources/bug-4969/json/foo.json b/yang/yang-data-codec-gson/src/test/resources/bug-4969/json/foo.json new file mode 100644 index 0000000000..f75065c4fb --- /dev/null +++ b/yang/yang-data-codec-gson/src/test/resources/bug-4969/json/foo.json @@ -0,0 +1,8 @@ +{ + "foo:root" : { + "ref1": "a", + "ref2": "a b", + "ref3": "a b c", + "ref4": "a b c d" + } +} \ No newline at end of file diff --git a/yang/yang-data-codec-gson/src/test/resources/bug-4969/yang/bar.yang b/yang/yang-data-codec-gson/src/test/resources/bug-4969/yang/bar.yang new file mode 100644 index 0000000000..b594d3d0a0 --- /dev/null +++ b/yang/yang-data-codec-gson/src/test/resources/bug-4969/yang/bar.yang @@ -0,0 +1,59 @@ +module bar { + namespace "bar"; + prefix bar; + + revision "2016-01-22" { + description "Initial version"; + } + + typedef ref1 { + type ref1-2; + } + + typedef ref2 { + type ref2-2; + } + + typedef ref3 { + type ref3-2; + } + + typedef ref1-2 { + type leafref { + path "/root/l1"; + } + } + + typedef ref2-2 { + type leafref { + path "/root/l2"; + } + } + + typedef ref3-2 { + type leafref { + path "/root/l3"; + } + } + + container root { + leaf l1 { + type bits { + bit a; + bit b; + bit c; + bit d; + } + } + leaf l2 { + type leafref { + path "/root/l1"; + } + } + leaf l3 { + type leafref { + path "../l1"; + } + } + } +} diff --git a/yang/yang-data-codec-gson/src/test/resources/bug-4969/yang/foo.yang b/yang/yang-data-codec-gson/src/test/resources/bug-4969/yang/foo.yang new file mode 100644 index 0000000000..ad3ac16074 --- /dev/null +++ b/yang/yang-data-codec-gson/src/test/resources/bug-4969/yang/foo.yang @@ -0,0 +1,27 @@ +module foo { + namespace "foo"; + prefix foo; + + import bar { prefix bar; revision-date 2016-01-22; } + + revision "2016-01-22" { + description "Initial version"; + } + + container root { + leaf ref1 { + type bar:ref1; + } + leaf ref2 { + type bar:ref2; + } + leaf ref3 { + type bar:ref3; + } + leaf ref4 { + type leafref { + path "/bar:root/bar:l1"; + } + } + } +} diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/xml/XmlStreamUtils.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/xml/XmlStreamUtils.java index b1f9805517..eeda8954c7 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/xml/XmlStreamUtils.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/xml/XmlStreamUtils.java @@ -12,6 +12,7 @@ import com.google.common.annotations.Beta; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; import com.google.common.base.Preconditions; +import com.google.common.base.Verify; import java.net.URI; import java.util.Map.Entry; import javax.annotation.Nonnull; @@ -108,6 +109,7 @@ public class XmlStreamUtils { if (schemaContext.isPresent() && baseType instanceof LeafrefTypeDefinition) { LeafrefTypeDefinition leafrefTypeDefinition = (LeafrefTypeDefinition) baseType; baseType = SchemaContextUtil.getBaseTypeForLeafRef(leafrefTypeDefinition, schemaContext.get(), schemaNode); + Verify.verifyNotNull(baseType, "Unable to find base type for leafref node '%s'.", schemaNode.getPath()); } writeValue(writer, baseType, value, parent); diff --git a/yang/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/BaseTypes.java b/yang/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/BaseTypes.java index facfe3080c..18e58d41a5 100644 --- a/yang/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/BaseTypes.java +++ b/yang/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/BaseTypes.java @@ -9,6 +9,7 @@ package org.opendaylight.yangtools.yang.model.util; import com.google.common.base.Optional; import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; import java.net.URI; import java.util.Set; @@ -58,26 +59,26 @@ public final class BaseTypes { public static final QName UINT64_QNAME = constructQName("uint64"); public static final QName UNION_QNAME = constructQName("union"); - private static final Set BUILT_IN_TYPES = ImmutableSet.builder() - .add(BINARY_QNAME.getLocalName()) - .add(BITS_QNAME.getLocalName()) - .add(BOOLEAN_QNAME.getLocalName()) - .add(DECIMAL64_QNAME.getLocalName()) - .add(EMPTY_QNAME.getLocalName()) - .add(ENUMERATION_QNAME.getLocalName()) - .add(IDENTITYREF_QNAME.getLocalName()) - .add(INSTANCE_IDENTIFIER_QNAME.getLocalName()) - .add(INT8_QNAME.getLocalName()) - .add(INT16_QNAME.getLocalName()) - .add(INT32_QNAME.getLocalName()) - .add(INT64_QNAME.getLocalName()) - .add(LEAFREF_QNAME.getLocalName()) - .add(STRING_QNAME.getLocalName()) - .add(UINT8_QNAME.getLocalName()) - .add(UINT16_QNAME.getLocalName()) - .add(UINT32_QNAME.getLocalName()) - .add(UINT64_QNAME.getLocalName()) - .add(UNION_QNAME.getLocalName()) + private static final Set BUILT_IN_TYPES = ImmutableSet.builder() + .add(BINARY_QNAME) + .add(BITS_QNAME) + .add(BOOLEAN_QNAME) + .add(DECIMAL64_QNAME) + .add(EMPTY_QNAME) + .add(ENUMERATION_QNAME) + .add(IDENTITYREF_QNAME) + .add(INSTANCE_IDENTIFIER_QNAME) + .add(INT8_QNAME) + .add(INT16_QNAME) + .add(INT32_QNAME) + .add(INT64_QNAME) + .add(LEAFREF_QNAME) + .add(STRING_QNAME) + .add(UINT8_QNAME) + .add(UINT16_QNAME) + .add(UINT32_QNAME) + .add(UINT64_QNAME) + .add(UNION_QNAME) .build(); /** @@ -102,7 +103,26 @@ public final class BaseTypes { * @return true if type is built-in YANG Types. */ public static boolean isYangBuildInType(final String type) { - return BUILT_IN_TYPES.contains(type); + if (Strings.isNullOrEmpty(type)) { + return false; + } + return BUILT_IN_TYPES.contains(QName.create(YangConstants.RFC6020_YANG_MODULE, type)); + } + + /** + * Returns true if supplied type is representation of built-in YANG type as + * per RFC 6020. + * + * See package documentation for description of base types. + * + * @param type + * @return true if type is built-in YANG Types. + */ + public static boolean isYangBuildInType(final TypeDefinition type) { + if(type == null) { + return false; + } + return BUILT_IN_TYPES.contains(type.getQName()); } /** diff --git a/yang/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/SchemaContextUtil.java b/yang/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/SchemaContextUtil.java index 1059421183..7ad5070f19 100644 --- a/yang/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/SchemaContextUtil.java +++ b/yang/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/SchemaContextUtil.java @@ -640,7 +640,7 @@ public final class SchemaContextUtil { } } - Module parentModule = findParentModuleByType(schemaContext, baseSchema); + Module parentModule = findParentModuleOfReferencingType(schemaContext, baseSchema); final DataSchemaNode dataSchemaNode; if(pathStatement.isAbsolute()) { @@ -665,7 +665,33 @@ public final class SchemaContextUtil { } } + private static Module findParentModuleOfReferencingType(final SchemaContext schemaContext, + final SchemaNode schemaNode) { + Preconditions.checkArgument(schemaContext != null, "Schema Context reference cannot be NULL!"); + Preconditions.checkArgument(schemaNode != null, "Schema Node cannot be NULL!"); + TypeDefinition nodeType = null; + + if (schemaNode instanceof LeafSchemaNode) { + nodeType = ((LeafSchemaNode) schemaNode).getType(); + } else if (schemaNode instanceof LeafListSchemaNode) { + nodeType = ((LeafListSchemaNode) schemaNode).getType(); + } + + if (nodeType.getBaseType() != null) { + while (nodeType.getBaseType() != null) { + nodeType = nodeType.getBaseType(); + } + + final QNameModule typeDefModuleQname = nodeType.getQName().getModule(); + return schemaContext.findModuleByNamespaceAndRevision(typeDefModuleQname.getNamespace(), + typeDefModuleQname.getRevision()); + } + + return SchemaContextUtil.findParentModule(schemaContext, schemaNode); + } + /** + * @deprecated due to expensive lookup * Returns parent Yang Module for specified Schema Context in which Schema * Node is declared. If Schema Node is of type 'ExtendedType' it tries to find parent module * in which the type was originally declared (needed for correct leafref path resolution).
@@ -684,6 +710,7 @@ public final class SchemaContextUtil { * Schema Node is NOT present, the method will returns * null */ + @Deprecated public static Module findParentModuleByType(final SchemaContext schemaContext, final SchemaNode schemaNode) { Preconditions.checkArgument(schemaContext != null, "Schema Context reference cannot be NULL!"); Preconditions.checkArgument(schemaNode != null, "Schema Node cannot be NULL!"); @@ -695,8 +722,8 @@ public final class SchemaContextUtil { nodeType = ((LeafListSchemaNode) schemaNode).getType(); } - if (nodeType instanceof ExtendedType) { - while (nodeType.getBaseType() instanceof ExtendedType) { + if (!BaseTypes.isYangBuildInType(nodeType) && nodeType.getBaseType() != null) { + while (nodeType.getBaseType() != null && !BaseTypes.isYangBuildInType(nodeType.getBaseType())) { nodeType = nodeType.getBaseType(); } -- 2.36.6