summaryrefslogtreecommitdiffstats
path: root/infra/translate-utils
diff options
context:
space:
mode:
authorJan Srnicek <jsrnicek@cisco.com>2016-10-24 13:02:59 +0200
committerMaros Marsalek <mmarsale@cisco.com>2016-10-24 17:33:24 +0200
commitbb090e1254eacc95d7bd1dd45f6311967f81af86 (patch)
tree101df09cd4b8adad9d083a00442aecf90e9d9948 /infra/translate-utils
parent81c7ae0e263515b5bf8acb59b06d61ba446b8f0b (diff)
HONEYCOMB-255 - Cutting identifiers to prevent failing of reverts
Mapping allready processes changes for reverting by InstanceIdentifier instead of using KeyedInstanceIdentifier(to prevent failing to identify handleable nodes) Modified logging to prevent double/triple logging of detailed cause of failed bulk update Reusing WriteContext for revert(removed try with resource to prevent closing of write context before revert) Change-Id: Ie939ebe443629f9cdad5b5b449aa8c5dac40ea67 Signed-off-by: Jan Srnicek <jsrnicek@cisco.com> Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
Diffstat (limited to 'infra/translate-utils')
-rw-r--r--infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/TransactionWriteContext.java3
-rw-r--r--infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistry.java26
-rw-r--r--infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/DataObjects.java14
-rw-r--r--infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistryTest.java60
4 files changed, 81 insertions, 22 deletions
diff --git a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/TransactionWriteContext.java b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/TransactionWriteContext.java
index 0128ee4a8..dd397a605 100644
--- a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/TransactionWriteContext.java
+++ b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/TransactionWriteContext.java
@@ -108,9 +108,6 @@ public final class TransactionWriteContext implements WriteContext {
return mappingContext;
}
- /**
- * Does not close the transactions.
- */
@Override
public void close() {
ctx.close();
diff --git a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistry.java b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistry.java
index ab80eb4ac..e5b829d6a 100644
--- a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistry.java
+++ b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistry.java
@@ -17,6 +17,8 @@
package io.fd.honeycomb.translate.util.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;
@@ -26,9 +28,9 @@ import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
import io.fd.honeycomb.translate.TranslationException;
-import io.fd.honeycomb.translate.write.WriteContext;
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.WriterRegistry;
@@ -211,11 +213,11 @@ final class FlatWriterRegistry implements WriterRegistry {
LOG.trace("Update successful for type: {}", writerType);
LOG.debug("Update successful for: {}", singleUpdate);
} catch (Exception e) {
- LOG.error("Error while processing data change of: {} (updates={})", writerType, writersData, e);
+ // do not log this exception here,its logged in ModifiableDataTreeDelegator
final Reverter reverter = attemptRevert
- ? new ReverterImpl(processedNodes, updates, writersOrder, ctx)
- : () -> {}; // NOOP reverter
+ ? new ReverterImpl(processedNodes, updates, writersOrder)
+ : (final WriteContext writeContext) -> {};//NOOP reverter
// Find out which changes left unprocessed
final Set<InstanceIdentifier<?>> unprocessedChanges = updates.values().stream()
@@ -258,30 +260,29 @@ final class FlatWriterRegistry implements WriterRegistry {
private final Collection<InstanceIdentifier<?>> processedNodes;
private final Multimap<InstanceIdentifier<?>, ? extends DataObjectUpdate> updates;
private final Set<InstanceIdentifier<?>> revertDeleteOrder;
- private final WriteContext ctx;
ReverterImpl(final Collection<InstanceIdentifier<?>> processedNodes,
final Multimap<InstanceIdentifier<?>, ? extends DataObjectUpdate> updates,
- final Set<InstanceIdentifier<?>> writersOrderOriginal,
- final WriteContext ctx) {
+ 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;
- this.ctx = ctx;
}
@Override
- public void revert() throws RevertFailedException {
+ 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, ctx, true, revertDeleteOrder);
+ bulkUpdate(updatesToRevert, writeContext, true, revertDeleteOrder);
LOG.info("Revert successful");
} catch (BulkUpdateException e) {
LOG.error("Revert failed", e);
@@ -298,11 +299,12 @@ final class FlatWriterRegistry implements WriterRegistry {
final Collection<InstanceIdentifier<?>> processedNodes) {
final Multimap<InstanceIdentifier<?>, DataObjectUpdate> filtered = HashMultimap.create();
for (InstanceIdentifier<?> processedNode : processedNodes) {
- final InstanceIdentifier<?> wildcardedIid = RWUtils.makeIidWildcarded(processedNode);
+ final InstanceIdentifier<?> wildcardedIid = makeIidWildcarded(processedNode);
if (updates.containsKey(wildcardedIid)) {
updates.get(wildcardedIid).stream()
.filter(dataObjectUpdate -> processedNode.contains(dataObjectUpdate.getId()))
- .forEach(dataObjectUpdate -> filtered.put(processedNode, dataObjectUpdate.reverse()));
+ // putting under unkeyed identifier, to prevent failing of checkAllTypesCanBeHandled
+ .forEach(dataObjectUpdate -> filtered.put(wildcardedIid, dataObjectUpdate.reverse()));
}
}
return filtered;
diff --git a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/DataObjects.java b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/DataObjects.java
index f2f664efe..8dd3d2065 100644
--- a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/DataObjects.java
+++ b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/DataObjects.java
@@ -60,5 +60,17 @@ public class DataObjects {
}
}
- public static class DataObjectKey implements Identifier<DataObjectK> {}
+ public interface DataObject1ChildK extends DataObject, ChildOf<DataObject1>, Identifiable<DataObject1ChildKey> {
+ // needs to be defined like this to have paths totally equal after cutting path for internally keyed id inside infra
+ InstanceIdentifier<DataObject1ChildK> IID =
+ RWUtils.makeIidWildcarded(InstanceIdentifier.create(DataObject1.class).child(DataObject1ChildK.class));
+ InstanceIdentifier<DataObject1ChildK> INTERNALLY_KEYED_IID = InstanceIdentifier.create(DataObject1.class)
+ .child(DataObject1ChildK.class, new DataObject1ChildKey());
+ }
+
+ public static class DataObject1ChildKey implements Identifier<DataObject1ChildK> {
+ }
+
+ public static class DataObjectKey implements Identifier<DataObjectK> {
+ }
}
diff --git a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistryTest.java b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistryTest.java
index 6b75f923e..fdd2eaddb 100644
--- a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistryTest.java
+++ b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistryTest.java
@@ -1,10 +1,10 @@
package io.fd.honeycomb.translate.util.write.registry;
-import static org.hamcrest.CoreMatchers.hasItem;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
@@ -18,10 +18,11 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.Multimap;
import io.fd.honeycomb.translate.util.DataObjects;
+import io.fd.honeycomb.translate.util.DataObjects.DataObject1;
import io.fd.honeycomb.translate.write.DataObjectUpdate;
import io.fd.honeycomb.translate.write.WriteContext;
+import io.fd.honeycomb.translate.write.WriteFailedException;
import io.fd.honeycomb.translate.write.Writer;
-import io.fd.honeycomb.translate.util.DataObjects.DataObject1;
import io.fd.honeycomb.translate.write.registry.WriterRegistry;
import org.hamcrest.CoreMatchers;
import org.junit.Before;
@@ -41,7 +42,11 @@ public class FlatWriterRegistryTest {
@Mock
private Writer<DataObjects.DataObject3> writer3;
@Mock
+ private Writer<DataObjects.DataObject1ChildK> writer4;
+ @Mock
private WriteContext ctx;
+ @Mock
+ private WriteContext revertWriteContext;
@Before
public void setUp() throws Exception {
@@ -215,12 +220,13 @@ public class FlatWriterRegistryTest {
inOrder.verify(writer3)
.update(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), any(WriteContext.class));
- e.revertChanges();
+ e.revertChanges(revertWriteContext);
// Revert changes. Successful updates are iterated in reverse
+ // also binding other write context,to verify if update context is not reused
inOrder.verify(writer2)
- .update(iid2, after2, before2, ctx);
+ .update(iid2, after2, before2, revertWriteContext);
inOrder.verify(writer1)
- .update(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), any(WriteContext.class));
+ .update(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), eq(revertWriteContext));
verifyNoMoreInteractions(writer3);
}
}
@@ -248,7 +254,7 @@ public class FlatWriterRegistryTest {
doThrow(new RuntimeException()).when(writer1)
.update(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class), any(WriteContext.class));
try {
- e.revertChanges();
+ e.revertChanges(revertWriteContext);
} catch (WriterRegistry.Reverter.RevertFailedException e1) {
assertThat(e1.getNotRevertedChanges().size(), is(1));
assertThat(e1.getNotRevertedChanges(), CoreMatchers
@@ -257,6 +263,48 @@ public class FlatWriterRegistryTest {
}
}
+ @Test
+ public void testMutlipleUpdatesWithOneKeyedContainer() throws Exception {
+ final InstanceIdentifier internallyKeyedIdentifier = InstanceIdentifier.create(DataObject1.class)
+ .child(DataObjects.DataObject1ChildK.class, new DataObjects.DataObject1ChildKey());
+
+ final FlatWriterRegistry flatWriterRegistry =
+ new FlatWriterRegistry(
+ ImmutableMap.of(DataObjects.DataObject1.IID, writer1, DataObjects.DataObject1ChildK.IID,writer4));
+
+ // Writer1 always fails
+ doThrow(new RuntimeException()).when(writer1)
+ .update(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class),
+ any(WriteContext.class));
+
+ final Multimap<InstanceIdentifier<?>, DataObjectUpdate> updates = HashMultimap.create();
+ addKeyedUpdate(updates,DataObjects.DataObject1ChildK.class);
+ addUpdate(updates, DataObjects.DataObject1.class);
+ try {
+ flatWriterRegistry.update(new WriterRegistry.DataObjectUpdates(updates, ImmutableMultimap.of()), ctx);
+ fail("Bulk update should have failed on writer1");
+ } catch (WriterRegistry.BulkUpdateException e) {
+ // Writer1 always fails from now
+ doThrow(new RuntimeException()).when(writer1)
+ .update(any(InstanceIdentifier.class), any(DataObject.class), any(DataObject.class),
+ any(WriteContext.class));
+ try {
+ e.revertChanges(revertWriteContext);
+ } catch (WriterRegistry.Reverter.RevertFailedException e1) {
+ assertThat(e1.getNotRevertedChanges().size(), is(1));
+ assertThat(e1.getNotRevertedChanges(), CoreMatchers
+ .hasItem(InstanceIdentifier.create(DataObjects.DataObject1.class)));
+ }
+ }
+ }
+
+ private <D extends DataObject> void addKeyedUpdate(final Multimap<InstanceIdentifier<?>, DataObjectUpdate> updates,
+ final Class<D> type) throws Exception {
+ final InstanceIdentifier<D> iid = (InstanceIdentifier<D>) type.getDeclaredField("IID").get(null);
+ final InstanceIdentifier<D> keyedIid = (InstanceIdentifier<D>) type.getDeclaredField("INTERNALLY_KEYED_IID").get(null);
+ updates.put(iid, DataObjectUpdate.create(keyedIid, mock(type), mock(type)));
+ }
+
private <D extends DataObject> void addUpdate(final Multimap<InstanceIdentifier<?>, DataObjectUpdate> updates,
final Class<D> type) throws Exception {
final InstanceIdentifier<D> iid = (InstanceIdentifier<D>) type.getDeclaredField("IID").get(null);