From 9ac68bac54d95b0342cab52bf39a4321f1f42d79 Mon Sep 17 00:00:00 2001 From: Maros Marsalek Date: Tue, 12 Apr 2016 10:12:58 +0200 Subject: HONEYCOMB-9: Simplify reader APIs, remove list of DataObjects Change-Id: I0cb3f20ef4595b0143dcc7e0ad5475f121a9cc86 Signed-off-by: Maros Marsalek --- .../v3po/impl/data/VppOperationalDataTree.java | 17 ++-- .../v3po/impl/data/VppReaderRegistry.java | 3 +- .../honeycomb/v3po/impl/trans/r/ListVppReader.java | 44 +++++++++ .../fd/honeycomb/v3po/impl/trans/r/VppReader.java | 4 +- .../trans/r/impl/AbstractCompositeVppReader.java | 105 ++++++++++----------- .../impl/trans/r/impl/CompositeChildVppReader.java | 3 +- .../impl/trans/r/impl/CompositeListVppReader.java | 25 ++--- .../trans/r/util/DelegatingReaderRegistry.java | 21 ++++- 8 files changed, 132 insertions(+), 90 deletions(-) create mode 100644 v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/ListVppReader.java (limited to 'v3po/impl/src/main/java/io/fd') diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppOperationalDataTree.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppOperationalDataTree.java index c378365f3..3fddd108c 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppOperationalDataTree.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppOperationalDataTree.java @@ -28,7 +28,6 @@ import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; import io.fd.honeycomb.v3po.impl.trans.r.ReaderRegistry; import java.util.Collection; -import java.util.List; import java.util.Map; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -90,21 +89,17 @@ public final class VppOperationalDataTree implements ReadableVppDataTree { LOG.debug("VppOperationalDataProxy.read(), yangInstanceIdentifier={}", yangInstanceIdentifier); final InstanceIdentifier path = serializer.fromYangInstanceIdentifier(yangInstanceIdentifier); - if (path == null) { - // TODO try to translate wildcarded identifiers here as a workaround if it is expected to be used that way - // Currently its not possible to read list using wildcarded ID. SO we may not need this at all. - } checkNotNull(path, "Invalid instance identifier %s. Cannot create BA equivalent.", yangInstanceIdentifier); LOG.debug("VppOperationalDataProxy.read(), path={}", path); - final List dataObjects = readerRegistry.read(path); + final Optional dataObject = readerRegistry.read(path); - if (dataObjects.isEmpty()) { - return Futures.immediateCheckedFuture(Optional.>absent()); + if (dataObject.isPresent()) { + final NormalizedNode value = toNormalizedNodeFunction(path).apply(dataObject.get()); + return Futures.immediateCheckedFuture(Optional.>fromNullable(value)); } - final NormalizedNode value = wrapDataObjects(yangInstanceIdentifier, path, dataObjects); - return Futures.immediateCheckedFuture(Optional.>fromNullable(value)); + return Futures.immediateCheckedFuture(Optional.>absent()); } private DataSchemaNode getSchemaNode(final @Nonnull YangInstanceIdentifier yangInstanceIdentifier) { @@ -175,7 +170,7 @@ public final class VppOperationalDataTree implements ReadableVppDataTree { public NormalizedNode apply(@Nullable final DataObject dataObject) { LOG.trace("VppOperationalDataProxy.toNormalizedNode(), path={}, dataObject={}", path, dataObject); final Map.Entry> entry = - serializer.toNormalizedNode(path, dataObject); + serializer.toNormalizedNode(path, dataObject); LOG.trace("VppOperationalDataProxy.toNormalizedNode(), normalizedNodeEntry={}", entry); return entry.getValue(); diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppReaderRegistry.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppReaderRegistry.java index c5d4a8194..df7b6568c 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppReaderRegistry.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/data/VppReaderRegistry.java @@ -16,6 +16,7 @@ package io.fd.honeycomb.v3po.impl.data; +import com.google.common.base.Optional; import com.google.common.collect.Multimap; import io.fd.honeycomb.v3po.impl.trans.r.ChildVppReader; import io.fd.honeycomb.v3po.impl.trans.r.ReaderRegistry; @@ -100,7 +101,7 @@ public class VppReaderRegistry implements ReaderRegistry { @Nonnull @Override - public List read(@Nonnull final InstanceIdentifier id) { + public Optional read(@Nonnull final InstanceIdentifier id) { return reader.read(id); } diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/ListVppReader.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/ListVppReader.java new file mode 100644 index 000000000..8a7f29cb3 --- /dev/null +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/ListVppReader.java @@ -0,0 +1,44 @@ +/* + * 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.r; + +import com.google.common.annotations.Beta; +import java.util.List; +import javax.annotation.Nonnull; +import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.Identifiable; +import org.opendaylight.yangtools.yang.binding.Identifier; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; + +/** + * List VPP reader, allowing read of all the elements + * + * @param Specific DataObject derived type, that is handled by this reader + */ +@Beta +public interface ListVppReader, K extends Identifier> extends VppReader { + + /** + * Read all elements in this list + * + * @param id Wildcarded identifier of list managed by this reader + * + * @return List of all entries in this list + */ + @Nonnull + List readList(@Nonnull final InstanceIdentifier id); +} diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/VppReader.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/VppReader.java index 8af493854..1cc76a3a4 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/VppReader.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/VppReader.java @@ -17,8 +17,8 @@ package io.fd.honeycomb.v3po.impl.trans.r; import com.google.common.annotations.Beta; +import com.google.common.base.Optional; import io.fd.honeycomb.v3po.impl.trans.SubtreeManager; -import java.util.List; import javax.annotation.Nonnull; import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; @@ -44,6 +44,6 @@ public interface VppReader extends SubtreeManager { * @return List of DataObjects identified by id. If the ID points to a single node, it will be wrapped in a list */ @Nonnull - List read(@Nonnull final InstanceIdentifier id); + Optional read(@Nonnull final InstanceIdentifier id); } diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/AbstractCompositeVppReader.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/AbstractCompositeVppReader.java index 511192483..61f318b35 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/AbstractCompositeVppReader.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/AbstractCompositeVppReader.java @@ -16,11 +16,12 @@ package io.fd.honeycomb.v3po.impl.trans.r.impl; +import static com.google.common.base.Preconditions.checkArgument; + import com.google.common.annotations.Beta; import com.google.common.base.Optional; import com.google.common.base.Predicate; -import com.google.common.collect.Collections2; -import com.google.common.collect.Lists; +import com.google.common.collect.Iterables; import io.fd.honeycomb.v3po.impl.trans.r.ChildVppReader; import io.fd.honeycomb.v3po.impl.trans.r.VppReader; import io.fd.honeycomb.v3po.impl.trans.util.ReflectionUtils; @@ -67,7 +68,7 @@ abstract class AbstractCompositeVppReader readCurrent(final InstanceIdentifier id) { + protected Optional readCurrent(final InstanceIdentifier id) { LOG.debug("{}: Reading current: {}", this, id); final B builder = getBuilder(id); // Cache empty value to determine if anything has changed later TODO cache in a field @@ -89,9 +90,9 @@ abstract class AbstractCompositeVppReader read = built.equals(emptyValue) - ? Collections.emptyList() - : Collections.singletonList(built); + final Optional read = built.equals(emptyValue) + ? Optional.absent() + : Optional.of(built); LOG.debug("{}: Current node read successfully. Result: {}", this, read); return read; @@ -100,7 +101,7 @@ abstract class AbstractCompositeVppReader read(@Nonnull final InstanceIdentifier id) { + public Optional read(@Nonnull final InstanceIdentifier id) { LOG.trace("{}: Reading : {}", this, id); if (id.getTargetType().equals(getManagedDataObjectType().getTargetType())) { return readCurrent((InstanceIdentifier) id); @@ -109,7 +110,7 @@ abstract class AbstractCompositeVppReader readSubtree(final InstanceIdentifier id) { + private Optional readSubtree(final InstanceIdentifier id) { LOG.debug("{}: Reading subtree: {}", this, id); final Class next = VppRWUtils.getNextId(id, getManagedDataObjectType()).getType(); final ChildVppReader> vppReader = childReaders.get(next); @@ -121,11 +122,11 @@ abstract class AbstractCompositeVppReader currentId = VppRWUtils.cutId(id, getManagedDataObjectType()); - final List current = readCurrent(currentId); + final Optional current = readCurrent(currentId); // then perform post-reading filtering (return only requested sub-node) - final List readSubtree = current.isEmpty() - ? current - : filterSubtree(current, id, getManagedDataObjectType().getTargetType()); + final Optional readSubtree = current.isPresent() + ? filterSubtree(current.get(), id, getManagedDataObjectType().getTargetType()) + : current; LOG.debug("{}: Subtree: {} read successfully. Result: {}", this, id, readSubtree); return readSubtree; @@ -150,65 +151,59 @@ abstract class AbstractCompositeVppReader filterSubtree(@Nonnull final List built, + private static Optional filterSubtree(@Nonnull final DataObject parent, @Nonnull final InstanceIdentifier absolutPath, @Nonnull final Class managedType) { // TODO is there a better way than reflection ? e.g. convert into NN and filter out with a utility // FIXME this needs to be recursive. right now it expects only 1 additional element in ID + test - List filtered = Lists.newArrayList(); - for (DataObject parent : built) { - final InstanceIdentifier.PathArgument nextId = - VppRWUtils.getNextId(absolutPath, InstanceIdentifier.create(parent.getClass())); + final InstanceIdentifier.PathArgument nextId = + VppRWUtils.getNextId(absolutPath, InstanceIdentifier.create(parent.getClass())); + + Optional method = ReflectionUtils.findMethodReflex(managedType, "get", + Collections.>emptyList(), nextId.getType()); - Optional method = ReflectionUtils.findMethodReflex(managedType, "get", - Collections.>emptyList(), nextId.getType()); + if (method.isPresent()) { + return Optional.fromNullable(filterSingle(parent, nextId, method.get())); + } else { + // List child nodes + method = ReflectionUtils.findMethodReflex(managedType, + "get" + nextId.getType().getSimpleName(), Collections.>emptyList(), List.class); if (method.isPresent()) { - filterSingle(filtered, parent, nextId, method); + return filterList(parent, nextId, method.get()); } else { - // List child nodes - method = ReflectionUtils.findMethodReflex(managedType, - "get" + nextId.getType().getSimpleName(), Collections.>emptyList(), List.class); - - if (method.isPresent()) { - filterList(filtered, parent, nextId, method); - } else { - throw new IllegalStateException( - "Unable to filter " + nextId + " from " + parent + " getters not found using reflexion"); - } + throw new IllegalStateException( + "Unable to filter " + nextId + " from " + parent + " getters not found using reflexion"); } } - - return filtered; } @SuppressWarnings("unchecked") - private static void filterList(final List filtered, final DataObject parent, - final InstanceIdentifier.PathArgument nextId, final Optional method) { - final List invoke = (List)invoke(method.get(), nextId, parent); - - if (nextId instanceof InstanceIdentifier.IdentifiableItem) { - final Identifier key = ((InstanceIdentifier.IdentifiableItem) nextId).getKey(); - filtered.addAll(Collections2.filter(invoke, new Predicate() { - @Override - public boolean apply(@Nullable final DataObject input) { - final Optional keyGetter = - ReflectionUtils.findMethodReflex(nextId.getType(), "get", - Collections.>emptyList(), key.getClass()); - final Object actualKey; - actualKey = invoke(keyGetter.get(), nextId, input); - return key.equals(actualKey); - } - })); - } else { - filtered.addAll(invoke); - } + private static Optional filterList(final DataObject parent, + final InstanceIdentifier.PathArgument nextId, + final Method method) { + final List invoke = (List) invoke(method, nextId, parent); + + checkArgument(nextId instanceof InstanceIdentifier.IdentifiableItem, + "Unable to perform wildcarded read for %s", nextId); + final Identifier key = ((InstanceIdentifier.IdentifiableItem) nextId).getKey(); + return Iterables.tryFind(invoke, new Predicate() { + @Override + public boolean apply(@Nullable final DataObject input) { + final Optional keyGetter = + ReflectionUtils.findMethodReflex(nextId.getType(), "get", + Collections.>emptyList(), key.getClass()); + final Object actualKey; + actualKey = invoke(keyGetter.get(), nextId, input); + return key.equals(actualKey); + } + }); } - private static void filterSingle(final List filtered, final DataObject parent, - final InstanceIdentifier.PathArgument nextId, final Optional method) { - filtered.add(nextId.getType().cast(invoke(method.get(), nextId, parent))); + private static DataObject filterSingle(final DataObject parent, + final InstanceIdentifier.PathArgument nextId, final Method method) { + return nextId.getType().cast(invoke(method, nextId, parent)); } private static Object invoke(final Method method, diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/CompositeChildVppReader.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/CompositeChildVppReader.java index 8f425b09f..a64a72bc4 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/CompositeChildVppReader.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/CompositeChildVppReader.java @@ -79,8 +79,7 @@ public final class CompositeChildVppReader parentId, @Nonnull final Builder parentBuilder) { - final Optional read = Optional.fromNullable(readCurrent(VppRWUtils.appendTypeToId(parentId, - getManagedDataObjectType())).get(0)); + final Optional read = readCurrent(VppRWUtils.appendTypeToId(parentId, getManagedDataObjectType())); if(read.isPresent()) { customizer.merge(parentBuilder, read.get()); diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/CompositeListVppReader.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/CompositeListVppReader.java index 8d23aa4e0..7e3d8452e 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/CompositeListVppReader.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/impl/CompositeListVppReader.java @@ -17,10 +17,11 @@ package io.fd.honeycomb.v3po.impl.trans.r.impl; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkState; import com.google.common.annotations.Beta; +import com.google.common.base.Optional; import io.fd.honeycomb.v3po.impl.trans.r.ChildVppReader; +import io.fd.honeycomb.v3po.impl.trans.r.ListVppReader; import io.fd.honeycomb.v3po.impl.trans.r.impl.spi.ListVppReaderCustomizer; import io.fd.honeycomb.v3po.impl.trans.util.VppRWUtils; import java.util.ArrayList; @@ -47,7 +48,8 @@ import org.slf4j.LoggerFactory; @Beta @ThreadSafe public final class CompositeListVppReader, K extends Identifier, B extends Builder> - extends AbstractCompositeVppReader implements ChildVppReader { + extends AbstractCompositeVppReader implements ChildVppReader, ListVppReader +{ private static final Logger LOG = LoggerFactory.getLogger(CompositeListVppReader.class); @@ -88,11 +90,6 @@ public final class CompositeListVppReader customizer); } - @Override - protected List readCurrent(@Nonnull final InstanceIdentifier id) { - return shouldReadAll(id) ? readList(id) : super.readCurrent(id); - } - @Override public void read(@Nonnull final InstanceIdentifier id, @Nonnull final Builder parentBuilder) { @@ -103,7 +100,9 @@ public final class CompositeListVppReader customizer.merge(parentBuilder, ifcs); } - private List readList(@Nonnull final InstanceIdentifier id) { + @Override + @Nonnull + public List readList(@Nonnull final InstanceIdentifier id) { LOG.trace("{}: Reading all list entries", this); final List allIds = customizer.getAllIds(id); LOG.debug("{}: Reading list entries for: {}", this, allIds); @@ -113,20 +112,14 @@ public final class CompositeListVppReader final InstanceIdentifier.IdentifiableItem currentBdItem = VppRWUtils.getCurrentIdItem(id, key); final InstanceIdentifier keyedId = VppRWUtils.replaceLastInId(id, currentBdItem); - final List read = readCurrent(keyedId); - checkState(read.size() == 1); - final DataObject singleItem = read.get(0); + final Optional read = readCurrent(keyedId); + final DataObject singleItem = read.get(); checkArgument(getManagedDataObjectType().getTargetType().isAssignableFrom(singleItem.getClass())); allEntries.add(getManagedDataObjectType().getTargetType().cast(singleItem)); } return allEntries; } - private boolean shouldReadAll(@Nonnull final InstanceIdentifier id) { - final InstanceIdentifier instanceIdentifier = id.firstIdentifierOf(getManagedDataObjectType().getTargetType()); - return instanceIdentifier == null || instanceIdentifier.isWildcarded(); - } - @Override protected void readCurrentAttributes(@Nonnull final InstanceIdentifier id, @Nonnull final B builder) { customizer.readCurrentAttributes(id, builder); diff --git a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/util/DelegatingReaderRegistry.java b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/util/DelegatingReaderRegistry.java index 4e50e5aa8..0777425b2 100644 --- a/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/util/DelegatingReaderRegistry.java +++ b/v3po/impl/src/main/java/io/fd/honeycomb/v3po/impl/trans/r/util/DelegatingReaderRegistry.java @@ -18,12 +18,15 @@ package io.fd.honeycomb.v3po.impl.trans.r.util; import static com.google.common.base.Preconditions.checkNotNull; +import com.google.common.base.Optional; import com.google.common.collect.Iterables; import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.Multimap; +import io.fd.honeycomb.v3po.impl.trans.r.ListVppReader; import io.fd.honeycomb.v3po.impl.trans.r.ReaderRegistry; import io.fd.honeycomb.v3po.impl.trans.r.VppReader; import io.fd.honeycomb.v3po.impl.trans.util.VppRWUtils; +import java.util.Collections; import java.util.List; import java.util.Map; import javax.annotation.Nonnull; @@ -62,15 +65,27 @@ public final class DelegatingReaderRegistry implements ReaderRegistry { final Multimap, DataObject> objects = LinkedListMultimap.create(); for (VppReader rootReader : rootReaders.values()) { LOG.debug("Reading from delegate: {}", rootReader); - final List read = rootReader.read(rootReader.getManagedDataObjectType()); - objects.putAll(rootReader.getManagedDataObjectType(), read); + + if(rootReader instanceof ListVppReader) { + final List listEntries = + ((ListVppReader) rootReader).readList(rootReader.getManagedDataObjectType()); + if(!listEntries.isEmpty()) { + objects.putAll(rootReader.getManagedDataObjectType(), listEntries); + } + } else { + final Optional read = rootReader.read(rootReader.getManagedDataObjectType()); + if(read.isPresent()) { + objects.putAll(rootReader.getManagedDataObjectType(), Collections.singletonList(read.get())); + } + } } + return objects; } @Nonnull @Override - public List read(@Nonnull final InstanceIdentifier id) { + public Optional read(@Nonnull final InstanceIdentifier id) { final InstanceIdentifier.PathArgument first = checkNotNull( Iterables.getFirst(id.getPathArguments(), null), "Empty id"); final VppReader vppReader = rootReaders.get(first.getType()); -- cgit 1.2.3-korg