HONEYCOMB-405 - Revert fix for indirect updates

If indirect update(delete+create) fails in a way, that delete passed,
but update part failed, delete part must be reverted

Moves reverter creation to MDTG and test cases related too it to
ModifiableDataTreeDelegatorRevertTest

Fixes tracking of allready processed changes by tracking them
from perspective of processModifications() method

Introduces UpdateFailedException as replacement
for BulkUpdateException(now thrown also for single updates)

Separates ReverterImpl from FlatWriterRegistry and ads unit tests

Change-Id: If0066d0716d9476be89b1d99985b6745becac15e
Signed-off-by: Jan Srnicek <jsrnicek@cisco.com>

2 files changed, 202 insertions, 17 deletions

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 66dcbe5d9..e76c5b6e5 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
@@ -33,8 +33,12 @@ import io.fd.honeycomb.translate.util.TransactionMappingContext;
 import io.fd.honeycomb.translate.util.write.TransactionWriteContext;
 import io.fd.honeycomb.translate.write.DataObjectUpdate;
 import io.fd.honeycomb.translate.write.WriteContext;
+import io.fd.honeycomb.translate.write.registry.UpdateFailedException;
 import io.fd.honeycomb.translate.write.registry.WriterRegistry;
+import java.util.List;
 import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;
@@ -136,33 +140,37 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager
                     ((TransactionMappingContext) ctx.getMappingContext()).submit();
                 // Blocking on context data update
                 contextUpdateResult.checkedGet();
-
-            } catch (WriterRegistry.BulkUpdateException e) {
+            } catch (UpdateFailedException e) {
+                // TODO - HONEYCOMB-411
                 LOG.warn("Failed to apply all changes", e);
-                LOG.info("Trying to revert successful changes for current transaction");
-
-                try {
-                    // attempt revert with fresh context, to allow write logic used context-binded data
-                    e.revertChanges(getRevertTransactionContext(ctx.getMappingContext()));
-                    LOG.info("Changes successfully reverted");
-                } catch (WriterRegistry.Reverter.RevertFailedException revertFailedException) {
-                    // fail with failed revert
-                    LOG.error("Failed to revert successful(comitted) changes, failure occurred for: {}. State might be corrupted.",
-                            revertFailedException.getFailedUpdate(), revertFailedException);
-                    throw revertFailedException;
+                final List<DataObjectUpdate> processed = e.getProcessed();
+                if (processed.isEmpty()) {
+                    // nothing was processed, which means either very first operation failed, or it was single operation
+                    // update. In both cases, no revert is needed
+                    LOG.info("Nothing to revert");
+                    throw e;
+                } else {
+                    LOG.info("Trying to revert successful changes for current transaction");
+                    try {
+                        // attempt revert with fresh context, to allow write logic used context-binded data
+                        new Reverter(processed, writerRegistry)
+                                .revert(getRevertTransactionContext(ctx.getMappingContext()));
+                        LOG.info("Changes successfully reverted");
+                    } catch (Reverter.RevertFailedException revertFailedException) {
+                        // fail with failed revert
+                        LOG.error("Failed to revert successful(comitted) changes", revertFailedException);
+                        throw revertFailedException;
+                    }
                 }
                 // fail with success revert
                 // not passing the cause,its logged above and it would be logged after transaction
                 // ended again(prevent double logging of same error
-                throw new WriterRegistry.Reverter.RevertSuccessException(e.getUnrevertedSubtrees());
+                throw new Reverter.RevertSuccessException(getNonProcessedNodes(baUpdates, processed));
             } catch (TransactionCommitFailedException e) {
                 // TODO HONEYCOMB-162 revert should probably occur when context is not written successfully
                 final String msg = "Error while updating mapping context data";
                 LOG.error(msg, e);
                 throw new TranslationException(msg, e);
-            } catch (TranslationException e) {
-                LOG.error("Error while processing data change (updates={})", baUpdates, e);
-                throw e;
             } finally {
                 // Using finally instead of try-with-resources in order to leave ctx open for BulkUpdateException catch
                 // block. The context is needed there, but try-with-resources closes the resource before handling ex.
@@ -200,6 +208,14 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager
         }
     }
 
+    private static Set<InstanceIdentifier<?>> getNonProcessedNodes(final WriterRegistry.DataObjectUpdates allUpdates,
+                                                                   final List<DataObjectUpdate> alreadyProcessed) {
+        return allUpdates.getAllModifications().stream()
+                .filter(update -> !alreadyProcessed.contains(update))
+                .map(DataObjectUpdate::getId)
+                .collect(Collectors.toSet());
+    }
+
     @VisibleForTesting
     static WriterRegistry.DataObjectUpdates toBindingAware(
             final WriterRegistry registry,
diff --git a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/Reverter.java b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/Reverter.java
new file mode 100644
index 000000000..9c7262647
--- /dev/null
+++ b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/Reverter.java
@@ -0,0 +1,169 @@
+/*
+ * Copyright (c) 2017 Cisco and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.fd.honeycomb.data.impl;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static io.fd.honeycomb.translate.util.RWUtils.makeIidWildcarded;
+
+import com.google.common.annotations.Beta;
+import com.google.common.collect.LinkedHashMultimap;
+import com.google.common.collect.Multimap;
+import io.fd.honeycomb.translate.TranslationException;
+import io.fd.honeycomb.translate.write.DataObjectUpdate;
+import io.fd.honeycomb.translate.write.WriteContext;
+import io.fd.honeycomb.translate.write.registry.UpdateFailedException;
+import io.fd.honeycomb.translate.write.registry.WriterRegistry;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+import javax.annotation.Nonnull;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Takes list of changes, creates revert operations and writes them using writer registry
+ */
+final class Reverter {
+
+    private static final Logger LOG = LoggerFactory.getLogger(Reverter.class);
+
+    private final List<DataObjectUpdate> toBeReverted;
+    private final WriterRegistry writerRegistry;
+
+    /**
+     * @param toBeReverted   - list of changes in order they were processed, that should be reverted. Reverting order
+     *                       and data inside operations will be done by this reverter.
+     * @param writerRegistry - registry able to handle all nodes that should be reverted
+     */
+    Reverter(final List<DataObjectUpdate> toBeReverted,
+             final WriterRegistry writerRegistry) {
+        this.toBeReverted = toBeReverted;
+        this.writerRegistry = writerRegistry;
+    }
+
+    /**
+     * Reverts changes that were successfully applied during update before failure occurred. Changes are reverted in
+     * reverse order they were applied. Used {@code WriteContext} needs to be in non-closed state, creating fresh one
+     * for revert is recommended, same way as for write, to allow {@code Reverter} use same logic as write.
+     *
+     * @param writeContext Non-closed {@code WriteContext} to be used by reverting logic
+     * @throws RevertFailedException if not all of applied changes were successfully reverted
+     */
+    void revert(@Nonnull final WriteContext writeContext) throws RevertFailedException {
+        checkNotNull(writeContext, "Cannot revert changes for null context");
+
+        // create list of changes in revert order, and than switch data inside these chagnes to create opposite operations
+        final WriterRegistry.DataObjectUpdates revertedAndMapped = revertAndMapProcessed(revertOrder(toBeReverted));
+
+        LOG.info("Attempting revert for changes: {}", revertedAndMapped);
+        try {
+            // Perform reversed bulk update without revert attempt
+            writerRegistry.processModifications(revertedAndMapped, writeContext);
+            LOG.info("Revert successful");
+        } catch (UpdateFailedException e) {
+            // some of revert operations failed
+            // throws exception with all revert operations that failed
+            LOG.error("Revert failed", e);
+            final Set<DataObjectUpdate> nonReverted = revertedAndMapped.getAllModifications();
+            nonReverted.removeAll(e.getProcessed());
+            throw new RevertFailedException(e.getFailed(), nonReverted, e);
+        } catch (Exception e) {
+            // any other unexpected error
+            LOG.error("Revert failed with unexpected error");
+            throw new RevertFailedException(e);
+        }
+    }
+
+    /**
+     * Switching before and after data for each update.
+     */
+    private WriterRegistry.DataObjectUpdates revertAndMapProcessed(final List<DataObjectUpdate> updates) {
+        // uses linked maps to preserve order of insertion
+        final Multimap<InstanceIdentifier<?>, DataObjectUpdate> updatesMap = LinkedHashMultimap.create();
+        final Multimap<InstanceIdentifier<?>, DataObjectUpdate.DataObjectDelete> deleteMap =
+                LinkedHashMultimap.create();
+
+        updates.stream()
+                .map(DataObjectUpdate::reverse)
+                .forEach(reversed -> {
+                    // putting under unkeyed identifier, to prevent failing of checkAllTypesCanBeHandled
+                    final InstanceIdentifier<?> wildcardedIid = makeIidWildcarded(reversed.getId());
+                    if (reversed.getDataAfter() == null) {
+                        deleteMap.put(wildcardedIid, DataObjectUpdate.DataObjectDelete.class.cast(reversed));
+                    } else {
+                        updatesMap.put(wildcardedIid, reversed);
+                    }
+                });
+        return new WriterRegistry.DataObjectUpdates(updatesMap, deleteMap);
+    }
+
+    private List<DataObjectUpdate> revertOrder(final List<DataObjectUpdate> processingOrdered) {
+        final List<DataObjectUpdate> copy = new ArrayList<>(processingOrdered);
+        Collections.reverse(copy);
+        return copy;
+    }
+
+    /**
+     * Thrown when some of the changes applied during update were not reverted.
+     */
+    @Beta
+    static class RevertFailedException extends TranslationException {
+
+        /**
+         * Constructs a RevertFailedException with the list of changes that were not reverted.
+         *
+         * @param cause      the cause of revert failure
+         * @param failed     node that failed to revert
+         * @param unreverted unreverted changes
+         */
+        RevertFailedException(@Nonnull final DataObjectUpdate failed,
+                              @Nonnull final Set<DataObjectUpdate> unreverted,
+                              @Nonnull final Exception cause) {
+            super("Unable to revert changes after failure. Revert failed for "
+                    + failed + " unreverted subtrees: " + unreverted, cause);
+        }
+
+        RevertFailedException(@Nonnull final Exception cause) {
+            super("Unexpected error while reverting", cause);
+        }
+    }
+
+    /**
+     * Thrown after bulk operation was successfully reverted, to maintain marking of transaction as failed,without
+     * double logging of cause of update fail(its logged before reverting in ModifiableDataTreeDelegator
+     */
+    @Beta
+    static class RevertSuccessException extends TranslationException {
+        private final Set<InstanceIdentifier<?>> failedIds;
+
+        /**
+         * Constructs an RevertSuccessException.
+         *
+         * @param failedIds instance identifiers of the data objects that were not processed during bulk update.
+         */
+        public RevertSuccessException(@Nonnull final Set<InstanceIdentifier<?>> failedIds) {
+            super("Bulk update failed for: " + failedIds);
+            this.failedIds = failedIds;
+        }
+
+        public Set<InstanceIdentifier<?>> getFailedIds() {
+            return failedIds;
+        }
+    }
+}