From: Jamo Luhrsen Date: Sun, 31 May 2020 05:29:52 +0000 (-0700) Subject: Allow list elements to be interleaved X-Git-Tag: v4.0.10~5 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=yangtools.git;a=commitdiff_plain;h=81b78a9f481e47b1ab54a136f807e3ecb71a29a4 Allow list elements to be interleaved If a list node A is parsed then list node B and another list with A is given after, it fails to parse as a duplicate. Skip checking for duplicate nodes if the node being parsed is found to correspond to ListEffectiveStatement. JIRA: YANGTOOLS-1107 Change-Id: I4dae263a1e41444db7a6cce6eb958f397c801070 Signed-off-by: Jamo Luhrsen Signed-off-by: Robert Varga (cherry picked from commit eb4617a9867325921e0fd9660898c3fc4dfc8d11) --- diff --git a/yang/yang-data-codec-xml/src/main/java/org/opendaylight/yangtools/yang/data/codec/xml/XmlParserStream.java b/yang/yang-data-codec-xml/src/main/java/org/opendaylight/yangtools/yang/data/codec/xml/XmlParserStream.java index 4e7d88fde5..5afef61018 100644 --- a/yang/yang-data-codec-xml/src/main/java/org/opendaylight/yangtools/yang/data/codec/xml/XmlParserStream.java +++ b/yang/yang-data-codec-xml/src/main/java/org/opendaylight/yangtools/yang/data/codec/xml/XmlParserStream.java @@ -59,6 +59,7 @@ import org.opendaylight.yangtools.yang.data.util.AbstractNodeDataWithSchema; import org.opendaylight.yangtools.yang.data.util.AnyXmlNodeDataWithSchema; import org.opendaylight.yangtools.yang.data.util.AnydataNodeDataWithSchema; import org.opendaylight.yangtools.yang.data.util.CompositeNodeDataWithSchema; +import org.opendaylight.yangtools.yang.data.util.CompositeNodeDataWithSchema.ChildReusePolicy; import org.opendaylight.yangtools.yang.data.util.ContainerNodeDataWithSchema; import org.opendaylight.yangtools.yang.data.util.LeafListEntryNodeDataWithSchema; import org.opendaylight.yangtools.yang.data.util.LeafListNodeDataWithSchema; @@ -437,7 +438,9 @@ public final class XmlParserStream implements Closeable, Flushable { switch (in.nextTag()) { case XMLStreamConstants.START_ELEMENT: - // FIXME: why do we even need this tracker? either document it or remove it + // FIXME: 6.0.0: why do we even need this tracker? either document it or remove it. + // it looks like it is a crude duplicate finder, which should really be handled via + // ChildReusePolicy.REJECT final Set> namesakes = new HashSet<>(); while (in.hasNext()) { final String xmlElementName = in.getLocalName(); @@ -466,11 +469,7 @@ public final class XmlParserStream implements Closeable, Flushable { } final String elementNS = in.getNamespaceURI(); - if (!namesakes.add(new SimpleImmutableEntry<>(elementNS, xmlElementName))) { - throw new XMLStreamException(String.format( - "Duplicate namespace \"%s\" element \"%s\" in XML input", elementNS, xmlElementName), - in.getLocation()); - } + final boolean added = namesakes.add(new SimpleImmutableEntry<>(elementNS, xmlElementName)); final URI nsUri; try { @@ -483,8 +482,16 @@ public final class XmlParserStream implements Closeable, Flushable { final Deque childDataSchemaNodes = ParserStreamUtils.findSchemaNodeByNameAndNamespace(parentSchema, xmlElementName, nsUri); if (!childDataSchemaNodes.isEmpty()) { + final boolean elementList = isElementList(childDataSchemaNodes); + if (!added && !elementList) { + throw new XMLStreamException(String.format( + "Duplicate element \"%s\" in namespace \"%s\" with parent \"%s\" in XML input", + xmlElementName, elementNS, parent.getSchema()), in.getLocation()); + } + // We have a match, proceed with it - read(in, ((CompositeNodeDataWithSchema) parent).addChild(childDataSchemaNodes), rootElement); + read(in, ((CompositeNodeDataWithSchema) parent).addChild(childDataSchemaNodes, + elementList ? ChildReusePolicy.REUSE : ChildReusePolicy.NOOP), rootElement); continue; } @@ -544,6 +551,13 @@ public final class XmlParserStream implements Closeable, Flushable { } } + // Return true if schema represents a construct which uses multiple sibling elements to represent its content. The + // siblings MAY be interleaved as per RFC7950. + private static boolean isElementList(final Deque childDataSchemaNodes) { + final DataSchemaNode last = childDataSchemaNodes.getLast(); + return last instanceof ListSchemaNode || last instanceof LeafListSchemaNode; + } + private static void addMountPointChild(final MountPointData mount, final URI namespace, final String localName, final DOMSource source) { final DOMSourceMountPointChild child = new DOMSourceMountPointChild(source); diff --git a/yang/yang-data-codec-xml/src/test/java/org/opendaylight/yangtools/yang/data/codec/xml/XmlToNormalizedNodesTest.java b/yang/yang-data-codec-xml/src/test/java/org/opendaylight/yangtools/yang/data/codec/xml/XmlToNormalizedNodesTest.java index b25ad30e4f..1e7f9cc039 100644 --- a/yang/yang-data-codec-xml/src/test/java/org/opendaylight/yangtools/yang/data/codec/xml/XmlToNormalizedNodesTest.java +++ b/yang/yang-data-codec-xml/src/test/java/org/opendaylight/yangtools/yang/data/codec/xml/XmlToNormalizedNodesTest.java @@ -157,8 +157,8 @@ public class XmlToNormalizedNodesTest { xmlParser.parse(reader); fail("XMLStreamException should have been thrown because of duplicate leaf."); } catch (XMLStreamException ex) { - assertThat(ex.getMessage(), containsString("Duplicate namespace \"foo-namespace\" element " - + "\"decimal64-leaf\" in XML input")); + assertThat(ex.getMessage(), containsString("Duplicate element \"decimal64-leaf\" in namespace" + + " \"foo-namespace\" with parent \"container leaf-container\" in XML input")); } } @@ -177,7 +177,8 @@ public class XmlToNormalizedNodesTest { xmlParser.parse(reader); fail("XMLStreamException should have been thrown because of duplicate anyxml"); } catch (XMLStreamException ex) { - assertThat(ex.getMessage(), containsString("Duplicate namespace \"foo-namespace\" element \"my-anyxml\"")); + assertThat(ex.getMessage(), containsString("Duplicate element \"my-anyxml\" in namespace" + + " \"foo-namespace\" with parent \"container anyxml-container\" in XML input")); } } @@ -196,8 +197,8 @@ public class XmlToNormalizedNodesTest { xmlParser.parse(reader); fail("XMLStreamException should have been thrown because of duplicate container"); } catch (XMLStreamException ex) { - assertThat(ex.getMessage(), containsString("Duplicate namespace \"foo-namespace\" element " - + "\"leaf-container\" in XML input")); + assertThat(ex.getMessage(), containsString("Duplicate element \"leaf-container\" in namespace" + + " \"foo-namespace\" with parent \"container parent-container\" in XML input")); } } diff --git a/yang/yang-data-codec-xml/src/test/java/org/opendaylight/yangtools/yang/data/codec/xml/YT1107Test.java b/yang/yang-data-codec-xml/src/test/java/org/opendaylight/yangtools/yang/data/codec/xml/YT1107Test.java new file mode 100644 index 0000000000..a0bf8eee1f --- /dev/null +++ b/yang/yang-data-codec-xml/src/test/java/org/opendaylight/yangtools/yang/data/codec/xml/YT1107Test.java @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2020 PANTHEON.tech, 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.xml; + +import static org.junit.Assert.assertEquals; + +import java.io.IOException; +import java.io.InputStream; +import java.net.URISyntaxException; +import java.util.Arrays; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamReader; +import org.junit.Test; +import org.opendaylight.yangtools.util.xml.UntrustedXML; +import org.opendaylight.yangtools.yang.common.QName; +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.stream.NormalizedNodeStreamWriter; +import org.opendaylight.yangtools.yang.data.impl.schema.Builders; +import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; +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.EffectiveModelContext; +import org.opendaylight.yangtools.yang.model.util.SchemaContextUtil; +import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils; +import org.xml.sax.SAXException; + +public class YT1107Test { + private static final QName PARENT = QName.create("yt1107", "parent"); + private static final QName ADMIN = QName.create(PARENT, "admin"); + private static final QName NAME = QName.create(PARENT, "name"); + private static final QName USER = QName.create(PARENT, "user"); + + @Test + public void testInterleavingLists() throws XMLStreamException, URISyntaxException, IOException, SAXException { + final EffectiveModelContext schemaContext = YangParserTestUtils.parseYangResource("/yt1107/yt1107.yang"); + final InputStream resourceAsStream = XmlToNormalizedNodesTest.class.getResourceAsStream("/yt1107/yt1107.xml"); + final XMLStreamReader reader = UntrustedXML.createXMLStreamReader(resourceAsStream); + + final NormalizedNodeResult result = new NormalizedNodeResult(); + final NormalizedNodeStreamWriter streamWriter = ImmutableNormalizedNodeStreamWriter.from(result); + final XmlParserStream xmlParser = XmlParserStream.create(streamWriter, schemaContext, + SchemaContextUtil.findNodeInSchemaContext(schemaContext, Arrays.asList(PARENT))); + xmlParser.parse(reader); + + assertEquals(Builders.containerBuilder() + .withNodeIdentifier(new NodeIdentifier(PARENT)) + .withChild(Builders.mapBuilder() + .withNodeIdentifier(new NodeIdentifier(ADMIN)) + .withChild(Builders.mapEntryBuilder() + .withNodeIdentifier(NodeIdentifierWithPredicates.of(ADMIN, NAME, "John")) + .withChild(ImmutableNodes.leafNode(NAME, "John")) + .build()) + .build()) + .withChild(Builders.mapBuilder() + .withNodeIdentifier(new NodeIdentifier(USER)) + .withChild(Builders.mapEntryBuilder() + .withNodeIdentifier(NodeIdentifierWithPredicates.of(USER, NAME, "Freud")) + .withChild(ImmutableNodes.leafNode(NAME, "Freud")) + .build()) + .withChild(Builders.mapEntryBuilder() + .withNodeIdentifier(NodeIdentifierWithPredicates.of(USER, NAME, "Bob")) + .withChild(ImmutableNodes.leafNode(NAME, "Bob")) + .build()) + .build()) + .build(), result.getResult()); + } +} diff --git a/yang/yang-data-codec-xml/src/test/resources/yt1107/yt1107.xml b/yang/yang-data-codec-xml/src/test/resources/yt1107/yt1107.xml new file mode 100644 index 0000000000..f71e22175b --- /dev/null +++ b/yang/yang-data-codec-xml/src/test/resources/yt1107/yt1107.xml @@ -0,0 +1,11 @@ + + + Freud + + + John + + + Bob + + diff --git a/yang/yang-data-codec-xml/src/test/resources/yt1107/yt1107.yang b/yang/yang-data-codec-xml/src/test/resources/yt1107/yt1107.yang new file mode 100644 index 0000000000..5ff2fce8d5 --- /dev/null +++ b/yang/yang-data-codec-xml/src/test/resources/yt1107/yt1107.yang @@ -0,0 +1,19 @@ +module yt1107 { + namespace "yt1107"; + prefix "yt1107"; + container parent { + config true; + list user { + key name; + leaf name { + type string; + } + } + list admin { + key name; + leaf name { + type string; + } + } + } +} diff --git a/yang/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/ChoiceNodeDataWithSchema.java b/yang/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/ChoiceNodeDataWithSchema.java index bd89e1d45c..0523436632 100644 --- a/yang/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/ChoiceNodeDataWithSchema.java +++ b/yang/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/ChoiceNodeDataWithSchema.java @@ -28,15 +28,15 @@ final class ChoiceNodeDataWithSchema extends CompositeNodeDataWithSchema extends AbstractNodeDataWithSchema { + /** + * Policy on how child nodes should be treated when an attempt is made to add them multiple times. + */ + @Beta + public enum ChildReusePolicy { + /** + * Do not consider any existing nodes at all, just perform a straight append. Multiple occurrences of a child + * will result in multiple children being emitted. This is almost certainly the wrong policy unless the caller + * prevents such a situation from arising via some different mechanism. + */ + NOOP, + /** + * Do not allow duplicate definition of a child node. This would typically be used when a child cannot be + * encountered multiple times, but the caller does not make any provision to detect such a conflict. If a child + * node would end up being defined a second time, {@link DuplicateChildNodeRejectedException} is reported. + */ + REJECT { + @Override + AbstractNodeDataWithSchema appendChild(final Collection> view, + final AbstractNodeDataWithSchema newChild) { + final DataSchemaNode childSchema = newChild.getSchema(); + final AbstractNodeDataWithSchema existing = findExistingChild(view, childSchema); + if (existing != null) { + throw new DuplicateChildNodeRejectedException("Duplicate child " + childSchema.getQName()); + } + return super.appendChild(view, newChild); + } + }, + /** + * Reuse previously-defined child node. This is most appropriate when a child may be visited multiple times + * and the intent is to append content of each visit. A typical usage is list elements with RFC7950 XML + * encoding, where there is no encapsulating element and hence list entries may be interleaved with other + * children. + */ + REUSE { + @Override + AbstractNodeDataWithSchema appendChild(final Collection> view, + final AbstractNodeDataWithSchema newChild) { + final AbstractNodeDataWithSchema existing = findExistingChild(view, newChild.getSchema()); + return existing != null ? existing : super.appendChild(view, newChild); + } + }; + + AbstractNodeDataWithSchema appendChild(final Collection> view, + final AbstractNodeDataWithSchema newChild) { + view.add(newChild); + return newChild; + } + + static @Nullable AbstractNodeDataWithSchema findExistingChild( + final Collection> view, final DataSchemaNode childSchema) { + for (AbstractNodeDataWithSchema existing : view) { + if (childSchema.equals(existing.getSchema())) { + return existing; + } + } + return null; + } + } + /** * nodes which were added to schema via augmentation and are present in data input. */ @@ -55,24 +117,24 @@ public class CompositeNodeDataWithSchema extends Abstr super(schema); } - private AbstractNodeDataWithSchema addChild(final DataSchemaNode schema) { - AbstractNodeDataWithSchema newChild = addSimpleChild(schema); - return newChild == null ? addCompositeChild(schema) : newChild; - } - @Deprecated public void addChild(final AbstractNodeDataWithSchema newChild) { children.add(newChild); } + @Deprecated public AbstractNodeDataWithSchema addChild(final Deque schemas) { + return addChild(schemas, ChildReusePolicy.NOOP); + } + + public AbstractNodeDataWithSchema addChild(final Deque schemas, final ChildReusePolicy policy) { checkArgument(!schemas.isEmpty(), "Expecting at least one schema"); // Pop the first node... final DataSchemaNode schema = schemas.pop(); if (schemas.isEmpty()) { // Simple, direct node - return addChild(schema); + return addChild(schema, policy); } // The choice/case mess, reuse what we already popped @@ -105,13 +167,18 @@ public class CompositeNodeDataWithSchema extends Abstr if (caseNodeDataWithSchema == null) { ChoiceNodeDataWithSchema choiceNodeDataWithSchema = new ChoiceNodeDataWithSchema(choiceNode); childNodes.add(choiceNodeDataWithSchema); - caseNodeDataWithSchema = choiceNodeDataWithSchema.addCompositeChild(caseNode); + caseNodeDataWithSchema = choiceNodeDataWithSchema.addCompositeChild(caseNode, ChildReusePolicy.NOOP); } - return caseNodeDataWithSchema.addChild(schemas); + return caseNodeDataWithSchema.addChild(schemas, policy); } - private AbstractNodeDataWithSchema addSimpleChild(final DataSchemaNode schema) { + private AbstractNodeDataWithSchema addChild(final DataSchemaNode schema, final ChildReusePolicy policy) { + AbstractNodeDataWithSchema newChild = addSimpleChild(schema, policy); + return newChild == null ? addCompositeChild(schema, policy) : newChild; + } + + private AbstractNodeDataWithSchema addSimpleChild(final DataSchemaNode schema, final ChildReusePolicy policy) { final SimpleNodeDataWithSchema newChild; if (schema instanceof LeafSchemaNode) { newChild = new LeafNodeDataWithSchema((LeafSchemaNode) schema); @@ -133,6 +200,9 @@ public class CompositeNodeDataWithSchema extends Abstr } else { augSchema = null; } + + // FIXME: 6.0.0: use policy once we have removed addChild() visibility + if (augSchema != null) { augmentationsToChild.put(augSchema, newChild); } else { @@ -161,7 +231,7 @@ public class CompositeNodeDataWithSchema extends Abstr return null; } - AbstractNodeDataWithSchema addCompositeChild(final DataSchemaNode schema) { + AbstractNodeDataWithSchema addCompositeChild(final DataSchemaNode schema, final ChildReusePolicy policy) { final CompositeNodeDataWithSchema newChild; if (schema instanceof ListSchemaNode) { @@ -176,17 +246,16 @@ public class CompositeNodeDataWithSchema extends Abstr newChild = new CompositeNodeDataWithSchema<>(schema); } - addCompositeChild(newChild); - return newChild; + return addCompositeChild(newChild, policy); } - final void addCompositeChild(final CompositeNodeDataWithSchema newChild) { + final AbstractNodeDataWithSchema addCompositeChild(final CompositeNodeDataWithSchema newChild, + final ChildReusePolicy policy) { final AugmentationSchemaNode augSchema = findCorrespondingAugment(getSchema(), newChild.getSchema()); - if (augSchema != null) { - augmentationsToChild.put(augSchema, newChild); - } else { - addChild(newChild); - } + final Collection> view = augSchema == null ? children + : augmentationsToChild.get(augSchema); + + return policy.appendChild(view, newChild); } /** diff --git a/yang/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/DuplicateChildNodeRejectedException.java b/yang/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/DuplicateChildNodeRejectedException.java new file mode 100644 index 0000000000..f1a9078c29 --- /dev/null +++ b/yang/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/DuplicateChildNodeRejectedException.java @@ -0,0 +1,19 @@ +/* + * Copyright (c) 2020 PANTHEON.tech, 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.util; + +import com.google.common.annotations.Beta; + +@Beta +public class DuplicateChildNodeRejectedException extends IllegalStateException { + private static final long serialVersionUID = 1L; + + public DuplicateChildNodeRejectedException(final String message) { + super(message); + } +}