X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=blobdiff_plain;f=binding%2Fmdsal-binding-dom-codec%2Fsrc%2Fmain%2Fjava%2Forg%2Fopendaylight%2Fmdsal%2Fbinding%2Fdom%2Fcodec%2Fimpl%2FChoiceNodeCodecContext.java;h=6100d4db209e08b715689758243ce6da79fef5f3;hb=11408d627adca7eb71ac956c3ad01f75b6b91596;hp=7e7a821bc84415bb4f5fad34268a1c628e49f4ad;hpb=dc51015f64da8ca865df08fcf452d24eecccc62c;p=mdsal.git 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 7e7a821bc8..6100d4db20 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 @@ -7,51 +7,119 @@ */ package org.opendaylight.mdsal.binding.dom.codec.impl; -import com.google.common.base.Optional; -import com.google.common.base.Preconditions; +import static com.google.common.base.Preconditions.checkArgument; + +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.util.ArrayList; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Optional; import java.util.Set; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; +import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.eclipse.jdt.annotation.NonNull; +import org.opendaylight.mdsal.binding.model.api.JavaTypeName; +import org.opendaylight.mdsal.binding.runtime.api.CaseRuntimeType; +import org.opendaylight.mdsal.binding.runtime.api.ChoiceRuntimeType; +import org.opendaylight.mdsal.binding.spec.reflect.BindingReflections; import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier.PathArgument; -import org.opendaylight.yangtools.yang.binding.util.BindingReflections; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.ChoiceNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer; -import org.opendaylight.yangtools.yang.data.impl.schema.SchemaUtils; -import org.opendaylight.yangtools.yang.model.api.AugmentationSchema; -import org.opendaylight.yangtools.yang.model.api.ChoiceCaseNode; -import org.opendaylight.yangtools.yang.model.api.ChoiceSchemaNode; +import org.opendaylight.yangtools.yang.data.util.DataSchemaContextNode; +import org.opendaylight.yangtools.yang.data.util.NormalizedNodeSchemaUtils; +import org.opendaylight.yangtools.yang.model.api.AugmentationSchemaNode; import org.opendaylight.yangtools.yang.model.api.DataSchemaNode; +import org.opendaylight.yangtools.yang.model.api.DocumentedNode.WithStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -final class ChoiceNodeCodecContext extends DataContainerCodecContext { +/** + * This is a bit tricky. DataObject addressing does not take into account choice/case statements, and hence given: + * + * 
+ *   
+ *     container foo {
+ *       choice bar {
+ *         leaf baz;
+ *       }
+ *     }
+ *   
+ *
+ * we will see {@code Baz extends ChildOf}, which is how the users would address it in InstanceIdentifier terms. + * The implicit assumption being made is that {@code Baz} identifies a particular instantiation and hence provides + * unambiguous reference to an effective schema statement. + * + *

+ * Unfortunately this does not quite work with groupings, as their generation has changed: we do not have interfaces + * that would capture grouping instantiations, hence we do not have a proper addressing point and users need to specify + * the interfaces generated in the grouping's definition. These can be very much ambiguous, as a {@code grouping} can be + * used in multiple modules independently within an {@code augment} targeting {@code choice}, as each instantiation is + * guaranteed to have a unique namespace -- but we do not have the appropriate instantiations of those nodes. + * + *

+ * To address this issue we have a two-class lookup mechanism, which relies on the interface generated for the + * {@code case} statement to act as the namespace anchor bridging the nodes inside the grouping to the namespace in + * which they are instantiated. + * + *

+ * Furthermore downstream code relies on historical mechanics, which would guess what the instantiation is, silently + * assuming the ambiguity is theoretical and does not occur in practice. + * + *

+ * This leads to three classes of addressing, in order descending performance requirements. + *

    + *
  • Direct DataObject, where we name an exact child
    • + *
  • Case DataObject + Grouping DataObject
    • + *
  • Grouping DataObject, which is ambiguous
    • + *
+ * + * {@link #byCaseChildClass} supports direct DataObject mapping and contains only unambiguous children, while + * {@link #byClass} supports indirect mapping and contains {@code case} sub-statements. + * + * {@link #ambiguousByCaseChildClass} contains ambiguous mappings, for which we end up issuing warnings. We track each + * ambiguous reference and issue warn once when they are encountered -- tracking warning information in + * {@link #ambiguousByCaseChildWarnings}. + */ +final class ChoiceNodeCodecContext extends DataContainerCodecContext { private static final Logger LOG = LoggerFactory.getLogger(ChoiceNodeCodecContext.class); + private final ImmutableMap> byYangCaseChild; - private final ImmutableMap, DataContainerCodecPrototype> byClass; + private final ImmutableListMultimap, DataContainerCodecPrototype> ambiguousByCaseChildClass; private final ImmutableMap, DataContainerCodecPrototype> byCaseChildClass; + private final ImmutableMap, DataContainerCodecPrototype> byClass; + private final Set> ambiguousByCaseChildWarnings; - ChoiceNodeCodecContext(final DataContainerCodecPrototype prototype) { + ChoiceNodeCodecContext(final DataContainerCodecPrototype prototype) { super(prototype); final Map> byYangCaseChildBuilder = - new HashMap<>(); + new HashMap<>(); final Map, DataContainerCodecPrototype> byClassBuilder = new HashMap<>(); - final Map, DataContainerCodecPrototype> byCaseChildClassBuilder = new HashMap<>(); + final SetMultimap, DataContainerCodecPrototype> childToCase = + SetMultimapBuilder.hashKeys().hashSetValues().build(); final Set> potentialSubstitutions = new HashSet<>(); // Walks all cases for supplied choice in current runtime context - for (final Class caze : factory().getRuntimeContext().getCases(getBindingClass())) { + // FIXME: 9.0.0: factory short-circuits to prototype, just as getBindingClass() does + for (final Class caze : loadCaseClasses()) { // We try to load case using exact match thus name // and original schema must equals - final DataContainerCodecPrototype cazeDef = loadCase(caze); + final DataContainerCodecPrototype cazeDef = loadCase(caze); // If we have case definition, this case is instantiated // at current location and thus is valid in context of parent choice if (cazeDef != null) { @@ -60,19 +128,24 @@ final class ChoiceNodeCodecContext extends DataContainerCo @SuppressWarnings("unchecked") final Class cazeCls = (Class) caze; for (final Class cazeChild : BindingReflections.getChildrenClasses(cazeCls)) { - byCaseChildClassBuilder.put(cazeChild, cazeDef); + childToCase.put(cazeChild, cazeDef); } // Updates collection of YANG instance identifier to case - for (final DataSchemaNode cazeChild : cazeDef.getSchema().getChildNodes()) { - if (cazeChild.isAugmenting()) { - final AugmentationSchema augment = SchemaUtils.findCorrespondingAugment(cazeDef.getSchema(), - cazeChild); - if (augment != null) { - byYangCaseChildBuilder.put(SchemaUtils.getNodeIdentifierForAugmentation(augment), cazeDef); - continue; + for (var stmt : cazeDef.getType().statement().effectiveSubstatements()) { + if (stmt instanceof DataSchemaNode) { + final DataSchemaNode cazeChild = (DataSchemaNode) stmt; + if (cazeChild.isAugmenting()) { + final AugmentationSchemaNode augment = NormalizedNodeSchemaUtils.findCorrespondingAugment( + // FIXME: bad cast + (DataSchemaNode) cazeDef.getType().statement(), cazeChild); + if (augment != null) { + byYangCaseChildBuilder.put(DataSchemaContextNode.augmentationIdentifierFrom(augment), + cazeDef); + continue; + } } + byYangCaseChildBuilder.put(NodeIdentifier.create(cazeChild.getQName()), cazeDef); } - byYangCaseChildBuilder.put(NodeIdentifier.create(cazeChild.getQName()), cazeDef); } } else { /* @@ -82,6 +155,29 @@ final class ChoiceNodeCodecContext extends DataContainerCo potentialSubstitutions.add(caze); } } + 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(); + if (cases.size() != 1) { + // Sort all possibilities by their FQCN to retain semi-predictable results + final List> list = new ArrayList<>(e.getValue()); + list.sort(Comparator.comparing(proto -> proto.getBindingClass().getCanonicalName())); + ambiguousByCaseBuilder.putAll(e.getKey(), list); + } else { + unambiguousByCaseBuilder.put(e.getKey(), cases.iterator().next()); + } + } + byCaseChildClass = unambiguousByCaseBuilder.build(); + + // Setup ambiguous tracking, if needed + ambiguousByCaseChildClass = ambiguousByCaseBuilder.build(); + ambiguousByCaseChildWarnings = ambiguousByCaseChildClass.isEmpty() ? ImmutableSet.of() + : ConcurrentHashMap.newKeySet(); final Map, DataContainerCodecPrototype> bySubstitutionBuilder = new HashMap<>(); /* @@ -100,17 +196,37 @@ final class ChoiceNodeCodecContext extends DataContainerCo } } byClassBuilder.putAll(bySubstitutionBuilder); - byYangCaseChild = ImmutableMap.copyOf(byYangCaseChildBuilder); byClass = ImmutableMap.copyOf(byClassBuilder); - byCaseChildClass = ImmutableMap.copyOf(byCaseChildClassBuilder); + } + + private List> loadCaseClasses() { + final var context = factory().getRuntimeContext(); + final var type = getType(); + + return Stream.concat(type.validCaseChildren().stream(), type.additionalCaseChildren().stream()) + .map(caseChild -> { + final var caseName = caseChild.getIdentifier(); + try { + return context.loadClass(caseName); + } catch (ClassNotFoundException e) { + throw new IllegalStateException("Failed to load class for " + caseName, e); + } + }) + .collect(Collectors.toUnmodifiableList()); + } + + @Override + public WithStatus getSchema() { + // FIXME: Bad cast, we should be returning an EffectiveStatement perhaps? + return (WithStatus) getType().statement(); } @SuppressWarnings("unchecked") @Override public DataContainerCodecContext streamChild(final Class childClass) { final DataContainerCodecPrototype child = byClass.get(childClass); - return (DataContainerCodecContext) childNonNull(child, childClass, "Supplied class %s is not valid case", - childClass).get(); + return (DataContainerCodecContext) childNonNull(child, childClass, + "Supplied class %s is not valid case in %s", childClass, bindingArg()).get(); } @SuppressWarnings("unchecked") @@ -121,26 +237,25 @@ final class ChoiceNodeCodecContext extends DataContainerCo if (child != null) { return Optional.of((DataContainerCodecContext) child.get()); } - return Optional.absent(); + return Optional.empty(); } Iterable> getCaseChildrenClasses() { - return byCaseChildClass.keySet(); + return Iterables.concat(byCaseChildClass.keySet(), ambiguousByCaseChildClass.keySet()); } - protected DataContainerCodecPrototype loadCase(final Class childClass) { - final Optional childSchema = factory().getRuntimeContext().getCaseSchemaDefinition(getSchema(), - childClass); - if (childSchema.isPresent()) { - return DataContainerCodecPrototype.from(childClass, childSchema.get(), factory()); + protected DataContainerCodecPrototype loadCase(final Class childClass) { + final var child = getType().bindingCaseChild(JavaTypeName.create(childClass)); + if (child == null) { + LOG.debug("Supplied class {} is not valid case in schema {}", childClass, getSchema()); + return null; } - LOG.debug("Supplied class %s is not valid case in schema %s", childClass, getSchema()); - return null; + return DataContainerCodecPrototype.from(childClass, child, factory()); } @Override - public NodeCodecContext yangPathArgumentChild(final YangInstanceIdentifier.PathArgument arg) { + public NodeCodecContext yangPathArgumentChild(final YangInstanceIdentifier.PathArgument arg) { final DataContainerCodecPrototype cazeProto; if (arg instanceof YangInstanceIdentifier.NodeIdentifierWithPredicates) { cazeProto = byYangCaseChild.get(new NodeIdentifier(arg.getNodeType())); @@ -148,41 +263,34 @@ final class ChoiceNodeCodecContext extends DataContainerCo cazeProto = byYangCaseChild.get(arg); } - return childNonNull(cazeProto, arg,"Argument %s is not valid child of %s", arg, getSchema()).get() + return childNonNull(cazeProto, arg, "Argument %s is not valid child of %s", arg, getSchema()).get() .yangPathArgumentChild(arg); } - @SuppressWarnings("unchecked") @Override - @Nullable - public D deserialize(final NormalizedNode data) { - Preconditions.checkArgument(data instanceof ChoiceNode); - final NormalizedNodeContainer> casted = - (NormalizedNodeContainer>) data; - final NormalizedNode first = Iterables.getFirst(casted.getValue(), null); + @SuppressWarnings("unchecked") + @SuppressFBWarnings(value = "NP_NONNULL_RETURN_VIOLATION", justification = "See FIXME below") + public D deserialize(final NormalizedNode data) { + checkArgument(data instanceof ChoiceNode); + final ChoiceNode casted = (ChoiceNode) data; + final NormalizedNode first = Iterables.getFirst(casted.body(), null); if (first == null) { + // FIXME: this needs to be sorted out return null; } final DataContainerCodecPrototype caze = byYangCaseChild.get(first.getIdentifier()); return (D) caze.get().deserialize(data); } - DataContainerCodecContext getCazeByChildClass(final @Nonnull Class type) { - final DataContainerCodecPrototype protoCtx = - childNonNull(byCaseChildClass.get(type), type, "Class %s is not child of any cases for %s", type, - bindingArg()); - return protoCtx.get(); - } - @Override - protected Object deserializeObject(final NormalizedNode normalizedNode) { + protected Object deserializeObject(final NormalizedNode normalizedNode) { return deserialize(normalizedNode); } @Override public PathArgument deserializePathArgument(final YangInstanceIdentifier.PathArgument arg) { - Preconditions.checkArgument(getDomPathArgument().equals(arg)); + checkArgument(getDomPathArgument().equals(arg)); return null; } @@ -191,4 +299,25 @@ final class ChoiceNodeCodecContext extends DataContainerCo // FIXME: check for null, since binding container is null. return getDomPathArgument(); } + + DataContainerCodecContext getCaseByChildClass(final @NonNull Class type) { + DataContainerCodecPrototype result = byCaseChildClass.get(type); + if (result == null) { + // We have not found an unambiguous result, try ambiguous ones + final List> 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()); + } + } + } + + return childNonNull(result, type, "Class %s is not child of any cases for %s", type, bindingArg()).get(); + } }