From d1b102f6fafced3c7badb09ecc78fec590704c8a Mon Sep 17 00:00:00 2001 From: Marek Gradzki Date: Fri, 13 Apr 2018 13:38:16 +0200 Subject: HONEYCOMB-431: add validation support to Writers This patch introduces FlatWriterRegistry.validateModifications. Implementation iterates over writersOrder following bulkUpdate logic to properly support subtree writers case. Writers are now cabable of validating modifications. Commonly used implementations (GenericWriter and GenericListWriter) delegate validation capbility to Validators. Change-Id: If7a0bb0838c0b8f2c0393c989f3b03853a2ea679 Signed-off-by: Marek Gradzki --- .../translate/impl/write/GenericListWriter.java | 21 +++++-- .../translate/impl/write/GenericWriter.java | 9 ++- .../impl/write/registry/FlatWriterRegistry.java | 69 +++++++++++++++++----- .../impl/write/registry/SubtreeWriter.java | 8 +++ .../impl/write/GenericListWriterTest.java | 29 ++++++++- .../translate/impl/write/GenericWriterTest.java | 21 ++++++- .../write/registry/FlatWriterRegistryTest.java | 65 ++++++++++++++++++++ 7 files changed, 196 insertions(+), 26 deletions(-) (limited to 'infra/translate-impl') diff --git a/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/GenericListWriter.java b/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/GenericListWriter.java index 92467a8e8..5121eb793 100644 --- a/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/GenericListWriter.java +++ b/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/GenericListWriter.java @@ -19,12 +19,13 @@ package io.fd.honeycomb.translate.impl.write; import static io.fd.honeycomb.translate.impl.write.GenericWriter.isUpdateSupported; import io.fd.honeycomb.translate.spi.write.ListWriterCustomizer; +import io.fd.honeycomb.translate.spi.write.WriterCustomizer; import io.fd.honeycomb.translate.util.RWUtils; import io.fd.honeycomb.translate.util.write.AbstractGenericWriter; import io.fd.honeycomb.translate.write.ListWriter; +import io.fd.honeycomb.translate.write.Validator; import io.fd.honeycomb.translate.write.WriteContext; import io.fd.honeycomb.translate.write.WriteFailedException; -import io.fd.honeycomb.translate.spi.write.WriterCustomizer; import javax.annotation.Nonnull; import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.Identifiable; @@ -45,6 +46,13 @@ public final class GenericListWriter, K e this.customizer = customizer; } + public GenericListWriter(@Nonnull final InstanceIdentifier type, + @Nonnull final ListWriterCustomizer customizer, + @Nonnull final Validator validator) { + super(type, isUpdateSupported(customizer), validator); + this.customizer = customizer; + } + @Override protected void writeCurrentAttributes(@Nonnull final InstanceIdentifier id, @Nonnull final D data, @Nonnull final WriteContext ctx) throws WriteFailedException { @@ -79,22 +87,25 @@ public final class GenericListWriter, K e @Override protected void writeCurrent(final InstanceIdentifier id, final D data, final WriteContext ctx) throws WriteFailedException { - super.writeCurrent(getId(id, data), data, ctx); + super.writeCurrent(getManagedId(id, data), data, ctx); } @Override protected void updateCurrent(final InstanceIdentifier id, final D dataBefore, final D dataAfter, final WriteContext ctx) throws WriteFailedException { - super.updateCurrent(getId(id, dataBefore), dataBefore, dataAfter, ctx); + super.updateCurrent(getManagedId(id, dataBefore), dataBefore, dataAfter, ctx); } @Override protected void deleteCurrent(final InstanceIdentifier id, final D dataBefore, final WriteContext ctx) throws WriteFailedException { - super.deleteCurrent(getId(id, dataBefore), dataBefore, ctx); + super.deleteCurrent(getManagedId(id, dataBefore), dataBefore, ctx); } - private InstanceIdentifier getId(final InstanceIdentifier id, final D current) { + @Override + protected InstanceIdentifier getManagedId(@Nonnull final InstanceIdentifier currentId, + @Nonnull final D current) { + final InstanceIdentifier id = (InstanceIdentifier) currentId; // Make sure the key is present if (isWildcarded(id)) { return getSpecificId(id, current); diff --git a/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/GenericWriter.java b/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/GenericWriter.java index 086936e38..bfdf072c0 100644 --- a/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/GenericWriter.java +++ b/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/GenericWriter.java @@ -16,9 +16,10 @@ package io.fd.honeycomb.translate.impl.write; -import io.fd.honeycomb.translate.write.WriteContext; import io.fd.honeycomb.translate.spi.write.WriterCustomizer; import io.fd.honeycomb.translate.util.write.AbstractGenericWriter; +import io.fd.honeycomb.translate.write.Validator; +import io.fd.honeycomb.translate.write.WriteContext; import io.fd.honeycomb.translate.write.WriteFailedException; import javax.annotation.Nonnull; import org.opendaylight.yangtools.yang.binding.DataObject; @@ -39,7 +40,13 @@ public final class GenericWriter extends AbstractGenericWr @Nonnull final WriterCustomizer customizer) { super(type, isUpdateSupported(customizer)); this.customizer = customizer; + } + public GenericWriter(@Nonnull final InstanceIdentifier type, + @Nonnull final WriterCustomizer customizer, + @Nonnull final Validator validator) { + super(type, isUpdateSupported(customizer), validator); + this.customizer = customizer; } static boolean isUpdateSupported(final @Nonnull WriterCustomizer customizer) { 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 67a5da32a..9e36e593f 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 @@ -28,12 +28,14 @@ import com.google.common.collect.Sets; 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.DataValidationFailedException; 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.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -75,6 +77,33 @@ final class FlatWriterRegistry implements WriterRegistry { this.writers = writersById.entrySet().stream().map(Map.Entry::getValue).collect(Collectors.toSet()); } + @Override + public void validateModifications(@Nonnull final DataObjectUpdates updates, @Nonnull final WriteContext ctx) + throws DataValidationFailedException { + // Optimization: validation order is not relevant, so do not merge deletes and updates. + validateModifications(updates.getDeletes(), ctx); + validateModifications(updates.getUpdates(), ctx); + } + + private void validateModifications( + @Nonnull final Multimap, ? extends DataObjectUpdate> updates, + @Nonnull final WriteContext ctx) throws DataValidationFailedException { + if (updates.isEmpty()) { + return; + } + // Fail early if some handlers are missing. + checkAllTypesCanBeHandled(updates); + + // Validators do not modify anything, so order of validators is not important. + // We iterate over writersOrder instead of modifications for consistent handling of subtree writers case. + for (InstanceIdentifier writerType : writersOrder) { + final Writer writer = getWriter(writerType); + for (DataObjectUpdate singleUpdate : getWritersData(updates, writer, writerType, ctx)) { + writer.validate(singleUpdate.getId(), singleUpdate.getDataBefore(), singleUpdate.getDataAfter(), ctx); + } + } + } + @Override public void processModifications(@Nonnull final DataObjectUpdates updates, @Nonnull final WriteContext ctx) throws TranslationException { @@ -207,26 +236,11 @@ final class FlatWriterRegistry implements WriterRegistry { LOG.debug("Performing bulk update for: {}", updates.keySet()); // Iterate over all writers and call update if there are any related updates for (InstanceIdentifier writerType : writersOrder) { - Collection writersData = updates.get(writerType); final Writer writer = getWriter(writerType); - - if (writersData.isEmpty()) { - // If there are no data for current writer, but it is a SubtreeWriter and there are updates to - // its children, still invoke it with its root data - if (writer instanceof SubtreeWriter && isAffected((SubtreeWriter) writer, updates)) { - // Provide parent data for SubtreeWriter for further processing - writersData = getParentDataObjectUpdate(ctx, updates, writer); - } else { - // Skipping unaffected writer - // Alternative to this would be modification sort according to the order of writers - continue; - } - } - LOG.debug("Performing update for: {}", writerType); LOG.trace("Performing update with writer: {}", writer); - for (DataObjectUpdate singleUpdate : writersData) { + for (DataObjectUpdate singleUpdate : getWritersData(updates, writer, writerType, ctx)) { try { writer.processModification(singleUpdate.getId(), singleUpdate.getDataBefore(), singleUpdate.getDataAfter(), ctx); @@ -240,6 +254,29 @@ final class FlatWriterRegistry implements WriterRegistry { } } + private Collection getWritersData( + final Multimap, ? extends DataObjectUpdate> updates, final Writer writer, + final InstanceIdentifier writerType, final WriteContext ctx) { + Collection writersData = updates.get(writerType); + + if (writersData.isEmpty()) { + // If there are no data for current writer, but it is a SubtreeWriter and there are updates to + // its children, still invoke it with its root data. + // Notice that child updates will be ignored if the set of all modifications + // contain both parent update and child updates. But this is ok, since all changes are already expressed in + // the parent update. + if (writer instanceof SubtreeWriter && isAffected((SubtreeWriter) writer, updates)) { + // Provide parent data for SubtreeWriter for further processing + writersData = getParentDataObjectUpdate(ctx, updates, writer); + } else { + // Skipping unaffected writer + // Alternative to this would be modification sort according to the order of writers + return Collections.emptyList(); + } + } + return writersData; + } + private void checkAllTypesCanBeHandled( @Nonnull final Multimap, ? extends DataObjectUpdate> updates) { diff --git a/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/registry/SubtreeWriter.java b/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/registry/SubtreeWriter.java index a1a5f3fd0..e510bc33c 100644 --- a/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/registry/SubtreeWriter.java +++ b/infra/translate-impl/src/main/java/io/fd/honeycomb/translate/impl/write/registry/SubtreeWriter.java @@ -19,6 +19,7 @@ package io.fd.honeycomb.translate.impl.write.registry; import static com.google.common.base.Preconditions.checkArgument; import com.google.common.collect.Iterables; +import io.fd.honeycomb.translate.write.DataValidationFailedException; import io.fd.honeycomb.translate.write.WriteContext; import io.fd.honeycomb.translate.write.WriteFailedException; import io.fd.honeycomb.translate.write.Writer; @@ -67,6 +68,13 @@ final class SubtreeWriter implements Writer { return handledChildTypes; } + @Override + public void validate(@Nonnull final InstanceIdentifier id, + @Nullable final DataObject dataBefore, @Nullable final DataObject dataAfter, + @Nonnull final WriteContext ctx) throws DataValidationFailedException { + delegate.validate(id, dataBefore, dataAfter, ctx); + } + @Override public void processModification( @Nonnull final InstanceIdentifier id, diff --git a/infra/translate-impl/src/test/java/io/fd/honeycomb/translate/impl/write/GenericListWriterTest.java b/infra/translate-impl/src/test/java/io/fd/honeycomb/translate/impl/write/GenericListWriterTest.java index 91785b25e..20cabc5b4 100644 --- a/infra/translate-impl/src/test/java/io/fd/honeycomb/translate/impl/write/GenericListWriterTest.java +++ b/infra/translate-impl/src/test/java/io/fd/honeycomb/translate/impl/write/GenericListWriterTest.java @@ -22,6 +22,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import io.fd.honeycomb.translate.spi.write.ListWriterCustomizer; +import io.fd.honeycomb.translate.write.Validator; import io.fd.honeycomb.translate.write.WriteContext; import io.fd.honeycomb.translate.write.WriteFailedException; import java.util.Collections; @@ -42,7 +43,6 @@ public class GenericListWriterTest { private ListWriterCustomizer customizer; @Mock private WriteContext ctx; - private GenericListWriter writer; @Mock private IdentifiableDataObject before; @Mock @@ -51,11 +51,15 @@ public class GenericListWriterTest { private IdentifiableDataObject after; @Mock private DataObjectIdentifier keyAfter; + @Mock + private Validator validator; + + private GenericListWriter writer; @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); - writer = new GenericListWriter<>(DATA_OBJECT_ID, customizer); + writer = new GenericListWriter<>(DATA_OBJECT_ID, customizer, validator); when(before.getKey()).thenReturn(beforeKey); when(after.getKey()).thenReturn(keyAfter); } @@ -106,4 +110,25 @@ public class GenericListWriterTest { writer = new GenericListWriter<>(DATA_OBJECT_ID, customizer); writer.deleteCurrentAttributes(DATA_OBJECT_ID, before, ctx); } + + @Test + public void testValidate() throws Exception { + assertEquals(DATA_OBJECT_ID, writer.getManagedDataObjectType()); + + final InstanceIdentifier keyedIdBefore = + (InstanceIdentifier) InstanceIdentifier.create(Collections + .singleton(new InstanceIdentifier.IdentifiableItem<>(IdentifiableDataObject.class, beforeKey))); + final InstanceIdentifier keyedIdAfter = + (InstanceIdentifier) InstanceIdentifier.create(Collections + .singleton(new InstanceIdentifier.IdentifiableItem<>(IdentifiableDataObject.class, keyAfter))); + + writer.validate(DATA_OBJECT_ID, before, after, ctx); + verify(validator).validateUpdate(keyedIdBefore, before, after, ctx); + + writer.validate(DATA_OBJECT_ID, before, null, ctx); + verify(validator).validateDelete(keyedIdBefore, before, ctx); + + writer.validate(DATA_OBJECT_ID, null, after, ctx); + verify(validator).validateWrite(keyedIdAfter, after, ctx); + } } \ No newline at end of file diff --git a/infra/translate-impl/src/test/java/io/fd/honeycomb/translate/impl/write/GenericWriterTest.java b/infra/translate-impl/src/test/java/io/fd/honeycomb/translate/impl/write/GenericWriterTest.java index c9d381ad4..1c0927b89 100644 --- a/infra/translate-impl/src/test/java/io/fd/honeycomb/translate/impl/write/GenericWriterTest.java +++ b/infra/translate-impl/src/test/java/io/fd/honeycomb/translate/impl/write/GenericWriterTest.java @@ -23,6 +23,7 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.verify; import io.fd.honeycomb.translate.spi.write.WriterCustomizer; +import io.fd.honeycomb.translate.write.Validator; import io.fd.honeycomb.translate.write.WriteContext; import io.fd.honeycomb.translate.write.WriteFailedException; import org.junit.Before; @@ -40,16 +41,19 @@ public class GenericWriterTest { private WriterCustomizer customizer; @Mock private WriteContext ctx; - private GenericWriter writer; @Mock private DataObject before; @Mock private DataObject after; + @Mock + private Validator validator; + + private GenericWriter writer; @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); - writer = new GenericWriter<>(DATA_OBJECT_ID, customizer); + writer = new GenericWriter<>(DATA_OBJECT_ID, customizer, validator); } @Test @@ -94,4 +98,17 @@ public class GenericWriterTest { assertTrue(GenericWriter.isUpdateSupported(new NoopWriters.DirectUpdateWriterCustomizer())); assertTrue(GenericWriter.isUpdateSupported(new NoopWriters.ParentImplDirectUpdateWriterCustomizer())); } + + @Test + public void testValidate() throws Exception { + assertEquals(DATA_OBJECT_ID, writer.getManagedDataObjectType()); + writer.validate(DATA_OBJECT_ID, before, after, ctx); + verify(validator).validateUpdate(DATA_OBJECT_ID, before, after, ctx); + + writer.validate(DATA_OBJECT_ID, before, null, ctx); + verify(validator).validateDelete(DATA_OBJECT_ID, before, ctx); + + writer.validate(DATA_OBJECT_ID, null, after, ctx); + verify(validator).validateWrite(DATA_OBJECT_ID, after, ctx); + } } \ No newline at end of file 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 7f4b93e01..e06197fae 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 @@ -365,6 +365,71 @@ public class FlatWriterRegistryTest { } } + @Test(expected = IllegalArgumentException.class) + public void testValidateMissingWriter() throws Exception { + final FlatWriterRegistry flatWriterRegistry = + new FlatWriterRegistry(ImmutableMap.of(DataObject1.IID, writer1)); + + final Multimap, DataObjectUpdate> updates = HashMultimap.create(); + addUpdate(updates, DataObject1.class); + addUpdate(updates, DataObject2.class); + flatWriterRegistry.validateModifications(new WriterRegistry.DataObjectUpdates(updates, ImmutableMultimap.of()), ctx); + } + + @Test + public void testValidateSingleWriter() throws Exception { + final FlatWriterRegistry flatWriterRegistry = + new FlatWriterRegistry(ImmutableMap.of(DataObject1.IID, writer1, DataObject2.IID, writer2)); + + final Multimap, DataObjectUpdate> updates = HashMultimap.create(); + final InstanceIdentifier iid = InstanceIdentifier.create(DataObject1.class); + final InstanceIdentifier iid2 = InstanceIdentifier.create(DataObject1.class); + final DataObject1 dataObject = mock(DataObject1.class); + updates.put(DataObject1.IID, DataObjectUpdate.create(iid, dataObject, dataObject)); + updates.put(DataObject1.IID, DataObjectUpdate.create(iid2, dataObject, dataObject)); + flatWriterRegistry + .validateModifications(new WriterRegistry.DataObjectUpdates(updates, ImmutableMultimap.of()), ctx); + + verify(writer1).validate(iid, dataObject, dataObject, ctx); + verify(writer1).validate(iid2, dataObject, dataObject, ctx); + // Invoked when registry is being created + verifyNoMoreInteractions(writer1); + verifyZeroInteractions(writer2); + } + + @Test + public void testValidateMultipleWriters() throws Exception { + final FlatWriterRegistry flatWriterRegistry = + new FlatWriterRegistry(ImmutableMap.of(DataObject1.IID, writer1, DataObject2.IID, writer2)); + + final Multimap, DataObjectUpdate.DataObjectDelete> deletes = HashMultimap.create(); + final Multimap, DataObjectUpdate> updates = HashMultimap.create(); + final InstanceIdentifier iid = InstanceIdentifier.create(DataObject1.class); + final DataObject1 dataObject = mock(DataObject1.class); + // Writer 1 delete + deletes.put(DataObject1.IID, + ((DataObjectUpdate.DataObjectDelete) DataObjectUpdate.create(iid, dataObject, null))); + // Writer 1 create + updates.put(DataObject1.IID, DataObjectUpdate.create(iid, null, dataObject)); + final InstanceIdentifier iid2 = InstanceIdentifier.create(DataObject2.class); + final DataObject2 dataObject2 = mock(DataObject2.class); + // Writer 2 delete + deletes.put(DataObject2.IID, + ((DataObjectUpdate.DataObjectDelete) DataObjectUpdate.create(iid2, dataObject2, null))); + // Writer 2 update + updates.put(DataObject2.IID, DataObjectUpdate.create(iid2, dataObject2, dataObject2)); + flatWriterRegistry.validateModifications(new WriterRegistry.DataObjectUpdates(updates, deletes), ctx); + + // Ignore order + verify(writer1).validate(iid, dataObject, null, ctx); + verify(writer1).validate(iid, null, dataObject, ctx); + verify(writer2).validate(iid2, dataObject2, null, ctx); + verify(writer2).validate(iid2, dataObject2, dataObject2, ctx); + + verifyNoMoreInteractions(writer1); + verifyNoMoreInteractions(writer2); + } + private void addKeyedUpdate(final Multimap, DataObjectUpdate> updates, final Class type) throws Exception { final InstanceIdentifier iid = (InstanceIdentifier) type.getDeclaredField("IID").get(null); -- cgit 1.2.3-korg