summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMarek Gradzki <mgradzki@cisco.com>2018-04-13 13:38:16 +0200
committerMarek Gradzki <mgradzki@cisco.com>2018-08-17 10:17:15 +0000
commitfa641a3e06a905cb3222ebd15a2b4ab90b599efc (patch)
tree2043bcea16a4105a7b93aaf057e17e34adf1d607
parent2be001c5014010698ed930236496bb939df89cde (diff)
HONEYCOMB-431: delegate DataModification.validate to WriterRegistry
This patch introduces ModifiableDataTreeDelegator.validateCandidate that translates DataTreeCandidate to DataObjectUpdates and delegates validation to WriterRegistry (similarly as for bulk update). ModifiableDataTreeManager.commit implementation invokes validation before bulk update. To make it efficient, DataObjectUpdates are computed once and stored in DataTreeContext. Change-Id: If4bd558e64ed84c11c9c50c7a98a2aaa8db841bb Signed-off-by: Marek Gradzki <mgradzki@cisco.com>
-rw-r--r--infra/data-api/src/main/java/io/fd/honeycomb/data/DataModification.java2
-rw-r--r--infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegator.java96
-rw-r--r--infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeManager.java38
-rw-r--r--infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/WriteTransaction.java8
-rw-r--r--infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java7
-rw-r--r--infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/WriteTransactionTest.java1
-rw-r--r--infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/DataValidationFailedException.java79
-rw-r--r--infra/translate-api/src/main/java/io/fd/honeycomb/translate/write/registry/WriterRegistry.java11
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 a87983c..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 92c4b39..b9246f3 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 aa5b3e5..acaa6e5 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 8e7dca0..cd47c65 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 ef9d3d6..f40e0e6 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 89ecef2..86521f3 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 0000000..3c0392e
--- /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 e2924f8..aae62e1 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.