HONEYCOMB-170 Add more information to RevertFailedEx

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

3 files changed, 61 insertions, 22 deletions

diff --git a/infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/registry/WriterRegistry.java b/infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/registry/WriterRegistry.java
index 51e6415a2..5520c00ec 100644
--- a/infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/registry/WriterRegistry.java
+++ b/infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/registry/WriterRegistry.java
@@ -19,7 +19,6 @@ package io.fd.honeycomb.translate.write.registry;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.annotations.Beta;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Sets;
 import io.fd.honeycomb.translate.TranslationException;
@@ -134,18 +133,24 @@ public interface WriterRegistry {
     class BulkUpdateException extends TranslationException {
 
         private final transient Reverter reverter;
-        private final Set<InstanceIdentifier<?>> failedIds;
+        private final InstanceIdentifier<?> failedSubtree;
+        private final DataObjectUpdate failedData;
+        private final Set<InstanceIdentifier<?>> unrevertedSubtrees;
 
         /**
          * Constructs an BulkUpdateException.
-         * @param failedIds instance identifiers of the data objects that were not processed during bulk update.
+         * @param unhandledSubtrees instance identifiers of the data objects that were not processed during bulk update.
          * @param cause the cause of bulk update failure
          */
-        public BulkUpdateException(@Nonnull final Set<InstanceIdentifier<?>> failedIds,
+        public BulkUpdateException(@Nonnull final InstanceIdentifier<?> failedSubtree,
+                                   @Nonnull final DataObjectUpdate failedData,
+                                   @Nonnull final Set<InstanceIdentifier<?>> unhandledSubtrees,
                                    @Nonnull final Reverter reverter,
                                    @Nonnull final Throwable cause) {
-            super("Bulk update failed at: " + failedIds, cause);
-            this.failedIds = failedIds;
+            super("Bulk update failed at: " + failedSubtree + " ignored updates: " + unhandledSubtrees, cause);
+            this.failedSubtree = failedSubtree;
+            this.failedData = failedData;
+            this.unrevertedSubtrees = unhandledSubtrees;
             this.reverter = checkNotNull(reverter, "reverter should not be null");
         }
 
@@ -160,8 +165,16 @@ public interface WriterRegistry {
             reverter.revert(writeContext);
         }
 
-        public Set<InstanceIdentifier<?>> getFailedIds() {
-            return failedIds;
+        public Set<InstanceIdentifier<?>> getUnrevertedSubtrees() {
+            return unrevertedSubtrees;
+        }
+
+        public InstanceIdentifier<?> getFailedSubtree() {
+            return failedSubtree;
+        }
+
+        public DataObjectUpdate getFailedData() {
+            return failedData;
         }
     }
 
@@ -188,20 +201,14 @@ public interface WriterRegistry {
         @Beta
         class RevertFailedException extends TranslationException {
 
-            // TODO HONEYCOMB-170 change to list of VppDataModifications to make debugging easier
-            private final Set<InstanceIdentifier<?>> notRevertedChanges;
-
             /**
              * Constructs a RevertFailedException with the list of changes that were not reverted.
              *
-             * @param notRevertedChanges list of changes that were not reverted
              * @param cause              the cause of revert failure
              */
-            public RevertFailedException(@Nonnull final Set<InstanceIdentifier<?>> notRevertedChanges,
-                                         final Throwable cause) {
-                super(cause);
-                checkNotNull(notRevertedChanges, "notRevertedChanges should not be null");
-                this.notRevertedChanges = ImmutableSet.copyOf(notRevertedChanges);
+            public RevertFailedException(@Nonnull final BulkUpdateException cause) {
+                super("Unable to revert changes after failure. Revert failed for "
+                        + cause.getFailedSubtree() + " unreverted subtrees: " + cause.getUnrevertedSubtrees(), cause);
             }
 
             /**
@@ -211,7 +218,17 @@ public interface WriterRegistry {
              */
             @Nonnull
             public Set<InstanceIdentifier<?>> getNotRevertedChanges() {
-                return notRevertedChanges;
+                return ((BulkUpdateException) getCause()).getUnrevertedSubtrees();
+            }
+
+            /**
+             * Returns the update that caused the failure.
+             *
+             * @return update that caused the failure
+             */
+            @Nonnull
+            public DataObjectUpdate getFailedUpdate() {
+                return ((BulkUpdateException) getCause()).getFailedData();
             }
         }
 
diff --git a/infra/translate-api/src/test/java/io/fd/honeycomb/translate/write/registry/BulkUpdateExceptionTest.java b/infra/translate-api/src/test/java/io/fd/honeycomb/translate/write/registry/BulkUpdateExceptionTest.java
index ae0c36d5b..9623db5bc 100644
--- a/infra/translate-api/src/test/java/io/fd/honeycomb/translate/write/registry/BulkUpdateExceptionTest.java
+++ b/infra/translate-api/src/test/java/io/fd/honeycomb/translate/write/registry/BulkUpdateExceptionTest.java
@@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.verify;
 
 import com.google.common.collect.Sets;
+import io.fd.honeycomb.translate.write.DataObjectUpdate;
 import io.fd.honeycomb.translate.write.WriteContext;
 import java.util.HashSet;
 import org.junit.Before;
@@ -35,9 +36,12 @@ public class BulkUpdateExceptionTest {
 
     @Mock
     private WriteContext writeContext;
-
     @Mock
     private WriterRegistry.Reverter reverter;
+    @Mock
+    private DataObject before;
+    @Mock
+    private DataObject after;
 
     @Before
     public void setUp() throws Exception {
@@ -48,9 +52,10 @@ public class BulkUpdateExceptionTest {
     public void testRevert() throws Exception {
         final HashSet<InstanceIdentifier<?>> failedIds = Sets.newHashSet(id);
         final WriterRegistry.BulkUpdateException bulkUpdateException =
-                new WriterRegistry.BulkUpdateException(failedIds, reverter, new RuntimeException());
+                new WriterRegistry.BulkUpdateException(id, DataObjectUpdate.create(id, before, after),
+                        failedIds, reverter, new RuntimeException());
 
-        assertEquals(failedIds, bulkUpdateException.getFailedIds());
+        assertEquals(failedIds, bulkUpdateException.getUnrevertedSubtrees());
 
         bulkUpdateException.revertChanges(writeContext);
         verify(reverter).revert(writeContext);
diff --git a/infra/translate-api/src/test/java/io/fd/honeycomb/translate/write/registry/RevertFailedExceptionTest.java b/infra/translate-api/src/test/java/io/fd/honeycomb/translate/write/registry/RevertFailedExceptionTest.java
index 721941d1e..6502235d0 100644
--- a/infra/translate-api/src/test/java/io/fd/honeycomb/translate/write/registry/RevertFailedExceptionTest.java
+++ b/infra/translate-api/src/test/java/io/fd/honeycomb/translate/write/registry/RevertFailedExceptionTest.java
@@ -19,20 +19,37 @@ package io.fd.honeycomb.translate.write.registry;
 import static org.junit.Assert.assertEquals;
 
 import com.google.common.collect.Sets;
+import io.fd.honeycomb.translate.write.DataObjectUpdate;
 import java.util.Set;
+import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
 public class RevertFailedExceptionTest {
 
     private InstanceIdentifier<?> id = InstanceIdentifier.create(DataObject.class);
+    @Mock
+    private WriterRegistry.Reverter reverter;
+    @Mock
+    private DataObject before;
+    @Mock
+    private DataObject after;
+
+    @Before
+    public void setUp() throws Exception {
+        MockitoAnnotations.initMocks(this);
+    }
 
     @Test
     public void testNonRevert() throws Exception {
         final Set<InstanceIdentifier<?>> notReverted = Sets.newHashSet(id);
         final WriterRegistry.Reverter.RevertFailedException revertFailedException =
-                new WriterRegistry.Reverter.RevertFailedException(notReverted, new RuntimeException());
+                new WriterRegistry.Reverter.RevertFailedException(
+                        new WriterRegistry.BulkUpdateException(id, DataObjectUpdate.create(id, before, after),
+                        notReverted, reverter, new RuntimeException()));
         assertEquals(notReverted, revertFailedException.getNotRevertedChanges());
     }
 }
\ No newline at end of file