Skip to content

Commit

Permalink
Fixed feature include logic. Keep synthetic 'case' if leaf is disabled.
Browse files Browse the repository at this point in the history
  • Loading branch information
dhuebner committed Feb 23, 2024
1 parent ce08ba6 commit 9550044
Show file tree
Hide file tree
Showing 5 changed files with 231 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -14,20 +18,41 @@ public class FeatureEvaluationContext {

private Map<String, Boolean> cache = Maps.newHashMap();

private Set<String> include = Sets.newHashSet(), exclude = Sets.newHashSet();
private Map<String, Set<String>> include = Maps.newHashMap();
private Set<String> exclude = Sets.newHashSet();

public FeatureEvaluationContext(List<String> includedFeatures, List<String> excludedFeatures) {
include.addAll(includedFeatures);
for (var featureToInclude : includedFeatures) {
Pair<String, String[]> 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<String, String[]> 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;
}
Expand All @@ -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 <module>: means include none of <module> 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 <module>: 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 <module>: 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -99,9 +100,9 @@ public void serialize(ModuleData moduleData, Format format, StringBuilder output

protected ProcessedDataModel processInternal(List<AbstractModule> modules, List<String> includedFeatures,
List<String> 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));

Expand Down Expand Up @@ -353,7 +354,8 @@ protected void processAugment(Augment augment, FeatureEvaluationContext evalCtx)

protected Set<SchemaNode> collectCopies(SchemaNode schemaNode, Augment augmen, Set<SchemaNode> 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);
Expand Down Expand Up @@ -395,7 +397,18 @@ protected void processAugments(Collection<SchemaNode> 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);
Expand Down Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
@@ -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"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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))) {
Expand All @@ -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,
"[email protected]");

String expectation = null;

// CLI tree test expect output like:
// pyang -f tree -p . [email protected] --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 {

Expand All @@ -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))) {
Expand Down Expand Up @@ -150,6 +179,12 @@ private Optional<ModuleData> processData(boolean withDeviation) throws IOExcepti

private Optional<ModuleData> processData(boolean withDeviation, List<String> includedFeatures,
List<String> excludedFeatures) throws IOException {
return processData(withDeviation, includedFeatures, excludedFeatures, "ietf-system.yang");
}

private Optional<ModuleData> processData(boolean withDeviation, List<String> includedFeatures,
List<String> excludedFeatures, String entryFile) throws IOException {

var resourceMap = Maps.<String, Resource>newHashMap();
List<String> files = newArrayList();
try (InputStream in = this.getClass().getClassLoader().getResourceAsStream("processor");
Expand All @@ -163,7 +198,7 @@ private Optional<ModuleData> processData(boolean withDeviation, List<String> 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);
Expand All @@ -176,16 +211,16 @@ private Optional<ModuleData> processData(boolean withDeviation, List<String> 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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 9550044

Please sign in to comment.