HONEYCOMB-255 - Cutting identifiers to prevent failing of reverts

Mapping allready processes changes for reverting by InstanceIdentifier
instead of using KeyedInstanceIdentifier(to prevent failing to identify
handleable nodes)
Modified logging to prevent double/triple logging of detailed cause
of failed bulk update
Reusing WriteContext for revert(removed try with resource to prevent
closing of write context before revert)

Change-Id: Ie939ebe443629f9cdad5b5b449aa8c5dac40ea67
Signed-off-by: Jan Srnicek <jsrnicek@cisco.com>
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>

2 files changed, 14 insertions, 15 deletions

diff --git a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/TransactionWriteContext.java b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/TransactionWriteContext.java
index 0128ee4a8..dd397a605 100644
--- a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/TransactionWriteContext.java
+++ b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/TransactionWriteContext.java
@@ -108,9 +108,6 @@ public final class TransactionWriteContext implements WriteContext {
         return mappingContext;
     }
 
-    /**
-     * Does not close the transactions.
-     */
     @Override
     public void close() {
         ctx.close();
diff --git a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistry.java b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistry.java
index ab80eb4ac..e5b829d6a 100644
--- a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistry.java
+++ b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistry.java
@@ -17,6 +17,8 @@
 package io.fd.honeycomb.translate.util.write.registry;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static io.fd.honeycomb.translate.util.RWUtils.makeIidWildcarded;
 
 import com.google.common.base.Optional;
 import com.google.common.collect.HashMultimap;
@@ -26,9 +28,9 @@ import com.google.common.collect.Lists;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Sets;
 import io.fd.honeycomb.translate.TranslationException;
-import io.fd.honeycomb.translate.write.WriteContext;
 import io.fd.honeycomb.translate.util.RWUtils;
 import io.fd.honeycomb.translate.write.DataObjectUpdate;
+import io.fd.honeycomb.translate.write.WriteContext;
 import io.fd.honeycomb.translate.write.WriteFailedException;
 import io.fd.honeycomb.translate.write.Writer;
 import io.fd.honeycomb.translate.write.registry.WriterRegistry;
@@ -211,11 +213,11 @@ final class FlatWriterRegistry implements WriterRegistry {
                     LOG.trace("Update successful for type: {}", writerType);
                     LOG.debug("Update successful for: {}", singleUpdate);
                 } catch (Exception e) {
-                    LOG.error("Error while processing data change of: {} (updates={})", writerType, writersData, e);
+                    // do not log this exception here,its logged in ModifiableDataTreeDelegator
 
                     final Reverter reverter = attemptRevert
-                            ? new ReverterImpl(processedNodes, updates, writersOrder, ctx)
-                            : () -> {}; // NOOP reverter
+                            ? new ReverterImpl(processedNodes, updates, writersOrder)
+                            : (final WriteContext writeContext) -> {};//NOOP reverter
 
                     // Find out which changes left unprocessed
                     final Set<InstanceIdentifier<?>> unprocessedChanges = updates.values().stream()
@@ -258,30 +260,29 @@ final class FlatWriterRegistry implements WriterRegistry {
         private final Collection<InstanceIdentifier<?>> processedNodes;
         private final Multimap<InstanceIdentifier<?>, ? extends DataObjectUpdate> updates;
         private final Set<InstanceIdentifier<?>> revertDeleteOrder;
-        private final WriteContext ctx;
 
         ReverterImpl(final Collection<InstanceIdentifier<?>> processedNodes,
                      final Multimap<InstanceIdentifier<?>, ? extends DataObjectUpdate> updates,
-                     final Set<InstanceIdentifier<?>> writersOrderOriginal,
-                     final WriteContext ctx) {
+                     final Set<InstanceIdentifier<?>> writersOrderOriginal) {
             this.processedNodes = processedNodes;
             this.updates = updates;
             // Use opposite ordering when executing revert
             this.revertDeleteOrder =  writersOrderOriginal == FlatWriterRegistry.this.writersOrder
                     ? FlatWriterRegistry.this.writersOrderReversed
                     : FlatWriterRegistry.this.writersOrder;
-            this.ctx = ctx;
         }
 
         @Override
-        public void revert() throws RevertFailedException {
+        public void revert(@Nonnull final WriteContext writeContext) throws RevertFailedException {
+            checkNotNull(writeContext, "Cannot revert changes for null context");
+
             Multimap<InstanceIdentifier<?>, DataObjectUpdate> updatesToRevert =
                     filterAndRevertProcessed(updates, processedNodes);
 
             LOG.info("Attempting revert for changes: {}", updatesToRevert);
             try {
                 // Perform reversed bulk update without revert attempt
-                bulkUpdate(updatesToRevert, ctx, true, revertDeleteOrder);
+                bulkUpdate(updatesToRevert, writeContext, true, revertDeleteOrder);
                 LOG.info("Revert successful");
             } catch (BulkUpdateException e) {
                 LOG.error("Revert failed", e);
@@ -298,11 +299,12 @@ final class FlatWriterRegistry implements WriterRegistry {
                 final Collection<InstanceIdentifier<?>> processedNodes) {
             final Multimap<InstanceIdentifier<?>, DataObjectUpdate> filtered = HashMultimap.create();
             for (InstanceIdentifier<?> processedNode : processedNodes) {
-                final InstanceIdentifier<?> wildcardedIid = RWUtils.makeIidWildcarded(processedNode);
+                final InstanceIdentifier<?> wildcardedIid = makeIidWildcarded(processedNode);
                 if (updates.containsKey(wildcardedIid)) {
                     updates.get(wildcardedIid).stream()
                             .filter(dataObjectUpdate -> processedNode.contains(dataObjectUpdate.getId()))
-                            .forEach(dataObjectUpdate -> filtered.put(processedNode, dataObjectUpdate.reverse()));
+                            // putting under unkeyed identifier, to prevent failing of checkAllTypesCanBeHandled
+                            .forEach(dataObjectUpdate -> filtered.put(wildcardedIid, dataObjectUpdate.reverse()));
                 }
             }
             return filtered;