From 95500444088597a44534a6a0cdbcc1335b002a1f Mon Sep 17 00:00:00 2001 From: Dennis Huebner Date: Fri, 23 Feb 2024 14:51:47 +0100 Subject: [PATCH] Fixed feature include logic. Keep synthetic 'case' if leaf is disabled. --- .../processor/FeatureEvaluationContext.java | 47 ++++++++-- .../typefox/yang/processor/YangProcessor.java | 27 +++++- .../tests/processor/FeatureContextTest.java | 37 ++++++++ .../tests/processor/YangProcessorTest.java | 47 ++++++++-- .../expectation-one-feature-only-example.txt | 92 +++++++++++++++++++ 5 files changed, 231 insertions(+), 19 deletions(-) create mode 100644 yang-lsp/io.typefox.yang/src/test/java/io/typefox/yang/tests/processor/FeatureContextTest.java create mode 100644 yang-lsp/io.typefox.yang/src/test/resources/processor/expectation/expectation-one-feature-only-example.txt diff --git a/yang-lsp/io.typefox.yang/src/main/java/io/typefox/yang/processor/FeatureEvaluationContext.java b/yang-lsp/io.typefox.yang/src/main/java/io/typefox/yang/processor/FeatureEvaluationContext.java index ffb547fd..8316e659 100644 --- a/yang-lsp/io.typefox.yang/src/main/java/io/typefox/yang/processor/FeatureEvaluationContext.java +++ b/yang-lsp/io.typefox.yang/src/main/java/io/typefox/yang/processor/FeatureEvaluationContext.java @@ -3,6 +3,10 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; + +import org.eclipse.xtext.util.Pair; +import org.eclipse.xtext.util.Tuples; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -14,20 +18,41 @@ public class FeatureEvaluationContext { private Map cache = Maps.newHashMap(); - private Set include = Sets.newHashSet(), exclude = Sets.newHashSet(); + private Map> include = Maps.newHashMap(); + private Set exclude = Sets.newHashSet(); public FeatureEvaluationContext(List includedFeatures, List excludedFeatures) { - include.addAll(includedFeatures); + for (var featureToInclude : includedFeatures) { + Pair parsedFeature = parseFeature(featureToInclude); + if (parsedFeature != null) { + include.put(parsedFeature.getFirst(), Sets.newHashSet(parsedFeature.getSecond()).stream() + .map(it -> it.trim()).collect(Collectors.toSet())); + } + } exclude.addAll(excludedFeatures); } + private Pair parseFeature(String featureToInclude) { + var colonIdx = featureToInclude.indexOf(':'); + if (colonIdx > 0) { + var module = featureToInclude.substring(0, colonIdx + 1); + var features = featureToInclude.substring(colonIdx + 1).split(","); + return Tuples.pair(module, features); + } else { + System.out.println("Feature include '" + featureToInclude + + "' ignored. Must match the pattern: modulename:[feature(,feature)*]"); + } + return null; + } + public boolean isActive(Feature feature) { ElementIdentifier featureModule = ProcessorUtility.moduleIdentifier(feature); var featureQName = featureGlobalQName(featureModule, feature.getName()); if (cache.containsKey(featureQName)) { return cache.get(featureQName); } - var active = isActive(featureModule.name + ":", featureQName) && featureIfConditionsActive(feature); + var active = isActive(featureModule.name + ":", featureQName, feature.getName()) + && featureIfConditionsActive(feature); cache.put(featureQName, active); return active; } @@ -36,15 +61,17 @@ private boolean featureIfConditionsActive(Feature feature) { return ProcessorUtility.checkIfFeatures(ProcessorUtility.findIfFeatures(feature), this); } - private boolean isActive(String modulePrefix, String featureQName) { + public boolean isActive(String modulePrefix, String featureQName, String featureName) { // include : means include none of features. - if(include.contains(modulePrefix) && !include.contains(featureQName)) { - // include e.g. 'example-system-ext:' means any of example-system-ext module features should be included - return false; + var moduleEntry = include.get(modulePrefix); + // if : not listed in include, all features are enabled by default. + boolean included = true; + if (moduleEntry != null) { + // include e.g. 'example-system-ext:' means any of example-system-ext module + // features should be included + included = moduleEntry.contains(featureName); } - return (include.isEmpty() || include.contains(featureQName) - || !include.contains(modulePrefix)) // if : not listed in include, all features are enabled - && (exclude.isEmpty() || !(exclude.contains(featureQName) || exclude.contains(modulePrefix))); + return (included) && (exclude.isEmpty() || !(exclude.contains(featureQName) || exclude.contains(modulePrefix))); } private String featureGlobalQName(ElementIdentifier module, String featureName) { diff --git a/yang-lsp/io.typefox.yang/src/main/java/io/typefox/yang/processor/YangProcessor.java b/yang-lsp/io.typefox.yang/src/main/java/io/typefox/yang/processor/YangProcessor.java index daafbd62..93ea221e 100644 --- a/yang-lsp/io.typefox.yang/src/main/java/io/typefox/yang/processor/YangProcessor.java +++ b/yang-lsp/io.typefox.yang/src/main/java/io/typefox/yang/processor/YangProcessor.java @@ -47,6 +47,7 @@ import io.typefox.yang.yang.Input; import io.typefox.yang.yang.Leaf; import io.typefox.yang.yang.LeafList; +import io.typefox.yang.yang.Mandatory; import io.typefox.yang.yang.Notification; import io.typefox.yang.yang.Output; import io.typefox.yang.yang.Prefix; @@ -99,9 +100,9 @@ public void serialize(ModuleData moduleData, Format format, StringBuilder output protected ProcessedDataModel processInternal(List modules, List includedFeatures, List excludedFeatures) { - var evalCtx = new FeatureEvaluationContext(includedFeatures, excludedFeatures); ProcessedDataModel processedModel = new ProcessedDataModel(); collectResourceErrors(modules.get(0), processedModel); + var evalCtx = new FeatureEvaluationContext(includedFeatures, excludedFeatures); modules.forEach((module) -> expandUses(module, evalCtx)); @@ -353,7 +354,8 @@ protected void processAugment(Augment augment, FeatureEvaluationContext evalCtx) protected Set collectCopies(SchemaNode schemaNode, Augment augmen, Set collector) { CopiedObjectAdapter.findAll(schemaNode).map(a -> ((SchemaNode) a.getCopy())).forEach(copy -> { - if (!InsideUsesMutationAdapter.find(augmen) || Objects.equal(augmen.eContainer().eClass(), copy.eContainer().eClass())) { + if (!InsideUsesMutationAdapter.find(augmen) + || Objects.equal(augmen.eContainer().eClass(), copy.eContainer().eClass())) { // augment and the target node need to be in the same hierarchy, defined by a // potential `uses` expansion. collector.add(copy); @@ -395,7 +397,18 @@ protected void processAugments(Collection targets, Augment source) { } protected void processChildren(Statement statement, HasStatements parent, FeatureEvaluationContext evalCtx) { - statement.getSubstatements().stream().filter(ele -> ProcessorUtility.isEnabled(ele, evalCtx)).forEach((ele) -> { + statement.getSubstatements().stream().forEach((ele) -> { + if (!ProcessorUtility.isEnabled(ele, evalCtx)) { + if (ele instanceof Leaf && ele.eContainer() instanceof Choice + && isMandatory((Choice) ele.eContainer())) { + // Wrapped choice's direct non-case children into case Element + // See https://www.rfc-editor.org/rfc/rfc6020#section-7.9.2 + // exclude the leafe, but keep the synthetic case statement + parent.addToChildren(ElementData.createNamedWrapper(ProcessorUtility.qualifiedName((Leaf) ele), + ElementKind.Case)); + } + return; + } ElementData child = null; if (ele instanceof Container) { child = new ElementData((Container) ele, ElementKind.Container); @@ -443,6 +456,14 @@ protected void processChildren(Statement statement, HasStatements parent, Featur }); } + /** + * Checks if the node has "mandatory true" substatement + */ + private boolean isMandatory(SchemaNode node) { + return node.getSubstatements().stream() + .anyMatch(sub -> (sub instanceof Mandatory) && "true".equals(((Mandatory) sub).getIsMandatory())); + } + public static class ForeignModuleAdapter extends AdapterImpl { final ElementIdentifier moduleId; diff --git a/yang-lsp/io.typefox.yang/src/test/java/io/typefox/yang/tests/processor/FeatureContextTest.java b/yang-lsp/io.typefox.yang/src/test/java/io/typefox/yang/tests/processor/FeatureContextTest.java new file mode 100644 index 00000000..5948fa1c --- /dev/null +++ b/yang-lsp/io.typefox.yang/src/test/java/io/typefox/yang/tests/processor/FeatureContextTest.java @@ -0,0 +1,37 @@ +package io.typefox.yang.tests.processor; + +import static org.junit.Assert.*; + +import java.util.Collections; + +import org.junit.Test; + +import com.google.common.collect.Lists; + +import io.typefox.yang.processor.FeatureEvaluationContext; + +public class FeatureContextTest { + + @Test + public void testIncludeOneFeature() { + FeatureEvaluationContext ctx = new FeatureEvaluationContext(Lists.newArrayList("module:feature1"), + Collections.emptyList()); + assertTrue(ctx.isActive("module:", "module:feature1", "feature1")); + assertFalse(ctx.isActive("module:", "module:feature2", "feature2")); + } + + @Test + public void testIncludeNoFeature() { + FeatureEvaluationContext ctx = new FeatureEvaluationContext(Lists.newArrayList("module:"), + Collections.emptyList()); + assertFalse(ctx.isActive("module:", "module:feature1", "feature1")); + assertFalse(ctx.isActive("module:", "module:feature2", "feature2")); + } + @Test + public void testIncludeTwoFeatures() { + FeatureEvaluationContext ctx = new FeatureEvaluationContext(Lists.newArrayList("module:feature1,feature2"), + Collections.emptyList()); + assertTrue(ctx.isActive("module:", "module:feature1", "feature1")); + assertTrue(ctx.isActive("module:", "module:feature2", "feature2")); + } +} diff --git a/yang-lsp/io.typefox.yang/src/test/java/io/typefox/yang/tests/processor/YangProcessorTest.java b/yang-lsp/io.typefox.yang/src/test/java/io/typefox/yang/tests/processor/YangProcessorTest.java index 0bda2de8..ddabb42a 100644 --- a/yang-lsp/io.typefox.yang/src/test/java/io/typefox/yang/tests/processor/YangProcessorTest.java +++ b/yang-lsp/io.typefox.yang/src/test/java/io/typefox/yang/tests/processor/YangProcessorTest.java @@ -74,7 +74,8 @@ public void processModules_TreeTest_FeatureInclude() throws IOException { String expectation = null; // CLI tree test expect output like: - // pyang -f tree ietf-system.yang --deviation-module example-system-ext.yang -F example-system-ext: + // pyang -f tree ietf-system.yang --deviation-module example-system-ext.yang -F + // example-system-ext: try (InputStream in = this.getClass().getClassLoader() .getResourceAsStream("processor/expectation/expectation-feature-only-example.txt"); BufferedReader br = new BufferedReader(new InputStreamReader(in))) { @@ -86,6 +87,33 @@ public void processModules_TreeTest_FeatureInclude() throws IOException { } + /* + * enables ietf-keystore:local-definitions-supported feature. + * Which effectively disables other ietf-keystore: features: + * ietf-keystore:keystore-supported and ietf-keystore:key-generation + */ + @Test + public void processModules_TreeTest_FeatureIncludeOne() throws IOException { + + var sysModule = processData(true, newArrayList("ietf-keystore:local-definitions-supported"), null, + "ietf-keystore@2019-11-20.yang"); + + String expectation = null; + + // CLI tree test expect output like: + // pyang -f tree -p . ietf-keystore@2019-11-20.yang --deviation-module + // example-system-ext.yang -F ietf-keystore:local-definitions-supported + try (InputStream in = this.getClass().getClassLoader() + .getResourceAsStream("processor/expectation/expectation-one-feature-only-example.txt"); + BufferedReader br = new BufferedReader(new InputStreamReader(in))) { + var writer = new StringWriter(); + br.transferTo(writer); + expectation = writer.getBuffer().toString(); + } + assertEqualsReduceSpace(expectation, new DataTreeSerializer().serialize(sysModule.get()).toString()); + + } + @Test public void processModules_TreeTest_FeatureExclude() throws IOException { @@ -94,7 +122,8 @@ public void processModules_TreeTest_FeatureExclude() throws IOException { String expectation = null; // CLI tree test expect output like: - // pyang -f tree ietf-system.yang --deviation-module example-system-ext.yang -X example-system-ext:ldap-posix-filter + // pyang -f tree ietf-system.yang --deviation-module example-system-ext.yang -X + // example-system-ext:ldap-posix-filter try (InputStream in = this.getClass().getClassLoader() .getResourceAsStream("processor/expectation/expectation-feature-exclude-f.txt"); BufferedReader br = new BufferedReader(new InputStreamReader(in))) { @@ -150,6 +179,12 @@ private Optional processData(boolean withDeviation) throws IOExcepti private Optional processData(boolean withDeviation, List includedFeatures, List excludedFeatures) throws IOException { + return processData(withDeviation, includedFeatures, excludedFeatures, "ietf-system.yang"); + } + + private Optional processData(boolean withDeviation, List includedFeatures, + List excludedFeatures, String entryFile) throws IOException { + var resourceMap = Maps.newHashMap(); List files = newArrayList(); try (InputStream in = this.getClass().getClassLoader().getResourceAsStream("processor"); @@ -163,7 +198,7 @@ private Optional processData(boolean withDeviation, List inc files.stream().forEach(it -> resourceMap.put(it, resourceSet.createResource(URI.createFileURI("src/test/resources/processor/" + it)))); - var resource = resourceMap.get("ietf-system.yang"); + var resource = resourceMap.get(entryFile); resource.load(resourceSet.getLoadOptions()); EcoreUtil.resolveAll(resource); assertNoErrors(resource); @@ -176,16 +211,16 @@ private Optional processData(boolean withDeviation, List inc }); ProcessedDataModel dataTree = new YangProcessor().process(modules, includedFeatures, excludedFeatures); - var sysModule = dataTree.getModules().stream().filter(mod -> "ietf-system".equals(mod.getSimpleName())).findFirst(); + var sysModule = dataTree.getModules().stream().filter(mod -> mod.getUri().endsWith(entryFile)).findFirst(); return sysModule; } private void assertEqualsReduceSpace(String expectation, String actual) { - if(expectation != null) { + if (expectation != null) { expectation = expectation.replaceAll(" {4,}", " "); } - if(actual != null) { + if (actual != null) { actual = actual.replaceAll(" {4,}", " "); } assertEquals(expectation, actual); diff --git a/yang-lsp/io.typefox.yang/src/test/resources/processor/expectation/expectation-one-feature-only-example.txt b/yang-lsp/io.typefox.yang/src/test/resources/processor/expectation/expectation-one-feature-only-example.txt new file mode 100644 index 00000000..4ca377bd --- /dev/null +++ b/yang-lsp/io.typefox.yang/src/test/resources/processor/expectation/expectation-one-feature-only-example.txt @@ -0,0 +1,92 @@ +module: ietf-keystore + +--rw keystore + +--rw asymmetric-keys + | +--rw asymmetric-key* [name] + | | +--rw name string + | | +--rw algorithm iasa:asymmetric-algorithm-type + | | +--rw public-key-format? identityref + | | +--rw public-key binary + | | +--rw private-key-format? identityref + | | +--rw (private-key-type) + | | | +--:(private-key) + | | | | +--rw private-key? binary + | | | +--:(hidden-private-key) + | | | | +--rw hidden-private-key? empty + | | | +--:(ks:encrypted-private-key) + | | | +--rw ks:encrypted-private-key + | | | +--rw (ks:key-type) + | | | | +--:(ks:symmetric-key-ref) + | | | | +--:(ks:asymmetric-key-ref) + | | | +--rw ks:value? binary + | | +--rw certificates + | | | +--rw certificate* [name] + | | | +--rw name string + | | | +--rw cert? end-entity-cert-cms + | | | +---n certificate-expiration + | | | +-- expiration-date yang:date-and-time + | | +---x generate-certificate-signing-request + | | +---w input + | | | +---w subject binary + | | | +---w attributes? binary + | | +--ro output + | | +--ro certificate-signing-request binary + | +---x generate-asymmetric-key + | +---w input + | | +---w algorithm iasa:asymmetric-algorithm-type + | | +---w encrypt-with! + | | +---w (key-type) + | | +--:(symmetric-key-ref) + | | +--:(asymmetric-key-ref) + | +--ro output + | +--ro algorithm iasa:asymmetric-algorithm-type + | +--ro public-key-format? identityref + | +--ro public-key binary + | +--ro private-key-format? identityref + | +--ro (private-key-type) + | +--:(private-key) + | | +--rw private-key? binary + | +--:(hidden-private-key) + | | +--rw hidden-private-key? empty + | +--:(ks:encrypted-private-key) + | +--rw ks:encrypted-private-key + | +--rw (ks:key-type) + | | +--:(ks:symmetric-key-ref) + | | +--:(ks:asymmetric-key-ref) + | +--rw ks:value? binary + +--rw symmetric-keys + +--rw symmetric-key* [name] + | +--rw name string + | +--rw algorithm isa:symmetric-algorithm-type + | +--rw key-format? identityref + | +--rw (key-type) + | +--:(key) + | | +--rw key? binary + | +--:(hidden-key) + | | +--rw hidden-key? empty + | +--:(ks:encrypted-key) + | +--rw ks:encrypted-key + | +--rw (ks:key-type) + | | +--:(ks:symmetric-key-ref) + | | +--:(ks:asymmetric-key-ref) + | +--rw ks:value? binary + +---x generate-symmetric-key + +---w input + | +---w algorithm isa:symmetric-algorithm-type + | +---w encrypt-with! + | +---w (key-type) + | +--:(symmetric-key-ref) + | +--:(asymmetric-key-ref) + +--ro output + +--ro algorithm isa:symmetric-algorithm-type + +--ro key-format? identityref + +--ro (key-type) + +--:(key) + | +--rw key? binary + +--:(hidden-key) + | +--rw hidden-key? empty + +--:(ks:encrypted-key) + +--rw ks:encrypted-key + +--rw (ks:key-type) + | +--:(ks:symmetric-key-ref) + | +--:(ks:asymmetric-key-ref) + +--rw ks:value? binary