From 7ed542bb9956aaab64599153c0d4eeea2e61eaf1 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sun, 4 Jun 2023 20:59:58 +0200 Subject: [PATCH] Modernize ChoiceNodeCodecContext We know that Choice's components can only be addressable by NodeIdentifier, make sure we reflect in the byYangCaseChild Map. Also use local variable type inference and a text block. Change-Id: I7ac024b5791247685ecf05e9f77040919eec1219 Signed-off-by: Robert Varga --- .../codec/impl/ChoiceNodeCodecContext.java | 76 +++++++++---------- 1 file changed, 35 insertions(+), 41 deletions(-) diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ChoiceNodeCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ChoiceNodeCodecContext.java index 7a4a0d0621..639e2dc5f7 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ChoiceNodeCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ChoiceNodeCodecContext.java @@ -12,23 +12,17 @@ import static com.google.common.base.Preconditions.checkArgument; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableMap.Builder; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.MultimapBuilder.SetMultimapBuilder; import com.google.common.collect.Multimaps; -import com.google.common.collect.SetMultimap; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.Map.Entry; import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -46,6 +40,7 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier.PathArgument; import org.opendaylight.yangtools.yang.binding.contract.Naming; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates; import org.opendaylight.yangtools.yang.data.api.schema.ChoiceNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.model.api.DataSchemaNode; @@ -104,7 +99,7 @@ final class ChoiceNodeCodecContext extends DataContainerCo implements BindingDataObjectCodecTreeNode { private static final Logger LOG = LoggerFactory.getLogger(ChoiceNodeCodecContext.class); - private final ImmutableMap> byYangCaseChild; + private final ImmutableMap> byYangCaseChild; private final ImmutableListMultimap, DataContainerCodecPrototype> ambiguousByCaseChildClass; private final ImmutableMap, DataContainerCodecPrototype> byCaseChildClass; private final ImmutableMap, DataContainerCodecPrototype> byClass; @@ -112,11 +107,10 @@ final class ChoiceNodeCodecContext extends DataContainerCo ChoiceNodeCodecContext(final DataContainerCodecPrototype prototype) { super(prototype); - final Map> byYangCaseChildBuilder = - new HashMap<>(); - final Map, DataContainerCodecPrototype> byClassBuilder = new HashMap<>(); - final SetMultimap, DataContainerCodecPrototype> childToCase = - SetMultimapBuilder.hashKeys().hashSetValues().build(); + final var byYangCaseChildBuilder = new HashMap>(); + final var byClassBuilder = new HashMap, DataContainerCodecPrototype>(); + final var childToCase = SetMultimapBuilder.hashKeys().hashSetValues() + ., DataContainerCodecPrototype>build(); // Load case statements valid in this choice and keep track of their names final var choiceType = prototype.getType(); @@ -129,8 +123,8 @@ final class ChoiceNodeCodecContext extends DataContainerCo // Updates collection of case children @SuppressWarnings("unchecked") - final Class cazeCls = (Class) cazeDef.getBindingClass(); - for (final Class cazeChild : getChildrenClasses(cazeCls)) { + final var cazeCls = (Class) cazeDef.getBindingClass(); + for (var cazeChild : getChildrenClasses(cazeCls)) { childToCase.put(cazeChild, cazeDef); } // Updates collection of YANG instance identifier to case @@ -143,18 +137,17 @@ final class ChoiceNodeCodecContext extends DataContainerCo byYangCaseChild = ImmutableMap.copyOf(byYangCaseChildBuilder); // Move unambiguous child->case mappings to byCaseChildClass, removing them from childToCase - final ImmutableListMultimap.Builder, DataContainerCodecPrototype> ambiguousByCaseBuilder = - ImmutableListMultimap.builder(); - final Builder, DataContainerCodecPrototype> unambiguousByCaseBuilder = ImmutableMap.builder(); - for (Entry, Set>> e : Multimaps.asMap(childToCase).entrySet()) { - final Set> cases = e.getValue(); + final var ambiguousByCaseBuilder = ImmutableListMultimap., DataContainerCodecPrototype>builder(); + final var unambiguousByCaseBuilder = ImmutableMap., DataContainerCodecPrototype>builder(); + for (var entry : Multimaps.asMap(childToCase).entrySet()) { + final var cases = entry.getValue(); if (cases.size() != 1) { // Sort all possibilities by their FQCN to retain semi-predictable results - final List> list = new ArrayList<>(e.getValue()); + final var list = new ArrayList<>(entry.getValue()); list.sort(Comparator.comparing(proto -> proto.getBindingClass().getCanonicalName())); - ambiguousByCaseBuilder.putAll(e.getKey(), list); + ambiguousByCaseBuilder.putAll(entry.getKey(), list); } else { - unambiguousByCaseBuilder.put(e.getKey(), cases.iterator().next()); + unambiguousByCaseBuilder.put(entry.getKey(), cases.iterator().next()); } } byCaseChildClass = unambiguousByCaseBuilder.build(); @@ -172,7 +165,7 @@ final class ChoiceNodeCodecContext extends DataContainerCo * This is required due property of binding specification, that if choice is in grouping schema path location is * lost, and users may use incorrect case class using copy builders. */ - final Map, DataContainerCodecPrototype> bySubstitutionBuilder = new HashMap<>(); + final var bySubstitutionBuilder = new HashMap, DataContainerCodecPrototype>(); final var context = factory.getRuntimeContext(); for (var caseType : context.getTypes().allCaseChildren(choiceType)) { final var caseName = caseType.getIdentifier(); @@ -180,9 +173,9 @@ final class ChoiceNodeCodecContext extends DataContainerCo // FIXME: do not rely on class loading here, the check we are performing should be possible on // GeneratedType only -- or it can be provided by BindingRuntimeTypes -- i.e. rather than // 'allCaseChildren()' it would calculate additional mappings we can use off-the-bat. - final Class substitution = loadCase(context, caseType); + final var substitution = loadCase(context, caseType); - search: for (final Entry, DataContainerCodecPrototype> real : byClassBuilder.entrySet()) { + search: for (var real : byClassBuilder.entrySet()) { if (isSubstitutionFor(substitution, real.getKey())) { bySubstitutionBuilder.put(substitution, real.getValue()); break search; @@ -218,7 +211,7 @@ final class ChoiceNodeCodecContext extends DataContainerCo @SuppressWarnings("unchecked") @Override public DataContainerCodecContext streamChild(final Class childClass) { - final DataContainerCodecPrototype child = byClass.get(childClass); + final var child = byClass.get(childClass); return (DataContainerCodecContext) childNonNull(child, childClass, "Supplied class %s is not valid case in %s", childClass, bindingArg()).get(); } @@ -227,7 +220,7 @@ final class ChoiceNodeCodecContext extends DataContainerCo @Override public Optional> possibleStreamChild( final Class childClass) { - final DataContainerCodecPrototype child = byClass.get(childClass); + final var child = byClass.get(childClass); if (child != null) { return Optional.of((DataContainerCodecContext) child.get()); } @@ -241,7 +234,7 @@ final class ChoiceNodeCodecContext extends DataContainerCo @Override public NodeCodecContext yangPathArgumentChild(final YangInstanceIdentifier.PathArgument arg) { final DataContainerCodecPrototype cazeProto; - if (arg instanceof YangInstanceIdentifier.NodeIdentifierWithPredicates) { + if (arg instanceof NodeIdentifierWithPredicates) { cazeProto = byYangCaseChild.get(new NodeIdentifier(arg.getNodeType())); } else { cazeProto = byYangCaseChild.get(arg); @@ -255,14 +248,14 @@ final class ChoiceNodeCodecContext extends DataContainerCo @SuppressWarnings("unchecked") @SuppressFBWarnings(value = "NP_NONNULL_RETURN_VIOLATION", justification = "See FIXME below") public D deserialize(final NormalizedNode data) { - final ChoiceNode casted = checkDataArgument(ChoiceNode.class, data); - final NormalizedNode first = Iterables.getFirst(casted.body(), null); + final var casted = checkDataArgument(ChoiceNode.class, data); + final var first = Iterables.getFirst(casted.body(), null); if (first == null) { // FIXME: this needs to be sorted out return null; } - final DataContainerCodecPrototype caze = byYangCaseChild.get(first.name()); + final var caze = byYangCaseChild.get(first.name()); return (D) caze.getDataObject().deserialize(data); } @@ -295,19 +288,20 @@ final class ChoiceNodeCodecContext extends DataContainerCo } DataContainerCodecContext getCaseByChildClass(final @NonNull Class type) { - DataContainerCodecPrototype result = byCaseChildClass.get(type); + var result = byCaseChildClass.get(type); if (result == null) { // We have not found an unambiguous result, try ambiguous ones - final List> inexact = ambiguousByCaseChildClass.get(type); + final var inexact = ambiguousByCaseChildClass.get(type); if (!inexact.isEmpty()) { result = inexact.get(0); // Issue a warning, but only once so as not to flood the logs if (ambiguousByCaseChildWarnings.add(type)) { - LOG.warn("Ambiguous reference {} to child of {} resolved to {}, the first case in {} This mapping " - + "is not guaranteed to be stable and is subject to variations based on runtime " - + "circumstances. Please see the stack trace for hints about the source of ambiguity.", - type, bindingArg(), result.getBindingClass(), - Lists.transform(inexact, DataContainerCodecPrototype::getBindingClass), new Throwable()); + LOG.warn(""" + Ambiguous reference {} to child of {} resolved to {}, the first case in {} This mapping is \ + not guaranteed to be stable and is subject to variations based on runtime circumstances. \ + Please see the stack trace for hints about the source of ambiguity.""", + type, bindingArg(), result.getBindingClass(), + Lists.transform(inexact, DataContainerCodecPrototype::getBindingClass), new Throwable()); } } } @@ -327,9 +321,9 @@ final class ChoiceNodeCodecContext extends DataContainerCo private static Iterable> getChildrenClasses(final Class type) { checkArgument(type != null, "Target type must not be null"); checkArgument(DataContainer.class.isAssignableFrom(type), "Supplied type must be derived from DataContainer"); - List> ret = new LinkedList<>(); - for (Method method : type.getMethods()) { - Optional> entity = getYangModeledReturnType(method, Naming.GETTER_PREFIX); + final var ret = new LinkedList>(); + for (var method : type.getMethods()) { + final var entity = getYangModeledReturnType(method, Naming.GETTER_PREFIX); if (entity.isPresent()) { ret.add((Class) entity.orElseThrow()); } -- 2.36.6