From 2be001c5014010698ed930236496bb939df89cde Mon Sep 17 00:00:00 2001 From: Marek Gradzki Date: Fri, 13 Apr 2018 13:38:16 +0200 Subject: HONEYCOMB-431: make DataModification.validate idempotent This patch modifies contract of DataModification.validate to make it idempotent. ModifiableDataTreeManager.validate now invokes dataTree.validate on a copy of DataTreeModification. ModifiableDataTreeManager.validateCandidate was introduced to allow additional validation. Change-Id: I86fc101faff9b04afde2f3eb16fff4d4df2867ad Signed-off-by: Marek Gradzki --- .../io/fd/honeycomb/data/DataModification.java | 20 ++++--- .../data/impl/ModifiableDataTreeManager.java | 69 +++++++++++++++------- .../data/impl/ModifiableDataTreeDelegatorTest.java | 10 ++++ .../honeycomb/data/impl/WriteTransactionTest.java | 4 +- .../translate/ValidationFailedException.java | 54 +++++++++++++++++ 5 files changed, 125 insertions(+), 32 deletions(-) create mode 100644 infra/translate-api/src/main/java/io/fd/honeycomb/translate/ValidationFailedException.java 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 22fba0f53..a87983cdb 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 @@ -18,9 +18,9 @@ package io.fd.honeycomb.data; import com.google.common.annotations.Beta; import io.fd.honeycomb.translate.TranslationException; +import io.fd.honeycomb.translate.ValidationFailedException; 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.DataValidationFailedException; /** * Modification of a {@link ModifiableDataManager}. @@ -55,18 +55,21 @@ public interface DataModification extends ReadableDataManager, AutoCloseable { /** * Alters data tree using this modification. * - * @throws DataValidationFailedException if modification data is not valid - * @throws TranslationException if failed while updating data tree state + * @throws TranslationException if commit failed while updating data tree state */ - void commit() throws DataValidationFailedException, TranslationException; + void commit() throws TranslationException; /** - * Validate and prepare modification before commit. Besides commit, no further operation is expected after validate - * and the behaviour is undefined. + * Validates state of the {@link DataModification}. * - * @throws DataValidationFailedException if modification data is not valid + *

The operation does not have any side-effects on the modification state. + * + *

It can be executed many times, providing the same results + * if the state of the modification has not been changed. + * + * @throws ValidationFailedException if modification data is not valid */ - void validate() throws DataValidationFailedException; + void validate() throws ValidationFailedException; /** * Perform cleanup if necessary. @@ -75,4 +78,5 @@ public interface DataModification extends ReadableDataManager, AutoCloseable { default void close() { // by default, no cleanup is required } + } 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 f16172fa5..aa5b3e5cf 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 @@ -17,12 +17,14 @@ 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 com.google.common.base.Optional; import com.google.common.util.concurrent.CheckedFuture; import io.fd.honeycomb.data.DataModification; import io.fd.honeycomb.data.ModifiableDataManager; +import io.fd.honeycomb.translate.ValidationFailedException; import io.fd.honeycomb.translate.TranslationException; import javax.annotation.Nonnull; import org.apache.commons.lang3.builder.RecursiveToStringStyle; @@ -30,9 +32,12 @@ import org.apache.commons.lang3.builder.ReflectionToStringBuilder; import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException; 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.CursorAwareDataTreeModification; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModificationCursor; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeSnapshot; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -61,11 +66,12 @@ public class ModifiableDataTreeManager implements ModifiableDataManager { } protected class ConfigSnapshot implements DataModification { + private final DataTreeSnapshot snapshot; private final DataTreeModification modification; - private boolean validated = false; ConfigSnapshot() { - this.modification = dataTree.takeSnapshot().newModification(); + this.snapshot = dataTree.takeSnapshot(); + this.modification = snapshot.newModification(); } @Override @@ -95,39 +101,58 @@ public class ModifiableDataTreeManager implements ModifiableDataManager { } @Override - public final void commit() throws DataValidationFailedException, TranslationException { - if(!validated) { - validate(); - } - final DataTreeCandidate candidate = dataTree.prepare(modification); + public final void commit() throws TranslationException { + final DataTreeCandidate candidate = prepareCandidate(modification); + validateCandidate(candidate); processCandidate(candidate); dataTree.commit(candidate); } + private DataTreeCandidate prepareCandidate(final DataTreeModification dataTreeModification) + throws ValidationFailedException { + // Seal the modification (required to perform validate) + dataTreeModification.ready(); + + // Check if modification can be applied to data tree + try { + dataTree.validate(dataTreeModification); + } catch (DataValidationFailedException e) { + throw new ValidationFailedException(e); + } + + return dataTree.prepare(dataTreeModification); + } + + protected void validateCandidate(final DataTreeCandidate candidate) throws ValidationFailedException { + // NOOP + } + protected void processCandidate(final DataTreeCandidate candidate) throws TranslationException { // NOOP } @Override - public final void validate() throws DataValidationFailedException { - modification.ready(); - dataTree.validate(modification); - validated = true; + public final void validate() throws ValidationFailedException { + // Modification requires to be sealed before validation. + // Sealed modification cannot be altered, so create copy. + final CursorAwareDataTreeModification modificationCopy = + (CursorAwareDataTreeModification) snapshot.newModification(); + final DataTreeModificationCursor cursor = modificationCopy.createCursor(dataTree.getRootPath()); + checkState(cursor != null, "DataTreeModificationCursor for root path should not be null"); + modification.applyToCursor(cursor); + // Then validate it. + validateCandidate(prepareCandidate(modificationCopy)); } @Override public String toString() { - return "ConfigSnapshot{" + - "modification=" + - ReflectionToStringBuilder.toString( - modification, - RecursiveToStringStyle.MULTI_LINE_STYLE, - false, - false - ) + ", validated=" + validated + '}'; + return "ConfigSnapshot{modification=" + + ReflectionToStringBuilder.toString( + modification, + RecursiveToStringStyle.MULTI_LINE_STYLE, + false, + false + ) + '}'; } } } - - - 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 3065a94ff..ef9d3d6d2 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 @@ -67,6 +67,16 @@ public class ModifiableDataTreeDelegatorTest extends ModifiableDataTreeDelegator assertEquals(dataTree.takeSnapshot().readNode(TOP_CONTAINER_ID), Optional.toJavaUtil(normalizedNodeOptional)); } + @Test + public void testValidateTwice() throws Exception { + final MapNode nestedList = getNestedList("listEntry", "listValue"); + + final DataModification dataModification = configDataTree.newModification(); + dataModification.write(NESTED_LIST_ID, nestedList); + dataModification.validate(); + dataModification.validate(); + } + @Test public void testCommitSuccessful() throws Exception { final MapNode nestedList = getNestedList("listEntry", "listValue"); 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 1dab5524d..89ecef2f3 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 @@ -27,6 +27,7 @@ import static org.mockito.MockitoAnnotations.initMocks; import com.google.common.util.concurrent.CheckedFuture; import io.fd.honeycomb.data.DataModification; +import io.fd.honeycomb.translate.ValidationFailedException; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -34,7 +35,6 @@ import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; 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.DataValidationFailedException; public class WriteTransactionTest { @@ -104,7 +104,7 @@ public class WriteTransactionTest { @Test public void testSubmitFailed() throws Exception { - doThrow(mock(DataValidationFailedException.class)).when(configSnapshot).commit(); + doThrow(mock(ValidationFailedException.class)).when(configSnapshot).commit(); final CheckedFuture future = writeTx.submit(); try { future.get(); diff --git a/infra/translate-api/src/main/java/io/fd/honeycomb/translate/ValidationFailedException.java b/infra/translate-api/src/main/java/io/fd/honeycomb/translate/ValidationFailedException.java new file mode 100644 index 000000000..776716218 --- /dev/null +++ b/infra/translate-api/src/main/java/io/fd/honeycomb/translate/ValidationFailedException.java @@ -0,0 +1,54 @@ +/* + * 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; + +import javax.annotation.Nonnull; + +/** + * Thrown when validation fails at the translation layer. + */ +public class ValidationFailedException extends TranslationException { + private static final long serialVersionUID = 1L; + + /** + * Constructs an {@link ValidationFailedException} given exception detail message. + * + * @param message the exception detail message + */ + public ValidationFailedException(@Nonnull final String message) { + super(message); + } + + /** + * Constructs an {@link ValidationFailedException} given exception detail message and exception cause. + * + * @param cause the cause of validation failure + * @param message the exception detail message + */ + public ValidationFailedException(@Nonnull final String message, @Nonnull final Throwable cause) { + super(message, cause); + } + + /** + * Constructs an {@link ValidationFailedException} given exception cause. + * + * @param cause the cause of validated failure + */ + public ValidationFailedException(@Nonnull final Throwable cause) { + super(cause); + } +} -- cgit 1.2.3-korg