YANGTOOLS-766: add RFC7951JSONInstanceIdentifierCodec 74/66774/5
authorRobert Varga <robert.varga@pantheon.tech>
Wed, 27 Dec 2017 01:00:26 +0000 (02:00 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 28 Dec 2017 13:05:04 +0000 (14:05 +0100)
RFC7951 non-present prefixes result in last argument namespace reuse,
hence refactor AbstractStringInstanceIdentifierCodec to allow for QNames
to be instantiated using a memoized QNameModule.

The default implementation routes to createQName("", localName), which
preserves backwards compatibility. A specialized subclass
of JSONInstanceIdetifierCodec is created to use memoized QNameModule,
and it is wired to RFC7951 codec supplier.

A similar update is done in AbstractNamespaceCodec, where appendQName()
alternative, which is forwarded the last encountered QNameModule is
added -- the default routes to normal appendQName(), with RFC7951
comparing last QNameModule with current QName's and skipping prefix
serialization if they match.

This is sufficient to make related unit test pass, so remove @Ignore
from it and expand it to assert the decoded YangInstanceIdentifier.

Change-Id: Iec7996d0e1d759623c3cabcbaf0164737afa1649
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 88ed3390d89256f1857d921ccb7766ffe4b04555)

yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/JSONCodecFactory.java
yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/JSONCodecFactorySupplier.java
yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/JSONInstanceIdentifierCodec.java [moved from yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/JSONStringInstanceIdentifierCodec.java with 93% similarity]
yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/RFC7951JSONInstanceIdentifierCodec.java [new file with mode: 0644]
yang/yang-data-codec-gson/src/test/java/org/opendaylight/yangtools/yang/data/codec/gson/Bug8083Test.java
yang/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/AbstractNamespaceCodec.java
yang/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/AbstractStringInstanceIdentifierCodec.java
yang/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/XpathStringParsingPathArgumentBuilder.java

index 641a5f8af82d01bd899781d7eb70a651e2131269..e8f1ff18a0fa82ed0028516395356a17a434bbb9 100644 (file)
@@ -55,7 +55,7 @@ public final class JSONCodecFactory extends AbstractCodecFactory<JSONCodec<?>> {
     private final JSONCodec<?> iidCodec;
 
     JSONCodecFactory(final SchemaContext context, final CodecCache<JSONCodec<?>> cache,
-            final BiFunction<SchemaContext, JSONCodecFactory, JSONStringInstanceIdentifierCodec> iidCodecSupplier) {
+            final BiFunction<SchemaContext, JSONCodecFactory, JSONInstanceIdentifierCodec> iidCodecSupplier) {
         super(context, cache);
         iidCodec = verifyNotNull(iidCodecSupplier.apply(context, this));
     }
index 1b4d76636f4aef93bb874cf3b403fe249934791a..e75b515e14ab5685eee12dfd8c673f45789efc0f 100644 (file)
@@ -40,19 +40,19 @@ public enum JSONCodecFactorySupplier {
      * Source of {@link JSONCodecFactory} instances compliant with RFC7951.
      */
     // FIXME: YANGTOOLS-766: use a different codec
-    RFC7951(JSONStringInstanceIdentifierCodec::new),
+    RFC7951(RFC7951JSONInstanceIdentifierCodec::new),
     /**
      * Source of {@link JSONCodecFactory} instances compliant with RFC7951.
      */
-    DRAFT_LHOTKA_NETMOD_YANG_JSON_02(JSONStringInstanceIdentifierCodec::new);
+    DRAFT_LHOTKA_NETMOD_YANG_JSON_02(JSONInstanceIdentifierCodec::new);
 
     private static final Logger LOG = LoggerFactory.getLogger(JSONCodecFactorySupplier.class);
 
     private static final class EagerCacheLoader extends CacheLoader<SchemaContext, JSONCodecFactory> {
-        private final BiFunction<SchemaContext, JSONCodecFactory, JSONStringInstanceIdentifierCodec>
+        private final BiFunction<SchemaContext, JSONCodecFactory, JSONInstanceIdentifierCodec>
             iidCodecSupplier;
 
-        EagerCacheLoader(final BiFunction<SchemaContext, JSONCodecFactory, JSONStringInstanceIdentifierCodec>
+        EagerCacheLoader(final BiFunction<SchemaContext, JSONCodecFactory, JSONInstanceIdentifierCodec>
                 iidCodecSupplier) {
             this.iidCodecSupplier = requireNonNull(iidCodecSupplier);
         }
@@ -86,7 +86,7 @@ public enum JSONCodecFactorySupplier {
         }
     }
 
-    private final BiFunction<SchemaContext, JSONCodecFactory, JSONStringInstanceIdentifierCodec> iidCodecSupplier;
+    private final BiFunction<SchemaContext, JSONCodecFactory, JSONInstanceIdentifierCodec> iidCodecSupplier;
 
     // Weak keys to retire the entry when SchemaContext goes away
     private final LoadingCache<SchemaContext, JSONCodecFactory> precomputed;
@@ -95,7 +95,7 @@ public enum JSONCodecFactorySupplier {
     private final LoadingCache<SchemaContext, JSONCodecFactory> shared;
 
     JSONCodecFactorySupplier(
-            final BiFunction<SchemaContext, JSONCodecFactory, JSONStringInstanceIdentifierCodec> iidCodecSupplier) {
+            final BiFunction<SchemaContext, JSONCodecFactory, JSONInstanceIdentifierCodec> iidCodecSupplier) {
         this.iidCodecSupplier = requireNonNull(iidCodecSupplier);
         precomputed = CacheBuilder.newBuilder().weakKeys().build(new EagerCacheLoader(iidCodecSupplier));
         shared = CacheBuilder.newBuilder().weakKeys().build(new CacheLoader<SchemaContext, JSONCodecFactory>() {
@@ -20,13 +20,13 @@ import org.opendaylight.yangtools.yang.model.api.LeafSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.Module;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 
-final class JSONStringInstanceIdentifierCodec extends AbstractModuleStringInstanceIdentifierCodec
+class JSONInstanceIdentifierCodec extends AbstractModuleStringInstanceIdentifierCodec
         implements JSONCodec<YangInstanceIdentifier> {
     private final DataSchemaContextTree dataContextTree;
     private final JSONCodecFactory codecFactory;
     private final SchemaContext context;
 
-    JSONStringInstanceIdentifierCodec(final SchemaContext context, final JSONCodecFactory jsonCodecFactory) {
+    JSONInstanceIdentifierCodec(final SchemaContext context, final JSONCodecFactory jsonCodecFactory) {
         this.context = Preconditions.checkNotNull(context);
         this.dataContextTree = DataSchemaContextTree.from(context);
         this.codecFactory = Preconditions.checkNotNull(jsonCodecFactory);
diff --git a/yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/RFC7951JSONInstanceIdentifierCodec.java b/yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/RFC7951JSONInstanceIdentifierCodec.java
new file mode 100644 (file)
index 0000000..d7e716e
--- /dev/null
@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2017 Pantheon Technologies, s.r.o. 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;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import javax.annotation.Nullable;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.common.QNameModule;
+import org.opendaylight.yangtools.yang.model.api.SchemaContext;
+
+final class RFC7951JSONInstanceIdentifierCodec extends JSONInstanceIdentifierCodec {
+    RFC7951JSONInstanceIdentifierCodec(final SchemaContext context, final JSONCodecFactory jsonCodecFactory) {
+        super(context, jsonCodecFactory);
+    }
+
+    @Override
+    protected StringBuilder appendQName(final StringBuilder sb, final QName qname,
+            final @Nullable QNameModule lastModule) {
+        if (qname.getModule().equals(lastModule)) {
+            return sb.append(qname.getLocalName());
+        }
+
+        return super.appendQName(sb, qname, lastModule);
+    }
+
+    @Override
+    protected QName createQName(final @Nullable QNameModule lastModule, final String localName) {
+        checkArgument(lastModule != null, "Unprefixed leading name %s", localName);
+        return QName.create(lastModule, localName);
+    }
+}
index f94d0da75f9e1486a1de0eb792cf9136110dc841..a1d93bb9503f4b0483eb13187c59ab72823216e9 100644 (file)
@@ -8,13 +8,27 @@
 
 package org.opendaylight.yangtools.yang.data.codec.gson;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 import static org.opendaylight.yangtools.yang.data.codec.gson.TestUtils.loadTextFile;
 
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.gson.stream.JsonReader;
 import java.io.StringReader;
+import java.net.URI;
 import org.junit.Ignore;
 import org.junit.Test;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.common.QNameModule;
+import org.opendaylight.yangtools.yang.common.SimpleDateFormatUtil;
+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.NodeIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
+import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
+import org.opendaylight.yangtools.yang.data.api.schema.LeafNode;
 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.impl.schema.ImmutableNormalizedNodeStreamWriter;
@@ -23,8 +37,19 @@ import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils;
 
 public class Bug8083Test {
+    private static final QNameModule FOOMOD = QNameModule.create(URI.create("http://example.com/foomod"),
+        SimpleDateFormatUtil.DEFAULT_DATE_REV);
+    private static final QNameModule BARMOD = QNameModule.create(URI.create("http://example.com/barmod"),
+        SimpleDateFormatUtil.DEFAULT_DATE_REV);
+
+    private static final QName FOO_QNAME = QName.create(FOOMOD, "foo");
+    private static final QName FOOLIST_QNAME = QName.create(FOOMOD, "foo-list");
+    private static final QName NAME_QNAME = QName.create(FOOMOD, "name");
+    private static final QName TOP_QNAME = QName.create(FOOMOD, "top");
+
+    private static final QName BARCONTAINER_QNAME = QName.create(BARMOD, "bar-container");
+    private static final QName BARLEAF_QNAME = QName.create(BARMOD, "bar-leaf");
 
-    @Ignore("Instance-identifier codec has to be modified according to the RFC7951 to correctly parse this.")
     @Test
     public void testRFC7951InstanceIdentifierPath() throws Exception {
         final SchemaContext schemaContext = YangParserTestUtils.parseYangSources("/bug8083/yang/");
@@ -37,7 +62,20 @@ public class Bug8083Test {
             JSONCodecFactorySupplier.RFC7951.getShared(schemaContext));
         jsonParser.parse(new JsonReader(new StringReader(inputJson)));
         final NormalizedNode<?, ?> transformedInput = result.getResult();
-        assertNotNull(transformedInput);
+
+        assertTrue(transformedInput instanceof ContainerNode);
+        final ContainerNode container = (ContainerNode) transformedInput;
+        final NormalizedNode<?, ?> child = container.getChild(new NodeIdentifier(FOO_QNAME)).get();
+        assertTrue(child instanceof LeafNode);
+        final YangInstanceIdentifier expected = YangInstanceIdentifier.builder()
+                .node(TOP_QNAME)
+                .node(FOOLIST_QNAME)
+                .node(new NodeIdentifierWithPredicates(FOOLIST_QNAME, ImmutableMap.of(NAME_QNAME, "key-value")))
+                .node(new AugmentationIdentifier(ImmutableSet.of(BARCONTAINER_QNAME)))
+                .node(BARCONTAINER_QNAME)
+                .node(BARLEAF_QNAME)
+                .build();
+        assertEquals(expected, child.getValue());
     }
 
     @Ignore("JSONEmptyCodec needs to be fixed first.")
index 31d95bfe34b542d26bbb8c7cc5e34318ca8c5305..500015cd37d7d8a6b253bad23630bb0c64eb4c6c 100644 (file)
@@ -14,6 +14,7 @@ import java.util.Iterator;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.common.QNameModule;
 
 abstract class AbstractNamespaceCodec {
     private static final Splitter COLON_SPLITTER = Splitter.on(':');
@@ -50,6 +51,20 @@ abstract class AbstractNamespaceCodec {
         return sb;
     }
 
+    /**
+     * Append a QName, potentially taking into account last QNameModule encountered in the serialized path.
+     *
+     * @param sb target StringBuilder
+     * @param qname QName to append
+     * @param lastModule last QNameModule encountered, may be null
+     * @return target StringBuilder
+     */
+    protected StringBuilder appendQName(final StringBuilder sb, final QName qname,
+            final @Nullable QNameModule lastModule) {
+        // Covers XML and uncompressed JSON codec
+        return appendQName(sb, qname);
+    }
+
     protected final QName parseQName(final String str) {
         final String xPathPartTrimmed = getIdAndPrefixAsStr(str).trim();
         final Iterator<String> it = COLON_SPLITTER.split(xPathPartTrimmed).iterator();
index 5f6c9d12e2a7e2a26c43a8167e2b355d58fa3c5d..ba5a8d1f6d374e1b38ce31846d742a082a7546cf 100644 (file)
@@ -9,9 +9,12 @@ package org.opendaylight.yangtools.yang.data.util;
 
 import com.google.common.annotations.Beta;
 import com.google.common.base.Preconditions;
-import java.util.Map;
+import java.util.Map.Entry;
 import javax.annotation.Nonnull;
+import javax.xml.XMLConstants;
+import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.common.QNameModule;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue;
@@ -29,8 +32,9 @@ public abstract class AbstractStringInstanceIdentifierCodec extends AbstractName
 
     @Override
     public final String serialize(final YangInstanceIdentifier data) {
-        StringBuilder sb = new StringBuilder();
+        final StringBuilder sb = new StringBuilder();
         DataSchemaContextNode<?> current = getDataContextTree().getRoot();
+        QNameModule lastModule = null;
         for (PathArgument arg : data.getPathArguments()) {
             current = current.getChild(arg);
             Preconditions.checkArgument(current != null,
@@ -47,13 +51,15 @@ public abstract class AbstractStringInstanceIdentifierCodec extends AbstractName
                 continue;
             }
 
+            final QName qname = arg.getNodeType();
             sb.append('/');
-            appendQName(sb, arg.getNodeType());
+            appendQName(sb, qname, lastModule);
+            lastModule = qname.getModule();
 
             if (arg instanceof NodeIdentifierWithPredicates) {
-                for (Map.Entry<QName, Object> entry : ((NodeIdentifierWithPredicates) arg).getKeyValues().entrySet()) {
+                for (Entry<QName, Object> entry : ((NodeIdentifierWithPredicates) arg).getKeyValues().entrySet()) {
                     sb.append('[');
-                    appendQName(sb, entry.getKey());
+                    appendQName(sb, entry.getKey(), lastModule);
                     sb.append("='");
                     sb.append(String.valueOf(entry.getValue()));
                     sb.append("']");
@@ -98,4 +104,16 @@ public abstract class AbstractStringInstanceIdentifierCodec extends AbstractName
         return YangInstanceIdentifier.create(builder.build());
     }
 
+    /**
+     * Create QName from unprefixed name, potentially taking last QNameModule encountered into account.
+     *
+     * @param lastModule last QNameModule encountered, potentially null
+     * @param localName Local name string
+     * @return A newly-created QName
+     */
+    protected QName createQName(final @Nullable QNameModule lastModule, final String localName) {
+        // This implementation handles both XML encoding, where we follow XML namespace rules and old JSON encoding,
+        // which is the same thing: always encode prefixes
+        return createQName(XMLConstants.DEFAULT_NS_PREFIX, localName);
+    }
 }
index efd64a930b22bac0b2dfb57ff42f870e23bf7373..22d0d3325b307b9d0e26043ddf4e5d2b2cf0c805 100644 (file)
@@ -12,10 +12,10 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 import org.opendaylight.yangtools.concepts.Builder;
 import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.common.QNameModule;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
@@ -29,7 +29,7 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgum
  * is not correctly serialized or does not represent instance identifier valid
  * for associated schema context.
  */
-class XpathStringParsingPathArgumentBuilder implements Builder<Collection<PathArgument>> {
+final class XpathStringParsingPathArgumentBuilder implements Builder<List<PathArgument>> {
 
     /**
      * Matcher matching WSP YANG ABNF token.
@@ -57,12 +57,12 @@ class XpathStringParsingPathArgumentBuilder implements Builder<Collection<PathAr
     private static final char PRECONDITION_START = '[';
     private static final char PRECONDITION_END = ']';
 
+    private final List<PathArgument> product = new ArrayList<>();
     private final AbstractStringInstanceIdentifierCodec codec;
     private final String data;
 
-    private final List<PathArgument> product = new ArrayList<>();
-
     private DataSchemaContextNode<?> current;
+    private QNameModule lastModule;
     private int offset;
 
     XpathStringParsingPathArgumentBuilder(final AbstractStringInstanceIdentifierCodec codec, final String data) {
@@ -73,7 +73,7 @@ class XpathStringParsingPathArgumentBuilder implements Builder<Collection<PathAr
     }
 
     @Override
-    public Collection<PathArgument> build() {
+    public List<PathArgument> build() {
         while (!allCharactersConsumed()) {
             product.add(computeNextArgument());
         }
@@ -84,7 +84,9 @@ class XpathStringParsingPathArgumentBuilder implements Builder<Collection<PathAr
         checkValid(SLASH == currentChar(), "Identifier must start with '/'.");
         skipCurrentChar();
         checkValid(!allCharactersConsumed(), "Identifier cannot end with '/'.");
-        QName name = nextQName();
+        final QName name = nextQName();
+        // Memoize module
+        lastModule = name.getModule();
         if (allCharactersConsumed() || SLASH == currentChar()) {
             return computeIdentifier(name);
         }
@@ -168,18 +170,13 @@ class XpathStringParsingPathArgumentBuilder implements Builder<Collection<PathAr
     private QName nextQName() {
         // Consume prefix or identifier
         final String maybePrefix = nextIdentifier();
-        final String prefix;
-        final String localName;
-        if (COLON == currentChar()) {
-            // previous token is prefix;
-            prefix = maybePrefix;
+        if (!allCharactersConsumed() && COLON == currentChar()) {
+            // previous token is prefix
             skipCurrentChar();
-            localName = nextIdentifier();
-        } else {
-            prefix = "";
-            localName = maybePrefix;
+            return codec.createQName(maybePrefix, nextIdentifier());
         }
-        return codec.createQName(prefix, localName);
+
+        return codec.createQName(lastModule, maybePrefix);
     }
 
     /**