diff options
| author |   Jan Srnicek <jsrnicek@cisco.com> | 2017-10-23 10:57:13 +0200 |
|---|---|---|
| committer |   Marek Gradzki <mgradzki@cisco.com> | 2017-10-23 12:26:02 +0000 |
| commit | 5503731d866d318e9d5a2183608092a9d332dfe6 (patch) | |
| tree | 10470b27b8ddb1a7776f78c733546be4d1a48b29 /infra | |
| parent | 0762f9aa1a7894056c2ddbc72421b933e9ea8dcf (diff) | |
HONEYCOMB-405 - Revert fix for indirect updates
If indirect update(delete+create) fails in a way, that delete passed,
but update part failed, delete part must be reverted
Moves reverter creation to MDTG and test cases related too it to
ModifiableDataTreeDelegatorRevertTest
Fixes tracking of allready processed changes by tracking them
from perspective of processModifications() method
Introduces UpdateFailedException as replacement
for BulkUpdateException(now thrown also for single updates)
Separates ReverterImpl from FlatWriterRegistry and ads unit tests
Change-Id: If0066d0716d9476be89b1d99985b6745becac15e
Signed-off-by: Jan Srnicek <jsrnicek@cisco.com>
Diffstat (limited to 'infra')
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 |