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 <mgradzki@cisco.com>

5 files changed, 125 insertions, 32 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 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
+     * <p>The operation does not have any side-effects on the modification state.
+     *
+     * <p>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
@@ -68,6 +68,16 @@ public class ModifiableDataTreeDelegatorTest extends ModifiableDataTreeDelegator
     }
 
     @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<Void, TransactionCommitFailedException> 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);
+    }
+}