diff options
| author |   Maros Marsalek <mmarsale@cisco.com> | 2016-04-12 10:12:46 +0200 |
|---|---|---|
| committer | Maros Marsalek <mmarsale@cisco.com> | 2016-04-12 10:12:46 +0200 |
| commit | 8d2528dc50ce506964ca6792d5c3e14477b4cd5b (patch) | |
| tree | c7fbf07c2286d941bf4dc915eafe6b7684300b3c | |
| parent | 3362d3a7c877e8fb597f2e619c0ca3dded301a6c (diff) | |
HONEYCOMB-9: Simplify writer APIs, remove list of DataObjects
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, 606 insertions, 401 deletions
diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppConfigDataTree.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppConfigDataTree.java index 6fdeea348..f14bec3aa 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppConfigDataTree.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppConfigDataTree.java @@ -17,21 +17,16 @@ 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); - } - } - } - } } diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppDataTree.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppDataTree.java index a3a4fae65..7fd9b2e3a 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppDataTree.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppDataTree.java @@ -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. diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppWriteTransaction.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppWriteTransaction.java index b7aa2f854..299898bd3 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppWriteTransaction.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppWriteTransaction.java @@ -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( diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppWriterRegistry.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppWriterRegistry.java index 67b45cfd2..83186c2de 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppWriterRegistry.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppWriterRegistry.java @@ -16,10 +16,12 @@ 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); + } } diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/VppApiInvocationException.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/VppApiInvocationException.java index b0076cd9c..0bb7c2b19 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/VppApiInvocationException.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/VppApiInvocationException.java @@ -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 index 000000000..aa18ae705 --- /dev/null +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/VppException.java @@ -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); + } +} diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/ChildVppReaderCustomizer.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/ChildVppReaderCustomizer.java index e462118aa..900f8f8c6 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/ChildVppReaderCustomizer.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/ChildVppReaderCustomizer.java @@ -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 diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/ListVppReaderCustomizer.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/ListVppReaderCustomizer.java index 5646ab992..694f21c9d 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/ListVppReaderCustomizer.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/ListVppReaderCustomizer.java @@ -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 diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/RootVppReaderCustomizer.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/RootVppReaderCustomizer.java index b6e1155b3..a09ed0488 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/RootVppReaderCustomizer.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/spi/RootVppReaderCustomizer.java @@ -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>> { diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/ChildVppWriter.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/ChildVppWriter.java index 93c1092b6..243a6528d 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/ChildVppWriter.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/ChildVppWriter.java @@ -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); } diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/VppWriter.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/VppWriter.java index 617e11cc1..f8a49a271 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/VppWriter.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/VppWriter.java @@ -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; } diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/WriteContext.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/WriteContext.java index 49759f20f..3aaf83250 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/WriteContext.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/WriteContext.java @@ -16,16 +16,42 @@ 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 index 000000000..4b09ff29e --- /dev/null +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/WriterRegistry.java @@ -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; + } +} diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/AbstractCompositeVppWriter.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/AbstractCompositeVppWriter.java index 44690bd28..6e8f8bc85 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/AbstractCompositeVppWriter.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/AbstractCompositeVppWriter.java @@ -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; diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeChildVppWriter.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeChildVppWriter.java index 919bbaa0f..d94e4fedf 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeChildVppWriter.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeChildVppWriter.java @@ -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; } diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeListVppWriter.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeListVppWriter.java index 8914d3757..1722b4652 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeListVppWriter.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeListVppWriter.java @@ -16,11 +16,16 @@ 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 diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeRootVppWriter.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeRootVppWriter.java index 6be5651b3..a7139e57a 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeRootVppWriter.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/CompositeRootVppWriter.java @@ -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, diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/ChildVppWriterCustomizer.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/ChildVppWriterCustomizer.java index 5ea504209..1e79c6830 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/ChildVppWriterCustomizer.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/ChildVppWriterCustomizer.java @@ -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); diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/ListVppWriterCustomizer.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/ListVppWriterCustomizer.java index edd8de930..6e72fc719 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/ListVppWriterCustomizer.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/ListVppWriterCustomizer.java @@ -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); diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/RootVppWriterCustomizer.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/RootVppWriterCustomizer.java index 6a2d0c2bf..0fa89d2af 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/RootVppWriterCustomizer.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/impl/spi/RootVppWriterCustomizer.java @@ -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); diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/DelegatingWriterRegistry.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/DelegatingWriterRegistry.java index 6df960626..d20e69a8b 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/DelegatingWriterRegistry.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/DelegatingWriterRegistry.java @@ -16,39 +16,60 @@ 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(); + } + } + } + } } diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/NoopWriterCustomizer.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/NoopWriterCustomizer.java index 4f02a1d5e..7691f558b 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/NoopWriterCustomizer.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/NoopWriterCustomizer.java @@ -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 diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/TransactionWriteContext.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/TransactionWriteContext.java index 7c1a63da6..8efcc6189 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/TransactionWriteContext.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/w/util/TransactionWriteContext.java @@ -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); } diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/vppstate/BridgeDomainCustomizer.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/vppstate/BridgeDomainCustomizer.java index 07193a16c..8784175b4 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/vppstate/BridgeDomainCustomizer.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/vppstate/BridgeDomainCustomizer.java @@ -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) { diff --git a/v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/data/VPPConfigDataTreeTest.java b/v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/data/VPPConfigDataTreeTest.java index 6aa9da57c..963df735b 100644 --- a/v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/data/VPPConfigDataTreeTest.java +++ b/v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/data/VPPConfigDataTreeTest.java @@ -16,37 +16,31 @@ 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; - } - } - } diff --git a/v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/trans/w/util/TransactionWriteContextTest.java b/v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/trans/w/util/TransactionWriteContextTest.java index 1a6cf3a5b..0f906d20c 100644 --- a/v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/trans/w/util/TransactionWriteContextTest.java +++ b/v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/trans/w/util/TransactionWriteContextTest.java @@ -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)); diff --git a/v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/vpp/VppTest.java b/v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/vpp/VppTest.java index bf7b9de4b..c9533a0e7 100644 --- a/v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/vpp/VppTest.java +++ b/v3po/impl/src/test/java/io/fd/honeycomb/v3po/impl/vpp/VppTest.java @@ -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 |