From 5503731d866d318e9d5a2183608092a9d332dfe6 Mon Sep 17 00:00:00 2001 From: Jan Srnicek Date: Mon, 23 Oct 2017 10:57:13 +0200 Subject: 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 --- .../write/registry/FlatWriterRegistryTest.java | 107 ++++++++------------- 1 file changed, 38 insertions(+), 69 deletions(-) (limited to 'infra/translate-impl/src/test') diff --git a/infra/translate-impl/src/test/java/io/fd/honeycomb/translate/impl/write/registry/FlatWriterRegistryTest.java b/infra/translate-impl/src/test/java/io/fd/honeycomb/translate/impl/write/registry/FlatWriterRegistryTest.java index 72a91cb00..151436975 100644 --- a/infra/translate-impl/src/test/java/io/fd/honeycomb/translate/impl/write/registry/FlatWriterRegistryTest.java +++ b/infra/translate-impl/src/test/java/io/fd/honeycomb/translate/impl/write/registry/FlatWriterRegistryTest.java @@ -16,13 +16,12 @@ package io.fd.honeycomb.translate.impl.write.registry; -import static org.hamcrest.CoreMatchers.hasItem; -import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.Matchers.hasSize; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; @@ -41,7 +40,9 @@ import io.fd.honeycomb.translate.util.DataObjects.DataObject2; import io.fd.honeycomb.translate.write.DataObjectUpdate; import io.fd.honeycomb.translate.write.WriteContext; import io.fd.honeycomb.translate.write.Writer; +import io.fd.honeycomb.translate.write.registry.UpdateFailedException; import io.fd.honeycomb.translate.write.registry.WriterRegistry; +import java.util.List; import org.junit.Before; import org.junit.Test; import org.mockito.InOrder; @@ -185,7 +186,7 @@ public class FlatWriterRegistryTest { } @Test - public void testMultipleUpdatesOneFailing() throws Exception { + public void testMultipleUpdatesFirstFailing() throws Exception { final FlatWriterRegistry flatWriterRegistry = new FlatWriterRegistry(ImmutableMap.of(DataObject1.IID, writer1, DataObject2.IID, writer2)); @@ -199,59 +200,38 @@ public class FlatWriterRegistryTest { try { flatWriterRegistry.processModifications(new WriterRegistry.DataObjectUpdates(updates, ImmutableMultimap.of()), ctx); - fail("Bulk update should have failed on writer1"); - } catch (WriterRegistry.BulkUpdateException e) { - assertThat(e.getUnrevertedSubtrees(), hasSize(2)); - assertThat(e.getUnrevertedSubtrees(), hasItem(InstanceIdentifier.create(DataObject2.class))); - assertThat(e.getUnrevertedSubtrees(), hasItem(InstanceIdentifier.create(DataObject1.class))); + fail("Bulk update should have failed on writer1 with UpdateFailedException"); + } catch (UpdateFailedException e) { + assertThat(e.getProcessed(), hasSize(0));// very first update failed } } @Test - public void testMultipleUpdatesOneFailingThenRevertWithSuccess() throws Exception { + public void testMultipleUpdatesSecondFailing() throws Exception { final FlatWriterRegistry flatWriterRegistry = - new FlatWriterRegistry( - ImmutableMap.of(DataObject1.IID, writer1, DataObject2.IID, writer2, DataObjects.DataObject3.IID, writer3)); + new FlatWriterRegistry(ImmutableMap.of(DataObject1.IID, writer1, DataObject2.IID, writer2)); - // Writer1 always fails - doThrow(new RuntimeException()).when(writer3) + // Writer2 always fails + doThrow(new RuntimeException()).when(writer2) .processModification(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), any(WriteContext.class)); final Multimap, DataObjectUpdate> updates = HashMultimap.create(); addUpdate(updates, DataObject1.class); - addUpdate(updates, DataObjects.DataObject3.class); - final InstanceIdentifier iid2 = InstanceIdentifier.create(DataObject2.class); - final DataObject2 before2 = mock(DataObject2.class); - final DataObject2 after2 = mock(DataObject2.class); - updates.put(DataObject2.IID, DataObjectUpdate.create(iid2, before2, after2)); + addUpdate(updates, DataObject2.class); try { flatWriterRegistry.processModifications(new WriterRegistry.DataObjectUpdates(updates, ImmutableMultimap.of()), ctx); - fail("Bulk update should have failed on writer1"); - } catch (WriterRegistry.BulkUpdateException e) { - assertThat(e.getUnrevertedSubtrees().size(), is(1)); - - final InOrder inOrder = inOrder(writer1, writer2, writer3); - inOrder.verify(writer1) - .processModification(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), any(WriteContext.class)); - inOrder.verify(writer2) - .processModification(iid2, before2, after2, ctx); - inOrder.verify(writer3) - .processModification(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), any(WriteContext.class)); - - e.revertChanges(revertWriteContext); - // Revert changes. Successful updates are iterated in reverse - // also binding other write context,to verify if update context is not reused - inOrder.verify(writer2) - .processModification(iid2, after2, before2, revertWriteContext); - inOrder.verify(writer1) - .processModification(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), eq(revertWriteContext)); - verifyNoMoreInteractions(writer3); + fail("Bulk update should have failed on writer1 with UpdateFailedException"); + } catch (UpdateFailedException e) { + final List alreadyProcessed = e.getProcessed(); + assertThat(alreadyProcessed, hasSize(1));// very first update failed + assertEquals(updateData(DataObject1.class, DataObject1.IID), + e.getProcessed().iterator().next()); } } @Test - public void testMultipleUpdatesOneFailingThenRevertWithFail() throws Exception { + public void testMultipleUpdatesLastFailing() throws Exception { final FlatWriterRegistry flatWriterRegistry = new FlatWriterRegistry( ImmutableMap.of(DataObject1.IID, writer1, DataObject2.IID, writer2, DataObjects.DataObject3.IID, writer3)); @@ -267,18 +247,12 @@ public class FlatWriterRegistryTest { try { flatWriterRegistry.processModifications(new WriterRegistry.DataObjectUpdates(updates, ImmutableMultimap.of()), ctx); - fail("Bulk update should have failed on writer1"); - } catch (WriterRegistry.BulkUpdateException e) { - // Writer1 always fails from now - doThrow(new RuntimeException()).when(writer1) - .processModification(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), any(WriteContext.class)); - try { - e.revertChanges(revertWriteContext); - } catch (WriterRegistry.Reverter.RevertFailedException e1) { - assertThat(e1.getNotRevertedChanges().size(), is(1)); - assertThat(e1.getNotRevertedChanges(), - hasItem(InstanceIdentifier.create(DataObject1.class))); - } + fail("Bulk update should have failed on writer1 with UpdateFailedException"); + } catch (UpdateFailedException e) { + final List alreadyProcessed = e.getProcessed(); + assertEquals(2, alreadyProcessed.size()); + assertTrue(alreadyProcessed.contains(updateData(DataObject1.class, DataObject1.IID))); + assertTrue(alreadyProcessed.contains(updateData(DataObject2.class, DataObject2.IID))); } } @@ -289,7 +263,7 @@ public class FlatWriterRegistryTest { final FlatWriterRegistry flatWriterRegistry = new FlatWriterRegistry( - ImmutableMap.of(DataObject1.IID, writer1, DataObjects.DataObject1ChildK.IID,writer4)); + ImmutableMap.of(DataObject1.IID, writer1, DataObjects.DataObject1ChildK.IID, writer4)); // Writer1 always fails doThrow(new RuntimeException()).when(writer1) @@ -297,23 +271,13 @@ public class FlatWriterRegistryTest { any(WriteContext.class)); final Multimap, DataObjectUpdate> updates = HashMultimap.create(); - addKeyedUpdate(updates,DataObjects.DataObject1ChildK.class); + addKeyedUpdate(updates, DataObjects.DataObject1ChildK.class); addUpdate(updates, DataObject1.class); try { flatWriterRegistry.processModifications(new WriterRegistry.DataObjectUpdates(updates, ImmutableMultimap.of()), ctx); - fail("Bulk update should have failed on writer1"); - } catch (WriterRegistry.BulkUpdateException e) { - // Writer1 always fails from now - doThrow(new RuntimeException()).when(writer1) - .processModification(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), - any(WriteContext.class)); - try { - e.revertChanges(revertWriteContext); - } catch (WriterRegistry.Reverter.RevertFailedException e1) { - assertThat(e1.getNotRevertedChanges().size(), is(1)); - assertThat(e1.getNotRevertedChanges(), - hasItem(InstanceIdentifier.create(DataObject1.class))); - } + fail("Bulk update should have failed on writer1 with UpdateFailedException"); + } catch (UpdateFailedException e) { + assertTrue(e.getProcessed().isEmpty()); } } @@ -325,8 +289,13 @@ public class FlatWriterRegistryTest { } private void addUpdate(final Multimap, DataObjectUpdate> updates, - final Class type) throws Exception { + final Class type) throws Exception { final InstanceIdentifier iid = (InstanceIdentifier) type.getDeclaredField("IID").get(null); - updates.put(iid, DataObjectUpdate.create(iid, mock(type), mock(type))); + updates.put(iid, updateData(type, iid)); + } + + private static DataObjectUpdate updateData(final Class type, + final InstanceIdentifier iid) { + return DataObjectUpdate.create(iid, mock(type), mock(type)); } } \ No newline at end of file -- cgit 1.2.3-korg