From 23fefaa6f0341c29b057a6fbc8d4041ffb19d0a5 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 15 Oct 2021 18:01:22 +0200 Subject: [PATCH] Cleanup AbstractBindingRuntimeContext.getAvailableAugmentationTypes() We are performing lookups along augmentation instantiation axis using APIs which are going away. Cleanup the code and move the traversal to BindingRuntimeTypes. Leave a FIXME for getting the required intelligence from GeneratorReactor as well as some sorely-needed documentation as to what exactly is going on. JIRA: MDSAL-695 Change-Id: I6dd7ad7807546091bfbc5cced48b574d6bff49e2 Signed-off-by: Robert Varga --- .../reactor/AbstractCompositeGenerator.java | 14 +++++++++++ .../api/AbstractBindingRuntimeContext.java | 25 +++++++------------ .../runtime/api/BindingRuntimeContext.java | 8 ++++++ .../runtime/api/BindingRuntimeTypes.java | 25 +++++++++++++++++++ 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/AbstractCompositeGenerator.java b/binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/AbstractCompositeGenerator.java index 14889c53dd..f75c0e6ac3 100644 --- a/binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/AbstractCompositeGenerator.java +++ b/binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/AbstractCompositeGenerator.java @@ -317,6 +317,20 @@ abstract class AbstractCompositeGenerator> ex } else if (stmt instanceof TypedefEffectiveStatement) { tmp.add(new TypedefGenerator((TypedefEffectiveStatement) stmt, this)); } else if (stmt instanceof AugmentEffectiveStatement) { + // FIXME: MDSAL-695: So here we are ignoring any augment which is not in a module, while the 'uses' + // processing takes care of the rest. There are two problems here: + // + // 1) this could be an augment introduced through uses -- in this case we are picking + // confusing it with this being its declaration site, we should probably be + // ignoring it, but then + // + // 2) we are losing track of AugmentEffectiveStatement for which we do not generate + // interfaces -- and recover it at runtime through explicit walk along the + // corresponding AugmentationSchemaNode.getOriginalDefinition() pointer + // + // So here is where we should decide how to handle this augment, and make sure we + // retain information about this being an alias. That will serve as the base for keys + // in the augment -> original map we provide to BindingRuntimeTypes. if (this instanceof ModuleGenerator) { tmpAug.add(new ModuleAugmentGenerator((AugmentEffectiveStatement) stmt, this)); } diff --git a/binding/mdsal-binding-runtime-api/src/main/java/org/opendaylight/mdsal/binding/runtime/api/AbstractBindingRuntimeContext.java b/binding/mdsal-binding-runtime-api/src/main/java/org/opendaylight/mdsal/binding/runtime/api/AbstractBindingRuntimeContext.java index 876cdd42a7..0cf67bc68f 100644 --- a/binding/mdsal-binding-runtime-api/src/main/java/org/opendaylight/mdsal/binding/runtime/api/AbstractBindingRuntimeContext.java +++ b/binding/mdsal-binding-runtime-api/src/main/java/org/opendaylight/mdsal/binding/runtime/api/AbstractBindingRuntimeContext.java @@ -284,25 +284,18 @@ public abstract class AbstractBindingRuntimeContext implements BindingRuntimeCon public final ImmutableMap getAvailableAugmentationTypes( final DataNodeContainer container) { if (container instanceof AugmentationTarget) { - final Map identifierToType = new HashMap<>(); - final BindingRuntimeTypes types = getTypes(); - for (final AugmentationSchemaNode augment : ((AugmentationTarget) container).getAvailableAugmentations()) { - // Augmentation must have child nodes if is to be used with Binding classes - AugmentationSchemaNode augOrig = augment; - while (augOrig.getOriginalDefinition().isPresent()) { - augOrig = augOrig.getOriginalDefinition().get(); - } - - if (!augment.getChildNodes().isEmpty()) { - final Optional augType = types.findType(augOrig); - if (augType.isPresent()) { - identifierToType.put(getAugmentationIdentifier(augment), augType.get()); - } + final var augmentations = ((AugmentationTarget) container).getAvailableAugmentations(); + if (!augmentations.isEmpty()) { + final var identifierToType = new HashMap(); + final var types = getTypes(); + for (var augment : augmentations) { + types.findOriginalAugmentationType(augment).ifPresent(augType -> { + identifierToType.put(getAugmentationIdentifier(augment), augType); + }); } + return ImmutableMap.copyOf(identifierToType); } - return ImmutableMap.copyOf(identifierToType); } - return ImmutableMap.of(); } diff --git a/binding/mdsal-binding-runtime-api/src/main/java/org/opendaylight/mdsal/binding/runtime/api/BindingRuntimeContext.java b/binding/mdsal-binding-runtime-api/src/main/java/org/opendaylight/mdsal/binding/runtime/api/BindingRuntimeContext.java index 5ee85731b1..66830d30e5 100644 --- a/binding/mdsal-binding-runtime-api/src/main/java/org/opendaylight/mdsal/binding/runtime/api/BindingRuntimeContext.java +++ b/binding/mdsal-binding-runtime-api/src/main/java/org/opendaylight/mdsal/binding/runtime/api/BindingRuntimeContext.java @@ -134,6 +134,14 @@ public interface BindingRuntimeContext extends EffectiveModelContextProvider, Im @NonNull Class getClassForSchema(SchemaNode childSchema); + /** + * Return the mapping of a particular {@link DataNodeContainer}'s available augmentations. This method deals with + * resolving {@code uses foo { augment bar { ... } } } scenarios by returning the augmentation created for + * {@code grouping foo}'s Binding representation. + * + * @param container {@link DataNodeContainer} to examine + * @return a mapping from local {@link AugmentationIdentifier}s to their corresponding Binding augmentations + */ @NonNull ImmutableMap getAvailableAugmentationTypes(DataNodeContainer container); @NonNull Class getIdentityClass(QName input); diff --git a/binding/mdsal-binding-runtime-api/src/main/java/org/opendaylight/mdsal/binding/runtime/api/BindingRuntimeTypes.java b/binding/mdsal-binding-runtime-api/src/main/java/org/opendaylight/mdsal/binding/runtime/api/BindingRuntimeTypes.java index 2367c33d52..55d6f150de 100644 --- a/binding/mdsal-binding-runtime-api/src/main/java/org/opendaylight/mdsal/binding/runtime/api/BindingRuntimeTypes.java +++ b/binding/mdsal-binding-runtime-api/src/main/java/org/opendaylight/mdsal/binding/runtime/api/BindingRuntimeTypes.java @@ -161,6 +161,31 @@ public final class BindingRuntimeTypes implements EffectiveModelContextProvider, return Optional.ofNullable(schemaToType.get(schema)); } + public Optional findOriginalAugmentationType(final AugmentationSchemaNode augment) { + // If the augment statement does not contain any child nodes, we did not generate an augmentation, as it would + // be plain littering. + // FIXME: MDSAL-695: this check is rather costly (involves filtering), can we just rely on the not being found + // in the end? all we are saving is essentially two map lookups after all... + if (augment.getChildNodes().isEmpty()) { + return Optional.empty(); + } + + // FIXME: MDSAL-695: We should have enough information from mdsal-binding-generator to receive a (sparse) Map + // for current -> original lookup. When combined with schemaToType, this amounts to the + // inverse view of what 'typeToSchema' holds + AugmentationSchemaNode current = augment; + while (true) { + // If this augmentation has been added through 'uses foo { augment bar { ... } }', we need to invert that + // walk and arrive at the original declaration site, as that is where we generated 'grouping foo's + // augmentation. That site may have a different module, hence the augment namespace may be different. + final Optional original = current.getOriginalDefinition(); + if (original.isEmpty()) { + return findType(current); + } + current = original.orElseThrow(); + } + } + public Multimap getChoiceToCases() { return choiceToCases; } -- 2.36.6