From 130c716105017c7b20b4779973b915968b3dc322 Mon Sep 17 00:00:00 2001 From: Jan Srnicek Date: Wed, 16 Aug 2017 09:21:24 +0200 Subject: HONEYCOMB-386 - Make update optional If customizer does not support update directly, updates for its handled nodes are broken up to delete + create pairs. Change-Id: I2929109e8c9a1db0bef108367cf7d839135ce173 Signed-off-by: Jan Srnicek --- .../data/impl/ModifiableDataTreeDelegator.java | 24 ++++++-- .../data/impl/ModifiableDataTreeDelegatorTest.java | 68 +++++++++++++++++++--- 2 files changed, 79 insertions(+), 13 deletions(-) (limited to 'infra/data-impl') 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 ccc40576a..66dcbe5d9 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 @@ -124,12 +124,13 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager LOG.debug("ConfigDataTree.modify() diff: {}", modificationDiff); // Distinguish between updates (create + update) and deletes - final WriterRegistry.DataObjectUpdates baUpdates = toBindingAware(modificationDiff.getUpdates()); + final WriterRegistry.DataObjectUpdates baUpdates = + toBindingAware(writerRegistry, modificationDiff.getUpdates()); LOG.debug("ConfigDataTree.modify() extracted updates={}", baUpdates); WriteContext ctx = getTransactionWriteContext(); try { - writerRegistry.update(baUpdates, ctx); + writerRegistry.processModifications(baUpdates, ctx); final CheckedFuture contextUpdateResult = ((TransactionMappingContext) ctx.getMappingContext()).submit(); @@ -193,14 +194,15 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager return new TransactionWriteContext(serializer, beforeTx, afterTx, mappingContext); } - private WriterRegistry.DataObjectUpdates toBindingAware( + private WriterRegistry.DataObjectUpdates toBindingAware(final WriterRegistry registry, final Map biNodes) { - return ModifiableDataTreeDelegator.toBindingAware(biNodes, serializer); + return ModifiableDataTreeDelegator.toBindingAware(registry, biNodes, serializer); } } @VisibleForTesting static WriterRegistry.DataObjectUpdates toBindingAware( + final WriterRegistry registry, final Map biNodes, final BindingNormalizedNodeSerializer serializer) { @@ -209,15 +211,27 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager HashMultimap.create(); for (Map.Entry biEntry : biNodes.entrySet()) { + final InstanceIdentifier keyedId = serializer.fromYangInstanceIdentifier(biEntry.getKey()); final InstanceIdentifier unkeyedIid = - RWUtils.makeIidWildcarded(serializer.fromYangInstanceIdentifier(biEntry.getKey())); + RWUtils.makeIidWildcarded(keyedId); NormalizedNodeUpdate normalizedNodeUpdate = biEntry.getValue(); final DataObjectUpdate dataObjectUpdate = toDataObjectUpdate(normalizedNodeUpdate, serializer); if (dataObjectUpdate != null) { if (dataObjectUpdate instanceof DataObjectUpdate.DataObjectDelete) { + // is delete dataObjectDeletes.put(unkeyedIid, (DataObjectUpdate.DataObjectDelete) dataObjectUpdate); + } else if (dataObjectUpdate.getDataBefore() != null && !registry.writerSupportsUpdate(unkeyedIid)) { + // is update and direct update operation is not supported + // breaks update to delete + create pair + + dataObjectDeletes.put(unkeyedIid, + (DataObjectUpdate.DataObjectDelete) DataObjectUpdate.DataObjectDelete + .create(keyedId, dataObjectUpdate.getDataBefore(), null)); + dataObjectUpdates + .put(unkeyedIid, DataObjectUpdate.create(keyedId, null, dataObjectUpdate.getDataAfter())); } else { + // is create dataObjectUpdates.put(unkeyedIid, dataObjectUpdate); } } diff --git a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java index 432833be3..ccd35a93d 100644 --- a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java +++ b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java @@ -128,8 +128,9 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { dataModification.commit(); final Multimap, DataObjectUpdate> map = HashMultimap.create(); - map.put(DEFAULT_ID, DataObjectUpdate.create(DEFAULT_ID, DEFAULT_DATA_OBJECT, DEFAULT_DATA_OBJECT)); - verify(writer).update(eq(new WriterRegistry.DataObjectUpdates(map, ImmutableMultimap.of())), any(WriteContext.class)); + // data before should be null as it is create + map.put(DEFAULT_ID, DataObjectUpdate.create(DEFAULT_ID, null, DEFAULT_DATA_OBJECT)); + verify(writer).processModifications(eq(new WriterRegistry.DataObjectUpdates(map, ImmutableMultimap.of())), any(WriteContext.class)); assertEquals(nestedList, dataTree.takeSnapshot().readNode(NESTED_LIST_ID).get()); } @@ -147,7 +148,7 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { final WriterRegistry.Reverter reverter = mock(WriterRegistry.Reverter.class); final TranslationException failedOnUpdateException = new TranslationException("update failed"); doThrow(new WriterRegistry.BulkUpdateException(DEFAULT_ID, update, Collections.singleton(DEFAULT_ID), reverter, failedOnUpdateException)) - .when(writer).update(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); + .when(writer).processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); try { // Run the test @@ -157,7 +158,7 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { dataModification.commit(); fail("WriterRegistry.RevertSuccessException was expected"); } catch (WriterRegistry.Reverter.RevertSuccessException e) { - verify(writer).update(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); + verify(writer).processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); assertThat(e.getFailedIds(), hasItem(DEFAULT_ID)); verify(reverter).revert(any(WriteContext.class)); } @@ -173,7 +174,7 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { final WriterRegistry.BulkUpdateException bulkFailEx = new WriterRegistry.BulkUpdateException(DEFAULT_ID, update, Collections.singleton(DEFAULT_ID), reverter, failedOnUpdateException); - doThrow(bulkFailEx).when(writer).update(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); + doThrow(bulkFailEx).when(writer).processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); // Fail on revert: doThrow(new WriterRegistry.Reverter.RevertFailedException(bulkFailEx)) @@ -187,7 +188,7 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { dataModification.commit(); fail("WriterRegistry.Reverter.RevertFailedException was expected"); } catch (WriterRegistry.Reverter.RevertFailedException e) { - verify(writer).update(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); + verify(writer).processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); verify(reverter).revert(any(WriteContext.class)); assertEquals(bulkFailEx, e.getCause()); } @@ -200,7 +201,7 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { @Test public void testToBindingAware() throws Exception { when(serializer.fromNormalizedNode(any(YangInstanceIdentifier.class), eq(null))).thenReturn(null); - + when(writer.writerSupportsUpdate(any())).thenReturn(true); final Map biNodes = new HashMap<>(); // delete final QName nn1 = QName.create("namespace", "nn1"); @@ -229,7 +230,7 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { biNodes.put(yid3, NormalizedNodeUpdate.create(yid3, nn3B, nn3A)); final WriterRegistry.DataObjectUpdates dataObjectUpdates = - ModifiableDataTreeDelegator.toBindingAware(biNodes, serializer); + ModifiableDataTreeDelegator.toBindingAware(writer, biNodes, serializer); assertThat(dataObjectUpdates.getDeletes().size(), is(1)); assertThat(dataObjectUpdates.getDeletes().keySet(), hasItem(((InstanceIdentifier) iid1))); @@ -245,6 +246,57 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { assertThat(dataObjectUpdates.getTypeIntersection().size(), is(3)); } + @Test + public void testToBindingAwareUpdateNotSupported() throws Exception { + when(serializer.fromNormalizedNode(any(YangInstanceIdentifier.class), eq(null))).thenReturn(null); + when(writer.writerSupportsUpdate(any())).thenReturn(false); + final Map biNodes = new HashMap<>(); + // delete + final QName nn1 = QName.create("namespace", "nn1"); + final YangInstanceIdentifier yid1 = mockYid(nn1); + final InstanceIdentifier iid1 = mockIid(yid1, DataObject1.class); + final NormalizedNode nn1B = mockNormalizedNode(nn1); + final DataObject1 do1B = mockDataObject(yid1, iid1, nn1B, DataObject1.class); + biNodes.put(yid1, NormalizedNodeUpdate.create(yid1, nn1B, null)); + + // create + final QName nn2 = QName.create("namespace", "nn1"); + final YangInstanceIdentifier yid2 = mockYid(nn2); + final InstanceIdentifier iid2 = mockIid(yid2, DataObject2.class);; + final NormalizedNode nn2A = mockNormalizedNode(nn2); + final DataObject2 do2A = mockDataObject(yid2, iid2, nn2A, DataObject2.class); + biNodes.put(yid2, NormalizedNodeUpdate.create(yid2, null, nn2A)); + + // processModifications + final QName nn3 = QName.create("namespace", "nn1"); + final YangInstanceIdentifier yid3 = mockYid(nn3); + final InstanceIdentifier iid3 = mockIid(yid3, DataObject3.class); + final NormalizedNode nn3B = mockNormalizedNode(nn3); + final DataObject3 do3B = mockDataObject(yid3, iid3, nn3B, DataObject3.class); + final NormalizedNode nn3A = mockNormalizedNode(nn3); + final DataObject3 do3A = mockDataObject(yid3, iid3, nn3A, DataObject3.class);; + biNodes.put(yid3, NormalizedNodeUpdate.create(yid3, nn3B, nn3A)); + + final WriterRegistry.DataObjectUpdates dataObjectUpdates = + ModifiableDataTreeDelegator.toBindingAware(writer, biNodes, serializer); + + // should have also id and data for delete as delete + create pair was created + assertThat(dataObjectUpdates.getDeletes().size(), is(2)); + assertThat(dataObjectUpdates.getDeletes().keySet(), + hasItems(((InstanceIdentifier) iid1), (InstanceIdentifier) iid3)); + assertThat(dataObjectUpdates.getDeletes().values(), hasItems( + ((DataObjectUpdate.DataObjectDelete) DataObjectUpdate.create(iid1, do1B, null)), + ((DataObjectUpdate.DataObjectDelete) DataObjectUpdate.create(iid3, do3B, null)))); + + assertThat(dataObjectUpdates.getUpdates().size(), is(2)); + assertThat(dataObjectUpdates.getUpdates().keySet(), hasItems( (InstanceIdentifier) iid2, (InstanceIdentifier) iid3)); + assertThat(dataObjectUpdates.getUpdates().values(), hasItems( + DataObjectUpdate.create(iid2, null, do2A), + DataObjectUpdate.create(iid3, null, do3A))); + + assertThat(dataObjectUpdates.getTypeIntersection().size(), is(3)); + } + private D mockDataObject(final YangInstanceIdentifier yid1, final InstanceIdentifier iid1, final NormalizedNode nn1B, -- cgit 1.2.3-korg