HONEYCOMB-261 Make ModificationDiff schema aware

to check presence statement on containers for starters
+ update all checks in ModificationDiff to be performed on schema
instead of data

Change-Id: I6ec1a0f9e1ca821ef9f6835072d075dd0994bdb6
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>

6 files changed, 238 insertions, 105 deletions

diff --git a/infra/data-impl/pom.xml b/infra/data-impl/pom.xml
index 2d6f9fd61..5e8ab37eb 100644
--- a/infra/data-impl/pom.xml
+++ b/infra/data-impl/pom.xml
@@ -64,6 +64,11 @@
             <artifactId>mockito-core</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.skinny-framework</groupId>
+            <artifactId>skinny-logback</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>
diff --git a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegator.java b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegator.java
index 7f8b53919..213208064 100644
--- a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegator.java
+++ b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegator.java
@@ -46,6 +46,7 @@ import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateNode;
+import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -62,6 +63,7 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager
     private final org.opendaylight.controller.md.sal.binding.api.DataBroker contextBroker;
     // TODO HONEYCOMB-161 what to use instead of deprecated BindingNormalizedNodeSerializer ?
     private final BindingNormalizedNodeSerializer serializer;
+    private final SchemaContext schema;
 
     /**
      * Creates configuration data tree instance.
@@ -73,12 +75,14 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager
      */
     public ModifiableDataTreeDelegator(@Nonnull final BindingNormalizedNodeSerializer serializer,
                                        @Nonnull final DataTree dataTree,
+                                       @Nonnull final SchemaContext schema,
                                        @Nonnull final WriterRegistry writerRegistry,
                                        @Nonnull final org.opendaylight.controller.md.sal.binding.api.DataBroker contextBroker) {
         super(dataTree);
         this.contextBroker = checkNotNull(contextBroker, "contextBroker should not be null");
         this.serializer = checkNotNull(serializer, "serializer should not be null");
         this.writerRegistry = checkNotNull(writerRegistry, "writerRegistry should not be null");
+        this.schema = checkNotNull(schema, "schema should not be null");
     }
 
     @Override
@@ -115,7 +119,7 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager
                 rootPath, rootNode, rootNode.getDataBefore(), rootNode.getDataAfter());
 
             final ModificationDiff modificationDiff =
-                    ModificationDiff.recursivelyFromCandidateRoot(rootNode);
+                    ModificationDiff.recursivelyFromCandidateRoot(rootNode, schema);
             LOG.debug("ConfigDataTree.modify() diff: {}", modificationDiff);
 
             // Distinguish between updates (create + update) and deletes
diff --git a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModificationDiff.java b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModificationDiff.java
index e78bb876b..723bb88ad 100644
--- a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModificationDiff.java
+++ b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModificationDiff.java
@@ -18,31 +18,50 @@ package io.fd.honeycomb.data.impl;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType.APPEARED;
+import static org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType.DELETE;
+import static org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType.DISAPPEARED;
+import static org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType.WRITE;
 
 import com.google.common.collect.ImmutableMap;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
-import org.opendaylight.yangtools.yang.data.api.schema.AugmentationNode;
-import org.opendaylight.yangtools.yang.data.api.schema.ChoiceNode;
-import org.opendaylight.yangtools.yang.data.api.schema.LeafNode;
 import org.opendaylight.yangtools.yang.data.api.schema.MixinNode;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateNode;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType;
+import org.opendaylight.yangtools.yang.model.api.AugmentationSchema;
+import org.opendaylight.yangtools.yang.model.api.AugmentationTarget;
+import org.opendaylight.yangtools.yang.model.api.ChoiceSchemaNode;
+import org.opendaylight.yangtools.yang.model.api.ContainerSchemaNode;
+import org.opendaylight.yangtools.yang.model.api.DataNodeContainer;
+import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
+import org.opendaylight.yangtools.yang.model.api.LeafSchemaNode;
+import org.opendaylight.yangtools.yang.model.api.ListSchemaNode;
+import org.opendaylight.yangtools.yang.model.api.SchemaContext;
+import org.opendaylight.yangtools.yang.model.api.SchemaNode;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Recursively collects and provides all unique and non-null modifications (modified normalized nodes).
  */
 final class ModificationDiff {
 
+    private static final Logger LOG = LoggerFactory.getLogger(ModificationDiff.class);
+
     private static final ModificationDiff EMPTY_DIFF = new ModificationDiff(Collections.emptyMap());
-    private static final EnumSet VALID_MODIFICATIONS = EnumSet.of(ModificationType.WRITE, ModificationType.DELETE);
-    private static final EnumSet IGNORED_MODIFICATIONS = EnumSet.of(ModificationType.APPEARED, ModificationType.DISAPPEARED);
+    private static final EnumSet VALID_MODIFICATIONS = EnumSet.of(WRITE, DELETE);
+    private static final EnumSet IGNORED_MODIFICATIONS = EnumSet.of(APPEARED, DISAPPEARED);
 
     private final Map<YangInstanceIdentifier, NormalizedNodeUpdate> updates;
 
@@ -71,16 +90,17 @@ final class ModificationDiff {
         return new ModificationDiff(join(updates, other.updates));
     }
 
-    private static Map<YangInstanceIdentifier, NormalizedNodeUpdate> join(Map<YangInstanceIdentifier, NormalizedNodeUpdate> first,
-                                                                          Map<YangInstanceIdentifier, NormalizedNodeUpdate> second) {
+    private static Map<YangInstanceIdentifier, NormalizedNodeUpdate> join(
+            Map<YangInstanceIdentifier, NormalizedNodeUpdate> first,
+            Map<YangInstanceIdentifier, NormalizedNodeUpdate> second) {
         final Map<YangInstanceIdentifier, NormalizedNodeUpdate> merged = new HashMap<>();
         merged.putAll(first);
         merged.putAll(second);
         return merged;
     }
 
-    private static ModificationDiff create(YangInstanceIdentifier id, DataTreeCandidateNode candidate) {
-        return new ModificationDiff(ImmutableMap.of(id, NormalizedNodeUpdate.create(id, candidate)));
+    private static ModificationDiff create(Modification modification) {
+        return new ModificationDiff(ImmutableMap.of(modification.getId(), NormalizedNodeUpdate.create(modification)));
     }
 
     /**
@@ -88,140 +108,98 @@ final class ModificationDiff {
      * are complex nodes which direct leaves were not modified.
      */
     @Nonnull
-    static ModificationDiff recursivelyFromCandidate(@Nonnull final YangInstanceIdentifier yangIid,
-                                                     @Nonnull final DataTreeCandidateNode currentCandidate) {
+    static ModificationDiff recursivelyFromCandidate(@Nonnull final Modification modification) {
         // recursively process child nodes for exact modifications
-        return recursivelyChildrenFromCandidate(yangIid, currentCandidate)
+        return recursivelyChildrenFromCandidate(modification)
                 // also add modification on current level, if elligible
-                .merge(isModification(currentCandidate)
-                        ? ModificationDiff.create(yangIid, currentCandidate)
+                .merge(isModification(modification)
+                        ? ModificationDiff.create(modification)
                         : EMPTY_DIFF);
     }
 
     /**
-     * Same as {@link #recursivelyFromCandidate(YangInstanceIdentifier, DataTreeCandidateNode)} but does not process
-     * the root node for modifications, since it's the artificial data root, that has no child leaves but always is
-     * marked as SUBTREE_MODIFIED.
+     * Same as {@link #recursivelyFromCandidate(Modification)} but does
+     * not process the root node for modifications, since it's the artificial data root, that has no child leaves but
+     * always is marked as SUBTREE_MODIFIED.
      */
     @Nonnull
-    static ModificationDiff recursivelyFromCandidateRoot(@Nonnull final DataTreeCandidateNode currentCandidate) {
-        return recursivelyChildrenFromCandidate(YangInstanceIdentifier.EMPTY, currentCandidate);
+    static ModificationDiff recursivelyFromCandidateRoot(@Nonnull final DataTreeCandidateNode currentCandidate,
+                                                         @Nonnull final SchemaContext ctx) {
+        return recursivelyChildrenFromCandidate(new Modification(YangInstanceIdentifier.EMPTY, currentCandidate, ctx));
     }
 
     /**
      * Check whether current node was modified. {@link MixinNode}s are ignored
      * and only nodes which direct leaves(or choices) are modified are considered a modification.
      */
-    private static Boolean isModification(@Nonnull final DataTreeCandidateNode currentCandidate) {
+    private static Boolean isModification(@Nonnull final Modification modification) {
         // Disappear is not a modification
-        if (IGNORED_MODIFICATIONS.contains(currentCandidate.getModificationType())) {
+        if (IGNORED_MODIFICATIONS.contains(modification.getModificationType())) {
             return false;
         // Mixin nodes are not considered modifications
-        } else if (isMixin(currentCandidate) && !isAugment(currentCandidate)) {
+        } else if (modification.isMixin() && !modification.is(AugmentationSchema.class)) {
             return false;
         } else {
-            return isCurrentModified(currentCandidate);
+            return isCurrentModified(modification);
         }
     }
 
-    private static Boolean isCurrentModified(final @Nonnull DataTreeCandidateNode currentCandidate) {
+    private static Boolean isCurrentModified(@Nonnull final Modification modification) {
         // First check if it's an empty presence node
-        if (isEmptyPresenceNode(currentCandidate)) {
-            return true;
-        }
+        final boolean emptyPresenceNode = isEmptyPresenceNode(modification);
 
         // Check if there are any modified leaves and if so, consider current node as modified
-        final Boolean directLeavesModified = currentCandidate.getChildNodes().stream()
-                .filter(ModificationDiff::isLeaf)
+        final Boolean directLeavesModified = emptyPresenceNode
+                || modification.streamChildren()
+                .filter(child -> child.is(LeafSchemaNode.class))
                 // For some reason, we get modifications on unmodified list keys
                 // and that messes up our modifications collection here, so we need to skip
-                .filter(ModificationDiff::isBeforeAndAfterDifferent)
+                .filter(Modification::isBeforeAndAfterDifferent)
                 .filter(child -> VALID_MODIFICATIONS.contains(child.getModificationType()))
                 .findFirst()
                 .isPresent();
 
-        return directLeavesModified
-                // Also check choices (choices do not exist in BA world and if anything within a choice was modified,
-                // consider its parent as being modified)
-                || currentCandidate.getChildNodes().stream()
-                        .filter(ModificationDiff::isChoice)
-                        // Recursively check each choice if there was any change to it
-                        .filter(ModificationDiff::isCurrentModified)
-                        .findFirst()
-                        .isPresent();
+        // Also as fallback check choices (choices do not exist in BA world and if anything within a choice was modified,
+        // consider its parent as being modified)
+        final boolean modified = directLeavesModified
+                || modification.streamChildren()
+                .filter(child -> child.is(ChoiceSchemaNode.class))
+                // Recursively check each choice if there was any change to it
+                .filter(ModificationDiff::isCurrentModified)
+                .findFirst()
+                .isPresent();
+
+        if (modified) {
+            LOG.debug("Modification detected as {} at {}",
+                    modification.getModificationType(), modification.getId());
+        }
+
+        return modified;
     }
 
     /**
      * Check if new data are empty but still to be considered as a modification, meaning it's presence has a meaning
      * e.g. containers with presence statement.
      */
-    private static boolean isEmptyPresenceNode(final @Nonnull DataTreeCandidateNode currentCandidate) {
-        return currentCandidate.getChildNodes().isEmpty()
-                && VALID_MODIFICATIONS.contains(currentCandidate.getModificationType());
+    private static boolean isEmptyPresenceNode(@Nonnull final Modification modification) {
+        return modification.is(ContainerSchemaNode.class)
+                && ((ContainerSchemaNode) modification.getSchemaNode()).isPresenceContainer()
+                && modification.getChildNodes().isEmpty()
+                && VALID_MODIFICATIONS.contains(modification.getModificationType());
     }
 
     /**
      * Process all non-leaf child nodes recursively, creating aggregated {@link ModificationDiff}.
      */
-    private static ModificationDiff recursivelyChildrenFromCandidate(final @Nonnull YangInstanceIdentifier yangIid,
-                                                                     final @Nonnull DataTreeCandidateNode currentCandidate) {
+    private static ModificationDiff recursivelyChildrenFromCandidate(@Nonnull final Modification modification) {
         // recursively process child nodes for specific modifications
-        return currentCandidate.getChildNodes().stream()
-                // not interested in modifications to leaves
-                .filter(child -> !isLeaf(child))
-                .map(candidate -> recursivelyFromCandidate(yangIid.node(candidate.getIdentifier()), candidate))
+        return modification.streamChildren()
+                .filter(child -> !child.is(LeafSchemaNode.class))
+                .map(ModificationDiff::recursivelyFromCandidate)
                 .reduce(ModificationDiff::merge)
                 .orElse(EMPTY_DIFF);
     }
 
-    /**
-     * Check whether candidate.before and candidate.after is different. If not return false.
-     */
-    private static boolean isBeforeAndAfterDifferent(@Nonnull final DataTreeCandidateNode candidateNode) {
-        if (candidateNode.getDataBefore().isPresent()) {
-            return !candidateNode.getDataBefore().get().equals(candidateNode.getDataAfter().orNull());
-        }
-
-        // considering not a modification if data after is also null
-        return candidateNode.getDataAfter().isPresent();
-    }
-
-    /**
-     * Check whether candidate node is for a leaf type node.
-     */
-    private static boolean isLeaf(final DataTreeCandidateNode candidateNode) {
-        // orNull intentional, some candidate nodes have both data after and data before null
-        return candidateNode.getDataAfter().orNull() instanceof LeafNode<?>
-                || candidateNode.getDataBefore().orNull() instanceof LeafNode<?>;
-    }
-
-    /**
-     * Check whether candidate node is for a Mixin type node.
-     */
-    private static boolean isMixin(final DataTreeCandidateNode candidateNode) {
-        // orNull intentional, some candidate nodes have both data after and data before null
-        return candidateNode.getDataAfter().orNull() instanceof MixinNode
-                || candidateNode.getDataBefore().orNull() instanceof MixinNode;
-    }
-
-    /**
-     * Check whether candidate node is for an Augmentation type node.
-     */
-    private static boolean isAugment(final DataTreeCandidateNode candidateNode) {
-        // orNull intentional, some candidate nodes have both data after and data before null
-        return candidateNode.getDataAfter().orNull() instanceof AugmentationNode
-                || candidateNode.getDataBefore().orNull() instanceof AugmentationNode;
-    }
-
-    /**
-     * Check whether candidate node is for a Choice type node.
-     */
-    private static boolean isChoice(final DataTreeCandidateNode candidateNode) {
-        // orNull intentional, some candidate nodes have both data after and data before null
-        return candidateNode.getDataAfter().orNull() instanceof ChoiceNode
-                || candidateNode.getDataBefore().orNull() instanceof ChoiceNode;
-    }
-
     @Override
     public String toString() {
         return "ModificationDiff{updates=" + updates + '}';
@@ -262,15 +240,19 @@ final class ModificationDiff {
             return id;
         }
 
-        static NormalizedNodeUpdate create(@Nonnull final YangInstanceIdentifier id,
-                                           @Nonnull final DataTreeCandidateNode candidate) {
-            return create(id, candidate.getDataBefore().orNull(), candidate.getDataAfter().orNull());
+        static NormalizedNodeUpdate create(@Nonnull final Modification modification) {
+            final com.google.common.base.Optional<NormalizedNode<?, ?>> beforeData =
+                    modification.getDataBefore();
+            final com.google.common.base.Optional<NormalizedNode<?, ?>> afterData =
+                    modification.getDataAfter();
+            checkArgument(beforeData.isPresent() || afterData.isPresent(),
+                    "Both before and after data are null for $s", modification.getId());
+            return NormalizedNodeUpdate.create(modification.getId(), beforeData.orNull(), afterData.orNull());
         }
 
         static NormalizedNodeUpdate create(@Nonnull final YangInstanceIdentifier id,
                                            @Nullable final NormalizedNode<?, ?> dataBefore,
                                            @Nullable final NormalizedNode<?, ?> dataAfter) {
-            checkArgument(!(dataBefore == null && dataAfter == null), "Both before and after data are null");
             return new NormalizedNodeUpdate(id, dataBefore, dataAfter);
         }
 
@@ -303,4 +285,118 @@ final class ModificationDiff {
         }
     }
 
+    /**
+     * Intermediate representation of a modification + its schema.
+     */
+    private static final class Modification {
+        private final YangInstanceIdentifier id;
+        private final DataTreeCandidateNode dataCandidate;
+        // Using Object as type for schema node since it's the only type that's a parent to all schema node types from
+        // yangtools. The hierarchy does not use e.g. SchemaNode class for all types
+        private final Object schemaNode;
+
+        Modification(final YangInstanceIdentifier id,
+                     final DataTreeCandidateNode dataCandidate,
+                     final Object schemaNode) {
+            this.id = id;
+            this.dataCandidate = dataCandidate;
+            this.schemaNode = schemaNode;
+        }
+
+        Stream<Modification> streamChildren() {
+            return dataCandidate.getChildNodes().stream()
+                    .map(child -> new Modification(id.node(child.getIdentifier()), child, schemaChild(schemaNode, child.getIdentifier())));
+        }
+
+        List<Modification> getChildNodes() {
+            return streamChildren().collect(Collectors.toList());
+        }
+
+        YangInstanceIdentifier getId() {
+            return id;
+        }
+
+        ModificationType getModificationType() {
+            return dataCandidate.getModificationType();
+        }
+
+        com.google.common.base.Optional<NormalizedNode<?, ?>> getDataBefore() {
+            return dataCandidate.getDataBefore();
+        }
+
+        com.google.common.base.Optional<NormalizedNode<?, ?>> getDataAfter() {
+            return dataCandidate.getDataAfter();
+        }
+
+        Object getSchemaNode() {
+            return schemaNode;
+        }
+
+        boolean is(final Class<?> schemaType) {
+            return schemaType.isAssignableFrom(schemaNode.getClass());
+        }
+
+        boolean isMixin() {
+            // Checking whether node is a mixin is not performed on schema, but on data since mixin is
+            // only a NormalizedNode concept, not a schema concept
+            return dataCandidate.getDataBefore().orNull() instanceof MixinNode ||
+                    dataCandidate.getDataAfter().orNull() instanceof MixinNode;
+        }
+
+        private boolean isBeforeAndAfterDifferent() {
+            if (dataCandidate.getDataBefore().isPresent()) {
+                return !dataCandidate.getDataBefore().get().equals(dataCandidate.getDataAfter().orNull());
+            }
+
+            // considering not a modification if data after is also null
+            return dataCandidate.getDataAfter().isPresent();
+        }
+
+        /**
+         * Find next schema node in hierarchy.
+         */
+        private Object schemaChild(final Object schema, final YangInstanceIdentifier.PathArgument identifier) {
+            Object found = null;
+
+            if (identifier instanceof YangInstanceIdentifier.AugmentationIdentifier) {
+                if (schema instanceof AugmentationTarget) {
+                    // Find matching augmentation
+                    found = ((AugmentationTarget) schema).getAvailableAugmentations().stream()
+                            .filter(aug -> identifier.equals(new YangInstanceIdentifier.AugmentationIdentifier(
+                                    aug.getChildNodes().stream()
+                                            .map(SchemaNode::getQName)
+                                            .collect(Collectors.toSet()))))
+                            .findFirst()
+                            .orElse(null);
+                }
+            } else if (schema instanceof DataNodeContainer) {
+                // Special handling for list aggregator nodes. If we are at list aggregator node e.g. MapNode and
+                // we are searching for schema for a list entry e.g. MapEntryNode just return the same schema
+                if (schema instanceof ListSchemaNode &&
+                        ((SchemaNode) schema).getQName().equals(identifier.getNodeType())) {
+                    found = schema;
+                } else {
+                    found = ((DataNodeContainer) schema).getDataChildByName(identifier.getNodeType());
+                }
+            } else if (schema instanceof ChoiceSchemaNode) {
+                // For choices, iterate through all the cases
+                final Optional<DataSchemaNode> maybeChild = ((ChoiceSchemaNode) schema).getCases().stream()
+                        .flatMap(cas -> cas.getChildNodes().stream())
+                        .filter(child -> child.getQName().equals(identifier.getNodeType()))
+                        .findFirst();
+                if (maybeChild.isPresent()) {
+                    found = maybeChild.get();
+                }
+            }
+
+            return checkNotNull(found, "Unable to find child node in: %s identifiable by: %s", schema, identifier);
+        }
+
+        @Override
+        public String toString() {
+            return "Modification{" +
+                    "id=" + id +
+                    '}';
+        }
+    }
 }
diff --git a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java
index dc962662d..0a7c85b3b 100644
--- a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java
+++ b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java
@@ -92,7 +92,7 @@ public class ModifiableDataTreeDelegatorTest {
         final Map.Entry<InstanceIdentifier<?>, DataObject> parsed = new AbstractMap.SimpleEntry<>(DEFAULT_ID, DEFAULT_DATA_OBJECT);
         when(serializer.fromNormalizedNode(any(YangInstanceIdentifier.class), any(NormalizedNode.class))).thenReturn(parsed);
 
-        configDataTree = new ModifiableDataTreeDelegator(serializer, dataTree, writer, contextBroker);
+        configDataTree = new ModifiableDataTreeDelegator(serializer, dataTree, ModificationDiffTest.getSchemaCtx(), writer, contextBroker);
     }
 
     @Test
diff --git a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModificationDiffTest.java b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModificationDiffTest.java
index cc00f2dc6..029457331 100644
--- a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModificationDiffTest.java
+++ b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModificationDiffTest.java
@@ -41,6 +41,8 @@ public class ModificationDiffTest {
     static final QName TEXT_LEAF_QNAME = QName.create(TOP_CONTAINER_QNAME, "text");
     static final QName NESTED_LIST_QNAME = QName.create(TOP_CONTAINER_QNAME, "nested-list");
     static final QName DEEP_LIST_QNAME = QName.create(TOP_CONTAINER_QNAME, "deep-list");
+    static final QName EMPTY_QNAME = QName.create(TOP_CONTAINER_QNAME, "empty");
+    static final QName IN_EMPTY_QNAME = QName.create(TOP_CONTAINER_QNAME, "in-empty");
 
     static final QName WITH_CHOICE_CONTAINER_QNAME =
             QName.create("urn:opendaylight:params:xml:ns:yang:test:diff", "2015-01-05", "with-choice");
@@ -133,6 +135,26 @@ public class ModificationDiffTest {
         assertThat(modificationDiff.getUpdates().size(), is(0));
     }
 
+    @Test
+    public void testWriteNonPresenceEmptyNestedContainer() throws Exception {
+        final TipProducingDataTree dataTree = getDataTree();
+        final DataTreeModification dataTreeModification = getModification(dataTree);
+        final NormalizedNode<?, ?> topContainer = Builders.containerBuilder()
+                .withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(TOP_CONTAINER_QNAME))
+                .withChild(ImmutableNodes.containerNode(EMPTY_QNAME))
+                .withChild(ImmutableNodes.leafNode(STRING_LEAF_QNAME, "1"))
+                .build();
+        dataTreeModification.write(TOP_CONTAINER_ID, topContainer);
+        final DataTreeCandidateTip prepare = prepareModification(dataTree, dataTreeModification);
+
+        final ModificationDiff modificationDiff = getModificationDiff(prepare);
+        dataTree.commit(prepare);
+
+        // Only the STRING_LEAF_QNAME is considered a modification, the EMPTY_QNAME container is ignored since it is
+        // not a presence container
+        assertThat(modificationDiff.getUpdates().size(), is(1));
+    }
+
     private DataTreeCandidateTip prepareModification(final TipProducingDataTree dataTree,
                                                      final DataTreeModification dataTreeModification)
             throws DataValidationFailedException {
@@ -159,8 +181,8 @@ public class ModificationDiffTest {
         assertUpdate(updates.values().iterator().next(), TOP_CONTAINER_ID, topContainer, topContainerAfter);
     }
 
-    private ModificationDiff getModificationDiff(final DataTreeCandidateTip prepare) {
-        return ModificationDiff.recursivelyFromCandidateRoot(prepare.getRootNode());
+    private ModificationDiff getModificationDiff(final DataTreeCandidateTip prepare) throws ReactorException {
+        return ModificationDiff.recursivelyFromCandidateRoot(prepare.getRootNode(), getSchemaCtx());
     }
 
     @Test
@@ -447,7 +469,7 @@ public class ModificationDiffTest {
                 .build();
     }
 
-    private static SchemaContext getSchemaCtx() throws ReactorException {
+    static SchemaContext getSchemaCtx() throws ReactorException {
         final CrossSourceStatementReactor.BuildAction buildAction = YangInferencePipeline.RFC6020_REACTOR.newBuild();
         buildAction.addSource(new YangStatementSourceImpl(ModificationDiffTest.class.getResourceAsStream("/test-diff.yang")));
         return buildAction.buildEffective();
diff --git a/infra/data-impl/src/test/resources/test-diff.yang b/infra/data-impl/src/test/resources/test-diff.yang
index 6c27ddc17..57708589d 100644
--- a/infra/data-impl/src/test/resources/test-diff.yang
+++ b/infra/data-impl/src/test/resources/test-diff.yang
@@ -16,6 +16,12 @@ module test-diff {
             type string;
         }
 
+        container empty {
+            leaf in-empty {
+                type string;
+            }
+        }
+
         list nested-list {
             key "name";