Fix attribute namespace lookup 87/57587/6
authorRobert Varga <robert.varga@pantheon.tech>
Sun, 21 May 2017 13:36:42 +0000 (15:36 +0200)
committerRobert Varga <nite@hq.sk>
Tue, 23 May 2017 13:22:39 +0000 (13:22 +0000)
During review process of Id214b78849998cf54e087685dcc78e3ded74ab69
we have broken writeAttributes() in the sense it is now sensitive
to attribute namespaces not hahving a prefix assigned in the writer.

Attributes are seldom used, but NETCONF uses them without emitting
them in the writer, hence the codec is not a drop-in replacement.

Add back the ability to map attributes, but also emit a warning if
the caller has not set up the namespace. The warning is emitted
only once for each namespace.

Also modify RandomPrefix to consult the NamespaceContext for already
existing prefix mappings.

Change-Id: Iaffcbfde06b7dffbf6e6ed02c0ba7d73fd053d6a
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
yang/yang-data-codec-xml/src/main/java/org/opendaylight/yangtools/yang/data/codec/xml/RandomPrefix.java
yang/yang-data-codec-xml/src/main/java/org/opendaylight/yangtools/yang/data/codec/xml/RandomPrefixInstanceIdentifierSerializer.java
yang/yang-data-codec-xml/src/main/java/org/opendaylight/yangtools/yang/data/codec/xml/SchemaAwareXMLStreamNormalizedNodeStreamWriter.java
yang/yang-data-codec-xml/src/main/java/org/opendaylight/yangtools/yang/data/codec/xml/SchemaAwareXMLStreamWriterUtils.java
yang/yang-data-codec-xml/src/main/java/org/opendaylight/yangtools/yang/data/codec/xml/SchemalessXMLStreamNormalizedNodeStreamWriter.java
yang/yang-data-codec-xml/src/main/java/org/opendaylight/yangtools/yang/data/codec/xml/XMLStreamNormalizedNodeStreamWriter.java
yang/yang-data-codec-xml/src/test/java/org/opendaylight/yangtools/yang/data/codec/xml/RandomPrefixTest.java
yang/yang-data-codec-xml/src/test/java/org/opendaylight/yangtools/yang/data/codec/xml/XmlStreamUtilsTest.java

index 80505dd54659180a6d3c4d9d8b792e78a4ba85a4..039ecd0d885ad949455ab337b7ad396ee54f1a62 100644 (file)
@@ -28,12 +28,8 @@ class RandomPrefix {
     private final BiMap<URI, String> prefixes = HashBiMap.create();
     private final NamespaceContext context;
 
-    RandomPrefix() {
-        this.context = null;
-    }
-
     RandomPrefix(final NamespaceContext context) {
-        this.context = Preconditions.checkNotNull(context);
+        this.context = context;
     }
 
     Iterable<Entry<URI, String>> getPrefixes() {
@@ -46,6 +42,13 @@ class RandomPrefix {
             return prefix;
         }
 
+        if (context != null) {
+            prefix = context.getPrefix(namespace.toString());
+            if (prefix != null) {
+                return prefix;
+            }
+        }
+
         do {
             prefix = encode(counter);
             counter++;
index 5e3a7a46e78253583241e59e7b5ce88ec2e926fe..550bb9b1a289fe73fbdcb3d44d6e46a1c1d036ec 100644 (file)
@@ -10,18 +10,19 @@ package org.opendaylight.yangtools.yang.data.codec.xml;
 import java.net.URI;
 import java.util.Map.Entry;
 import javax.annotation.Nonnull;
+import javax.xml.namespace.NamespaceContext;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.util.AbstractStringInstanceIdentifierCodec;
 import org.opendaylight.yangtools.yang.data.util.DataSchemaContextTree;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 
 final class RandomPrefixInstanceIdentifierSerializer extends AbstractStringInstanceIdentifierCodec {
-    private final RandomPrefix prefixes = new RandomPrefix();
     private final DataSchemaContextTree schemaTree;
+    private final RandomPrefix prefixes;
 
-
-    RandomPrefixInstanceIdentifierSerializer(SchemaContext ctx) {
-        schemaTree = DataSchemaContextTree.from(ctx);
+    RandomPrefixInstanceIdentifierSerializer(final SchemaContext schemaContext, final NamespaceContext nsContext) {
+        schemaTree = DataSchemaContextTree.from(schemaContext);
+        prefixes = new RandomPrefix(nsContext);
     }
 
     Iterable<Entry<URI, String>> getPrefixes() {
index b625eaa4f12417a4b499ed46204ee2a8b0a0f2a7..857ea91dd93200d00a93cc682f3a3b35f5df2f15 100644 (file)
@@ -8,11 +8,9 @@
  */
 package org.opendaylight.yangtools.yang.data.codec.xml;
 
-import com.google.common.base.Strings;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.Map;
-import java.util.Map.Entry;
 import javax.annotation.Nonnull;
 import javax.xml.stream.XMLStreamException;
 import javax.xml.stream.XMLStreamWriter;
@@ -47,23 +45,6 @@ final class SchemaAwareXMLStreamNormalizedNodeStreamWriter extends XMLStreamNorm
         return new SchemaAwareXMLStreamNormalizedNodeStreamWriter(writer, context, path);
     }
 
-    @Override
-    protected void writeAttributes(@Nonnull final Map<QName, String> attributes) throws IOException {
-        for (final Entry<QName, String> qNameStringEntry : attributes.entrySet()) {
-            try {
-                final String namespace = qNameStringEntry.getKey().getNamespace().toString();
-
-                if (Strings.isNullOrEmpty(namespace)) {
-                    writer.writeAttribute(qNameStringEntry.getKey().getLocalName(), qNameStringEntry.getValue());
-                } else {
-                    writer.writeAttribute(namespace, qNameStringEntry.getKey().getLocalName(), qNameStringEntry.getValue());
-                }
-            } catch (final XMLStreamException e) {
-                throw new IOException("Unable to emit attribute " + qNameStringEntry, e);
-            }
-        }
-    }
-
     @Override
     protected void writeValue(final XMLStreamWriter xmlWriter, final QName qname, @Nonnull final Object value,
             final SchemaNode schemaNode) throws IOException, XMLStreamException {
@@ -103,7 +84,7 @@ final class SchemaAwareXMLStreamNormalizedNodeStreamWriter extends XMLStreamNorm
     }
 
     @Override
-    public void leafNode(NodeIdentifier name, Object value, Map<QName, String> attributes) throws IOException {
+    public void leafNode(final NodeIdentifier name, final Object value, final Map<QName, String> attributes) throws IOException {
         final LeafSchemaNode schema = tracker.leafNode(name);
         writeElement(schema.getQName(), value, attributes, schema);
     }
index 5f7f5bb8db19d9511ea7fc4dde64875bea93f133..4e25cca2aa97eb2fe41e68638f0dc663c51dedaf 100644 (file)
@@ -36,7 +36,8 @@ final class SchemaAwareXMLStreamWriterUtils extends XMLStreamWriterUtils {
     @Override
     void writeInstanceIdentifier(final XMLStreamWriter writer, final YangInstanceIdentifier value)
             throws XMLStreamException {
-        RandomPrefixInstanceIdentifierSerializer iiCodec = new RandomPrefixInstanceIdentifierSerializer(schemaContext);
+        RandomPrefixInstanceIdentifierSerializer iiCodec = new RandomPrefixInstanceIdentifierSerializer(schemaContext,
+            writer.getNamespaceContext());
         String serializedValue = iiCodec.serialize(value);
 
         for (Entry<URI, String> e : iiCodec.getPrefixes()) {
index 4466b6bb88c4f9f31f4d69ab329c0740ec9748ac..9450c7a14b56ba333c6805a57eed5e55015f7b5a 100644 (file)
@@ -7,13 +7,11 @@
  */
 package org.opendaylight.yangtools.yang.data.codec.xml;
 
-import com.google.common.base.Strings;
 import java.io.IOException;
 import java.util.ArrayDeque;
 import java.util.Collections;
 import java.util.Deque;
 import java.util.Map;
-import java.util.Map.Entry;
 import javax.annotation.Nonnull;
 import javax.xml.stream.XMLStreamException;
 import javax.xml.stream.XMLStreamWriter;
@@ -35,113 +33,92 @@ class SchemalessXMLStreamNormalizedNodeStreamWriter extends XMLStreamNormalizedN
     }
 
     private final Deque<ContainerType> containerTypeStack = new ArrayDeque<>();
-    private final RandomPrefix randomPrefix;
 
-    private SchemalessXMLStreamNormalizedNodeStreamWriter(XMLStreamWriter writer) {
+    private SchemalessXMLStreamNormalizedNodeStreamWriter(final XMLStreamWriter writer) {
         super(writer);
-        randomPrefix = new RandomPrefix();
     }
 
-    static NormalizedNodeStreamWriter newInstance(XMLStreamWriter writer) {
+    static NormalizedNodeStreamWriter newInstance(final XMLStreamWriter writer) {
         return new SchemalessXMLStreamNormalizedNodeStreamWriter(writer);
     }
 
     @Override
-    public void leafNode(NodeIdentifier name, Object value, Map<QName, String> attributes) throws IOException {
+    public void leafNode(final NodeIdentifier name, final Object value, final Map<QName, String> attributes) throws IOException {
         writeElement(name.getNodeType(), value, attributes, null);
     }
 
     @Override
-    public void leafSetEntryNode(QName name, Object value, Map<QName, String> attributes) throws IOException {
+    public void leafSetEntryNode(final QName name, final Object value, final Map<QName, String> attributes) throws IOException {
         writeElement(name, value, attributes, null);
     }
 
     @Override
-    public void leafNode(NodeIdentifier name, Object value) throws IOException {
+    public void leafNode(final NodeIdentifier name, final Object value) throws IOException {
         writeElement(name.getNodeType(), value, Collections.emptyMap(), null);
     }
 
     @Override
-    public void leafSetEntryNode(QName name, Object value) throws IOException {
+    public void leafSetEntryNode(final QName name, final Object value) throws IOException {
         writeElement(name, value, Collections.emptyMap(), null);
     }
 
     @Override
-    public void startLeafSet(NodeIdentifier name, int childSizeHint) throws IOException {
+    public void startLeafSet(final NodeIdentifier name, final int childSizeHint) throws IOException {
         containerTypeStack.push(ContainerType.LEAF_SET);
     }
 
     @Override
-    public void startOrderedLeafSet(NodeIdentifier name, int childSizeHint)
+    public void startOrderedLeafSet(final NodeIdentifier name, final int childSizeHint)
             throws IOException, IllegalArgumentException {
         containerTypeStack.push(ContainerType.LEAF_SET);
     }
 
     @Override
-    public void startContainerNode(NodeIdentifier name, int childSizeHint) throws IOException {
+    public void startContainerNode(final NodeIdentifier name, final int childSizeHint) throws IOException {
         containerTypeStack.push(ContainerType.CONTAINER);
         startElement(name.getNodeType());
     }
 
     @Override
-    public void startChoiceNode(NodeIdentifier name, int childSizeHint) throws IOException {
+    public void startChoiceNode(final NodeIdentifier name, final int childSizeHint) throws IOException {
         containerTypeStack.push(ContainerType.CHOICE);
     }
 
     @Override
-    public void startAugmentationNode(AugmentationIdentifier identifier) throws IOException {
+    public void startAugmentationNode(final AugmentationIdentifier identifier) throws IOException {
         containerTypeStack.push(ContainerType.AUGMENTATION);
     }
 
     @Override
-    public void anyxmlNode(NodeIdentifier name, Object value) throws IOException {
+    public void anyxmlNode(final NodeIdentifier name, final Object value) throws IOException {
         anyxmlNode(name.getNodeType(), value);
     }
 
     @Override
-    public void startYangModeledAnyXmlNode(NodeIdentifier name, int childSizeHint) throws IOException {
+    public void startYangModeledAnyXmlNode(final NodeIdentifier name, final int childSizeHint) throws IOException {
         containerTypeStack.push(ContainerType.ANY_XML);
         startElement(name.getNodeType());
     }
 
     @Override
-    protected void writeAttributes(@Nonnull final Map<QName, String> attributes) throws IOException {
-        for (final Entry<QName, String> qNameStringEntry : attributes.entrySet()) {
-            try {
-                final String namespace = qNameStringEntry.getKey().getNamespace().toString();
-
-                if (Strings.isNullOrEmpty(namespace)) {
-                    writer.writeAttribute(qNameStringEntry.getKey().getLocalName(), qNameStringEntry.getValue());
-                } else {
-                    final String prefix = randomPrefix.encodePrefix(qNameStringEntry.getKey().getNamespace());
-                    writer.writeAttribute(prefix, namespace, qNameStringEntry.getKey().getLocalName(), qNameStringEntry
-                            .getValue());
-                }
-            } catch (final XMLStreamException e) {
-                throw new IOException("Unable to emit attribute " + qNameStringEntry, e);
-            }
-        }
-    }
-
-    @Override
-    protected void writeValue(XMLStreamWriter xmlWriter, QName qname, @Nonnull Object value, Object context)
+    protected void writeValue(final XMLStreamWriter xmlWriter, final QName qname, @Nonnull final Object value, final Object context)
             throws XMLStreamException {
         xmlWriter.writeCharacters(value.toString());
     }
 
     @Override
-    protected void startList(NodeIdentifier name) {
+    protected void startList(final NodeIdentifier name) {
         containerTypeStack.push(ContainerType.LIST);
     }
 
     @Override
-    protected void startListItem(PathArgument name) throws IOException {
+    protected void startListItem(final PathArgument name) throws IOException {
         containerTypeStack.push(ContainerType.LIST_ITEM);
         startElement(name.getNodeType());
     }
 
     @Override
-    protected void endNode(XMLStreamWriter xmlWriter) throws IOException, XMLStreamException {
+    protected void endNode(final XMLStreamWriter xmlWriter) throws IOException, XMLStreamException {
         ContainerType type = containerTypeStack.pop();
         switch (type) {
         case CONTAINER:
index bc8810379af39942f5926b93bbf7b22b4689e0ff..6eb16b97ebe2f46afeabf28e398c81759451ee74 100644 (file)
@@ -8,9 +8,14 @@
 package org.opendaylight.yangtools.yang.data.codec.xml;
 
 import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
 import java.io.IOException;
 import java.io.StringWriter;
+import java.net.URI;
 import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.xml.XMLConstants;
@@ -63,10 +68,14 @@ public abstract class XMLStreamNormalizedNodeStreamWriter<T> implements Normaliz
         TRANSFORMER_FACTORY = f;
     }
 
+    private static final Set<String> BROKEN_NAMESPACES = ConcurrentHashMap.newKeySet();
+
+    private final RandomPrefix prefixes;
     final XMLStreamWriter writer;
 
     XMLStreamNormalizedNodeStreamWriter(final XMLStreamWriter writer) {
         this.writer = Preconditions.checkNotNull(writer);
+        this.prefixes = new RandomPrefix(writer.getNamespaceContext());
     }
 
     /**
@@ -77,7 +86,7 @@ public abstract class XMLStreamNormalizedNodeStreamWriter<T> implements Normaliz
      * @return A new {@link NormalizedNodeStreamWriter}
      */
     public static NormalizedNodeStreamWriter create(final XMLStreamWriter writer, final SchemaContext context) {
-        return create( writer, context, SchemaPath.ROOT);
+        return create(writer, context, SchemaPath.ROOT);
     }
 
     /**
@@ -106,8 +115,6 @@ public abstract class XMLStreamNormalizedNodeStreamWriter<T> implements Normaliz
         return SchemalessXMLStreamNormalizedNodeStreamWriter.newInstance(writer);
     }
 
-    abstract void writeAttributes(@Nonnull final Map<QName, String> attributes) throws IOException;
-
     abstract void writeValue(final XMLStreamWriter xmlWriter, final QName qname,
             @Nonnull final Object value, T context) throws IOException, XMLStreamException;
 
@@ -117,6 +124,40 @@ public abstract class XMLStreamNormalizedNodeStreamWriter<T> implements Normaliz
 
     abstract void endNode(XMLStreamWriter xmlWriter) throws IOException, XMLStreamException;
 
+    private void writeAttributes(@Nonnull final Map<QName, String> attributes) throws IOException {
+        for (final Entry<QName, String> qNameStringEntry : attributes.entrySet()) {
+            try {
+                final QName qname = qNameStringEntry.getKey();
+                final String namespace = qname.getNamespace().toString();
+
+                if (!Strings.isNullOrEmpty(namespace)) {
+                    final String prefix = getPrefix(qname.getNamespace(), namespace);
+                    writer.writeAttribute(prefix, namespace, qname.getLocalName(), qNameStringEntry.getValue());
+                } else {
+                    writer.writeAttribute(qname.getLocalName(), qNameStringEntry.getValue());
+                }
+            } catch (final XMLStreamException e) {
+                throw new IOException("Unable to emit attribute " + qNameStringEntry, e);
+            }
+        }
+    }
+
+    private String getPrefix(final URI uri, final String str) throws XMLStreamException {
+        final String prefix = writer.getPrefix(str);
+        if (prefix != null) {
+            return prefix;
+        }
+
+        // This is needed to recover from attributes emitted while the namespace was not declared. Ordinarily
+        // attribute namespaces would be bound in the writer, so the resulting XML is efficient, but we cannot rely
+        // on that having been done.
+        if (BROKEN_NAMESPACES.add(str)) {
+            LOG.info("Namespace {} was not bound, please fix the caller", str, new Throwable());
+        }
+
+        return prefixes.encodePrefix(uri);
+    }
+
     private void writeStartElement(final QName qname) throws XMLStreamException {
         String ns = qname.getNamespace().toString();
         writer.writeStartElement(XMLConstants.DEFAULT_NS_PREFIX, qname.getLocalName(), ns);
index 81505b8b6b4511ae57e440eb40e0b1b9a3671c94..28b7710a9e80241936cb75bde6906dd99aea411d 100644 (file)
@@ -11,10 +11,10 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
 
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
 import java.net.URI;
+import java.util.ArrayList;
 import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
 import org.hamcrest.CoreMatchers;
 import org.junit.Test;
@@ -27,7 +27,7 @@ public class RandomPrefixTest {
     @Test
     public void testEncodeDecode() throws Exception {
 
-        final List<String> allGenerated = Lists.newArrayList();
+        final List<String> allGenerated = new ArrayList<>(MAX_COUNTER);
         for (int i = 0; i < MAX_COUNTER; i++) {
             final String encoded = RandomPrefix.encode(i);
             assertEquals(RandomPrefix.decode(encoded), i);
@@ -37,14 +37,14 @@ public class RandomPrefixTest {
         assertEquals(allGenerated.size(), MAX_COUNTER);
         assertEquals("dPT", allGenerated.get(MAX_COUNTER - 1));
         assertEquals("a", allGenerated.get(0));
-        assertEquals(allGenerated.size(), Sets.newHashSet(allGenerated).size());
+        assertEquals(allGenerated.size(), new HashSet<>(allGenerated).size());
     }
 
     @Test
     public void testQNameWithPrefix() throws Exception {
-        final RandomPrefix a = new RandomPrefix();
+        final RandomPrefix a = new RandomPrefix(null);
 
-        final List<String> allGenerated = Lists.newArrayList();
+        final List<String> allGenerated = new ArrayList<>();
         for (int i = 0; i < MAX_COUNTER; i++) {
             final String prefix = RandomPrefix.encode(i);
             final URI uri = new URI("localhost:" + prefix);
@@ -65,7 +65,7 @@ public class RandomPrefixTest {
 
     @Test
     public void test2QNames1Namespace() throws Exception {
-        final RandomPrefix a = new RandomPrefix();
+        final RandomPrefix a = new RandomPrefix(null);
 
         final URI uri = URI.create("localhost");
         final QName qName = QName.create(QNameModule.create(uri, new Date()), "local-name");
@@ -76,7 +76,7 @@ public class RandomPrefixTest {
 
     @Test
     public void testQNameNoPrefix() throws Exception {
-        final RandomPrefix a = new RandomPrefix();
+        final RandomPrefix a = new RandomPrefix(null);
 
         final URI uri = URI.create("localhost");
         QName qName = QName.create(uri, new Date(), "local-name");
index dc0c919098b1a827347dc05f89bf90de779f0be9..2b975184cd447e1c499853bee821aada8c05cd97 100644 (file)
@@ -76,7 +76,7 @@ public class XmlStreamUtilsTest {
         name = getAttrQName("namespace2", "2012-12-12", "attr", Optional.absent());
         final Map.Entry<QName, String> attributeEntryNoPrefix = new AbstractMap.SimpleEntry<>(name, "value");
 
-        final RandomPrefix randomPrefix = new RandomPrefix();
+        final RandomPrefix randomPrefix = new RandomPrefix(null);
         XMLStreamWriterUtils.writeAttribute(writer, attributeEntry, randomPrefix);
         XMLStreamWriterUtils.writeAttribute(writer, attributeEntryNoPrefix, randomPrefix);