From bb090e1254eacc95d7bd1dd45f6311967f81af86 Mon Sep 17 00:00:00 2001 From: Jan Srnicek Date: Mon, 24 Oct 2016 13:02:59 +0200 Subject: 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 Signed-off-by: Maros Marsalek --- .../fd/honeycomb/translate/util/DataObjects.java | 14 ++++- .../write/registry/FlatWriterRegistryTest.java | 60 +++++++++++++++++++--- 2 files changed, 67 insertions(+), 7 deletions(-) (limited to 'infra/translate-utils/src/test/java/io/fd/honeycomb') diff --git a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/DataObjects.java b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/DataObjects.java index f2f664efe..8dd3d2065 100644 --- a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/DataObjects.java +++ b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/DataObjects.java @@ -60,5 +60,17 @@ public class DataObjects { } } - public static class DataObjectKey implements Identifier {} + public interface DataObject1ChildK extends DataObject, ChildOf, Identifiable { + // needs to be defined like this to have paths totally equal after cutting path for internally keyed id inside infra + InstanceIdentifier IID = + RWUtils.makeIidWildcarded(InstanceIdentifier.create(DataObject1.class).child(DataObject1ChildK.class)); + InstanceIdentifier INTERNALLY_KEYED_IID = InstanceIdentifier.create(DataObject1.class) + .child(DataObject1ChildK.class, new DataObject1ChildKey()); + } + + public static class DataObject1ChildKey implements Identifier { + } + + public static class DataObjectKey implements Identifier { + } } diff --git a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistryTest.java b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistryTest.java index 6b75f923e..fdd2eaddb 100644 --- a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistryTest.java +++ b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistryTest.java @@ -1,10 +1,10 @@ package io.fd.honeycomb.translate.util.write.registry; -import static org.hamcrest.CoreMatchers.hasItem; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertThat; 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; @@ -18,10 +18,11 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.Multimap; import io.fd.honeycomb.translate.util.DataObjects; +import io.fd.honeycomb.translate.util.DataObjects.DataObject1; 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.util.DataObjects.DataObject1; import io.fd.honeycomb.translate.write.registry.WriterRegistry; import org.hamcrest.CoreMatchers; import org.junit.Before; @@ -41,7 +42,11 @@ public class FlatWriterRegistryTest { @Mock private Writer writer3; @Mock + private Writer writer4; + @Mock private WriteContext ctx; + @Mock + private WriteContext revertWriteContext; @Before public void setUp() throws Exception { @@ -215,12 +220,13 @@ public class FlatWriterRegistryTest { inOrder.verify(writer3) .update(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), any(WriteContext.class)); - e.revertChanges(); + 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) - .update(iid2, after2, before2, ctx); + .update(iid2, after2, before2, revertWriteContext); inOrder.verify(writer1) - .update(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), any(WriteContext.class)); + .update(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), eq(revertWriteContext)); verifyNoMoreInteractions(writer3); } } @@ -248,7 +254,7 @@ public class FlatWriterRegistryTest { doThrow(new RuntimeException()).when(writer1) .update(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), any(WriteContext.class)); try { - e.revertChanges(); + e.revertChanges(revertWriteContext); } catch (WriterRegistry.Reverter.RevertFailedException e1) { assertThat(e1.getNotRevertedChanges().size(), is(1)); assertThat(e1.getNotRevertedChanges(), CoreMatchers @@ -257,6 +263,48 @@ public class FlatWriterRegistryTest { } } + @Test + public void testMutlipleUpdatesWithOneKeyedContainer() throws Exception { + final InstanceIdentifier internallyKeyedIdentifier = InstanceIdentifier.create(DataObject1.class) + .child(DataObjects.DataObject1ChildK.class, new DataObjects.DataObject1ChildKey()); + + final FlatWriterRegistry flatWriterRegistry = + new FlatWriterRegistry( + ImmutableMap.of(DataObjects.DataObject1.IID, writer1, DataObjects.DataObject1ChildK.IID,writer4)); + + // Writer1 always fails + doThrow(new RuntimeException()).when(writer1) + .update(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), + any(WriteContext.class)); + + final Multimap, DataObjectUpdate> updates = HashMultimap.create(); + addKeyedUpdate(updates,DataObjects.DataObject1ChildK.class); + addUpdate(updates, DataObjects.DataObject1.class); + try { + flatWriterRegistry.update(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) + .update(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(), CoreMatchers + .hasItem(InstanceIdentifier.create(DataObjects.DataObject1.class))); + } + } + } + + private void addKeyedUpdate(final Multimap, DataObjectUpdate> updates, + final Class type) throws Exception { + final InstanceIdentifier iid = (InstanceIdentifier) type.getDeclaredField("IID").get(null); + final InstanceIdentifier keyedIid = (InstanceIdentifier) type.getDeclaredField("INTERNALLY_KEYED_IID").get(null); + updates.put(iid, DataObjectUpdate.create(keyedIid, mock(type), mock(type))); + } + private void addUpdate(final Multimap, DataObjectUpdate> updates, final Class type) throws Exception { final InstanceIdentifier iid = (InstanceIdentifier) type.getDeclaredField("IID").get(null); -- cgit 1.2.3-korg