summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegator.java50
-rw-r--r--infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/Reverter.java169
-rw-r--r--infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorBaseTest.java134
-rw-r--r--infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorRevertTest.java257
-rw-r--r--infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java156
-rw-r--r--infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ReverterTest.java192
-rw-r--r--infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/registry/UpdateFailedException.java58
-rw-r--r--infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/registry/WriterRegistry.java147
-rw-r--r--infra/translate-api/src/test/java/io/fd/honeycomb/translate/write/registry/BulkUpdateExceptionTest.java63
-rw-r--r--infra/translate-api/src/test/java/io/fd/honeycomb/translate/write/registry/RevertFailedExceptionTest.java55
-rw-r--r--infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/registry/FlatWriterRegistry.java157
-rw-r--r--infra/translate-impl/src/test/java/io/fd/honeycomb/translate/impl/write/registry/FlatWriterRegistryTest.java107
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 66dcbe5..e76c5b6 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 0000000..9c72626
--- /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 0000000..5690e78
--- /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 0000000..2787cdf
--- /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 ccd35a9..56c0c61 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 0000000..db3fdba
--- /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 0000000..82a08e4
--- /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 a297b6d..e2924f8 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 9623db5..0000000
--- 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 6502235..0000000
--- 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 e21297a..146ddb9 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 72a91cb..1514369 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