From 132a0d79e7e063cb7f5bc4545dc4dc22af8e3cc3 Mon Sep 17 00:00:00 2001
From: Robert Varga <rovarga@cisco.com>
Date: Tue, 18 Nov 2014 18:48:47 +0100
Subject: [PATCH] BUG-2402: add FIXMEs and javadoc

This adds an explanation about the interfaces being used and makes an
explicit warning for users to not use the exposed method.

Change-Id: I07753bea7380f7d92165b8ecb1840ee07d36dc06
Signed-off-by: Robert Varga <rovarga@cisco.com>
---
 ...tractImmutableDataContainerNodeBuilder.java | 18 ++++++++++++++++++
 .../AbstractImmutableDataContainerNode.java    |  9 +++++++++
 2 files changed, 27 insertions(+)

diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/AbstractImmutableDataContainerNodeBuilder.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/AbstractImmutableDataContainerNodeBuilder.java
index 669c4d05ab..cb50fefc8d 100644
--- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/AbstractImmutableDataContainerNodeBuilder.java
+++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/AbstractImmutableDataContainerNodeBuilder.java
@@ -43,6 +43,17 @@ abstract class AbstractImmutableDataContainerNodeBuilder<I extends YangInstanceI
 
     protected AbstractImmutableDataContainerNodeBuilder(final AbstractImmutableDataContainerNode<I> node) {
         this.nodeIdentifier = node.getIdentifier();
+
+        /*
+         * FIXME: BUG-2402: this call is not what we actually want. We are the
+         *        only user of getChildren(), and we really want this to be a
+         *        zero-copy operation if we happen to not modify the children.
+         *        If we do, we want to perform an efficient copy-on-write before
+         *        we make the change.
+         *
+         *        With this interface we end up creating a lot of short-lived
+         *        objects in case we modify the map -- see checkDirty().
+         */
         this.value = node.getChildren();
         this.dirty = true;
     }
@@ -62,6 +73,13 @@ abstract class AbstractImmutableDataContainerNodeBuilder<I extends YangInstanceI
 
     private void checkDirty() {
         if (dirty) {
+            /*
+             * FIXME: BUG-2402: This is the second part of the above. Note
+             *        that value here is usually a read-only view. Invocation
+             *        of this constructor will force instantiation of a wrapper
+             *        Map.Entry object, just to make sure this read path does
+             *        not modify the map.
+             */
             value = new HashMap<>(value);
             dirty = false;
         }
diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java
index a4b4268858..b6ac22617a 100644
--- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java
+++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java
@@ -39,6 +39,15 @@ public abstract class AbstractImmutableDataContainerNode<K extends PathArgument>
         return children.hashCode();
     }
 
+    /**
+     * DO NOT USE THIS METHOD.
+     *
+     * This is an implementation-internal API and no outside users should use it. If you do,
+     * you are asking for trouble, as the returned object is not guaranteed to conform to
+     * java.util.Map interface.
+     *
+     * @return An unmodifiable view if this node's children.
+     */
     public final Map<PathArgument, DataContainerChild<? extends PathArgument, ?>> getChildren() {
         return children;
     }
-- 
2.36.6