diff options
8 files changed, 206 insertions, 36 deletions
diff --git a/infra/data-api/src/main/java/io/fd/honeycomb/data/DataModification.java b/infra/data-api/src/main/java/io/fd/honeycomb/data/DataModification.java index a87983cdb..9fef3b48c 100644 --- a/infra/data-api/src/main/java/io/fd/honeycomb/data/DataModification.java +++ b/infra/data-api/src/main/java/io/fd/honeycomb/data/DataModification.java @@ -55,6 +55,8 @@ public interface DataModification extends ReadableDataManager, AutoCloseable { /** * Alters data tree using this modification. * + * <p>Modification is validated before application. + * * @throws TranslationException if commit failed while updating data tree state */ void commit() throws TranslationException; 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 92c4b393c..b9246f3c4 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 @@ -18,6 +18,8 @@ package io.fd.honeycomb.data.impl; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.util.concurrent.Futures.immediateCheckedFuture; +import static io.fd.honeycomb.data.impl.ModifiableDataTreeDelegator.DataTreeWriteContextFactory.DataTreeWriteContext; +import static io.fd.honeycomb.data.impl.ModifiableDataTreeManager.DataTreeContextFactory.DataTreeContext; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; @@ -28,6 +30,7 @@ import io.fd.honeycomb.data.DataModification; import io.fd.honeycomb.data.ReadableDataManager; import io.fd.honeycomb.translate.MappingContext; import io.fd.honeycomb.translate.TranslationException; +import io.fd.honeycomb.translate.ValidationFailedException; import io.fd.honeycomb.translate.util.RWUtils; import io.fd.honeycomb.translate.util.TransactionMappingContext; import io.fd.honeycomb.translate.util.write.TransactionWriteContext; @@ -35,6 +38,7 @@ 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 io.fd.honeycomb.translate.write.registry.WriterRegistry.DataObjectUpdates; import java.util.List; import java.util.Map; import java.util.Set; @@ -83,7 +87,7 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager @Nonnull final SchemaContext schema, @Nonnull final WriterRegistry writerRegistry, @Nonnull final DataBroker contextBroker) { - super(dataTree); + super(dataTree, new DataTreeWriteContextFactory()); this.contextBroker = checkNotNull(contextBroker, "contextBroker should not be null"); this.serializer = checkNotNull(serializer, "serializer should not be null"); this.writerRegistry = checkNotNull(writerRegistry, "writerRegistry should not be null"); @@ -109,28 +113,50 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager this.untouchedModification = untouchedModification; } - /** - * Pass the changes to underlying writer layer. - * Transform from BI to BA. - * Revert(Write data before to subtrees that have been successfully modified before failure) in case of failure. - */ @Override - protected void processCandidate(final DataTreeCandidate candidate) - throws TranslationException { + protected void validateCandidate(final DataTreeContext dataTreeContext) throws ValidationFailedException { + final DataObjectUpdates baUpdates = + getUpdates((DataTreeWriteContextFactory.DataTreeWriteContext) dataTreeContext); + LOG.debug("ModifiableDataTreeDelegator.validateCandidate() extracted updates={}", baUpdates); - final DataTreeCandidateNode rootNode = candidate.getRootNode(); - final YangInstanceIdentifier rootPath = candidate.getRootPath(); - LOG.trace("ConfigDataTree.modify() rootPath={}, rootNode={}, dataBefore={}, dataAfter={}", - rootPath, rootNode, rootNode.getDataBefore(), rootNode.getDataAfter()); + try (final WriteContext ctx = getTransactionWriteContext()) { + writerRegistry.validateModifications(baUpdates, ctx); + } + } - final ModificationDiff modificationDiff = new ModificationDiff.ModificationDiffBuilder() + private DataObjectUpdates getUpdates(final DataTreeWriteContext dataTreeContext) { + DataObjectUpdates updates = dataTreeContext.getUpdates(); + if (updates != null) { + return updates; + } else { + final DataTreeCandidate candidate = dataTreeContext.getCandidate(); + final DataTreeCandidateNode rootNode = candidate.getRootNode(); + final YangInstanceIdentifier rootPath = candidate.getRootPath(); + LOG.trace("ConfigDataTree.getUpdates() rootPath={}, rootNode={}, dataBefore={}, dataAfter={}", + rootPath, rootNode, rootNode.getDataBefore(), rootNode.getDataAfter()); + + final ModificationDiff modificationDiff = new ModificationDiff.ModificationDiffBuilder() .setCtx(schema).build(rootNode); - LOG.debug("ConfigDataTree.modify() diff: {}", modificationDiff); + LOG.debug("ConfigDataTree.getUpdates() diff: {}", modificationDiff); - // Distinguish between updates (create + update) and deletes - final WriterRegistry.DataObjectUpdates baUpdates = - toBindingAware(writerRegistry, modificationDiff.getUpdates()); - LOG.debug("ConfigDataTree.modify() extracted updates={}", baUpdates); + // Distinguish between updates (create + update) and deletes + updates = toBindingAware(writerRegistry, modificationDiff.getUpdates()); + dataTreeContext.setUpdates(updates); + return updates; + } + } + + /** + * Pass the changes to underlying writer layer. Transform from BI to BA. Revert(Write data before to subtrees + * that have been successfully modified before failure) in case of failure. + */ + @Override + protected void processCandidate(final DataTreeContext dataTreeContext) throws TranslationException { + final DataTreeWriteContextFactory.DataTreeWriteContext dataTreeWriteContext = + (DataTreeWriteContextFactory.DataTreeWriteContext) dataTreeContext; + + final DataObjectUpdates baUpdates = dataTreeWriteContext.getUpdates(); + LOG.debug("ModifiableDataTreeDelegator.processCandidate() extracted updates={}", baUpdates); final WriteContext ctx = getTransactionWriteContext(); final MappingContext mappingContext = ctx.getMappingContext(); @@ -297,6 +323,40 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager return dataObject; } + static final class DataTreeWriteContextFactory implements ModifiableDataTreeManager.DataTreeContextFactory { + @Nonnull + @Override + public DataTreeWriteContext create(@Nonnull final DataTreeCandidate candidate) { + return new DataTreeWriteContext(candidate); + } + + /** + * Implementation of {@link DataTreeContext} that, in addition to {@link DataTreeCandidate}, stores also + * {@DataObjectUpdates}. The {@link DataObjectUpdates} are shared by the validation and translation process. + */ + static final class DataTreeWriteContext implements DataTreeContext { + private final DataTreeCandidate candidate; + private DataObjectUpdates updates; + + DataTreeWriteContext(@Nonnull final DataTreeCandidate candidate) { + this.candidate = candidate; + } + + @Nonnull + @Override + public DataTreeCandidate getCandidate() { + return candidate; + } + + public void setUpdates(@Nonnull final DataObjectUpdates updates) { + this.updates = updates; + } + + DataObjectUpdates getUpdates() { + return updates; + } + } + } } diff --git a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeManager.java b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeManager.java index aa5b3e5cf..acaa6e5ee 100644 --- a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeManager.java +++ b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeManager.java @@ -19,6 +19,7 @@ package io.fd.honeycomb.data.impl; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static com.google.common.util.concurrent.Futures.immediateCheckedFuture; +import static io.fd.honeycomb.data.impl.ModifiableDataTreeManager.DataTreeContextFactory.DataTreeContext; import com.google.common.base.Optional; import com.google.common.util.concurrent.CheckedFuture; @@ -50,9 +51,16 @@ public class ModifiableDataTreeManager implements ModifiableDataManager { private static final Logger LOG = LoggerFactory.getLogger(ModifiableDataTreeManager.class); private final DataTree dataTree; + private final DataTreeContextFactory contextFactory; public ModifiableDataTreeManager(@Nonnull final DataTree dataTree) { + this(dataTree, candidate -> (() -> candidate)); + } + + public ModifiableDataTreeManager(@Nonnull final DataTree dataTree, + @Nonnull final DataTreeContextFactory contextFactory) { this.dataTree = checkNotNull(dataTree, "dataTree should not be null"); + this.contextFactory = contextFactory; } @Override @@ -102,13 +110,13 @@ public class ModifiableDataTreeManager implements ModifiableDataManager { @Override public final void commit() throws TranslationException { - final DataTreeCandidate candidate = prepareCandidate(modification); - validateCandidate(candidate); - processCandidate(candidate); - dataTree.commit(candidate); + final DataTreeContext candidateContext = prepareCandidateContext(modification); + validateCandidate(candidateContext); + processCandidate(candidateContext); + dataTree.commit(candidateContext.getCandidate()); } - private DataTreeCandidate prepareCandidate(final DataTreeModification dataTreeModification) + private DataTreeContext prepareCandidateContext(final DataTreeModification dataTreeModification) throws ValidationFailedException { // Seal the modification (required to perform validate) dataTreeModification.ready(); @@ -120,14 +128,14 @@ public class ModifiableDataTreeManager implements ModifiableDataManager { throw new ValidationFailedException(e); } - return dataTree.prepare(dataTreeModification); + return contextFactory.create(dataTree.prepare(dataTreeModification)); } - protected void validateCandidate(final DataTreeCandidate candidate) throws ValidationFailedException { + protected void validateCandidate(final DataTreeContext dataTreeContext) throws ValidationFailedException { // NOOP } - protected void processCandidate(final DataTreeCandidate candidate) throws TranslationException { + protected void processCandidate(final DataTreeContext dataTreeContext) throws TranslationException { // NOOP } @@ -141,7 +149,7 @@ public class ModifiableDataTreeManager implements ModifiableDataManager { checkState(cursor != null, "DataTreeModificationCursor for root path should not be null"); modification.applyToCursor(cursor); // Then validate it. - validateCandidate(prepareCandidate(modificationCopy)); + validateCandidate(prepareCandidateContext(modificationCopy)); } @Override @@ -155,4 +163,16 @@ public class ModifiableDataTreeManager implements ModifiableDataManager { ) + '}'; } } + + interface DataTreeContextFactory { + DataTreeContext create(final DataTreeCandidate candidate); + + /** + * Stores DataTreeCandidate. Implementation may also store additional data to optimize validation + * and translation process. + */ + interface DataTreeContext { + DataTreeCandidate getCandidate(); + } + } } diff --git a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/WriteTransaction.java b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/WriteTransaction.java index 8e7dca07b..cd47c65db 100644 --- a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/WriteTransaction.java +++ b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/WriteTransaction.java @@ -125,14 +125,6 @@ final class WriteTransaction implements DOMDataWriteTransaction { try { status = SUBMITED; - // Validate first to catch any issues before attempting commit - if (configModification != null) { - configModification.validate(); - } - if (operationalModification != null) { - operationalModification.validate(); - } - if(configModification != null) { configModification.commit(); } 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 ef9d3d6d2..f40e0e673 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 @@ -24,6 +24,7 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -75,6 +76,12 @@ public class ModifiableDataTreeDelegatorTest extends ModifiableDataTreeDelegator dataModification.write(NESTED_LIST_ID, nestedList); dataModification.validate(); dataModification.validate(); + + final Multimap<InstanceIdentifier<?>, DataObjectUpdate> map = HashMultimap.create(); + map.put(DEFAULT_ID, DataObjectUpdate.create(DEFAULT_ID, null, DEFAULT_DATA_OBJECT)); + final WriterRegistry.DataObjectUpdates updates = + new WriterRegistry.DataObjectUpdates(map, ImmutableMultimap.of()); + verify(writer, times(2)).validateModifications(eq(updates), any(WriteContext.class)); } @Test diff --git a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/WriteTransactionTest.java b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/WriteTransactionTest.java index 89ecef2f3..86521f39b 100644 --- a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/WriteTransactionTest.java +++ b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/WriteTransactionTest.java @@ -98,7 +98,6 @@ public class WriteTransactionTest { @Test public void testSubmit() throws Exception { writeTx.submit(); - verify(configSnapshot).validate(); verify(configSnapshot).commit(); } diff --git a/infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/DataValidationFailedException.java b/infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/DataValidationFailedException.java new file mode 100644 index 000000000..3c0392efa --- /dev/null +++ b/infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/DataValidationFailedException.java @@ -0,0 +1,79 @@ +/* + * Copyright (c) 2018 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; + +import static com.google.common.base.Preconditions.checkNotNull; + +import io.fd.honeycomb.translate.ValidationFailedException; +import javax.annotation.Nonnull; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; + +/** + * Thrown when a writer fails to validate data update. + */ +public class DataValidationFailedException extends ValidationFailedException { + private static final long serialVersionUID = 1; + + private final InstanceIdentifier<?> failedId; + + /** + * Constructs an ValidationFailedException given data id, exception detail message and exception cause. + * + * @param failedId instance identifier of the data object that could not be validated + * @param cause the cause of validation failure + * @param message the exception detail message + */ + public DataValidationFailedException(@Nonnull final InstanceIdentifier<?> failedId, + @Nonnull final String message, + @Nonnull final Throwable cause) { + super(message, cause); + this.failedId = checkNotNull(failedId, "failedId should not be null"); + } + + /** + * Constructs an ValidationFailedException given data id. + * + * @param failedId instance identifier of the data object that could not be validated + */ + public DataValidationFailedException(@Nonnull final InstanceIdentifier<?> failedId, + @Nonnull final String message) { + super(message); + this.failedId = checkNotNull(failedId, "failedId should not be null"); + } + + /** + * Constructs an ValidationFailedException given data id and exception cause. + * + * @param failedId instance identifier of the data object that could not be validated + * @param cause the cause of validated failure + */ + public DataValidationFailedException(@Nonnull final InstanceIdentifier<?> failedId, + @Nonnull final Throwable cause) { + super(cause); + this.failedId = checkNotNull(failedId, "failedId should not be null"); + } + + /** + * Returns id of the data object that could not be validated. + * + * @return data object instance identifier + */ + @Nonnull + public InstanceIdentifier<?> getFailedId() { + return failedId; + } +} 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 e2924f84a..aae62e1fe 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 @@ -21,6 +21,7 @@ import com.google.common.collect.Multimap; import com.google.common.collect.Sets; import io.fd.honeycomb.translate.TranslationException; 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 java.util.Set; @@ -34,6 +35,16 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; */ @Beta public interface WriterRegistry { + /** + * Validates provided DataObject updates. + * + * @param updates Updates to be validated + * @param ctx Write context that provides information about current state of DataTree. + * @throws DataValidationFailedException if validation failed. + */ + default void validateModifications(@Nonnull DataObjectUpdates updates, @Nonnull WriteContext ctx) throws + DataValidationFailedException { + } /** * Performs bulk update. |