HONEYCOMB-9: Simplify writer APIs, remove list of DataObjects 60/660/6
authorMaros Marsalek <mmarsale@cisco.com>
Tue, 12 Apr 2016 08:12:46 +0000 (10:12 +0200)
committerMaros Marsalek <mmarsale@cisco.com>
Tue, 12 Apr 2016 08:12:46 +0000 (10:12 +0200)
Change-Id: I139a883da167f9ab388b41b3ede50e48adc22d0b
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
Signed-off-by: Marek Gradzki <mgradzki@cisco.com>
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
27 files changed:
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppConfigDataTree.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppDataTree.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppWriteTransaction.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppWriterRegistry.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/VppApiInvocationException.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/VppException.java [new file with mode: 0644]
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/ChildVppReaderCustomizer.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/ListVppReaderCustomizer.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/RootVppReaderCustomizer.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/ChildVppWriter.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/VppWriter.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/WriteContext.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/WriterRegistry.java [new file with mode: 0644]
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/AbstractCompositeVppWriter.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeChildVppWriter.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeListVppWriter.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeRootVppWriter.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/ChildVppWriterCustomizer.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/ListVppWriterCustomizer.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/RootVppWriterCustomizer.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/DelegatingWriterRegistry.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/NoopWriterCustomizer.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/TransactionWriteContext.java
v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/vppstate/BridgeDomainCustomizer.java
v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/data/VPPConfigDataTreeTest.java
v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/trans/w/util/TransactionWriteContextTest.java
v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/vpp/VppTest.java

index 6fdeea3..f14bec3 100644 (file)
 package io.fd.honeycomb.v3po.impl.data;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-import static java.util.Collections.singletonList;
 
 import com.google.common.base.Optional;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.Futures;
+import io.fd.honeycomb.v3po.impl.trans.VppException;
 import io.fd.honeycomb.v3po.impl.trans.w.WriteContext;
+import io.fd.honeycomb.v3po.impl.trans.w.WriterRegistry;
 import io.fd.honeycomb.v3po.impl.trans.w.util.TransactionWriteContext;
-import io.fd.honeycomb.v3po.impl.trans.VppApiInvocationException;
-import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
-import java.util.ListIterator;
 import java.util.Map;
-import java.util.Set;
 import javax.annotation.Nonnull;
 import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataReadOnlyTransaction;
@@ -90,7 +85,7 @@ public final class VppConfigDataTree implements VppDataTree {
 
     @Override
     public void commit(final DataTreeModification modification)
-            throws DataValidationFailedException, VppApiInvocationException {
+            throws DataValidationFailedException, VppException {
         dataTree.validate(modification);
 
         final DataTreeCandidate candidate = dataTree.prepare(modification);
@@ -114,25 +109,29 @@ public final class VppConfigDataTree implements VppDataTree {
         final DOMDataReadOnlyTransaction afterTx = new VppReadOnlyTransaction(EMPTY_OPERATIONAL, modificationSnapshot);
         final WriteContext ctx = new TransactionWriteContext(serializer, beforeTx, afterTx);
 
-        final ChangesProcessor processor = new ChangesProcessor(writer, nodesBefore, nodesAfter, ctx);
         try {
-            processor.applyChanges();
-        } catch (VppApiInvocationException e) {
-            LOG.warn("Failed to apply changes", e);
+            writer.update(nodesBefore, nodesAfter, ctx);
+        } catch (WriterRegistry.BulkUpdateException e) {
+            LOG.warn("Failed to apply all changes", e);
             LOG.info("Trying to revert successful changes for current transaction");
 
             try {
-                processor.revertChanges();
+                e.revertChanges();
                 LOG.info("Changes successfully reverted");
-            } catch (VppApiInvocationException e2) {
+            } catch (VppException | RuntimeException e2) {
                 LOG.error("Failed to revert successful changes", e2);
             }
 
             // rethrow as we can't do anything more about it
+            // FIXME we need to throw a different kind of exception here to differentiate between:
+            // fail with success revert
+            // fail with failed revert (this one needs to contain IDs of changes that were not reverted)
+            throw e;
+        } catch (VppException e) {
+            LOG.error("Error while processing data change (before={}, after={})", nodesBefore, nodesAfter, e);
             throw e;
         }
 
-
         dataTree.commit(candidate);
     }
 
@@ -167,85 +166,6 @@ public final class VppConfigDataTree implements VppDataTree {
             return snapshot.newModification();
         }
     }
-
-    private static final class ChangesProcessor {
-        private final VppWriterRegistry writer;
-        private final List<InstanceIdentifier<?>> processedNodes;
-        private final Map<InstanceIdentifier<?>, DataObject> nodesBefore;
-        private final Map<InstanceIdentifier<?>, DataObject> nodesAfter;
-        private final WriteContext ctx;
-
-        ChangesProcessor(@Nonnull final VppWriterRegistry writer,
-                         final Map<InstanceIdentifier<?>, DataObject> nodesBefore,
-                         final Map<InstanceIdentifier<?>, DataObject> nodesAfter,
-                         @Nonnull final WriteContext writeContext) {
-            this.ctx = checkNotNull(writeContext, "writeContext is null!");
-            this.writer = checkNotNull(writer, "VppWriter is null!");
-            this.nodesBefore = checkNotNull(nodesBefore, "nodesBefore is null!");
-            this.nodesAfter = checkNotNull(nodesAfter, "nodesAfter is null!");
-            processedNodes = new ArrayList<>();
-        }
-
-        void applyChanges() throws VppApiInvocationException {
-            // TODO we should care about the order of modified subtrees
-            // TODO maybe WriterRegistry could provide writeAll method and it will process the updates
-            // in order in which it child writers are registered
-            final Set<InstanceIdentifier<?>> allNodes = new HashSet<>();
-            allNodes.addAll(nodesBefore.keySet());
-            allNodes.addAll(nodesAfter.keySet());
-            LOG.debug("ChangesProcessor.applyChanges() all extracted nodes: {}", allNodes);
-
-            for (InstanceIdentifier<?> node : allNodes) {
-                LOG.debug("ChangesProcessor.applyChanges() processing node={}", node);
-                final DataObject dataBefore = nodesBefore.get(node);
-                final DataObject dataAfter = nodesAfter.get(node);
-                LOG.debug("ChangesProcessor.applyChanges() processing dataBefore={}, dataAfter={}", dataBefore,
-                        dataAfter);
-
-                try {
-                    // TODO is List as input argument really necessary for writer ?
-                    final List<DataObject> dataObjectsBefore = dataBefore == null
-                            ? Collections.<DataObject>emptyList()
-                            : singletonList(dataBefore);
-                    final List<DataObject> dataObjectsAfter = dataAfter == null
-                            ? Collections.<DataObject>emptyList()
-                            : singletonList(dataAfter);
-                    LOG.debug("ChangesProcessor.applyChanges() processing dataObjectsBefore={}, dataObjectsAfter={}",
-                            dataObjectsBefore, dataObjectsAfter);
-                    writer.update(node, dataObjectsBefore, dataObjectsAfter, ctx);
-                    processedNodes.add(node);
-                } catch (RuntimeException e) {
-                    LOG.error("Error while processing data change (before={}, after={})", dataBefore, dataAfter, e);
-                    // FIXME ex handling
-                    throw new VppApiInvocationException("", 1, -1);
-                }
-            }
-        }
-
-        void revertChanges() throws VppApiInvocationException {
-            checkNotNull(writer, "VppWriter is null!");
-
-            // revert changes in reverse order they were applied
-            final ListIterator<InstanceIdentifier<?>> iterator = processedNodes.listIterator(processedNodes.size());
-
-            while (iterator.hasPrevious()) {
-                final InstanceIdentifier<?> node = iterator.previous();
-                LOG.debug("ChangesProcessor.revertChanges() processing node={}", node);
-
-                final DataObject dataBefore = nodesBefore.get(node);
-                final DataObject dataAfter = nodesAfter.get(node);
-
-                // revert a change by invoking writer with reordered arguments
-                try {
-                    // TODO is List as input argument really necessary for writer ?
-                    writer.update(node, singletonList(dataAfter), singletonList(dataBefore), ctx);
-                } catch (RuntimeException e) {
-                    // FIXME ex handling
-                    throw new VppApiInvocationException("", 1, -1);
-                }
-            }
-        }
-    }
 }
 
 
index a3a4fae..7fd9b2e 100644 (file)
@@ -17,7 +17,7 @@
 package io.fd.honeycomb.v3po.impl.data;
 
 import com.google.common.annotations.Beta;
-import io.fd.honeycomb.v3po.impl.trans.VppApiInvocationException;
+import io.fd.honeycomb.v3po.impl.trans.VppException;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException;
 
@@ -31,10 +31,10 @@ public interface VppDataTree {
      *
      * @param modification VPP data tree modification
      * @throws DataValidationFailedException if modification data is not valid
-     * @throws VppApiInvocationException if commit failed while updating VPP state
+     * @throws VppException if commit failed while updating VPP state
      */
     void commit(final DataTreeModification modification) throws DataValidationFailedException,
-            VppApiInvocationException;
+        VppException;
 
     /**
      * Creates read-only snapshot of a VppDataTree.
index b7aa2f8..299898b 100644 (file)
@@ -26,7 +26,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
-import io.fd.honeycomb.v3po.impl.trans.VppApiInvocationException;
+import io.fd.honeycomb.v3po.impl.trans.VppException;
 import javax.annotation.Nonnull;
 import javax.annotation.concurrent.NotThreadSafe;
 import org.opendaylight.controller.md.sal.common.api.TransactionStatus;
@@ -121,7 +121,7 @@ final class VppWriteTransaction implements DOMDataWriteTransaction {
         try {
             configDataTree.commit(modification);
             status = COMMITED;
-        } catch (DataValidationFailedException | VppApiInvocationException e) {
+        } catch (DataValidationFailedException | VppException e) {
             status = FAILED;
             LOG.error("Failed to commit VPP state modification", e);
             return Futures.immediateFailedCheckedFuture(
index 67b45cf..83186c2 100644 (file)
 
 package io.fd.honeycomb.v3po.impl.data;
 
+import io.fd.honeycomb.v3po.impl.trans.VppException;
 import io.fd.honeycomb.v3po.impl.trans.util.VppRWUtils;
 import io.fd.honeycomb.v3po.impl.trans.w.ChildVppWriter;
 import io.fd.honeycomb.v3po.impl.trans.w.VppWriter;
 import io.fd.honeycomb.v3po.impl.trans.w.WriteContext;
+import io.fd.honeycomb.v3po.impl.trans.w.WriterRegistry;
 import io.fd.honeycomb.v3po.impl.trans.w.impl.CompositeChildVppWriter;
 import io.fd.honeycomb.v3po.impl.trans.w.impl.CompositeListVppWriter;
 import io.fd.honeycomb.v3po.impl.trans.w.impl.CompositeRootVppWriter;
@@ -29,7 +31,9 @@ import io.fd.honeycomb.v3po.impl.trans.w.util.ReflexiveChildWriterCustomizer;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.v3po.rev150105.Vpp;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.v3po.rev150105.vpp.BridgeDomains;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.v3po.rev150105.vpp.bridge.domains.BridgeDomain;
@@ -40,7 +44,7 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.openvpp.vppjapi.vppApi;
 
 // TODO use some DI framework instead of singleton
-public class VppWriterRegistry implements VppWriter<DataObject> {
+public class VppWriterRegistry implements WriterRegistry {
 
     private static VppWriterRegistry instance;
 
@@ -85,8 +89,16 @@ public class VppWriterRegistry implements VppWriter<DataObject> {
 
     @Override
     public void update(@Nonnull final InstanceIdentifier<? extends DataObject> id,
-                       @Nonnull final List<? extends DataObject> dataBefore,
-                       @Nonnull final List<? extends DataObject> data, @Nonnull final WriteContext ctx) {
+                       @Nullable final DataObject dataBefore,
+                       @Nullable final DataObject data, @Nonnull final WriteContext ctx) throws VppException {
         writer.update(id, dataBefore, data, ctx);
     }
+
+    @Override
+    public void update(@Nonnull final Map<InstanceIdentifier<?>, DataObject> dataBefore,
+                       @Nonnull final Map<InstanceIdentifier<?>, DataObject> dataAfter,
+                       @Nonnull final WriteContext ctx)
+        throws VppException, BulkUpdateException {
+        writer.update(dataBefore, dataAfter, ctx);
+    }
 }
index b0076cd..0bb7c2b 100644 (file)
@@ -24,7 +24,7 @@ import javax.annotation.Nonnull;
  * Throws when Vpp jAPI method invocation failed.
  */
 @Beta
-public class VppApiInvocationException extends Exception {
+public class VppApiInvocationException extends VppException {
     private final String methodName;
     private final int ctxId;
     private final int errorCode;
diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/VppException.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/VppException.java
new file mode 100644 (file)
index 0000000..aa18ae7
--- /dev/null
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2016 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.v3po.impl.trans;
+
+import com.google.common.annotations.Beta;
+
+/**
+ * Base exception for Vpp writers
+ */
+@Beta
+public class VppException extends Exception {
+
+    public VppException(final String s) {
+        super(s);
+    }
+
+    public VppException(final String s, final Throwable throwable) {
+        super(s, throwable);
+    }
+
+    public VppException(final Throwable throwable) {
+        super(throwable);
+    }
+}
index e462118..900f8f8 100644 (file)
@@ -22,7 +22,10 @@ import org.opendaylight.yangtools.concepts.Builder;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 
 /**
- * io.fd.honeycomb.v3po.impl.trans.r.impl.CompositeChildVppReader SPI to customize its behavior
+ * {@link io.fd.honeycomb.v3po.impl.trans.r.impl.CompositeChildVppReader} SPI to customize its behavior
+ *
+ * @param <C> Specific DataObject derived type (Identifiable), that is handled by this customizer
+ * @param <B> Specific Builder for handled type (C)
  */
 @Beta
 public interface ChildVppReaderCustomizer<C extends DataObject, B extends Builder<C>> extends
index 5646ab9..694f21c 100644 (file)
@@ -26,7 +26,11 @@ import org.opendaylight.yangtools.yang.binding.Identifier;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
 /**
- * io.fd.honeycomb.v3po.impl.trans.r.impl.CompositeListVppReader SPI to customize its behavior
+ * {@link io.fd.honeycomb.v3po.impl.trans.r.impl.CompositeListVppReader} SPI to customize its behavior
+ *
+ * @param <C> Specific DataObject derived type (Identifiable), that is handled by this customizer
+ * @param <K> Specific Identifier for handled type (C)
+ * @param <B> Specific Builder for handled type (C)
  */
 @Beta
 public interface ListVppReaderCustomizer<C extends DataObject & Identifiable<K>, K extends Identifier<C>, B extends Builder<C>>
@@ -39,6 +43,7 @@ public interface ListVppReaderCustomizer<C extends DataObject & Identifiable<K>,
      */
     @Nonnull
     List<K> getAllIds(@Nonnull final InstanceIdentifier<C> id);
+    // TODO does it make sense with vpp APIs ? Should we replace it with a simple readAll ?
 
     /**
      * Merge read data into provided parent builder
index b6e1155..a09ed04 100644 (file)
@@ -23,7 +23,10 @@ import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
 /**
- * io.fd.honeycomb.v3po.impl.trans.r.impl.CompositeRootVppReader SPI to customize its behavior
+ * {@link io.fd.honeycomb.v3po.impl.trans.r.impl.CompositeRootVppReader} SPI to customize its behavior
+ *
+ * @param <C> Specific DataObject derived type, that is handled by this customizer
+ * @param <B> Specific Builder for handled type (C)
  */
 @Beta
 public interface RootVppReaderCustomizer<C extends DataObject, B extends Builder<C>> {
index 93c1092..243a652 100644 (file)
@@ -21,19 +21,46 @@ import javax.annotation.Nonnull;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
+/**
+ * Child VPP writer allowing its parent to pass the builder object
+ *
+ * @param <C> Specific DataObject derived type, that is handled by this writer
+ */
 @Beta
 public interface ChildVppWriter<D extends DataObject> extends VppWriter<D> {
 
+    /**
+     * Extract data object managed by this writer from parent data and perform write.
+     *
+     * @param parentId Id of parent node
+     * @param parentDataAfter Parent data from modification to extract data object from
+     * @param ctx Write context for current modification
+     */
     void writeChild(@Nonnull final InstanceIdentifier<? extends DataObject> parentId,
                     @Nonnull final DataObject parentDataAfter,
-                    @Nonnull WriteContext ctx);
+                    @Nonnull final WriteContext ctx);
 
+    /**
+     * Extract data object managed by this writer(if necessary) from parent data and perform delete.
+     *
+     * @param parentId Id of parent node
+     * @param parentDataBefore Parent data before modification to extract data object from
+     * @param ctx Write context for current modification
+     */
     void deleteChild(@Nonnull final InstanceIdentifier<? extends DataObject> parentId,
                      @Nonnull final DataObject parentDataBefore,
-                     @Nonnull WriteContext ctx);
+                     @Nonnull final WriteContext ctx);
 
+    /**
+     * Extract data object managed by this writer(if necessary) from parent data and perform delete.
+     *
+     * @param parentId Id of parent node
+     * @param parentDataBefore Parent data before modification to extract data object from
+     * @param parentDataAfter Parent data from modification to extract data object from
+     * @param ctx Write context for current modification
+     */
     void updateChild(@Nonnull final InstanceIdentifier<? extends DataObject> parentId,
                      @Nonnull final DataObject parentDataBefore,
                      @Nonnull final DataObject parentDataAfter,
-                     @Nonnull WriteContext ctx);
+                     @Nonnull final WriteContext ctx);
 }
index 617e11c..f8a49a2 100644 (file)
@@ -18,16 +18,30 @@ package io.fd.honeycomb.v3po.impl.trans.w;
 
 import com.google.common.annotations.Beta;
 import io.fd.honeycomb.v3po.impl.trans.SubtreeManager;
-import java.util.List;
+import io.fd.honeycomb.v3po.impl.trans.VppException;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
+/**
+ * Base VPP writer, responsible for translation between DataObjects and VPP APIs. Handling all update operations(create, update, delete)
+ *
+ * @param <D> Specific DataObject derived type, that is handled by this writer
+ */
 @Beta
 public interface VppWriter<D extends DataObject> extends SubtreeManager<D> {
 
+    /**
+     * Handle update operation. U from CRUD.
+     *
+     * @param id Identifier(from root) of data being written
+     * @param dataBefore Old data
+     * @param dataAfter New, updated data
+     * @param ctx Write context enabling writer to get information about candidate data as well as current data
+     */
     void update(@Nonnull final InstanceIdentifier<? extends DataObject> id,
-                @Nonnull final List<? extends DataObject> dataBefore,
-                @Nonnull final List<? extends DataObject> data,
-                @Nonnull final WriteContext ctx);
+                @Nullable final DataObject dataBefore,
+                @Nullable final DataObject dataAfter,
+                @Nonnull final WriteContext ctx) throws VppException;
 }
index 49759f2..3aaf832 100644 (file)
 
 package io.fd.honeycomb.v3po.impl.trans.w;
 
+import com.google.common.annotations.Beta;
+import com.google.common.base.Optional;
 import io.fd.honeycomb.v3po.impl.trans.util.Context;
-import java.util.List;
+import javax.annotation.Nonnull;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
+/**
+ * Context providing information about current state of DataTree to writers
+ */
+@Beta
 public interface WriteContext {
 
-    List<? extends DataObject> readBefore(InstanceIdentifier<? extends DataObject> currentId);
+    /**
+     * Read any data object before current modification was applied
+     *
+     * @param currentId Id of an object to read
+     *
+     * @return Data before the modification was applied
+     */
+    Optional<DataObject> readBefore(@Nonnull final InstanceIdentifier<? extends DataObject> currentId);
 
-    List<? extends DataObject> readAfter(InstanceIdentifier<? extends DataObject> currentId);
+    /**
+     * Read any data object from current modification
+     *
+     * @param currentId Id of an object to read
+     *
+     * @return Data from the modification
+     */
+    Optional<DataObject> readAfter(@Nonnull final InstanceIdentifier<? extends DataObject> currentId);
 
+    /**
+     * Get key value storage for customizers
+     *
+     * @return Context for customizers
+     */
+    @Nonnull
     Context getContext();
 }
diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/WriterRegistry.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/WriterRegistry.java
new file mode 100644 (file)
index 0000000..4b09ff2
--- /dev/null
@@ -0,0 +1,64 @@
+/*
+ * Copyright (c) 2016 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.v3po.impl.trans.w;
+
+import com.google.common.annotations.Beta;
+import io.fd.honeycomb.v3po.impl.trans.VppException;
+import java.util.Map;
+import javax.annotation.Nonnull;
+import org.opendaylight.yangtools.yang.binding.DataObject;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+
+/**
+ * Special {@link VppWriter} capable of performing bulk updates
+ */
+@Beta
+public interface WriterRegistry extends VppWriter<DataObject> {
+
+    /**
+     * Performs bulk update
+     *
+     * @throws BulkUpdateException in case bulk update fails
+     */
+    void update(@Nonnull final Map<InstanceIdentifier<?>, DataObject> dataBefore,
+                @Nonnull final Map<InstanceIdentifier<?>, DataObject> dataAfter,
+                @Nonnull final WriteContext ctx) throws VppException, BulkUpdateException;
+
+    @Beta
+    public class BulkUpdateException extends VppException {
+
+        private final Revert runnable;
+
+        public BulkUpdateException(final InstanceIdentifier<?> id, final RuntimeException e, final Revert runnable) {
+            super("Bulk edit failed at " + id, e);
+            this.runnable = runnable;
+        }
+
+        public void revertChanges() throws VppException {
+            runnable.revert();
+        }
+    }
+
+    /**
+     * Abstraction over revert mechanism in cast of a bulk update failure
+     */
+    @Beta
+    public interface Revert {
+
+        public void revert() throws VppException;
+    }
+}
index 44690bd..6e8f8bc 100644 (file)
@@ -18,10 +18,9 @@ package io.fd.honeycomb.v3po.impl.trans.w.impl;
 
 import static com.google.common.base.Preconditions.checkArgument;
 
-import com.google.common.base.Function;
+import com.google.common.base.Optional;
 import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
+import io.fd.honeycomb.v3po.impl.trans.VppException;
 import io.fd.honeycomb.v3po.impl.trans.util.VppRWUtils;
 import io.fd.honeycomb.v3po.impl.trans.w.ChildVppWriter;
 import io.fd.honeycomb.v3po.impl.trans.w.VppWriter;
@@ -32,10 +31,10 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.opendaylight.yangtools.yang.binding.Augmentation;
 import org.opendaylight.yangtools.yang.binding.ChildOf;
 import org.opendaylight.yangtools.yang.binding.DataObject;
-import org.opendaylight.yangtools.yang.binding.Identifiable;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -44,25 +43,16 @@ public abstract class AbstractCompositeVppWriter<D extends DataObject> implement
 
     private static final Logger LOG = LoggerFactory.getLogger(AbstractCompositeVppWriter.class);
 
-    public static final Function<DataObject, Object> INDEX_FUNCTION = new Function<DataObject, Object>() {
-        @Override
-        public Object apply(final DataObject input) {
-            return input instanceof Identifiable<?>
-                ? ((Identifiable<?>) input).getKey()
-                : input;
-        }
-    };
-
-    private final Map<Class<? extends DataObject>, ChildVppWriter<? extends ChildOf<D>>> childReaders;
-    private final Map<Class<? extends DataObject>, ChildVppWriter<? extends Augmentation<D>>> augReaders;
+    private final Map<Class<? extends DataObject>, ChildVppWriter<? extends ChildOf<D>>> childWriters;
+    private final Map<Class<? extends DataObject>, ChildVppWriter<? extends Augmentation<D>>> augWriters;
     private final InstanceIdentifier<D> instanceIdentifier;
 
     public AbstractCompositeVppWriter(final Class<D> type,
-                                      final List<ChildVppWriter<? extends ChildOf<D>>> childReaders,
-                                      final List<ChildVppWriter<? extends Augmentation<D>>> augReaders) {
+                                      final List<ChildVppWriter<? extends ChildOf<D>>> childWriters,
+                                      final List<ChildVppWriter<? extends Augmentation<D>>> augWriters) {
         this.instanceIdentifier = InstanceIdentifier.create(type);
-        this.childReaders = VppRWUtils.uniqueLinkedIndex(childReaders, VppRWUtils.MANAGER_CLASS_FUNCTION);
-        this.augReaders = VppRWUtils.uniqueLinkedIndex(augReaders, VppRWUtils.MANAGER_CLASS_AUG_FUNCTION);
+        this.childWriters = VppRWUtils.uniqueLinkedIndex(childWriters, VppRWUtils.MANAGER_CLASS_FUNCTION);
+        this.augWriters = VppRWUtils.uniqueLinkedIndex(augWriters, VppRWUtils.MANAGER_CLASS_AUG_FUNCTION);
     }
 
     protected void writeCurrent(final InstanceIdentifier<D> id, final D data, final WriteContext ctx) {
@@ -71,12 +61,12 @@ public abstract class AbstractCompositeVppWriter<D extends DataObject> implement
         LOG.trace("{}: Writing current attributes", this);
         writeCurrentAttributes(id, data, ctx);
 
-        for (ChildVppWriter<? extends ChildOf<D>> child : childReaders.values()) {
+        for (ChildVppWriter<? extends ChildOf<D>> child : childWriters.values()) {
             LOG.debug("{}: Writing child in: {}", this, child);
             child.writeChild(id, data, ctx);
         }
 
-        for (ChildVppWriter<? extends Augmentation<D>> child : augReaders.values()) {
+        for (ChildVppWriter<? extends Augmentation<D>> child : augWriters.values()) {
             LOG.debug("{}: Writing augment in: {}", this, child);
             child.writeChild(id, data, ctx);
         }
@@ -97,12 +87,12 @@ public abstract class AbstractCompositeVppWriter<D extends DataObject> implement
         LOG.trace("{}: Updating current attributes", this);
         updateCurrentAttributes(id, dataBefore, dataAfter, ctx);
 
-        for (ChildVppWriter<? extends ChildOf<D>> child : childReaders.values()) {
+        for (ChildVppWriter<? extends ChildOf<D>> child : childWriters.values()) {
             LOG.debug("{}: Updating child in: {}", this, child);
             child.updateChild(id, dataBefore, dataAfter, ctx);
         }
 
-        for (ChildVppWriter<? extends Augmentation<D>> child : augReaders.values()) {
+        for (ChildVppWriter<? extends Augmentation<D>> child : augWriters.values()) {
             LOG.debug("{}: Updating augment in: {}", this, child);
             child.updateChild(id, dataBefore, dataAfter, ctx);
         }
@@ -114,12 +104,12 @@ public abstract class AbstractCompositeVppWriter<D extends DataObject> implement
         LOG.debug("{}: Deleting current: {} dataBefore: {}", this, id, dataBefore);
 
         // delete in reversed order
-        for (ChildVppWriter<? extends Augmentation<D>> child : reverseCollection(augReaders.values())) {
+        for (ChildVppWriter<? extends Augmentation<D>> child : reverseCollection(augWriters.values())) {
             LOG.debug("{}: Deleting augment in: {}", this, child);
             child.deleteChild(id, dataBefore, ctx);
         }
 
-        for (ChildVppWriter<? extends ChildOf<D>> child : reverseCollection(childReaders.values())) {
+        for (ChildVppWriter<? extends ChildOf<D>> child : reverseCollection(childWriters.values())) {
             LOG.debug("{}: Deleting child in: {}", this, child);
             child.deleteChild(id, dataBefore, ctx);
         }
@@ -131,20 +121,20 @@ public abstract class AbstractCompositeVppWriter<D extends DataObject> implement
     @SuppressWarnings("unchecked")
     @Override
     public void update(@Nonnull final InstanceIdentifier<? extends DataObject> id,
-                       @Nonnull final List<? extends DataObject> dataBefore,
-                       @Nonnull final List<? extends DataObject> dataAfter,
-                       @Nonnull final WriteContext ctx) {
+                       @Nullable final DataObject dataBefore,
+                       @Nullable final DataObject dataAfter,
+                       @Nonnull final WriteContext ctx) throws VppException {
         LOG.debug("{}: Updating : {}", this, id);
         LOG.trace("{}: Updating : {}, from: {} to: {}", this, id, dataBefore, dataAfter);
 
         if (idPointsToCurrent(id)) {
             if(isWrite(dataBefore, dataAfter)) {
-                writeAll((InstanceIdentifier<D>) id, dataAfter, ctx);
+                writeCurrent((InstanceIdentifier<D>) id, castToManaged(dataAfter), ctx);
             } else if(isDelete(dataBefore, dataAfter)) {
-                deleteAll((InstanceIdentifier<D>) id, dataBefore, ctx);
+                deleteCurrent((InstanceIdentifier<D>) id, castToManaged(dataBefore), ctx);
             } else {
-                checkArgument(!dataBefore.isEmpty() && !dataAfter.isEmpty(), "No data to process");
-                updateAll((InstanceIdentifier<D>) id, dataBefore, dataAfter, ctx);
+                checkArgument(dataBefore != null && dataAfter != null, "No data to process");
+                updateCurrent((InstanceIdentifier<D>) id, castToManaged(dataBefore), castToManaged(dataAfter), ctx);
             }
         } else {
             if (isWrite(dataBefore, dataAfter)) {
@@ -152,89 +142,47 @@ public abstract class AbstractCompositeVppWriter<D extends DataObject> implement
             } else if (isDelete(dataBefore, dataAfter)) {
                 deleteSubtree(id, dataBefore, ctx);
             } else {
-                checkArgument(!dataBefore.isEmpty() && !dataAfter.isEmpty(), "No data to process");
+                checkArgument(dataBefore != null && dataAfter != null, "No data to process");
                 updateSubtree(id, dataBefore, dataAfter, ctx);
             }
         }
     }
 
-    protected void updateAll(final @Nonnull InstanceIdentifier<D> id,
-                             final @Nonnull List<? extends DataObject> dataBefore,
-                             final @Nonnull List<? extends DataObject> dataAfter, final WriteContext ctx) {
-        LOG.trace("{}: Updating all : {}", this, id);
-
-        final Map<Object, ? extends DataObject> indexedAfter = indexData(dataAfter);
-        final Map<Object, ? extends DataObject> indexedBefore = indexData(dataBefore);
-
-        for (Map.Entry<Object, ? extends DataObject> after : indexedAfter.entrySet()) {
-            final DataObject before = indexedBefore.get(after.getKey());
-            if(before == null) {
-                writeCurrent(id, castToManaged(after.getValue()), ctx);
-            } else {
-                updateCurrent(id, castToManaged(before), castToManaged(after.getValue()), ctx);
-            }
-        }
-
-        // Delete the rest in dataBefore
-        for (Object deletedNodeKey : Sets.difference(indexedBefore.keySet(), indexedAfter.keySet())) {
-            final DataObject deleted = indexedBefore.get(deletedNodeKey);
-            deleteCurrent(id, castToManaged(deleted), ctx);
-        }
-    }
-
-    private static Map<Object, ? extends DataObject> indexData(final List<? extends DataObject> data) {
-        return Maps.uniqueIndex(data, INDEX_FUNCTION);
-    }
-
-    protected void deleteAll(final @Nonnull InstanceIdentifier<D> id,
-                             final @Nonnull List<? extends DataObject> dataBefore, final WriteContext ctx) {
-        LOG.trace("{}: Deleting all : {}", this, id);
-        for (DataObject singleValue : dataBefore) {
-            checkArgument(getManagedDataObjectType().getTargetType().isAssignableFrom(singleValue.getClass()));
-            deleteCurrent(id, castToManaged(singleValue), ctx);
-        }
+    private void checkDataType(final @Nullable DataObject dataAfter) {
+        checkArgument(getManagedDataObjectType().getTargetType().isAssignableFrom(dataAfter.getClass()));
     }
 
     private D castToManaged(final DataObject data) {
-        checkArgument(getManagedDataObjectType().getTargetType().isAssignableFrom(data.getClass()));
+        checkDataType(data);
         return getManagedDataObjectType().getTargetType().cast(data);
     }
 
-    protected void writeAll(final @Nonnull InstanceIdentifier<D> id,
-                            final @Nonnull List<? extends DataObject> dataAfter, final WriteContext ctx) {
-        LOG.trace("{}: Writing all : {}", this, id);
-        for (DataObject singleValue : dataAfter) {
-            checkArgument(getManagedDataObjectType().getTargetType().isAssignableFrom(singleValue.getClass()));
-            writeCurrent(id, castToManaged(singleValue), ctx);
-        }
-    }
-
-    private static boolean isWrite(final List<? extends DataObject> dataBefore,
-                                    final List<? extends DataObject> dataAfter) {
-        return dataBefore.isEmpty() && !dataAfter.isEmpty();
+    private static boolean isWrite(final DataObject dataBefore,
+                                    final DataObject dataAfter) {
+        return dataBefore == null && dataAfter != null;
     }
 
-    private static boolean isDelete(final List<? extends DataObject> dataBefore,
-                                    final List<? extends DataObject> dataAfter) {
-        return dataAfter.isEmpty() && !dataBefore.isEmpty();
+    private static boolean isDelete(final DataObject dataBefore,
+                                    final DataObject dataAfter) {
+        return dataAfter == null && dataBefore != null;
     }
 
     private void writeSubtree(final InstanceIdentifier<? extends DataObject> id,
-                              final List<? extends DataObject> dataAfter, final WriteContext ctx) {
+                              final DataObject dataAfter, final WriteContext ctx) throws VppException {
         LOG.debug("{}: Writing subtree: {}", this, id);
         final VppWriter<? extends ChildOf<D>> vppWriter = getNextWriter(id);
 
         if (vppWriter != null) {
             LOG.debug("{}: Writing subtree: {} in: {}", this, id, vppWriter);
-            vppWriter.update(id, Collections.<DataObject>emptyList(), dataAfter, ctx);
+            vppWriter.update(id, null, dataAfter, ctx);
         } else {
             // If there's no dedicated writer, use write current
             // But we need current data after to do so
             final InstanceIdentifier<D> currentId = VppRWUtils.cutId(id, getManagedDataObjectType());
-            List<? extends DataObject> currentDataAfter = ctx.readAfter(currentId);
+            Optional<DataObject> currentDataAfter = ctx.readAfter(currentId);
             LOG.debug("{}: Dedicated subtree writer missing for: {}. Writing current.", this,
                 VppRWUtils.getNextId(id, getManagedDataObjectType()).getType(), currentDataAfter);
-            writeAll(currentId, currentDataAfter, ctx);
+            writeCurrent(currentId, castToManaged(currentDataAfter.get()), ctx);
         }
     }
 
@@ -244,32 +192,34 @@ public abstract class AbstractCompositeVppWriter<D extends DataObject> implement
 
     @SuppressWarnings("unchecked")
     private void deleteSubtree(final InstanceIdentifier<? extends DataObject> id,
-                               final List<? extends DataObject> dataBefore, final WriteContext ctx) {
+                               final DataObject dataBefore, final WriteContext ctx) throws VppException {
         LOG.debug("{}: Deleting subtree: {}", this, id);
         final VppWriter<? extends ChildOf<D>> vppWriter = getNextWriter(id);
 
         if (vppWriter != null) {
             LOG.debug("{}: Deleting subtree: {} in: {}", this, id, vppWriter);
-            vppWriter.update(id, dataBefore, Collections.<DataObject>emptyList(), ctx);
+            vppWriter.update(id, dataBefore, null, ctx);
         } else {
             updateSubtreeFromCurrent(id, ctx);
         }
     }
 
+    @SuppressWarnings("unchecked")
     private void updateSubtreeFromCurrent(final InstanceIdentifier<? extends DataObject> id, final WriteContext ctx) {
         final InstanceIdentifier<D> currentId = VppRWUtils.cutId(id, getManagedDataObjectType());
-        List<? extends DataObject> currentDataBefore = ctx.readBefore(currentId);
-        List<? extends DataObject> currentDataAfter = ctx.readAfter(currentId);
+        Optional<DataObject> currentDataBefore = ctx.readBefore(currentId);
+        Optional<DataObject> currentDataAfter = ctx.readAfter(currentId);
         LOG.debug("{}: Dedicated subtree writer missing for: {}. Updating current without subtree", this,
             VppRWUtils.getNextId(id, getManagedDataObjectType()).getType(), currentDataAfter);
-        updateAll((InstanceIdentifier<D>) id, currentDataBefore, currentDataAfter, ctx);
+        updateCurrent((InstanceIdentifier<D>) id, castToManaged(currentDataBefore.orNull()),
+            castToManaged(currentDataAfter.orNull()), ctx);
     }
 
     @SuppressWarnings("unchecked")
     private void updateSubtree(final InstanceIdentifier<? extends DataObject> id,
-                               final List<? extends DataObject> dataBefore,
-                               final List<? extends DataObject> dataAfter,
-                               final WriteContext ctx) {
+                               final DataObject dataBefore,
+                               final DataObject dataAfter,
+                               final WriteContext ctx) throws VppException {
         LOG.debug("{}: Updating subtree: {}", this, id);
         final VppWriter<? extends ChildOf<D>> vppWriter = getNextWriter(id);
 
@@ -283,11 +233,11 @@ public abstract class AbstractCompositeVppWriter<D extends DataObject> implement
 
     private VppWriter<? extends ChildOf<D>> getNextWriter(final InstanceIdentifier<? extends DataObject> id) {
         final Class<? extends DataObject> next = VppRWUtils.getNextId(id, getManagedDataObjectType()).getType();
-        return childReaders.get(next);
+        return childWriters.get(next);
     }
 
     private static <T> List<T> reverseCollection(final Collection<T> original) {
-        // TODO find a better reverse mechanism (probably a different collection for child readers is necessary)
+        // TODO find a better reverse mechanism (probably a different collection for child writers is necessary)
         final ArrayList<T> list = Lists.newArrayList(original);
         Collections.reverse(list);
         return list;
index 919bbaa..d94e4fe 100644 (file)
@@ -35,9 +35,9 @@ public class CompositeChildVppWriter<D extends DataObject> extends AbstractCompo
 
     public CompositeChildVppWriter(@Nonnull final Class<D> type,
                                    @Nonnull final List<ChildVppWriter<? extends ChildOf<D>>> childWriters,
-                                   @Nonnull final List<ChildVppWriter<? extends Augmentation<D>>> augReaders,
+                                   @Nonnull final List<ChildVppWriter<? extends Augmentation<D>>> augWriters,
                                    @Nonnull final ChildVppWriterCustomizer<D> customizer) {
-        super(type, childWriters, augReaders);
+        super(type, childWriters, augWriters);
         this.customizer = customizer;
     }
 
index 8914d37..1722b46 100644 (file)
 
 package io.fd.honeycomb.v3po.impl.trans.w.impl;
 
+import com.google.common.base.Function;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
 import io.fd.honeycomb.v3po.impl.trans.util.VppRWUtils;
 import io.fd.honeycomb.v3po.impl.trans.w.ChildVppWriter;
 import io.fd.honeycomb.v3po.impl.trans.w.WriteContext;
 import io.fd.honeycomb.v3po.impl.trans.w.impl.spi.ListVppWriterCustomizer;
 import java.util.List;
+import java.util.Map;
 import javax.annotation.Nonnull;
 import org.opendaylight.yangtools.yang.binding.Augmentation;
 import org.opendaylight.yangtools.yang.binding.ChildOf;
@@ -32,20 +37,30 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 public class CompositeListVppWriter<D extends DataObject & Identifiable<K>, K extends Identifier<D>> extends AbstractCompositeVppWriter<D>
     implements ChildVppWriter<D> {
 
+    public static final Function<DataObject, Object> INDEX_FUNCTION = new Function<DataObject, Object>() {
+        @Override
+        public Object apply(final DataObject input) {
+            return input instanceof Identifiable<?>
+                ? ((Identifiable<?>) input).getKey()
+                : input;
+        }
+    };
+
+
     private final ListVppWriterCustomizer<D, K> customizer;
 
     public CompositeListVppWriter(@Nonnull final Class<D> type,
-                                  @Nonnull final List<ChildVppWriter<? extends ChildOf<D>>> childReaders,
-                                  @Nonnull final List<ChildVppWriter<? extends Augmentation<D>>> augReaders,
+                                  @Nonnull final List<ChildVppWriter<? extends ChildOf<D>>> childWriters,
+                                  @Nonnull final List<ChildVppWriter<? extends Augmentation<D>>> augWriters,
                                   @Nonnull final ListVppWriterCustomizer<D, K> customizer) {
-        super(type, childReaders, augReaders);
+        super(type, childWriters, augWriters);
         this.customizer = customizer;
     }
 
     public CompositeListVppWriter(@Nonnull final Class<D> type,
-                                  @Nonnull final List<ChildVppWriter<? extends ChildOf<D>>> childReaders,
+                                  @Nonnull final List<ChildVppWriter<? extends ChildOf<D>>> childWriters,
                                   @Nonnull final ListVppWriterCustomizer<D, K> customizer) {
-        this(type, childReaders, VppRWUtils.<D>emptyAugWriterList(), customizer);
+        this(type, childWriters, VppRWUtils.<D>emptyAugWriterList(), customizer);
     }
 
     public CompositeListVppWriter(@Nonnull final Class<D> type,
@@ -78,7 +93,9 @@ public class CompositeListVppWriter<D extends DataObject & Identifiable<K>, K ex
                            @Nonnull final WriteContext ctx) {
         final InstanceIdentifier<D> currentId = VppRWUtils.appendTypeToId(parentId, getManagedDataObjectType());
         final List<D> currentData = customizer.extract(currentId, parentData);
-        writeAll(currentId, currentData, ctx);
+        for (D entry : currentData) {
+            writeCurrent(currentId, entry, ctx);
+        }
     }
 
     @Override
@@ -87,7 +104,9 @@ public class CompositeListVppWriter<D extends DataObject & Identifiable<K>, K ex
                             @Nonnull final WriteContext ctx) {
         final InstanceIdentifier<D> currentId = VppRWUtils.appendTypeToId(parentId, getManagedDataObjectType());
         final List<D> dataBefore = customizer.extract(currentId, parentDataBefore);
-        deleteAll(currentId, dataBefore, ctx);
+        for (D entry : dataBefore) {
+            deleteCurrent(currentId, entry, ctx);
+        }
     }
 
     @Override
@@ -95,9 +114,26 @@ public class CompositeListVppWriter<D extends DataObject & Identifiable<K>, K ex
                             @Nonnull final DataObject parentDataBefore, @Nonnull final DataObject parentDataAfter,
                             @Nonnull final WriteContext ctx) {
         final InstanceIdentifier<D> currentId = VppRWUtils.appendTypeToId(parentId, getManagedDataObjectType());
-        final List<D> dataBefore = customizer.extract(currentId, parentDataBefore);
-        final List<D> dataAfter = customizer.extract(currentId, parentDataAfter);
-        updateAll(currentId, dataBefore, dataAfter, ctx);
+        final ImmutableMap<Object, D>
+            dataBefore = Maps.uniqueIndex(customizer.extract(currentId, parentDataBefore), INDEX_FUNCTION);
+        final ImmutableMap<Object, D>
+            dataAfter = Maps.uniqueIndex(customizer.extract(currentId, parentDataAfter), INDEX_FUNCTION);
+
+        for (Map.Entry<Object, D> after : dataAfter.entrySet()) {
+            final D before = dataBefore.get(after.getKey());
+            if(before == null) {
+                writeCurrent(currentId, after.getValue(), ctx);
+            } else {
+                updateCurrent(currentId, before, after.getValue(), ctx);
+            }
+        }
+
+        // Delete the rest in dataBefore
+        for (Object deletedNodeKey : Sets.difference(dataBefore.keySet(), dataAfter.keySet())) {
+            final D deleted = dataBefore.get(deletedNodeKey);
+            deleteCurrent(currentId, deleted, ctx);
+        }
+
     }
 
     @Override
index 6be5651..a7139e5 100644 (file)
@@ -32,17 +32,17 @@ public class CompositeRootVppWriter<D extends DataObject> extends AbstractCompos
     private final RootVppWriterCustomizer<D> customizer;
 
     public CompositeRootVppWriter(@Nonnull final Class<D> type,
-                                  @Nonnull final List<ChildVppWriter<? extends ChildOf<D>>> childReaders,
-                                  @Nonnull final List<ChildVppWriter<? extends Augmentation<D>>> augReaders,
+                                  @Nonnull final List<ChildVppWriter<? extends ChildOf<D>>> childWriters,
+                                  @Nonnull final List<ChildVppWriter<? extends Augmentation<D>>> augWriters,
                                   @Nonnull final RootVppWriterCustomizer<D> customizer) {
-        super(type, childReaders, augReaders);
+        super(type, childWriters, augWriters);
         this.customizer = customizer;
     }
 
     public CompositeRootVppWriter(@Nonnull final Class<D> type,
-                                  @Nonnull final List<ChildVppWriter<? extends ChildOf<D>>> childReaders,
+                                  @Nonnull final List<ChildVppWriter<? extends ChildOf<D>>> childWriters,
                                   @Nonnull final RootVppWriterCustomizer<D> customizer) {
-        this(type, childReaders, VppRWUtils.<D>emptyAugWriterList(), customizer);
+        this(type, childWriters, VppRWUtils.<D>emptyAugWriterList(), customizer);
     }
 
     public CompositeRootVppWriter(@Nonnull final Class<D> type,
index 5ea5042..1e79c68 100644 (file)
@@ -22,9 +22,20 @@ import javax.annotation.Nonnull;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
+/**
+ * {@link io.fd.honeycomb.v3po.impl.trans.w.impl.CompositeChildVppWriter} SPI to customize its behavior
+ *
+ * @param <D> Specific DataObject derived type (Identifiable), that is handled by this customizer
+ */
 @Beta
 public interface ChildVppWriterCustomizer<D extends DataObject> extends RootVppWriterCustomizer<D> {
 
+    /**
+     * Get child of parentData identified by currentId
+     *
+     * @param currentId Identifier(from root) of data being extracted
+     * @param parentData Parent data object from which managed data object must be extracted
+     */
     @Nonnull
     Optional<D> extract(@Nonnull final InstanceIdentifier<D> currentId, @Nonnull final DataObject parentData);
 
index edd8de9..6e72fc7 100644 (file)
@@ -24,9 +24,21 @@ import org.opendaylight.yangtools.yang.binding.Identifiable;
 import org.opendaylight.yangtools.yang.binding.Identifier;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
+/**
+ * {@link io.fd.honeycomb.v3po.impl.trans.w.impl.CompositeListVppWriter} SPI to customize its behavior
+ *
+ * @param <C> Specific DataObject derived type (Identifiable), that is handled by this customizer
+ * @param <K> Specific Identifier for handled type (C)
+ */
 @Beta
 public interface ListVppWriterCustomizer<C extends DataObject & Identifiable<K>, K extends Identifier<C>> extends RootVppWriterCustomizer<C> {
 
+    /**
+     * Get children of parentData identified by currentId
+     *
+     * @param currentId Identifier(from root) of data being extracted
+     * @param parentData Parent data object from which managed data object must be extracted
+     */
     @Nonnull
     List<C> extract(@Nonnull final InstanceIdentifier<C> currentId, @Nonnull final DataObject parentData);
 
index 6a2d0c2..0fa89d2 100644 (file)
@@ -22,18 +22,45 @@ import javax.annotation.Nonnull;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
+/**
+ * {@link io.fd.honeycomb.v3po.impl.trans.w.impl.CompositeRootVppWriter} SPI to customize its behavior
+ *
+ * @param <D> Specific DataObject derived type, that is handled by this customizer
+ */
 @Beta
 public interface RootVppWriterCustomizer<D extends DataObject> {
 
+    /**
+     * Handle write operation. C from CRUD.
+     *
+     * @param id Identifier(from root) of data being written
+     * @param dataAfter New data to be written
+     * @param writeContext Write context can be used to store any useful information and then utilized by other customizers
+     */
     void writeCurrentAttributes(@Nonnull final InstanceIdentifier<D> id,
                                 @Nonnull final D dataAfter,
                                 @Nonnull final Context writeContext);
 
+    /**
+     * Handle update operation. U from CRUD.
+     *
+     * @param id Identifier(from root) of data being written
+     * @param dataBefore Old data
+     * @param dataAfter New, updated data
+     * @param writeContext Write context can be used to store any useful information and then utilized by other customizers
+     */
     void updateCurrentAttributes(@Nonnull final InstanceIdentifier<D> id,
                                  @Nonnull final D dataBefore,
                                  @Nonnull final D dataAfter,
                                  @Nonnull final Context writeContext);
 
+    /**
+     * Handle delete operation. D from CRUD.
+     *
+     * @param id Identifier(from root) of data being written
+     * @param dataBefore Old data being deleted
+     * @param writeContext Write context can be used to store any useful information and then utilized by other customizers
+     */
     void deleteCurrentAttributes(@Nonnull final InstanceIdentifier<D> id,
                                  @Nonnull final D dataBefore,
                                  @Nonnull final Context writeContext);
index 6df9606..d20e69a 100644 (file)
 
 package io.fd.honeycomb.v3po.impl.trans.w.util;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 
+import com.google.common.base.Function;
+import com.google.common.collect.Collections2;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import io.fd.honeycomb.v3po.impl.trans.VppException;
 import io.fd.honeycomb.v3po.impl.trans.util.VppRWUtils;
 import io.fd.honeycomb.v3po.impl.trans.w.VppWriter;
 import io.fd.honeycomb.v3po.impl.trans.w.WriteContext;
+import io.fd.honeycomb.v3po.impl.trans.w.WriterRegistry;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Map;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
- * Simple reader registry able to perform and aggregated read (ROOT read) on top of all
- * provided readers. Also able to delegate a specific read to one of the delegate readers.
+ * Simple writer registry able to perform and aggregated read (ROOT write) on top of all
+ * provided writers. Also able to delegate a specific read to one of the delegate writers.
  *
- * This could serve as a utility to hold & hide all available readers in upper layers.
+ * This could serve as a utility to hold & hide all available writers in upper layers.
  */
-public final class DelegatingWriterRegistry implements VppWriter<DataObject> {
+public final class DelegatingWriterRegistry implements WriterRegistry {
 
-    private final Map<Class<? extends DataObject>, VppWriter<? extends DataObject>> rootReaders;
+    private static final Logger LOG = LoggerFactory.getLogger(DelegatingWriterRegistry.class);
+
+    private static final Function<InstanceIdentifier<?>, Class<? extends DataObject>> ID_TO_CLASS =
+        new Function<InstanceIdentifier<?>, Class<? extends DataObject>>() {
+        @Override
+        public Class<? extends DataObject> apply(final InstanceIdentifier<?> input) {
+            return input.getTargetType();
+        }
+    };
+
+    private final Map<Class<? extends DataObject>, VppWriter<? extends DataObject>> rootWriters;
 
     /**
      * Create new {@link DelegatingWriterRegistry}
      *
-     * @param rootReaders List of delegate readers
+     * @param rootWriters List of delegate writers
      */
-    public DelegatingWriterRegistry(@Nonnull final List<VppWriter<? extends DataObject>> rootReaders) {
-        this.rootReaders = VppRWUtils.uniqueLinkedIndex(checkNotNull(rootReaders), VppRWUtils.MANAGER_CLASS_FUNCTION);
+    public DelegatingWriterRegistry(@Nonnull final List<VppWriter<? extends DataObject>> rootWriters) {
+        this.rootWriters = VppRWUtils.uniqueLinkedIndex(checkNotNull(rootWriters), VppRWUtils.MANAGER_CLASS_FUNCTION);
     }
 
     /**
-     * @throws UnsupportedOperationException This getter is not supported for reader registry since it does not manage a
+     * @throws UnsupportedOperationException This getter is not supported for writer registry since it does not manage a
      *                                       specific node type
      */
     @Nonnull
@@ -59,14 +80,95 @@ public final class DelegatingWriterRegistry implements VppWriter<DataObject> {
 
     @Override
     public void update(@Nonnull final InstanceIdentifier<? extends DataObject> id,
-                       @Nonnull final List<? extends DataObject> dataBefore,
-                       @Nonnull final List<? extends DataObject> dataAfter,
-                       @Nonnull final WriteContext ctx) {
+                       @Nullable final DataObject dataBefore,
+                       @Nullable final DataObject dataAfter,
+                       @Nonnull final WriteContext ctx) throws VppException {
         final InstanceIdentifier.PathArgument first = checkNotNull(
             Iterables.getFirst(id.getPathArguments(), null), "Empty id");
-        final VppWriter<? extends DataObject> vppWriter = rootReaders.get(first.getType());
+        final VppWriter<? extends DataObject> vppWriter = rootWriters.get(first.getType());
         checkNotNull(vppWriter,
-            "Unable to write %s. Missing writer. Current writers for: %s", id, rootReaders.keySet());
+            "Unable to write %s. Missing writer. Current writers for: %s", id, rootWriters.keySet());
         vppWriter.update(id, dataBefore, dataAfter, ctx);
     }
+
+    @Override
+    public void update(@Nonnull final Map<InstanceIdentifier<?>, DataObject> nodesBefore,
+                       @Nonnull final Map<InstanceIdentifier<?>, DataObject> nodesAfter,
+                       @Nonnull final WriteContext ctx) throws VppException {
+        checkAllWritersPresent(nodesBefore);
+        checkAllWritersPresent(nodesAfter);
+
+        final List<InstanceIdentifier<?>> processedNodes = Lists.newArrayList();
+
+        for (Map.Entry<Class<? extends DataObject>, VppWriter<? extends DataObject>> rootWriterEntry : rootWriters
+            .entrySet()) {
+
+            final InstanceIdentifier<? extends DataObject> id = rootWriterEntry.getValue().getManagedDataObjectType();
+
+            final DataObject dataBefore = nodesBefore.get(id);
+            final DataObject dataAfter = nodesAfter.get(id);
+
+            // No change to current writer
+            if(dataBefore == null && dataAfter == null) {
+                continue;
+            }
+
+            LOG.debug("ChangesProcessor.applyChanges() processing dataBefore={}, dataAfter={}", dataBefore, dataAfter);
+
+            try {
+                update(id, dataBefore, dataAfter, ctx);
+                processedNodes.add(id);
+            } catch (RuntimeException e) {
+                LOG.error("Error while processing data change of: {} (before={}, after={})", id, dataBefore, dataAfter, e);
+                throw new BulkUpdateException(id, e, new RevertImpl(this, processedNodes, nodesBefore, nodesAfter, ctx));
+            }
+        }
+
+    }
+
+    private void checkAllWritersPresent(final @Nonnull Map<InstanceIdentifier<?>, DataObject> nodesBefore) {
+        checkArgument(rootWriters.keySet().containsAll(Collections2.transform(nodesBefore.keySet(), ID_TO_CLASS)),
+            "Unable to handle all changes. Missing dedicated writers for: %s",
+            Sets.difference(nodesBefore.keySet(), rootWriters.keySet()));
+    }
+
+    private static final class RevertImpl implements Revert {
+        private final WriterRegistry delegatingWriterRegistry;
+        private final List<InstanceIdentifier<?>> processedNodes;
+        private final Map<InstanceIdentifier<?>, DataObject> nodesBefore;
+        private final Map<InstanceIdentifier<?>, DataObject> nodesAfter;
+        private final WriteContext ctx;
+
+        public RevertImpl(final WriterRegistry delegatingWriterRegistry,
+                          final List<InstanceIdentifier<?>> processedNodes,
+                          final Map<InstanceIdentifier<?>, DataObject> nodesBefore,
+                          final Map<InstanceIdentifier<?>, DataObject> nodesAfter, final WriteContext ctx) {
+            this.delegatingWriterRegistry = delegatingWriterRegistry;
+            this.processedNodes = processedNodes;
+            this.nodesBefore = nodesBefore;
+            this.nodesAfter = nodesAfter;
+            this.ctx = ctx;
+        }
+
+        @Override
+        public void revert() throws VppException {
+
+            final ListIterator<InstanceIdentifier<?>> iterator = processedNodes.listIterator(processedNodes.size());
+
+            while (iterator.hasPrevious()) {
+                final InstanceIdentifier<?> node = iterator.previous();
+                LOG.debug("ChangesProcessor.revertChanges() processing node={}", node);
+
+                final DataObject dataBefore = nodesBefore.get(node);
+                final DataObject dataAfter = nodesAfter.get(node);
+
+                // revert a change by invoking writer with reordered arguments
+                try {
+                    delegatingWriterRegistry.update(node, dataAfter, dataBefore, ctx);
+                } catch (RuntimeException e) {
+                    throw new RuntimeException();
+                }
+            }
+        }
+    }
 }
index 4f02a1d..7691f55 100644 (file)
@@ -22,6 +22,10 @@ import javax.annotation.Nonnull;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
+/**
+ * Customizer not performing any changes on current level. Suitable for nodes that don't have any leaves and all of
+ * its child nodes are managed by dedicated writers
+ */
 public class NoopWriterCustomizer<D extends DataObject> implements RootVppWriterCustomizer<D> {
 
     @Override
index 7c1a63d..8efcc61 100644 (file)
@@ -20,9 +20,8 @@ import com.google.common.base.Optional;
 import com.google.common.util.concurrent.CheckedFuture;
 import io.fd.honeycomb.v3po.impl.trans.util.Context;
 import io.fd.honeycomb.v3po.impl.trans.w.WriteContext;
-import java.util.Collections;
-import java.util.List;
 import java.util.Map;
+import javax.annotation.Nonnull;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataReadOnlyTransaction;
@@ -32,7 +31,10 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 
-public class TransactionWriteContext implements WriteContext, AutoCloseable {
+/**
+ * Transaction based WriteContext
+ */
+public final class TransactionWriteContext implements WriteContext, AutoCloseable {
 
     private final DOMDataReadOnlyTransaction beforeTx;
     private final DOMDataReadOnlyTransaction afterTx;
@@ -48,39 +50,39 @@ public class TransactionWriteContext implements WriteContext, AutoCloseable {
         this.ctx = new Context();
     }
 
+    // TODO make this asynchronous
+
     @Override
-    public List<? extends DataObject> readBefore(final InstanceIdentifier<? extends DataObject> currentId) {
+    public Optional<DataObject> readBefore(@Nonnull final InstanceIdentifier<? extends DataObject> currentId) {
         return read(currentId, beforeTx);
     }
 
-    private List<? extends DataObject> read(final InstanceIdentifier<? extends DataObject> currentId,
+    private Optional<DataObject> read(final InstanceIdentifier<? extends DataObject> currentId,
                                             final DOMDataReadOnlyTransaction tx) {
-        // FIXME how to read all for list (using wildcarded ID) ?
-
         final YangInstanceIdentifier path = serializer.toYangInstanceIdentifier(currentId);
 
         final CheckedFuture<Optional<NormalizedNode<?, ?>>, ReadFailedException> read =
                 tx.read(LogicalDatastoreType.CONFIGURATION, path);
 
         try {
+            // TODO once the APIs are asynchronous use just Futures.transform
             final Optional<NormalizedNode<?, ?>> optional = read.checkedGet();
 
             if (!optional.isPresent()) {
-                return Collections.<DataObject>emptyList();
+                return Optional.absent();
             }
 
             final NormalizedNode<?, ?> data = optional.get();
-            final Map.Entry<InstanceIdentifier<?>, DataObject> entry =
-                    serializer.fromNormalizedNode(path, data);
+            final Map.Entry<InstanceIdentifier<?>, DataObject> entry = serializer.fromNormalizedNode(path, data);
 
-            return Collections.singletonList(entry.getValue());
+            return Optional.of(entry.getValue());
         } catch (ReadFailedException e) {
             throw new IllegalStateException("Unable to perform read", e);
         }
     }
 
     @Override
-    public List<? extends DataObject> readAfter(final InstanceIdentifier<? extends DataObject> currentId) {
+    public Optional<DataObject> readAfter(@Nonnull final InstanceIdentifier<? extends DataObject> currentId) {
         return read(currentId, afterTx);
     }
 
index 07193a1..8784175 100644 (file)
@@ -65,8 +65,7 @@ public final class BridgeDomainCustomizer extends VppApiCustomizer
 
         builder.setInterface(getIfcs(bridgeDomainDetails));
 
-        // final vppL2Fib[] vppL2Fibs = getVppApi().l2FibTableDump(bdId); FIXME we need writer for L2Fib
-        final vppL2Fib[] vppL2Fibs = getL2Fibs(bdId);
+        final vppL2Fib[] vppL2Fibs = getVppApi().l2FibTableDump(bdId);
 
         final List<L2Fib> l2Fibs = Lists.newArrayListWithCapacity(vppL2Fibs.length);
         for (vppL2Fib vppL2Fib : vppL2Fibs) {
@@ -83,22 +82,6 @@ public final class BridgeDomainCustomizer extends VppApiCustomizer
         builder.setL2Fib(l2Fibs);
     }
 
-    // FIXME remove when list read is implemented
-    // updating L2Fib was BD was is implemented
-    // this was added to test reading list
-    private vppL2Fib[] getL2Fibs(final int bdId) {
-        if (bdId == 0) {
-            return new vppL2Fib[]{
-                    new vppL2Fib(new byte[]{1, 2, 3, 4, 5, 6}, true, "ifc1", true, true)
-            };
-        } else {
-            return new vppL2Fib[]{
-                    new vppL2Fib(new byte[]{1, 2, 3, 4, 5, 6}, true, "ifc1", true, true),
-                    new vppL2Fib(new byte[]{2, 2, 3, 4, 5, 6}, true, "ifc2", true, true),
-            };
-        }
-    }
-
     private static String getMacAddress(byte[] mac) {
         StringBuilder sb = new StringBuilder(18);
         for (byte b : mac) {
index 6aa9da5..963df73 100644 (file)
 
 package io.fd.honeycomb.v3po.impl.data;
 
-import static java.util.Collections.singletonList;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.anyListOf;
+import static org.mockito.Matchers.anyMap;
 import static org.mockito.Matchers.eq;
-import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.mockito.MockitoAnnotations.initMocks;
 
 import com.google.common.base.Optional;
 import com.google.common.util.concurrent.CheckedFuture;
+import io.fd.honeycomb.v3po.impl.trans.VppException;
 import io.fd.honeycomb.v3po.impl.trans.w.WriteContext;
-import io.fd.honeycomb.v3po.impl.trans.VppApiInvocationException;
-import java.util.AbstractMap;
+import io.fd.honeycomb.v3po.impl.trans.w.WriterRegistry;
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import org.junit.Before;
 import org.junit.Test;
-import org.mockito.Matchers;
 import org.mockito.Mock;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
 import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
 import org.opendaylight.mdsal.binding.dom.codec.api.BindingNormalizedNodeSerializer;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.v3po.rev150105.interfaces._interface.Ethernet;
@@ -121,8 +115,8 @@ public class VPPConfigDataTreeTest {
 
     @Test
     public void testCommitSuccessful() throws Exception {
-        final DataObject dataBefore = mock(Ethernet.class);
-        final DataObject dataAfter = mock(Ethernet.class);
+        final DataObject dataBefore = mockDataObject("before", Ethernet.class);
+        final DataObject dataAfter = mockDataObject("after", Ethernet.class);
 
         // Prepare modification:
         final DataTreeCandidateNode rootNode = mockRootNode();
@@ -137,30 +131,42 @@ public class VPPConfigDataTreeTest {
         proxy.commit(modification);
 
         // Verify all changes were processed:
-        verify(vppWriter).update(Matchers.<InstanceIdentifier<?>>any(), eq(singletonList(dataBefore)),
-            eq(singletonList(dataAfter)), any(WriteContext.class));
+        verify(vppWriter).update(
+            mapOf(dataBefore, Ethernet.class),
+            mapOf(dataAfter, Ethernet.class),
+            any(WriteContext.class));
 
         // Verify modification was validated
         verify(dataTree).validate(modification);
     }
 
+    private Map<InstanceIdentifier<?>, DataObject> mapOf(final DataObject dataBefore, final Class<Ethernet> type) {
+        return eq(
+            Collections.<InstanceIdentifier<?>, DataObject>singletonMap(InstanceIdentifier.create(type), dataBefore));
+    }
+
+    private DataObject mockDataObject(final String name, final Class<? extends DataObject> classToMock) {
+        final DataObject dataBefore = mock(classToMock, name);
+        doReturn(classToMock).when(dataBefore).getImplementedInterface();
+        return dataBefore;
+    }
+
     @Test
     public void testCommitUndoSuccessful() throws Exception {
         // Prepare data changes:
-        final DataObject dataBefore1 = mock(Ethernet.class);
-        final DataObject dataAfter1 = mock(Ethernet.class);
+        final DataObject dataBefore1 = mockDataObject("before", Ethernet.class);
+        final DataObject dataAfter1 = mockDataObject("after", Ethernet.class);
 
-        final DataObject dataBefore2 = mock(Vxlan.class);
-        final DataObject dataAfter2 = mock(Vxlan.class);
+        final DataObject dataBefore2 = mockDataObject("before", Vxlan.class);
+        final DataObject dataAfter2 = mockDataObject("after", Vxlan.class);
 
-        final DataObject dataBefore3 = mock(L2.class);
-        final DataObject dataAfter3 = mock(L2.class);
+        final DataObject dataBefore3 = mockDataObject("before", L2.class);
+        final DataObject dataAfter3 = mockDataObject("after", L2.class);
 
-        final List<Map.Entry<DataObject, DataObject>> processedChanges = new ArrayList<>();
         // reject third applied change
-        final Answer answer = new VppWriterAnswer(processedChanges, Arrays.asList(1,2), singletonList(3));
-        doAnswer(answer).when(vppWriter).update(Matchers.<InstanceIdentifier<?>>any(), anyListOf(DataObject.class),
-            anyListOf(DataObject.class), any(WriteContext.class));
+        final WriterRegistry.Revert revert = mock(WriterRegistry.Revert.class);
+        doThrow(new WriterRegistry.BulkUpdateException(InstanceIdentifier.create(L2.class), new RuntimeException(),
+            revert)).when(vppWriter).update(anyMap(), anyMap(), any(WriteContext.class));
 
         // Prepare modification:
         final DataTreeCandidateNode rootNode = mockRootNode();
@@ -174,23 +180,9 @@ public class VPPConfigDataTreeTest {
         // Run the test
         try {
             proxy.commit(modification);
-        } catch (DataValidationFailedException | VppApiInvocationException e) {
-            // verify that all changes were processed:
-            verify(vppWriter).update(Matchers.<InstanceIdentifier<?>>any(), eq(singletonList(dataBefore1)),
-                eq(singletonList(dataAfter1)), any(WriteContext.class));
-            verify(vppWriter).update(Matchers.<InstanceIdentifier<?>>any(), eq(singletonList(dataBefore2)),
-                eq(singletonList(dataAfter2)), any(WriteContext.class));
-            verify(vppWriter).update(Matchers.<InstanceIdentifier<?>>any(), eq(singletonList(dataBefore3)),
-                eq(singletonList(dataAfter3)), any(WriteContext.class));
-
-            // verify that only two changes were processed successfully:
-            assertEquals(2, processedChanges.size());
-
-            // verify that successful changes were undone
-            for (final Map.Entry<DataObject, DataObject> change : processedChanges) {
-                verify(vppWriter).update(Matchers.<InstanceIdentifier<?>>any(), eq(singletonList(change.getValue())),
-                    eq(singletonList(change.getKey())), any(WriteContext.class));
-            }
+        } catch (DataValidationFailedException | VppException e) {
+            verify(vppWriter).update(anyMap(), anyMap(), any(WriteContext.class));
+            verify(revert).revert();
             return;
         }
 
@@ -200,20 +192,20 @@ public class VPPConfigDataTreeTest {
     @Test
     public void testCommitUndoFailed() throws Exception {
         // Prepare data changes:
-        final DataObject dataBefore1 = mock(Ethernet.class);
-        final DataObject dataAfter1 = mock(Ethernet.class);
+        final DataObject dataBefore1 = mockDataObject("before", Ethernet.class);
+        final DataObject dataAfter1 = mockDataObject("after", Ethernet.class);
 
-        final DataObject dataBefore2 = mock(Vxlan.class);
-        final DataObject dataAfter2 = mock(Vxlan.class);
+        final DataObject dataBefore2 = mockDataObject("before", Vxlan.class);
+        final DataObject dataAfter2 = mockDataObject("after", Vxlan.class);
 
-        final DataObject dataBefore3 = mock(L2.class);
-        final DataObject dataAfter3 = mock(L2.class);
+        final DataObject dataBefore3 = mockDataObject("before", L2.class);
+        final DataObject dataAfter3 = mockDataObject("after", L2.class);
 
-        // reject third applied change and fourth (first undo):
-        final List<Map.Entry<DataObject, DataObject>> processedChanges = new ArrayList<>();
-        final Answer answer = new VppWriterAnswer(processedChanges, Arrays.asList(1,2), Arrays.asList(3,4));
-        doAnswer(answer).when(vppWriter).update(Matchers.<InstanceIdentifier<?>>any(), anyListOf(DataObject.class),
-            anyListOf(DataObject.class), any(WriteContext.class));
+        // reject third applied change
+        final WriterRegistry.Revert revert = mock(WriterRegistry.Revert.class);
+        doThrow(new RuntimeException("revert failed")).when(revert).revert();
+        doThrow(new WriterRegistry.BulkUpdateException(InstanceIdentifier.create(L2.class), new RuntimeException(),
+            revert)).when(vppWriter).update(anyMap(), anyMap(), any(WriteContext.class));
 
         // Prepare modification:
         final DataTreeCandidateNode rootNode = mockRootNode();
@@ -227,27 +219,10 @@ public class VPPConfigDataTreeTest {
         // Run the test
         try {
             proxy.commit(modification);
-        } catch (DataValidationFailedException | VppApiInvocationException e) {
-            // verify that all changes were processed:
-            verify(vppWriter).update(Matchers.<InstanceIdentifier<?>>any(), eq(singletonList(dataBefore1)),
-                eq(singletonList(dataAfter1)), any(WriteContext.class));
-            verify(vppWriter).update(Matchers.<InstanceIdentifier<?>>any(), eq(singletonList(dataBefore2)),
-                eq(singletonList(dataAfter2)), any(WriteContext.class));
-            verify(vppWriter).update(Matchers.<InstanceIdentifier<?>>any(), eq(singletonList(dataBefore3)),
-                eq(singletonList(dataAfter3)), any(WriteContext.class));
-
-            // verify that only two changes were processed successfully:
-            assertEquals(2, processedChanges.size());
-
-            // verify we tried to undo the last successful change:
-            Map.Entry<DataObject, DataObject> change = processedChanges.get(1);
-            verify(vppWriter).update(Matchers.<InstanceIdentifier<?>>any(), eq(singletonList(change.getValue())),
-                eq(singletonList(change.getKey())), any(WriteContext.class));
-
-            // but failed, and did not try to undo the first:
-            change = processedChanges.get(0);
-            verify(vppWriter, never()).update(Matchers.<InstanceIdentifier<?>>any(), eq(singletonList(change.getValue())),
-                eq(singletonList(change.getKey())), any(WriteContext.class));
+        } catch (DataValidationFailedException | VppException e) {
+            // FIXME the behavior with successful and failed revert is the same from outside world
+            verify(vppWriter).update(anyMap(), anyMap(), any(WriteContext.class));
+            verify(revert).revert();
             return;
         }
 
@@ -282,7 +257,9 @@ public class VPPConfigDataTreeTest {
             list.add(child);
 
             final Map.Entry entry  = mock(Map.Entry.class);
-            final InstanceIdentifier<?> id = InstanceIdentifier.create(modification.getClass());
+            final Class<? extends DataObject> implementedInterface =
+                (Class<? extends DataObject>) modification.getImplementedInterface();
+            final InstanceIdentifier<?> id = InstanceIdentifier.create(implementedInterface);
 
             doReturn(id).when(entry).getKey();
             doReturn(modification).when(entry).getValue();
@@ -291,33 +268,4 @@ public class VPPConfigDataTreeTest {
         return node;
     }
 
-    private static final class VppWriterAnswer implements Answer {
-        private final List<Map.Entry<DataObject, DataObject>> capturedChanges;
-        private final Collection<Integer> toCapture;
-        private final Collection<Integer> toReject;
-        private int count = 0;
-
-        private VppWriterAnswer(final List<Map.Entry<DataObject, DataObject>> capturedChanges,
-                                final Collection<Integer> toCapture,
-                                final Collection<Integer> toReject) {
-            this.capturedChanges = capturedChanges;
-            this.toCapture = toCapture;
-            this.toReject = toReject;
-        }
-
-        @Override
-        public Object answer(final InvocationOnMock invocation) throws Throwable {
-            ++count;
-            if (toCapture.contains(count)) {
-                final DataObject dataBefore = (DataObject) ((List)invocation.getArguments()[1]).get(0);
-                final DataObject dataAfter = (DataObject) ((List)invocation.getArguments()[2]).get(0);
-                capturedChanges.add(new AbstractMap.SimpleImmutableEntry<>(dataBefore, dataAfter));
-            }
-            if (toReject.contains(count)) {
-                throw mock(RuntimeException.class);
-            }
-            return null;
-        }
-    }
-
 }
index 1a6cf3a..0f906d2 100644 (file)
@@ -14,7 +14,6 @@ import static org.mockito.MockitoAnnotations.initMocks;
 import com.google.common.base.Optional;
 import com.google.common.util.concurrent.CheckedFuture;
 import io.fd.honeycomb.v3po.impl.trans.util.Context;
-import java.util.List;
 import java.util.Map;
 import org.junit.Before;
 import org.junit.Test;
@@ -64,9 +63,9 @@ public class TransactionWriteContextTest {
         final InstanceIdentifier<BridgeDomain> instanceId =
                 InstanceIdentifier.create(Vpp.class).child(BridgeDomains.class).child(BridgeDomain.class);
 
-        final List<? extends DataObject> dataObjects = transactionWriteContext.readBefore(instanceId);
+        final Optional<DataObject> dataObjects = transactionWriteContext.readBefore(instanceId);
         assertNotNull(dataObjects);
-        assertTrue(dataObjects.isEmpty());
+        assertFalse(dataObjects.isPresent());
 
         verify(serializer).toYangInstanceIdentifier(instanceId);
         verify(serializer, never()).fromNormalizedNode(any(YangInstanceIdentifier.class), any(NormalizedNode.class));
@@ -85,10 +84,11 @@ public class TransactionWriteContextTest {
                 BridgeDomains.QNAME).node(BridgeDomain.QNAME).build();
         when(serializer.toYangInstanceIdentifier(any(InstanceIdentifier.class))).thenReturn(yangId);
         when(serializer.fromNormalizedNode(eq(yangId), any(NormalizedNode.class))).thenReturn(entry);
+        when(entry.getValue()).thenReturn(mock(DataObject.class));
 
-        final List<? extends DataObject> dataObjects = transactionWriteContext.readBefore(instanceId);
+        final Optional<DataObject> dataObjects = transactionWriteContext.readBefore(instanceId);
         assertNotNull(dataObjects);
-        assertFalse(dataObjects.isEmpty());
+        assertTrue(dataObjects.isPresent());
 
         verify(serializer).toYangInstanceIdentifier(instanceId);
         verify(serializer).fromNormalizedNode(eq(yangId), any(NormalizedNode.class));
index bf7b9de..c9533a0 100644 (file)
@@ -82,20 +82,31 @@ public class VppTest {
     public void writeVpp() throws Exception {
         rootRegistry.update(
             InstanceIdentifier.create(Vpp.class),
-            Collections.<DataObject>emptyList(),
-            Lists.newArrayList(new VppBuilder().setBridgeDomains(getBridgeDomains("bdn1")).build()),
+            null,
+            new VppBuilder().setBridgeDomains(getBridgeDomains("bdn1")).build(),
             ctx);
 
         verify(api).bridgeDomainAddDel(1, flood, forward, learn, uuf, arpTerm, add);
 
         vppWriter.update(InstanceIdentifier.create(Vpp.class),
-            Collections.<DataObject>emptyList(),
-            Lists.newArrayList(new VppBuilder().setBridgeDomains(getBridgeDomains("bdn1")).build()),
+            null,
+            new VppBuilder().setBridgeDomains(getBridgeDomains("bdn1")).build(),
             ctx);
 
         verify(api, times(2)).bridgeDomainAddDel(1, flood, forward, learn, uuf, arpTerm, add);
     }
 
+    @Test
+    public void writeVppFromRoot() throws Exception {
+        final Vpp vpp = new VppBuilder().setBridgeDomains(getBridgeDomains("bdn1")).build();
+
+        rootRegistry.update(Collections.<InstanceIdentifier<?>, DataObject>emptyMap(),
+            Collections.<InstanceIdentifier<?>, DataObject>singletonMap(InstanceIdentifier.create(Vpp.class),
+                vpp), ctx);
+
+        verify(api).bridgeDomainAddDel(1, flood, forward, learn, uuf, arpTerm, add);
+    }
+
     private BridgeDomains getBridgeDomains(String... name) {
         final List<BridgeDomain> bdmns = Lists.newArrayList();
         for (String s : name) {
@@ -116,8 +127,8 @@ public class VppTest {
     public void deleteVpp() throws Exception {
         rootRegistry.update(
             InstanceIdentifier.create(Vpp.class),
-            Collections.singletonList(new VppBuilder().setBridgeDomains(getBridgeDomains("bdn1")).build()),
-            Collections.<DataObject>emptyList(),
+            new VppBuilder().setBridgeDomains(getBridgeDomains("bdn1")).build(),
+            null,
             ctx);
 
         final byte zero = (byte) 0;
@@ -129,26 +140,33 @@ public class VppTest {
     public void updateVppNoActualChange() throws Exception {
         rootRegistry.update(
             InstanceIdentifier.create(Vpp.class),
-            Collections.singletonList(new VppBuilder().setBridgeDomains(getBridgeDomains("bdn1")).build()),
-            Collections.singletonList(new VppBuilder().setBridgeDomains(getBridgeDomains("bdn1")).build()),
+            new VppBuilder().setBridgeDomains(getBridgeDomains("bdn1")).build(),
+            new VppBuilder().setBridgeDomains(getBridgeDomains("bdn1")).build(),
             ctx);
 
         verifyZeroInteractions(api);
     }
 
     @Test
-    public void writeBridgeDomain() throws Exception {
+    public void writeUpdate() throws Exception {
+        final BridgeDomains domainsBefore = getBridgeDomains("bdn1");
+        final BridgeDomain bdn1Before = domainsBefore.getBridgeDomain().get(0);
+
+        final BridgeDomain bdn1After = new BridgeDomainBuilder(bdn1Before).setFlood(!bdn1Before.isFlood()).build();
+        final BridgeDomains domainsAfter = new BridgeDomainsBuilder()
+            .setBridgeDomain(Collections.singletonList(bdn1After))
+            .build();
+
         rootRegistry.update(
-            InstanceIdentifier.create(Vpp.class).child(BridgeDomains.class).child(BridgeDomain.class),
-            getBridgeDomains("bdn1", "bdn2").getBridgeDomain(),
-            getBridgeDomains("bdn1", "bdn3").getBridgeDomain(),
+            InstanceIdentifier.create(Vpp.class),
+            new VppBuilder().setBridgeDomains(domainsBefore).build(),
+            new VppBuilder().setBridgeDomains(domainsAfter).build(),
             ctx);
 
-        // bdn1 is untouched
-        // bdn3 is added
-        verify(api).bridgeDomainAddDel(3, flood, forward, learn, uuf, arpTerm, add);
-        // bdn2 is deleted
-        verify(api).bridgeDomainAddDel(2, zero, zero, zero, zero, zero, zero);
+        final int bdn1Id = 1;
+
+        // bdn1 is created with negated flood value
+        verify(api).bridgeDomainAddDel(bdn1Id, (byte) (flood ^ 1), forward, learn, uuf, arpTerm, add);
     }
 
     // TODO test unkeyed list