BUG-1440: improve CompositeNodeDataWithSchema 42/10542/1
authorRobert Varga <rovarga@cisco.com>
Sat, 30 Aug 2014 21:54:37 +0000 (23:54 +0200)
committerRobert Varga <rovarga@cisco.com>
Sun, 31 Aug 2014 01:01:19 +0000 (03:01 +0200)
- Hide internal state, making fields final
- Preserve field order in augmentations
- Use preconditions and simplify checks
- Emit correct child sizing hints
- Speed up predicate/augmentation builder

Change-Id: I2192d9093aa3909a960f4172468d971d2fd51aec
Signed-off-by: Robert Varga <rovarga@cisco.com>
yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/ChoiceNodeDataWithSchema.java
yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/CompositeNodeDataWithSchema.java
yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/ContainerNodeDataWithSchema.java
yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/LeafListNodeDataWithSchema.java
yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/ListEntryNodeDataWithSchema.java
yang/yang-data-codec-gson/src/main/java/org/opendaylight/yangtools/yang/data/codec/gson/ListNodeDataWithSchema.java

index d68cbf9234cd83bbb3235d2b9f9e30c27bd76415..6f840bb3d67ab96ee89829b69a9eadd7f247c66e 100644 (file)
@@ -7,8 +7,6 @@
  */
 package org.opendaylight.yangtools.yang.data.codec.gson;
 
-import static org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter.UNKNOWN_SIZE;
-
 import java.io.IOException;
 
 import org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter;
@@ -30,7 +28,7 @@ class ChoiceNodeDataWithSchema extends CompositeNodeDataWithSchema {
     }
 
     @Override
-    public CompositeNodeDataWithSchema addCompositeChild(final DataSchemaNode schema) {
+    protected CompositeNodeDataWithSchema addCompositeChild(final DataSchemaNode schema) {
         CaseNodeDataWithSchema newChild = new CaseNodeDataWithSchema((ChoiceCaseNode) schema);
         caseNodeDataWithSchema = newChild;
         addCompositeChild(newChild);
@@ -43,8 +41,9 @@ class ChoiceNodeDataWithSchema extends CompositeNodeDataWithSchema {
 
     @Override
     public void write(final NormalizedNodeStreamWriter writer) throws IOException {
-        writer.startChoiceNode(provideNodeIdentifier(), UNKNOWN_SIZE);
+        writer.startChoiceNode(provideNodeIdentifier(), childSizeHint());
         super.write(writer);
         writer.endNode();
     }
+
 }
index 46c9507c12694ca05220a94564dc99e2da1edd13..4c38c3120ceb7434d1f38560e82cd0dabeae84ce 100644 (file)
@@ -7,17 +7,21 @@
  */
 package org.opendaylight.yangtools.yang.data.codec.gson;
 
+import com.google.common.base.Function;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Collections2;
+import com.google.common.collect.ImmutableSet;
+
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Deque;
-import java.util.HashMap;
-import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.Set;
+
+import javax.annotation.Nonnull;
 
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.AugmentationIdentifier;
@@ -33,23 +37,76 @@ import org.opendaylight.yangtools.yang.model.api.LeafListSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.LeafSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.ListSchemaNode;
 
+/**
+ * A node which is composed of multiple simpler nodes.
+ */
 class CompositeNodeDataWithSchema extends AbstractNodeDataWithSchema {
+    private static final Function<DataSchemaNode, QName> QNAME_FUNCTION = new Function<DataSchemaNode, QName>() {
+        @Override
+        public QName apply(@Nonnull final DataSchemaNode input) {
+            return input.getQName();
+        }
+    };
 
     /**
      * nodes which were added to schema via augmentation and are present in data input
      */
-    protected Map<AugmentationSchema, List<AbstractNodeDataWithSchema>> augmentationsToChild = new HashMap<>();
+    private final Map<AugmentationSchema, List<AbstractNodeDataWithSchema>> augmentationsToChild = new LinkedHashMap<>();
 
     /**
-     * remaining data nodes (which aren't added via augment). Every of them should have the same QName
+     * remaining data nodes (which aren't added via augment). Every of one them should have the same QName.
      */
-    protected List<AbstractNodeDataWithSchema> childs = new ArrayList<>();
+    private final List<AbstractNodeDataWithSchema> children = new ArrayList<>();
 
     public CompositeNodeDataWithSchema(final DataSchemaNode schema) {
         super(schema);
     }
 
-    public AbstractNodeDataWithSchema addSimpleChild(final DataSchemaNode schema) {
+    public AbstractNodeDataWithSchema addChild(final Deque<DataSchemaNode> schemas) {
+        Preconditions.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);
+        }
+
+        // The choice/case mess, reuse what we already popped
+        final DataSchemaNode choiceCandidate = schema;
+        Preconditions.checkArgument(choiceCandidate instanceof ChoiceNode,
+            "Expected node of type ChoiceNode but was %s", choiceCandidate.getClass().getSimpleName());
+        final ChoiceNode choiceNode = (ChoiceNode) choiceCandidate;
+
+        final DataSchemaNode caseCandidate = schemas.pop();
+        Preconditions.checkArgument(caseCandidate instanceof ChoiceCaseNode,
+            "Expected node of type ChoiceCaseNode but was %s", caseCandidate.getClass().getSimpleName());
+        final ChoiceCaseNode caseNode = (ChoiceCaseNode) caseCandidate;
+
+        AugmentationSchema augSchema = null;
+        if (choiceCandidate.isAugmenting()) {
+            augSchema = findCorrespondingAugment(getSchema(), choiceCandidate);
+        }
+
+        // looking for existing choice
+        final List<AbstractNodeDataWithSchema> childNodes;
+        if (augSchema != null) {
+            childNodes = augmentationsToChild.get(augSchema);
+        } else {
+            childNodes = children;
+        }
+
+        CompositeNodeDataWithSchema caseNodeDataWithSchema = findChoice(childNodes, choiceCandidate, caseCandidate);
+        if (caseNodeDataWithSchema == null) {
+            ChoiceNodeDataWithSchema choiceNodeDataWithSchema = new ChoiceNodeDataWithSchema(choiceNode);
+            addChild(choiceNodeDataWithSchema);
+            caseNodeDataWithSchema = choiceNodeDataWithSchema.addCompositeChild(caseNode);
+        }
+
+        return caseNodeDataWithSchema.addChild(schemas);
+    }
+
+    private AbstractNodeDataWithSchema addSimpleChild(final DataSchemaNode schema) {
         SimpleNodeDataWithSchema newChild = null;
         if (schema instanceof LeafSchemaNode) {
             newChild = new LeafNodeDataWithSchema(schema);
@@ -82,75 +139,26 @@ class CompositeNodeDataWithSchema extends AbstractNodeDataWithSchema {
         childsInAugment.add(newChild);
     }
 
-    public AbstractNodeDataWithSchema addChild(final Deque<DataSchemaNode> schemas) {
-        if (schemas.size() == 1) {
-            final DataSchemaNode childDataSchemaNode = schemas.pop();
-            return addChild(childDataSchemaNode);
-        } else {
-            DataSchemaNode choiceCandidate = schemas.pop();
-            DataSchemaNode caseCandidate = schemas.pop();
-            ChoiceNode choiceNode = null;
-            ChoiceCaseNode caseNode = null;
-            if (choiceCandidate instanceof ChoiceNode) {
-                choiceNode = (ChoiceNode) choiceCandidate;
-            } else {
-                throw new IllegalArgumentException("Awaited node of type ChoiceNode but was "
-                        + choiceCandidate.getClass().getSimpleName());
-            }
-
-            if (caseCandidate instanceof ChoiceCaseNode) {
-                caseNode = (ChoiceCaseNode) caseCandidate;
-            } else {
-                throw new IllegalArgumentException("Awaited node of type ChoiceCaseNode but was "
-                        + caseCandidate.getClass().getSimpleName());
-            }
-
-            AugmentationSchema augSchema = null;
-            if (choiceCandidate.isAugmenting()) {
-                augSchema = findCorrespondingAugment(getSchema(), choiceCandidate);
-            }
-
-            // looking for existing choice
-            List<AbstractNodeDataWithSchema> childNodes = Collections.emptyList();
-            if (augSchema != null) {
-                childNodes = augmentationsToChild.get(augSchema);
-            } else {
-                childNodes = childs;
-            }
-
-            CompositeNodeDataWithSchema caseNodeDataWithSchema = findChoice(childNodes, choiceCandidate, caseCandidate);
-            if (caseNodeDataWithSchema == null) {
-                ChoiceNodeDataWithSchema choiceNodeDataWithSchema = new ChoiceNodeDataWithSchema(choiceNode);
-                addChild(choiceNodeDataWithSchema);
-                caseNodeDataWithSchema = choiceNodeDataWithSchema.addCompositeChild(caseNode);
-            }
-
-            return caseNodeDataWithSchema.addChild(schemas);
-        }
-
-    }
-
     private CaseNodeDataWithSchema findChoice(final List<AbstractNodeDataWithSchema> childNodes, final DataSchemaNode choiceCandidate,
             final DataSchemaNode caseCandidate) {
-        if (childNodes == null) {
-            return null;
-        }
-        for (AbstractNodeDataWithSchema nodeDataWithSchema : childNodes) {
-            if (nodeDataWithSchema instanceof ChoiceNodeDataWithSchema
-                    && nodeDataWithSchema.getSchema().getQName().equals(choiceCandidate.getQName())) {
-                CaseNodeDataWithSchema casePrevious = ((ChoiceNodeDataWithSchema) nodeDataWithSchema).getCase();
-                if (casePrevious.getSchema().getQName() != caseCandidate.getQName()) {
-                    throw new IllegalArgumentException("Data from case " + caseCandidate.getQName()
-                            + " are specified but other data from case " + casePrevious.getSchema().getQName()
-                            + " were specified erlier. Data aren't from the same case.");
+        if (childNodes != null) {
+            for (AbstractNodeDataWithSchema nodeDataWithSchema : childNodes) {
+                if (nodeDataWithSchema instanceof ChoiceNodeDataWithSchema
+                        && nodeDataWithSchema.getSchema().getQName().equals(choiceCandidate.getQName())) {
+                    CaseNodeDataWithSchema casePrevious = ((ChoiceNodeDataWithSchema) nodeDataWithSchema).getCase();
+
+                    Preconditions.checkArgument(casePrevious.getSchema().getQName().equals(caseCandidate.getQName()),
+                        "Data from case %s are specified but other data from case %s were specified erlier. Data aren't from the same case.",
+                        caseCandidate.getQName(), casePrevious.getSchema().getQName());
+
+                    return casePrevious;
                 }
-                return casePrevious;
             }
         }
         return null;
     }
 
-    public AbstractNodeDataWithSchema addCompositeChild(final DataSchemaNode schema) {
+    AbstractNodeDataWithSchema addCompositeChild(final DataSchemaNode schema) {
         CompositeNodeDataWithSchema newChild;
         if (schema instanceof ListSchemaNode) {
             newChild = new ListNodeDataWithSchema(schema);
@@ -165,7 +173,7 @@ class CompositeNodeDataWithSchema extends AbstractNodeDataWithSchema {
         return newChild;
     }
 
-    public void addCompositeChild(final CompositeNodeDataWithSchema newChild) {
+    void addCompositeChild(final CompositeNodeDataWithSchema newChild) {
         AugmentationSchema augSchema = findCorrespondingAugment(getSchema(), newChild.getSchema());
         if (augSchema != null) {
             addChildToAugmentation(augSchema, newChild);
@@ -180,14 +188,22 @@ class CompositeNodeDataWithSchema extends AbstractNodeDataWithSchema {
     }
 
     public void addChild(final AbstractNodeDataWithSchema newChild) {
-        childs.add(newChild);
+        children.add(newChild);
+    }
+
+    /**
+     * Return a hint about how may children we are going to generate.
+     * @return Size of currently-present node list.
+     */
+    protected final int childSizeHint() {
+        return children.size();
     }
 
     /**
      * Tries to find in {@code parent} which is dealed as augmentation target node with QName as {@code child}. If such
      * node is found then it is returned, else null.
      */
-    protected AugmentationSchema findCorrespondingAugment(final DataSchemaNode parent, final DataSchemaNode child) {
+    AugmentationSchema findCorrespondingAugment(final DataSchemaNode parent, final DataSchemaNode child) {
         if (parent instanceof AugmentationTarget) {
             for (AugmentationSchema augmentation : ((AugmentationTarget) parent).getAvailableAugmentations()) {
                 DataSchemaNode childInAugmentation = augmentation.getDataChildByName(child.getQName());
@@ -201,15 +217,13 @@ class CompositeNodeDataWithSchema extends AbstractNodeDataWithSchema {
 
     @Override
     public void write(final NormalizedNodeStreamWriter writer) throws IOException {
-        for (AbstractNodeDataWithSchema child : childs) {
+        for (AbstractNodeDataWithSchema child : children) {
             child.write(writer);
         }
         for (Entry<AugmentationSchema, List<AbstractNodeDataWithSchema>> augmentationToChild : augmentationsToChild.entrySet()) {
-
             final List<AbstractNodeDataWithSchema> childsFromAgumentation = augmentationToChild.getValue();
-
             if (!childsFromAgumentation.isEmpty()) {
-                writer.startAugmentationNode(toAugmentationIdentifier(augmentationToChild));
+                writer.startAugmentationNode(toAugmentationIdentifier(augmentationToChild.getKey()));
 
                 for (AbstractNodeDataWithSchema nodeDataWithSchema : childsFromAgumentation) {
                     nodeDataWithSchema.write(writer);
@@ -220,15 +234,8 @@ class CompositeNodeDataWithSchema extends AbstractNodeDataWithSchema {
         }
     }
 
-    private static AugmentationIdentifier toAugmentationIdentifier(
-            final Entry<AugmentationSchema, List<AbstractNodeDataWithSchema>> augmentationToChild) {
-        Collection<DataSchemaNode> nodes = augmentationToChild.getKey().getChildNodes();
-        Set<QName> nodesQNames = new HashSet<>();
-        for (DataSchemaNode node : nodes) {
-            nodesQNames.add(node.getQName());
-        }
-
-        return new AugmentationIdentifier(nodesQNames);
+    private static AugmentationIdentifier toAugmentationIdentifier(final AugmentationSchema schema) {
+        final Collection<QName> qnames = Collections2.transform(schema.getChildNodes(), QNAME_FUNCTION);
+        return new AugmentationIdentifier(ImmutableSet.copyOf(qnames));
     }
-
 }
index 4e9125e77dbfb323e6e6566d40479aa5de6268e5..b473e9e8a19c7ae1334186f5da1ffc5d60f9abe7 100644 (file)
@@ -7,8 +7,6 @@
  */
 package org.opendaylight.yangtools.yang.data.codec.gson;
 
-import static org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter.UNKNOWN_SIZE;
-
 import java.io.IOException;
 
 import org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter;
@@ -22,7 +20,7 @@ class ContainerNodeDataWithSchema extends CompositeNodeDataWithSchema {
 
     @Override
     public void write(final NormalizedNodeStreamWriter writer) throws IOException {
-        writer.startContainerNode(provideNodeIdentifier(), UNKNOWN_SIZE);
+        writer.startContainerNode(provideNodeIdentifier(), childSizeHint());
         super.write(writer);
         writer.endNode();
     }
index 2ae4404034d4dfa440793c4e31e50889f37761ed..20a607d5fe6c1ea31c6b374bd1a2fb314eba90fb 100644 (file)
@@ -7,8 +7,6 @@
  */
 package org.opendaylight.yangtools.yang.data.codec.gson;
 
-import static org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter.UNKNOWN_SIZE;
-
 import java.io.IOException;
 
 import org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter;
@@ -21,7 +19,7 @@ class LeafListNodeDataWithSchema extends CompositeNodeDataWithSchema {
 
     @Override
     public void write(final NormalizedNodeStreamWriter writer) throws IOException {
-        writer.startLeafSet(provideNodeIdentifier(), UNKNOWN_SIZE);
+        writer.startLeafSet(provideNodeIdentifier(), childSizeHint());
         super.write(writer);
         writer.endNode();
     }
index 80845b0cdb537386878ecda00ca02a071859255c..8f23f4c4f661bb5d35a09e7185f87eea6c4834e2 100644 (file)
@@ -7,13 +7,16 @@
  */
 package org.opendaylight.yangtools.yang.data.codec.gson;
 
-import static org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter.UNKNOWN_SIZE;
+import com.google.common.base.Function;
+import com.google.common.collect.Maps;
 
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import javax.annotation.Nonnull;
+
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
 import org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter;
@@ -22,6 +25,12 @@ import org.opendaylight.yangtools.yang.model.api.LeafSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.ListSchemaNode;
 
 class ListEntryNodeDataWithSchema extends CompositeNodeDataWithSchema {
+    private static final Function<SimpleNodeDataWithSchema, Object> VALUE_FUNCTION = new Function<SimpleNodeDataWithSchema, Object>() {
+        @Override
+        public Object apply(@Nonnull final SimpleNodeDataWithSchema input) {
+            return input.getValue();
+        }
+    };
 
     private final Map<QName, SimpleNodeDataWithSchema> qNameToKeys = new HashMap<>();
 
@@ -50,28 +59,19 @@ class ListEntryNodeDataWithSchema extends CompositeNodeDataWithSchema {
 
     @Override
     public void write(final NormalizedNodeStreamWriter writer) throws IOException {
-        int keyCount = ((ListSchemaNode) getSchema()).getKeyDefinition().size();
+        final int keyCount = ((ListSchemaNode) getSchema()).getKeyDefinition().size();
         if (keyCount == 0) {
-            writer.startUnkeyedListItem(provideNodeIdentifier(), UNKNOWN_SIZE);
+            writer.startUnkeyedListItem(provideNodeIdentifier(), childSizeHint());
             super.write(writer);
             writer.endNode();
         } else if (keyCount == qNameToKeys.size()) {
-            writer.startMapEntryNode(provideNodeIdentifierWithPredicates(), UNKNOWN_SIZE);
+            writer.startMapEntryNode(
+                new NodeIdentifierWithPredicates(getSchema().getQName(), Maps.transformValues(qNameToKeys, VALUE_FUNCTION)),
+                childSizeHint());
             super.write(writer);
             writer.endNode();
         } else {
             throw new IllegalStateException("Some of keys of " + getSchema().getQName() + " are missing in input.");
         }
     }
-
-    private NodeIdentifierWithPredicates provideNodeIdentifierWithPredicates() {
-        Map<QName, Object> qNameToPredicateValues = new HashMap<>();
-
-        for (SimpleNodeDataWithSchema simpleNodeDataWithSchema : qNameToKeys.values()) {
-            qNameToPredicateValues.put(simpleNodeDataWithSchema.getSchema().getQName(), simpleNodeDataWithSchema.getValue());
-        }
-
-        return new NodeIdentifierWithPredicates(getSchema().getQName(), qNameToPredicateValues);
-    }
-
 }
index bbf81bd4ca27bde8d6827cf1f480a13a55a9238a..d21cd9a77a01000a9e48cb3b941899e68dc7d06f 100644 (file)
@@ -7,8 +7,6 @@
  */
 package org.opendaylight.yangtools.yang.data.codec.gson;
 
-import static org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter.UNKNOWN_SIZE;
-
 import java.io.IOException;
 
 import org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter;
@@ -23,10 +21,10 @@ class ListNodeDataWithSchema extends CompositeNodeDataWithSchema {
 
     @Override
     public void write(final NormalizedNodeStreamWriter writer) throws IOException {
-        if (!((ListSchemaNode) getSchema()).getKeyDefinition().isEmpty()) {
-            writer.startMapNode(provideNodeIdentifier(), UNKNOWN_SIZE);
+        if (((ListSchemaNode) getSchema()).getKeyDefinition().isEmpty()) {
+            writer.startUnkeyedList(provideNodeIdentifier(), childSizeHint());
         } else {
-            writer.startUnkeyedList(provideNodeIdentifier(), UNKNOWN_SIZE);
+            writer.startMapNode(provideNodeIdentifier(), childSizeHint());
         }
         super.write(writer);
         writer.endNode();