diff options
12 files changed, 950 insertions, 595 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; + } + } +} diff --git a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorBaseTest.java b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorBaseTest.java new file mode 100644 index 000000000..5690e78c3 --- /dev/null +++ b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorBaseTest.java @@ -0,0 +1,134 @@ +/* + * 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 org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.google.common.util.concurrent.Futures; +import io.fd.honeycomb.translate.write.DataObjectUpdate; +import io.fd.honeycomb.translate.write.WriteContext; +import io.fd.honeycomb.translate.write.registry.WriterRegistry; +import java.util.AbstractMap; +import java.util.Map; +import org.junit.Before; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.opendaylight.controller.md.sal.binding.api.DataBroker; +import org.opendaylight.mdsal.binding.dom.codec.api.BindingNormalizedNodeSerializer; +import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; +import org.opendaylight.yangtools.yang.common.QName; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; + +abstract class ModifiableDataTreeDelegatorBaseTest extends ModificationBaseTest { + + @Mock + WriterRegistry writer; + @Mock + BindingNormalizedNodeSerializer serializer; + @Mock + org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification modification; + @Mock + DataBroker contextBroker; + @Mock + org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction tx; + + @Captor + ArgumentCaptor<WriteContext> writeContextCaptor; + + @Captor + ArgumentCaptor<WriterRegistry.DataObjectUpdates> updatesCaptor; + + final InstanceIdentifier<?> DEFAULT_ID = InstanceIdentifier.create(DataObject.class); + final DataObject DEFAULT_DATA_OBJECT = mockDataObject("serialized", DataObject.class); + + DataTree dataTree; + ModifiableDataTreeManager configDataTree; + DataObjectUpdate update = DataObjectUpdate.create(DEFAULT_ID, null, DEFAULT_DATA_OBJECT); + + @Before + public void setup() throws Exception { + MockitoAnnotations.initMocks(this); + dataTree = getDataTree(); + when(contextBroker.newReadWriteTransaction()).thenReturn(tx); + when(tx.submit()).thenReturn(Futures.immediateCheckedFuture(null)); + + when(serializer.fromYangInstanceIdentifier(any(YangInstanceIdentifier.class))) + .thenReturn(((InstanceIdentifier) DEFAULT_ID)); + final Map.Entry<InstanceIdentifier<?>, DataObject> parsed = + new AbstractMap.SimpleEntry<>(DEFAULT_ID, DEFAULT_DATA_OBJECT); + when(serializer.fromNormalizedNode(any(YangInstanceIdentifier.class), any(NormalizedNode.class))) + .thenReturn(parsed); + + configDataTree = new ModifiableDataTreeDelegator(serializer, dataTree, getSchemaCtx(), writer, contextBroker); + + additionalSetup(); + } + + void additionalSetup() throws Exception { + } + + private static DataObject mockDataObject(final String name, final Class<? extends DataObject> classToMock) { + final DataObject dataBefore = mock(classToMock, name); + doReturn(classToMock).when(dataBefore).getImplementedInterface(); + return dataBefore; + } + + abstract static class DataObject1 implements DataObject { + } + + abstract static class DataObject2 implements DataObject { + } + + abstract static class DataObject3 implements DataObject { + } + + <D extends DataObject> D mockDataObject(final YangInstanceIdentifier yid1, + final InstanceIdentifier iid1, + final NormalizedNode nn1B, + final Class<D> type) { + final D do1B = mock(type); + when(serializer.fromNormalizedNode(yid1, nn1B)).thenReturn(new AbstractMap.SimpleEntry<>(iid1, do1B)); + return do1B; + } + + NormalizedNode mockNormalizedNode(final QName nn1) { + final NormalizedNode nn1B = mock(NormalizedNode.class); + when(nn1B.getNodeType()).thenReturn(nn1); + return nn1B; + } + + InstanceIdentifier mockIid(final YangInstanceIdentifier yid1, + final Class<? extends DataObject> type) { + final InstanceIdentifier iid1 = InstanceIdentifier.create(type); + when(serializer.fromYangInstanceIdentifier(yid1)).thenReturn(iid1); + return iid1; + } + + YangInstanceIdentifier mockYid(final QName nn1) { + final YangInstanceIdentifier yid1 = mock(YangInstanceIdentifier.class); + when(yid1.getLastPathArgument()).thenReturn(new YangInstanceIdentifier.NodeIdentifier(nn1)); + return yid1; + } +} diff --git a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorRevertTest.java b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorRevertTest.java new file mode 100644 index 000000000..2787cdf1f --- /dev/null +++ b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorRevertTest.java @@ -0,0 +1,257 @@ +/* + * 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 org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import io.fd.honeycomb.data.DataModification; +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.Collections; +import java.util.List; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.schema.MapNode; + +public class ModifiableDataTreeDelegatorRevertTest extends ModifiableDataTreeDelegatorBaseTest { + + /** + * Test scenario when commit fails, but there's nothing to revert because very first crud operation failed + */ + @Test + public void testCommitFailedNoRevert() throws Exception { + final MapNode nestedList = getNestedList("listEntry", "listValue"); + + // Fail on update: + final TranslationException failedOnUpdateException = new TranslationException("update failed"); + doThrow(new UpdateFailedException(failedOnUpdateException, Collections.emptyList(), update))// fail on update + .when(writer) + .processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); + + try { + // Run the test + final DataModification dataModification = configDataTree.newModification(); + dataModification.write(NESTED_LIST_ID, nestedList); + dataModification.validate(); + dataModification.commit(); + fail("UpdateFailedException was expected"); + } catch (UpdateFailedException e) { + // writer was called only one for update, and it was only one operation so no revert needed + // exception was just rethrown + verify(writer).processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); + assertEquals(e.getFailed().getId(), DEFAULT_ID); + assertTrue(e.getProcessed().isEmpty()); + } + } + + /** + * Test whether + * - Correct operations were invoked(when creating and reverting) + * - Create and revert both failed + * - Correct exception has been thrown + * Steps: + * - Prepares state with nested list written + * - Attempts to rewrite that list with new list with value with different key + * - Simulates fail(both on modification and revert) + * - Checks modifications + * Asserts + * - index 0 - Represents create of original data + * - index 1 - Represents override with new list(fails)(include delete of data created by index 0 and create of new) + * - index 2 - Represents revert of removal of original data + */ + @Test + public void testCommitWithRevertFailed() throws Exception { + // configure initial state + final MapNode originalList = getNestedList("listEntryOriginal", "listValueOriginal"); + + final DataModification preModification = configDataTree.newModification(); + preModification.write(NESTED_LIST_ID, originalList); + preModification.validate(); + preModification.commit(); + + // then test + final MapNode nestedList = getNestedList("listEntry", "listValueNew"); + + // Fail on update: + final TranslationException failedOnUpdateException = new TranslationException("update failed"); + final DataObjectUpdate.DataObjectDelete mockRevertData = mock(DataObjectUpdate.DataObjectDelete.class); + final DataObjectUpdate.DataObjectDelete mockRevertDataReverted = mock(DataObjectUpdate.DataObjectDelete.class); + when(mockRevertData.getId()).thenReturn((InstanceIdentifier) InstanceIdentifier.create(DataObject.class)); + when(mockRevertDataReverted.getId()) + .thenReturn((InstanceIdentifier) InstanceIdentifier.create(DataObject.class)); + when(mockRevertData.getDataBefore()).thenReturn(DEFAULT_DATA_OBJECT);// to simulate that delete of original data + //should be reverted + when(mockRevertDataReverted.getDataAfter()) + .thenReturn(DEFAULT_DATA_OBJECT);// to simulate that delete of original data + //should be reverted + when(mockRevertData.reverse()).thenReturn(mockRevertDataReverted); + + final UpdateFailedException cause = + new UpdateFailedException(failedOnUpdateException, + Collections.singletonList(mockRevertData),//fail on new one + update); + doThrow(cause) + .when(writer) + .processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); + + try { + // Run the test + final DataModification dataModification = configDataTree.newModification(); + dataModification.write(NESTED_LIST_ID, nestedList); + dataModification.validate(); + dataModification.commit(); + fail("WriterRegistry.Reverter.RevertFailedException was expected"); + } catch (Reverter.RevertFailedException e) { + assertRewriteModificationWithRevert(writer, updatesCaptor, DEFAULT_DATA_OBJECT); + assertEquals(cause, e.getCause()); + } + } + + /** + * Test whether + * - Correct operations were invoked(when creating and reverting) + * - Create failed and revert passed + * - Correct exception has been thrown + * Steps: + * - Prepares state with nested list written + * - Attempts to rewrite that list with new list with value with different key + * - Simulates fail on create + * - Passes on revert + * - Checks modifications + * Asserts + * - index 0 - Represents create of original data + * - index 1 - Represents override with new list(fails)(include delete of data created by index 0 and create of new) + * - index 2 - Represents revert of removal of original data + */ + @Test + public void testCommitWithRevertSuccessfull() throws Exception { + // configure initial state + final MapNode originalList = getNestedList("listEntryOriginal", "listValueOriginal"); + + final DataModification preModification = configDataTree.newModification(); + preModification.write(NESTED_LIST_ID, originalList); + preModification.validate(); + preModification.commit(); + + // then test + final MapNode nestedList = getNestedList("listEntry", "listValueNew"); + + // Fail on update: + final TranslationException failedOnUpdateException = new TranslationException("update failed"); + final DataObjectUpdate.DataObjectDelete mockRevertData = mock(DataObjectUpdate.DataObjectDelete.class); + final DataObjectUpdate.DataObjectDelete mockRevertDataReverted = mock(DataObjectUpdate.DataObjectDelete.class); + when(mockRevertData.getId()).thenReturn((InstanceIdentifier) InstanceIdentifier.create(DataObject.class)); + when(mockRevertDataReverted.getId()) + .thenReturn((InstanceIdentifier) InstanceIdentifier.create(DataObject.class)); + when(mockRevertData.getDataBefore()).thenReturn(DEFAULT_DATA_OBJECT);// to simulate that delete of original data + //should be reverted + when(mockRevertDataReverted.getDataAfter()) + .thenReturn(DEFAULT_DATA_OBJECT);// to simulate that delete of original data + //should be reverted + when(mockRevertData.reverse()).thenReturn(mockRevertDataReverted); + + final UpdateFailedException cause = + new UpdateFailedException(failedOnUpdateException, + Collections.singletonList(mockRevertData),//fail on new one + update); + doThrow(cause) // fails on create + .doNothing()//to pass on revert + .when(writer) + .processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); + + try { + // Run the test + final DataModification dataModification = configDataTree.newModification(); + dataModification.write(NESTED_LIST_ID, nestedList); + dataModification.validate(); + dataModification.commit(); + fail("WriterRegistry.Reverter.RevertFailedException was expected"); + } catch (Reverter.RevertSuccessException e) { + assertRewriteModificationWithRevert(writer, updatesCaptor, DEFAULT_DATA_OBJECT); + assertNull(e.getCause()); + } + } + + private static void assertRewriteModificationWithRevert(final WriterRegistry writer, + final ArgumentCaptor<WriterRegistry.DataObjectUpdates> updatesCaptor, + final DataObject DEFAULT_DATA_OBJECT) + throws TranslationException { + verify(writer, times(3)).processModifications(updatesCaptor.capture(), any(WriteContext.class)); + final List<WriterRegistry.DataObjectUpdates> allUpdates = updatesCaptor.getAllValues(); + assertEquals(3, allUpdates.size()); + + // represent create of original data + final WriterRegistry.DataObjectUpdates originalCreate = allUpdates.get(0); + assertContainsOnlySingleUpdate(originalCreate); + + final DataObjectUpdate createOriginalData = originalCreate.getUpdates().values().iterator().next(); + assertCreateWithData(createOriginalData, DEFAULT_DATA_OBJECT); + + // delete of original data was successful + // create of new data - failed + final WriterRegistry.DataObjectUpdates originalDelete = allUpdates.get(1); + assertConstainsSingleUpdateAndDelete(originalDelete); + + final DataObjectUpdate.DataObjectDelete deleteOriginalData = + originalDelete.getDeletes().values().iterator().next(); + assertDeleteWithData(deleteOriginalData, DEFAULT_DATA_OBJECT); + + final DataObjectUpdate newCreate = originalDelete.getUpdates().values().iterator().next(); + assertCreateWithData(newCreate, DEFAULT_DATA_OBJECT); + + final WriterRegistry.DataObjectUpdates revert = allUpdates.get(2); + assertContainsOnlySingleUpdate(revert); + } + + private static void assertDeleteWithData(final DataObjectUpdate.DataObjectDelete deleteOriginalData, + final DataObject DEFAULT_DATA_OBJECT) { + assertNull(deleteOriginalData.getDataAfter()); + assertEquals(DEFAULT_DATA_OBJECT, deleteOriginalData.getDataBefore()); + } + + private static void assertCreateWithData(final DataObjectUpdate newCreate, final DataObject DEFAULT_DATA_OBJECT) { + assertNull(newCreate.getDataBefore()); + assertEquals(DEFAULT_DATA_OBJECT, newCreate.getDataAfter()); + } + + private static void assertContainsOnlySingleUpdate(final WriterRegistry.DataObjectUpdates originalCreate) { + assertThat(originalCreate.getDeletes().size(), is(0)); + assertThat(originalCreate.getUpdates().size(), is(1)); + } + + private static void assertConstainsSingleUpdateAndDelete(final WriterRegistry.DataObjectUpdates originalDelete) { + assertThat(originalDelete.getDeletes().size(), is(1)); + assertThat(originalDelete.getUpdates().size(), is(1)); + } + +} 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 ccd35a93d..56c0c610f 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 @@ -22,84 +22,32 @@ import static org.hamcrest.CoreMatchers.is; 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.doReturn; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.mockito.MockitoAnnotations.initMocks; import com.google.common.base.Optional; import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.Multimap; import com.google.common.util.concurrent.CheckedFuture; -import com.google.common.util.concurrent.Futures; import io.fd.honeycomb.data.DataModification; -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.WriterRegistry; -import java.util.AbstractMap; -import java.util.Collections; import java.util.HashMap; import java.util.Map; -import org.junit.Before; import org.junit.Test; -import org.mockito.ArgumentCaptor; -import org.mockito.Captor; -import org.mockito.Mock; -import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException; -import org.opendaylight.mdsal.binding.dom.codec.api.BindingNormalizedNodeSerializer; -import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; import org.opendaylight.yangtools.yang.data.api.schema.MapNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; -public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { - - @Mock - private WriterRegistry writer; - @Mock - private BindingNormalizedNodeSerializer serializer; - private DataTree dataTree; - @Mock - private org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification modification; - @Mock - private DataBroker contextBroker; - @Mock - private org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction tx; - - @Captor - private ArgumentCaptor<WriteContext> writeContextCaptor; - - private ModifiableDataTreeManager configDataTree; - private final DataObjectUpdate update = DataObjectUpdate.create(DEFAULT_ID, null, DEFAULT_DATA_OBJECT); - - static final InstanceIdentifier<?> DEFAULT_ID = InstanceIdentifier.create(DataObject.class); - static DataObject DEFAULT_DATA_OBJECT = mockDataObject("serialized", DataObject.class); - - @Before - public void setUp() throws Exception { - initMocks(this); - dataTree = getDataTree(); - when(contextBroker.newReadWriteTransaction()).thenReturn(tx); - when(tx.submit()).thenReturn(Futures.immediateCheckedFuture(null)); - - when(serializer.fromYangInstanceIdentifier(any(YangInstanceIdentifier.class))).thenReturn(((InstanceIdentifier) DEFAULT_ID)); - final Map.Entry<InstanceIdentifier<?>, DataObject> parsed = new AbstractMap.SimpleEntry<>(DEFAULT_ID, DEFAULT_DATA_OBJECT); - when(serializer.fromNormalizedNode(any(YangInstanceIdentifier.class), any(NormalizedNode.class))).thenReturn(parsed); - - configDataTree = new ModifiableDataTreeDelegator(serializer, dataTree, getSchemaCtx(), writer, contextBroker); - } +public class ModifiableDataTreeDelegatorTest extends ModifiableDataTreeDelegatorBaseTest { @Test public void testRead() throws Exception { @@ -134,70 +82,6 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { assertEquals(nestedList, dataTree.takeSnapshot().readNode(NESTED_LIST_ID).get()); } - private static DataObject mockDataObject(final String name, final Class<? extends DataObject> classToMock) { - final DataObject dataBefore = mock(classToMock, name); - doReturn(classToMock).when(dataBefore).getImplementedInterface(); - return dataBefore; - } - - @Test - public void testCommitUndoSuccessful() throws Exception { - final MapNode nestedList = getNestedList("listEntry", "listValue"); - - // Fail on update: - 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).processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); - - try { - // Run the test - final DataModification dataModification = configDataTree.newModification(); - dataModification.write(NESTED_LIST_ID, nestedList); - dataModification.validate(); - dataModification.commit(); - fail("WriterRegistry.RevertSuccessException was expected"); - } catch (WriterRegistry.Reverter.RevertSuccessException e) { - verify(writer).processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); - assertThat(e.getFailedIds(), hasItem(DEFAULT_ID)); - verify(reverter).revert(any(WriteContext.class)); - } - } - - @Test - public void testCommitUndoFailed() throws Exception { - final MapNode nestedList = getNestedList("listEntry", "listValue"); - - // Fail on update: - final WriterRegistry.Reverter reverter = mock(WriterRegistry.Reverter.class); - final TranslationException failedOnUpdateException = new TranslationException("update failed"); - final WriterRegistry.BulkUpdateException bulkFailEx = - new WriterRegistry.BulkUpdateException(DEFAULT_ID, update, Collections.singleton(DEFAULT_ID), reverter, - failedOnUpdateException); - doThrow(bulkFailEx).when(writer).processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); - - // Fail on revert: - doThrow(new WriterRegistry.Reverter.RevertFailedException(bulkFailEx)) - .when(reverter).revert(any(WriteContext.class)); - - try { - // Run the test - final DataModification dataModification = configDataTree.newModification(); - dataModification.write(NESTED_LIST_ID, nestedList); - dataModification.validate(); - dataModification.commit(); - fail("WriterRegistry.Reverter.RevertFailedException was expected"); - } catch (WriterRegistry.Reverter.RevertFailedException e) { - verify(writer).processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); - verify(reverter).revert(any(WriteContext.class)); - assertEquals(bulkFailEx, e.getCause()); - } - } - - private abstract static class DataObject1 implements DataObject {} - private abstract static class DataObject2 implements DataObject {} - private abstract static class DataObject3 implements DataObject {} - @Test public void testToBindingAware() throws Exception { when(serializer.fromNormalizedNode(any(YangInstanceIdentifier.class), eq(null))).thenReturn(null); @@ -214,7 +98,7 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { // create final QName nn2 = QName.create("namespace", "nn1"); final YangInstanceIdentifier yid2 = mockYid(nn2); - final InstanceIdentifier iid2 = mockIid(yid2, DataObject2.class);; + 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)); @@ -226,7 +110,7 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { 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);; + final DataObject3 do3A = mockDataObject(yid3, iid3, nn3A, DataObject3.class); biNodes.put(yid3, NormalizedNodeUpdate.create(yid3, nn3B, nn3A)); final WriterRegistry.DataObjectUpdates dataObjectUpdates = @@ -262,7 +146,7 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { // create final QName nn2 = QName.create("namespace", "nn1"); final YangInstanceIdentifier yid2 = mockYid(nn2); - final InstanceIdentifier iid2 = mockIid(yid2, DataObject2.class);; + 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)); @@ -274,7 +158,7 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { 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);; + final DataObject3 do3A = mockDataObject(yid3, iid3, nn3A, DataObject3.class); biNodes.put(yid3, NormalizedNodeUpdate.create(yid3, nn3B, nn3A)); final WriterRegistry.DataObjectUpdates dataObjectUpdates = @@ -289,39 +173,11 @@ public class ModifiableDataTreeDelegatorTest extends ModificationBaseTest { ((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().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 extends DataObject> D mockDataObject(final YangInstanceIdentifier yid1, - final InstanceIdentifier iid1, - final NormalizedNode nn1B, - final Class<D> type) { - final D do1B = mock(type); - when(serializer.fromNormalizedNode(yid1, nn1B)).thenReturn(new AbstractMap.SimpleEntry<>(iid1, do1B)); - return do1B; - } - - private NormalizedNode mockNormalizedNode(final QName nn1) { - final NormalizedNode nn1B = mock(NormalizedNode.class); - when(nn1B.getNodeType()).thenReturn(nn1); - return nn1B; - } - - private InstanceIdentifier mockIid(final YangInstanceIdentifier yid1, - final Class<? extends DataObject> type) { - final InstanceIdentifier iid1 = InstanceIdentifier.create(type); - when(serializer.fromYangInstanceIdentifier(yid1)).thenReturn(iid1); - return iid1; - } - - private YangInstanceIdentifier mockYid(final QName nn1) { - final YangInstanceIdentifier yid1 = mock(YangInstanceIdentifier.class); - when(yid1.getLastPathArgument()).thenReturn(new YangInstanceIdentifier.NodeIdentifier(nn1)); - return yid1; - } } diff --git a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ReverterTest.java b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ReverterTest.java new file mode 100644 index 000000000..db3fdbaf5 --- /dev/null +++ b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ReverterTest.java @@ -0,0 +1,192 @@ +/* + * 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 org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import com.google.common.collect.ImmutableList; +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.Collections; +import java.util.Iterator; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; + +public class ReverterTest { + + private static final InstanceIdentifier<DataObject> IID_0 = InstanceIdentifier.create(DataObject.class); + private static final InstanceIdentifier<DataObject1> IID_1 = InstanceIdentifier.create(DataObject1.class); + private static final InstanceIdentifier<DataObject2> IID_2 = InstanceIdentifier.create(DataObject2.class); + + @Mock + private WriterRegistry registry; + + @Mock + private WriteContext writeContext; + + @Captor + private ArgumentCaptor<WriterRegistry.DataObjectUpdates> updateCaptor; + + @Before + public void init() { + MockitoAnnotations.initMocks(this); + } + + @Test + public void revertSingle() throws Exception { + final DataObjectUpdate create = DataObjectUpdate.create(IID_0, null, mock(DataObject.class)); + + new Reverter(ImmutableList.of(create), registry).revert(writeContext); + assertSingleRevert(create); + } + + @Test + public void revertSingleFailed() throws TranslationException { + final DataObjectUpdate create = DataObjectUpdate.create(IID_0, null, mock(DataObject.class)); + final UpdateFailedException ex = + new UpdateFailedException(new IllegalStateException(), Collections.emptyList(), create); + doThrow(ex).when(registry) + .processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); + + try { + new Reverter(ImmutableList.of(create), registry).revert(writeContext); + } catch (Reverter.RevertFailedException e) { + assertEquals(ex, e.getCause()); + assertSingleRevert(create); + return; + } + fail("Reverter.RevertFailedException was expected"); + } + + + @Test + public void revertSingleFailedWithUnexpectedEx() throws TranslationException { + final DataObjectUpdate create = DataObjectUpdate.create(IID_0, null, mock(DataObject.class)); + final IllegalStateException ex = new IllegalStateException(); + doThrow(ex).when(registry) + .processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); + + try { + new Reverter(ImmutableList.of(create), registry).revert(writeContext); + } catch (Reverter.RevertFailedException e) { + assertEquals(ex, e.getCause()); + assertSingleRevert(create); + return; + } + fail("IllegalStateException was expected"); + } + + + @Test + public void revertMultiple() throws Exception { + final DataObjectUpdate create = DataObjectUpdate.create(IID_0, null, mock(DataObject.class)); + final DataObjectUpdate update = + DataObjectUpdate.create(IID_1, mock(DataObject1.class), mock(DataObject1.class)); + final DataObjectUpdate delete = DataObjectUpdate.create(IID_2, mock(DataObject2.class), null); + + new Reverter(ImmutableList.of(create, update, delete), registry).revert(writeContext); + assertMultiRevert(create, update, delete); + } + + + @Test + public void revertMultipleFailed() throws Exception { + final DataObjectUpdate create = DataObjectUpdate.create(IID_0, null, mock(DataObject.class)); + final DataObjectUpdate update = + DataObjectUpdate.create(IID_1, mock(DataObject1.class), mock(DataObject1.class)); + final DataObjectUpdate delete = DataObjectUpdate.create(IID_2, mock(DataObject2.class), null); + + final UpdateFailedException ex = + new UpdateFailedException(new IllegalStateException(), ImmutableList.of(create, update), create); + doThrow(ex).when(registry) + .processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); + + try { + new Reverter(ImmutableList.of(create, update, delete), registry).revert(writeContext); + } catch (Reverter.RevertFailedException e) { + assertEquals(ex, e.getCause()); + assertMultiRevert(create, update, delete); + return; + } + fail("Reverter.RevertFailedException was expected"); + } + + @Test + public void revertMultipleFailedWithUnnexpectedException() throws Exception { + final DataObjectUpdate create = DataObjectUpdate.create(IID_0, null, mock(DataObject.class)); + final DataObjectUpdate update = + DataObjectUpdate.create(IID_1, mock(DataObject1.class), mock(DataObject1.class)); + final DataObjectUpdate delete = DataObjectUpdate.create(IID_2, mock(DataObject2.class), null); + + final IllegalStateException ex = new IllegalStateException(); + doThrow(ex).when(registry) + .processModifications(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class)); + + try { + new Reverter(ImmutableList.of(create, update, delete), registry).revert(writeContext); + } catch (Reverter.RevertFailedException e) { + assertEquals(ex, e.getCause()); + assertMultiRevert(create, update, delete); + return; + } + fail("IllegalStateException was expected"); + } + + + private void assertSingleRevert(final DataObjectUpdate create) throws TranslationException { + verify(registry, times(1)).processModifications(updateCaptor.capture(), eq(writeContext)); + final WriterRegistry.DataObjectUpdates updates = updateCaptor.getValue(); + assertTrue(updates.getDeletes().containsValue(create.reverse())); + assertTrue(updates.getUpdates().isEmpty()); + } + + private void assertMultiRevert(final DataObjectUpdate create, final DataObjectUpdate update, + final DataObjectUpdate delete) throws TranslationException { + verify(registry, times(1)).processModifications(updateCaptor.capture(), eq(writeContext)); + final WriterRegistry.DataObjectUpdates updates = updateCaptor.getValue(); + final Iterator<DataObjectUpdate.DataObjectDelete> deletesIterator = updates.getDeletes().values().iterator(); + final Iterator<DataObjectUpdate> updatesIterator = updates.getUpdates().values().iterator(); + + assertEquals(updatesIterator.next(), delete.reverse()); + assertEquals(updatesIterator.next(), update.reverse()); + assertEquals(deletesIterator.next(), create.reverse()); + } + + + private interface DataObject1 extends DataObject { + } + + private interface DataObject2 extends DataObject { + } +}
\ No newline at end of file diff --git a/infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/registry/UpdateFailedException.java b/infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/registry/UpdateFailedException.java new file mode 100644 index 000000000..82a08e46c --- /dev/null +++ b/infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/registry/UpdateFailedException.java @@ -0,0 +1,58 @@ +/* + * 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.translate.write.registry; + +import io.fd.honeycomb.translate.TranslationException; +import io.fd.honeycomb.translate.write.DataObjectUpdate; +import java.util.List; +import javax.annotation.Nonnull; + +/** + * Thrown when CRUD operation fails. + */ +public class UpdateFailedException extends TranslationException { + + private final List<DataObjectUpdate> processed; + private final DataObjectUpdate failed; + + /** + * @param cause original cause of failure + * @param processed updates that were processed up until the point of failure + * @param failed update that cause the failure + */ + public UpdateFailedException(@Nonnull final Throwable cause, + @Nonnull final List<DataObjectUpdate> processed, + @Nonnull final DataObjectUpdate failed) { + super(cause); + this.processed = processed; + this.failed = failed; + } + + /** + * Returns set of nodes that has been processed by this operation till the failure happened, in execution order + */ + public List<DataObjectUpdate> getProcessed() { + return processed; + } + + /** + * Returns update that caused failure + */ + public DataObjectUpdate getFailed() { + return failed; + } +} 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 a297b6de8..e2924f84a 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 @@ -16,8 +16,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.Multimap; import com.google.common.collect.Sets; @@ -26,6 +24,8 @@ import io.fd.honeycomb.translate.write.DataObjectUpdate; import io.fd.honeycomb.translate.write.WriteContext; import io.fd.honeycomb.translate.write.Writer; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.annotation.Nonnull; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; @@ -38,8 +38,7 @@ public interface WriterRegistry { /** * Performs bulk update. * - * @throws BulkUpdateException in case bulk update fails - * @throws TranslationException in case some other error occurs while processing update request + * @throws TranslationException in case update fails or there was some other problem while processing */ void processModifications(@Nonnull DataObjectUpdates updates, @Nonnull WriteContext ctx) throws TranslationException; @@ -52,8 +51,8 @@ public interface WriterRegistry { boolean writerSupportsUpdate(@Nonnull InstanceIdentifier<?> type); /** - * Simple DTO containing updates for {@link WriterRegistry}. Currently only deletes and updates (create + - * update) are distinguished. + * Simple DTO containing updates for {@link WriterRegistry}. Currently only deletes and updates (create + update) + * are distinguished. */ @Beta final class DataObjectUpdates { @@ -97,6 +96,11 @@ public interface WriterRegistry { return Sets.union(deletes.keySet(), updates.keySet()); } + public Set<DataObjectUpdate> getAllModifications() { + return Stream.concat(updates.values().stream(), deletes.values().stream()) + .collect(Collectors.toSet()); + } + /** * Check whether there is only a single type of data object to be updated. * @@ -132,135 +136,4 @@ public interface WriterRegistry { } } - - /** - * Thrown when bulk update failed. - */ - @Beta - class BulkUpdateException extends TranslationException { - - private final transient Reverter reverter; - private final InstanceIdentifier<?> failedSubtree; - private final DataObjectUpdate failedData; - private final Set<InstanceIdentifier<?>> unrevertedSubtrees; - - /** - * Constructs an BulkUpdateException. - * @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 InstanceIdentifier<?> failedSubtree, - @Nonnull final DataObjectUpdate failedData, - @Nonnull final Set<InstanceIdentifier<?>> unhandledSubtrees, - @Nonnull final Reverter reverter, - @Nonnull final Throwable cause) { - 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"); - } - - /** - * Reverts changes that were successfully applied during bulk update before failure occurred. - * - * @param writeContext Non-closed {@code WriteContext} to be used by reverting logic.<br> <b>Do not use same - * write context as was used in previous write</b> - * @throws Reverter.RevertFailedException if revert fails - */ - public void revertChanges(@Nonnull final WriteContext writeContext) throws Reverter.RevertFailedException { - reverter.revert(writeContext); - } - - public Set<InstanceIdentifier<?>> getUnrevertedSubtrees() { - return unrevertedSubtrees; - } - - public InstanceIdentifier<?> getFailedSubtree() { - return failedSubtree; - } - - public DataObjectUpdate getFailedData() { - return failedData; - } - } - - /** - * Abstraction over revert mechanism in case of a bulk update failure. - */ - @Beta - interface Reverter { - - /** - * Reverts changes that were successfully applied during bulk 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; - - /** - * Thrown when some of the changes applied during bulk update were not reverted. - */ - @Beta - class RevertFailedException extends TranslationException { - - /** - * Constructs a RevertFailedException with the list of changes that were not reverted. - * - * @param cause the cause of revert failure - */ - public RevertFailedException(@Nonnull final BulkUpdateException cause) { - super("Unable to revert changes after failure. Revert failed for " - + cause.getFailedSubtree() + " unreverted subtrees: " + cause.getUnrevertedSubtrees(), cause); - } - - /** - * Returns the list of changes that were not reverted. - * - * @return list of changes that were not reverted - */ - @Nonnull - public Set<InstanceIdentifier<?>> getNotRevertedChanges() { - 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(); - } - } - - /** - * 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 - 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; - } - } - } }
\ No newline at end of file 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 deleted file mode 100644 index 9623db5bc..000000000 --- a/infra/translate-api/src/test/java/io/fd/honeycomb/translate/write/registry/BulkUpdateExceptionTest.java +++ /dev/null @@ -1,63 +0,0 @@ -/* - * Copyright (c) 2016 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.translate.write.registry; - -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; -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 BulkUpdateExceptionTest { - - private InstanceIdentifier<?> id = InstanceIdentifier.create(DataObject.class); - - @Mock - private WriteContext writeContext; - @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 testRevert() throws Exception { - final HashSet<InstanceIdentifier<?>> failedIds = Sets.newHashSet(id); - final WriterRegistry.BulkUpdateException bulkUpdateException = - new WriterRegistry.BulkUpdateException(id, DataObjectUpdate.create(id, before, after), - failedIds, reverter, new RuntimeException()); - - assertEquals(failedIds, bulkUpdateException.getUnrevertedSubtrees()); - - bulkUpdateException.revertChanges(writeContext); - verify(reverter).revert(writeContext); - } -}
\ No newline at end of file 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 deleted file mode 100644 index 6502235d0..000000000 --- a/infra/translate-api/src/test/java/io/fd/honeycomb/translate/write/registry/RevertFailedExceptionTest.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright (c) 2016 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.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( - new WriterRegistry.BulkUpdateException(id, DataObjectUpdate.create(id, before, after), - notReverted, reverter, new RuntimeException())); - assertEquals(notReverted, revertFailedException.getNotRevertedChanges()); - } -}
\ No newline at end of file diff --git a/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/registry/FlatWriterRegistry.java b/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/registry/FlatWriterRegistry.java index e21297aa3..146ddb9c5 100644 --- a/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/registry/FlatWriterRegistry.java +++ b/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/registry/FlatWriterRegistry.java @@ -18,10 +18,8 @@ package io.fd.honeycomb.translate.impl.write.registry; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import static io.fd.honeycomb.translate.util.RWUtils.makeIidWildcarded; import com.google.common.base.Optional; -import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; @@ -31,15 +29,15 @@ import io.fd.honeycomb.translate.TranslationException; import io.fd.honeycomb.translate.util.RWUtils; 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.write.registry.UpdateFailedException; import io.fd.honeycomb.translate.write.registry.WriterRegistry; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; +import java.util.LinkedList; +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 javax.annotation.concurrent.ThreadSafe; @@ -66,9 +64,9 @@ final class FlatWriterRegistry implements WriterRegistry { /** * Create flat registry instance. * - * @param writers immutable, ordered map of writers to use to process updates. Order of the writers has to be - * one in which create and update operations should be handled. Deletes will be handled in reversed - * order. All deletes are handled before handling all the updates. + * @param writers immutable, ordered map of writers to use to process updates. Order of the writers has to be one in + * which create and update operations should be handled. Deletes will be handled in reversed order. + * All deletes are handled before handling all the updates. */ FlatWriterRegistry(@Nonnull final ImmutableMap<InstanceIdentifier<?>, Writer<?>> writers) { this.writers = writers; @@ -98,17 +96,22 @@ final class FlatWriterRegistry implements WriterRegistry { return; } - // Optimization + // ordered set of already processed nodes + final List<DataObjectUpdate> alreadyProcessed = new LinkedList<>(); + + // Optimization for single type updates, less consuming for pairing update with responsible writer,etc if (updates.containsOnlySingleType()) { // First process delete - singleUpdate(updates.getDeletes(), ctx); + singleUpdate(updates.getDeletes(), alreadyProcessed, ctx); + // Next is update - singleUpdate(updates.getUpdates(), ctx); + singleUpdate(updates.getUpdates(), alreadyProcessed, ctx); } else { // First process deletes - bulkUpdate(updates.getDeletes(), ctx, true, writersOrderReversed); + bulkUpdate(updates.getDeletes(), alreadyProcessed, ctx, writersOrderReversed); + // Next are updates - bulkUpdate(updates.getUpdates(), ctx, true, writersOrder); + bulkUpdate(updates.getUpdates(), alreadyProcessed, ctx, writersOrder); } LOG.debug("Update successful for types: {}", updates.getTypeIntersection()); @@ -119,19 +122,22 @@ final class FlatWriterRegistry implements WriterRegistry { public boolean writerSupportsUpdate(@Nonnull final InstanceIdentifier<?> type) { Writer writer = getWriter(type); - if(writer == null){ + if (writer == null) { writer = getSubtreeWriterResponsible(type); } return checkNotNull(writer, "Unable to find writer for %s", type).supportsDirectUpdate(); } - private void singleUpdate(@Nonnull final Multimap<InstanceIdentifier<?>, ? extends DataObjectUpdate> updates, - @Nonnull final WriteContext ctx) throws WriteFailedException { + private void singleUpdate( + @Nonnull final Multimap<InstanceIdentifier<?>, ? extends DataObjectUpdate> updates, + @Nonnull final List<DataObjectUpdate> alreadyProcessed, + @Nonnull final WriteContext ctx) throws UpdateFailedException { if (updates.isEmpty()) { return; } + DataObjectUpdate current = null; final InstanceIdentifier<?> singleType = updates.keySet().iterator().next(); LOG.debug("Performing single type update for: {}", singleType); Collection<? extends DataObjectUpdate> singleTypeUpdates = updates.get(singleType); @@ -145,9 +151,18 @@ final class FlatWriterRegistry implements WriterRegistry { singleTypeUpdates = getParentDataObjectUpdate(ctx, updates, writer); } - LOG.trace("Performing single type update with writer: {}", writer); - for (DataObjectUpdate singleUpdate : singleTypeUpdates) { - writer.processModification(singleUpdate.getId(), singleUpdate.getDataBefore(), singleUpdate.getDataAfter(), ctx); + try { + LOG.trace("Performing single type update with writer: {}", writer); + + for (DataObjectUpdate singleUpdate : singleTypeUpdates) { + current = singleUpdate; + writer.processModification(singleUpdate.getId(), singleUpdate.getDataBefore(), + singleUpdate.getDataAfter(), + ctx); + alreadyProcessed.add(singleUpdate); + } + } catch (Exception e) { + throw new UpdateFailedException(e, alreadyProcessed, current); } } @@ -181,21 +196,20 @@ final class FlatWriterRegistry implements WriterRegistry { DataObjectUpdate.create(parentKeyedId, parentBefore.orNull(), parentAfter.orNull())); } - private void bulkUpdate(@Nonnull final Multimap<InstanceIdentifier<?>, ? extends DataObjectUpdate> updates, - @Nonnull final WriteContext ctx, - final boolean attemptRevert, - @Nonnull final Set<InstanceIdentifier<?>> writersOrder) throws BulkUpdateException { + private void bulkUpdate( + @Nonnull final Multimap<InstanceIdentifier<?>, ? extends DataObjectUpdate> updates, + @Nonnull final List<DataObjectUpdate> alreadyProcessed, + @Nonnull final WriteContext ctx, + @Nonnull final Set<InstanceIdentifier<?>> writersOrder) throws UpdateFailedException { if (updates.isEmpty()) { return; } - LOG.debug("Performing bulk update with revert attempt: {} for: {}", attemptRevert, updates.keySet()); - // Check that all updates can be handled checkAllTypesCanBeHandled(updates); - // Capture all changes successfully processed in case revert is needed - final Set<InstanceIdentifier<?>> processedNodes = new HashSet<>(); + LOG.debug("Performing bulk update for: {}", updates.keySet()); + DataObjectUpdate current = null; // Iterate over all writers and call update if there are any related updates for (InstanceIdentifier<?> writerType : writersOrder) { @@ -215,29 +229,20 @@ final class FlatWriterRegistry implements WriterRegistry { } } - LOG.debug("Performing update for: {}", writerType); + LOG.debug("Performing update for: {}", writerType); LOG.trace("Performing update with writer: {}", writer); for (DataObjectUpdate singleUpdate : writersData) { + current = singleUpdate; try { - writer.processModification(singleUpdate.getId(), singleUpdate.getDataBefore(), singleUpdate.getDataAfter(), ctx); - processedNodes.add(singleUpdate.getId()); - LOG.trace("Update successful for type: {}", writerType); - LOG.debug("Update successful for: {}", singleUpdate); + writer.processModification(singleUpdate.getId(), singleUpdate.getDataBefore(), + singleUpdate.getDataAfter(), ctx); } catch (Exception e) { - // do not log this exception here,its logged in ModifiableDataTreeDelegator - - final Reverter reverter = attemptRevert - ? new ReverterImpl(processedNodes, updates, writersOrder) - : (final WriteContext writeContext) -> {};//NOOP reverter - - // Find out which changes left unprocessed - final Set<InstanceIdentifier<?>> unprocessedChanges = updates.values().stream() - .map(DataObjectUpdate::getId) - .filter(id -> !processedNodes.contains(id)) - .collect(Collectors.toSet()); - throw new BulkUpdateException(writerType, singleUpdate, unprocessedChanges, reverter, e); + throw new UpdateFailedException(e, alreadyProcessed, current); } + alreadyProcessed.add(singleUpdate); + LOG.trace("Update successful for type: {}", writerType); + LOG.debug("Update successful for: {}", singleUpdate); } } } @@ -254,11 +259,11 @@ final class FlatWriterRegistry implements WriterRegistry { /** * Check whether {@link SubtreeWriter} is affected by the updates. * - * @return true if there are any updates to SubtreeWriter's child nodes (those marked by SubtreeWriter - * as being taken care of) - * */ + * @return true if there are any updates to SubtreeWriter's child nodes (those marked by SubtreeWriter as being + * taken care of) + */ private static boolean isAffected(final SubtreeWriter<?> writer, - final Multimap<InstanceIdentifier<?>, ? extends DataObjectUpdate> updates) { + final Multimap<InstanceIdentifier<?>, ? extends DataObjectUpdate> updates) { return !Sets.intersection(writer.getHandledChildTypes(), updates.keySet()).isEmpty(); } @@ -267,60 +272,4 @@ final class FlatWriterRegistry implements WriterRegistry { return writers.get(singleType); } - private final class ReverterImpl implements Reverter { - - private final Collection<InstanceIdentifier<?>> processedNodes; - private final Multimap<InstanceIdentifier<?>, ? extends DataObjectUpdate> updates; - private final Set<InstanceIdentifier<?>> revertDeleteOrder; - - ReverterImpl(final Collection<InstanceIdentifier<?>> processedNodes, - final Multimap<InstanceIdentifier<?>, ? extends DataObjectUpdate> updates, - final Set<InstanceIdentifier<?>> writersOrderOriginal) { - this.processedNodes = processedNodes; - this.updates = updates; - // Use opposite ordering when executing revert - this.revertDeleteOrder = writersOrderOriginal == FlatWriterRegistry.this.writersOrder - ? FlatWriterRegistry.this.writersOrderReversed - : FlatWriterRegistry.this.writersOrder; - } - - @Override - public void revert(@Nonnull final WriteContext writeContext) throws RevertFailedException { - checkNotNull(writeContext, "Cannot revert changes for null context"); - - Multimap<InstanceIdentifier<?>, DataObjectUpdate> updatesToRevert = - filterAndRevertProcessed(updates, processedNodes); - - LOG.info("Attempting revert for changes: {}", updatesToRevert); - try { - // Perform reversed bulk update without revert attempt - bulkUpdate(updatesToRevert, writeContext, true, revertDeleteOrder); - LOG.info("Revert successful"); - } catch (BulkUpdateException e) { - LOG.error("Revert failed", e); - throw new RevertFailedException(e); - } - } - - /** - * Create new updates map, but only keep already processed changes. Switching before and after data for each - * update. - */ - private Multimap<InstanceIdentifier<?>, DataObjectUpdate> filterAndRevertProcessed( - final Multimap<InstanceIdentifier<?>, ? extends DataObjectUpdate> updates, - final Collection<InstanceIdentifier<?>> processedNodes) { - final Multimap<InstanceIdentifier<?>, DataObjectUpdate> filtered = HashMultimap.create(); - for (InstanceIdentifier<?> processedNode : processedNodes) { - final InstanceIdentifier<?> wildcardedIid = makeIidWildcarded(processedNode); - if (updates.containsKey(wildcardedIid)) { - updates.get(wildcardedIid).stream() - .filter(dataObjectUpdate -> processedNode.contains(dataObjectUpdate.getId())) - // putting under unkeyed identifier, to prevent failing of checkAllTypesCanBeHandled - .forEach(dataObjectUpdate -> filtered.put(wildcardedIid, dataObjectUpdate.reverse())); - } - } - return filtered; - } - } - } 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<InstanceIdentifier<?>, DataObjectUpdate> updates = HashMultimap.create(); addUpdate(updates, DataObject1.class); - addUpdate(updates, DataObjects.DataObject3.class); - final InstanceIdentifier<DataObject2> 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<DataObjectUpdate> 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<DataObjectUpdate> 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<InstanceIdentifier<?>, 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 <D extends DataObject> void addUpdate(final Multimap<InstanceIdentifier<?>, DataObjectUpdate> updates, - final Class<D> type) throws Exception { + final Class<D> type) throws Exception { final InstanceIdentifier<D> iid = (InstanceIdentifier<D>) type.getDeclaredField("IID").get(null); - updates.put(iid, DataObjectUpdate.create(iid, mock(type), mock(type))); + updates.put(iid, updateData(type, iid)); + } + + private static <D extends DataObject> DataObjectUpdate updateData(final Class<D> type, + final InstanceIdentifier<D> iid) { + return DataObjectUpdate.create(iid, mock(type), mock(type)); } }
\ No newline at end of file |