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 --- .../data/impl/ModifiableDataTreeManager.java | 69 +++++++++++++++------- .../data/impl/ModifiableDataTreeDelegatorTest.java | 10 ++++ .../honeycomb/data/impl/WriteTransactionTest.java | 4 +- 3 files changed, 59 insertions(+), 24 deletions(-) (limited to 'infra/data-impl') 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(); -- cgit 1.2.3-korg