Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smithy Rules engine #1356

Merged
merged 44 commits into from
Oct 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
5f52c06
Rules Engine Initial Port
skmcgrail Aug 17, 2022
b5172cf
Incorporate Latest Rules Engine Commits
skmcgrail Aug 20, 2022
f5160cc
Fix several cases where serializers were missing and add round-trip t…
rcoh Aug 22, 2022
0194b09
Change namespace from reterminus to language
skmcgrail Aug 22, 2022
97838df
Move resources to correct classpath locations
skmcgrail Aug 22, 2022
fbfcfdc
Mark unstable, make classes final that can be
skmcgrail Aug 23, 2022
521a9e8
Add UseGlobalEndpoint builtins for STS/S3
alextwoods Aug 23, 2022
c8a63f7
Make UseGlobalEndpoints default to false.
alextwoods Aug 23, 2022
3136498
Prune synthesizer components
skmcgrail Aug 24, 2022
0ef91c0
Update the PartitionFn to fallback to the AWS partition
rcoh Aug 24, 2022
da17266
Add more tests of hostLabel standard library function
rcoh Aug 24, 2022
a87b3ae
Add disable MRAP builtin
rcoh Aug 25, 2022
c4d5585
constrain substring to the set of ascii characters
rcoh Aug 25, 2022
33f6470
fix doc comment
rcoh Aug 25, 2022
cc31c46
Integrate Smithy Rules Engine Traits (#8)
skmcgrail Aug 26, 2022
e220145
Fixed additional trait issues (#9)
skmcgrail Aug 26, 2022
73df2b6
Fix checkstyle
skmcgrail Aug 28, 2022
a0efefb
Windows fixes
skmcgrail Aug 28, 2022
ebad2ef
Removed endpoints.json (#10)
skmcgrail Aug 29, 2022
7e884b5
add partition names to output (#11)
rcoh Aug 30, 2022
c0a84e9
Rename language to syntax
rcoh Sep 7, 2022
fe5a70e
Add aws-iso global regions.
alextwoods Aug 30, 2022
635104c
Fix tests to pass latest partitions.json (#13)
skmcgrail Aug 30, 2022
68dc7be
Add S3Control builtin + EndpointRuleset toBuilder (#14)
alextwoods Sep 2, 2022
5768c7f
Fix addRules -> rules usage
alextwoods Sep 2, 2022
cd15998
MandatorySourceLocation cleanups
rcoh Sep 7, 2022
afcb970
Lots of refactors + extract function registry
rcoh Sep 8, 2022
f29c8d5
refactor booleanequals and string equals
rcoh Sep 8, 2022
3fe3e38
Fixups
rcoh Sep 8, 2022
560f96b
More cleanups: good by eval trait
rcoh Sep 8, 2022
220b251
Delete FnVisitor & address CR feedback
rcoh Sep 9, 2022
1cff45a
Add more comments
rcoh Sep 9, 2022
91a14a3
Implement aws.IsVirtualHostableS3Bucket function. (#16)
alextwoods Sep 15, 2022
4d6d778
Add condition methods to GetAttr
alextwoods Sep 15, 2022
65e8e9b
Add functions for isVirtualHostableS3Bucket to Expr
alextwoods Sep 15, 2022
0f2b91c
Fix Parameters toBuilder
alextwoods Sep 16, 2022
1910262
Remove the EndpointRulesetCustomization interface (#17)
alextwoods Sep 16, 2022
5180a03
Add additional checks for invalid labels + ip addresses (#18)
alextwoods Sep 19, 2022
ff7a596
Add a test case for empty host label
alextwoods Sep 20, 2022
034bd7b
First pass at cleanup
skmcgrail Sep 30, 2022
957379a
Fix substring
skmcgrail Sep 30, 2022
d58ac06
Replace newlines in invalidTestCase tests (possible windows fix)
alextwoods Oct 7, 2022
238a1b8
Compact whitespace in IntegrationTests#invalidTestCases
alextwoods Oct 7, 2022
4f791a4
Replace test paths in IntegrationTest with OS independent seperators
alextwoods Oct 7, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.smithy.rulesengine;

import software.amazon.smithy.utils.SmithyUnstableApi;

/**
* An interface that describe a type that can be transformed to type T.
* @param <T> the type.
*/
@SmithyUnstableApi
public interface Into<T> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to only resolve into T being a Condition and it's only present on Function. Can it be removed to reduce the surface area?

T into();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.smithy.rulesengine;

import software.amazon.smithy.utils.SmithyUnstableApi;

/**
* An interface that describe a type that can convert itself into itself.
* @param <T> the type.
*/
@SmithyUnstableApi
public interface IntoSelf<T extends IntoSelf<T>> extends Into<T> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only used once, on Condition. Should remove this to reduce the surface area since this is public

@Override
default T into() {
return (T) this;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,247 @@
/*
* Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.smithy.rulesengine.analysis;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import software.amazon.smithy.rulesengine.language.EndpointRuleSet;
import software.amazon.smithy.rulesengine.language.eval.RuleEvaluator;
import software.amazon.smithy.rulesengine.language.eval.Value;
import software.amazon.smithy.rulesengine.language.syntax.Identifier;
import software.amazon.smithy.rulesengine.language.syntax.rule.Condition;
import software.amazon.smithy.rulesengine.language.syntax.rule.Rule;
import software.amazon.smithy.rulesengine.language.util.PathFinder;
import software.amazon.smithy.rulesengine.language.util.StringUtils;
import software.amazon.smithy.rulesengine.language.visit.TraversingVisitor;
import software.amazon.smithy.rulesengine.traits.EndpointTestCase;
import software.amazon.smithy.utils.SmithyUnstableApi;

/**
* Analyzer for determining coverage of a rule-set.
*/
@SmithyUnstableApi
public final class CoverageChecker {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this class supposed to be used? Is it for rule-set writers? Should it be integrated into a validation?

private final CoverageCheckerCore checkerCore;
private final EndpointRuleSet ruleSet;

public CoverageChecker(EndpointRuleSet ruleSet) {
this.ruleSet = ruleSet;
this.checkerCore = new CoverageCheckerCore();
}

/**
* Evaluates the rule-set with the given inputs to determine rule coverage.
*
* @param input the map parameters and inputs to test coverage.
*/
public void evaluateInput(Map<Identifier, Value> input) {
this.checkerCore.evaluateRuleSet(ruleSet, input);
}

/**
* Evaluate the rule-set using the given test case to determine rule coverage.
*
* @param testCase the test case to evaluate.
*/
public void evaluateTestCase(EndpointTestCase testCase) {
HashMap<Identifier, Value> map = new HashMap<>();
kstich marked this conversation as resolved.
Show resolved Hide resolved
testCase.getParams().getStringMap().forEach((s, node) -> map.put(Identifier.of(s), Value.fromNode(node)));
this.checkerCore.evaluateRuleSet(ruleSet, map);
}

/**
* Analyze and provides the coverage results for the rule-set.
*
* @return stream of {@link CoverageResult}.
*/
public Stream<CoverageResult> checkCoverage() {
Stream<Condition> conditions = new CollectConditions().visitRuleset(ruleSet);
return coverageForConditions(conditions);
}

/**
* Analyze and provides the coverage results for a specific rule.
*
* @return stream of {@link CoverageResult}.
*/
public Stream<CoverageResult> checkCoverageFromRule(Rule rule) {
kstich marked this conversation as resolved.
Show resolved Hide resolved
Stream<Condition> conditions = rule.accept(new CollectConditions());
return coverageForConditions(conditions);

kstich marked this conversation as resolved.
Show resolved Hide resolved
}

private Stream<CoverageResult> coverageForConditions(Stream<Condition> conditions) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method could do with some documentation internally. I'm not sure why it's doing what it's doing around branch results.

Are Streams the best interface for this or would a list be more ergonomic?

return conditions.distinct().flatMap(condition -> {
Wrapper<Condition> w = new Wrapper<>(condition);
ArrayList<BranchResult> conditionResults = checkerCore.conditionResults.getOrDefault(w, new ArrayList<>());
kstich marked this conversation as resolved.
Show resolved Hide resolved
List<Boolean> branches = conditionResults.stream()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be boxed or can it be a boolean?

.map(c -> c.result)
.distinct()
.collect(Collectors.toList());
if (branches.size() == 1) {
return Stream.of(new CoverageResult(condition, !branches.get(0), conditionResults.stream()
.map(c -> c.context.input)
.collect(Collectors.toList())));
} else if (branches.size() == 0) {
return Stream.of(new CoverageResult(condition, false, Collections.emptyList()),
new CoverageResult(condition, true, Collections.emptyList()));
} else {
return Stream.empty();
}
});
}

private static class Wrapper<T> {
kstich marked this conversation as resolved.
Show resolved Hide resolved
private final T inner;

Wrapper(T inner) {
this.inner = inner;
}

@Override
public int hashCode() {
return Objects.hash(inner);
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
Wrapper<?> wrapper = (Wrapper<?>) o;
return inner.equals(wrapper.inner);
}
}

public static class BranchResult {
kstich marked this conversation as resolved.
Show resolved Hide resolved
private final boolean result;
private final CoverageCheckerCore.Context context;

public BranchResult(boolean result, CoverageCheckerCore.Context context) {
this.result = result;
this.context = context;
}
}

static class CoverageCheckerCore extends RuleEvaluator {
HashMap<Wrapper<Condition>, ArrayList<BranchResult>> conditionResults = new HashMap<>();
kstich marked this conversation as resolved.
Show resolved Hide resolved
Context context = null;

@Override
public Value evaluateRuleSet(EndpointRuleSet ruleset, Map<Identifier, Value> parameterArguments) {
try {
context = new Context(parameterArguments);
return super.evaluateRuleSet(ruleset, parameterArguments);
} finally {
context = null;
}
}

@Override
public Value evaluateCondition(Condition condition) {
assert context != null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually avoid asserts in the main libraries. Is there another way this precondition can be handled safely? We use Objects.requireNonNull elsewhere, but this indicates a potential flow issue.

Value result = super.evaluateCondition(condition);
Wrapper<Condition> cond = new Wrapper<>(condition);
ArrayList<BranchResult> list = conditionResults.getOrDefault(cond, new ArrayList<>());
kstich marked this conversation as resolved.
Show resolved Hide resolved
if (result.isNone() || result.equals(Value.bool(false))) {
list.add(new BranchResult(false, context));
} else {
list.add(new BranchResult(true, context));
}
conditionResults.put(cond, list);
return result;
}

static class Context {
private final Map<Identifier, Value> input;

Context(Map<Identifier, Value> input) {
this.input = input;
}
}
}

static class CollectConditions extends TraversingVisitor<Condition> {
@Override
public Stream<Condition> visitConditions(List<Condition> conditions) {
return conditions.stream();
}
}

public static class CoverageResult {
kstich marked this conversation as resolved.
Show resolved Hide resolved
private final Condition condition;
private final boolean result;
private final List<Map<Identifier, Value>> otherUsages;

public CoverageResult(Condition condition, boolean result, List<Map<Identifier, Value>> otherUsages) {
this.condition = condition;
this.result = result;
this.otherUsages = otherUsages;
}

public Condition condition() {
return condition;
}

public boolean result() {
return result;
}

public List<Map<Identifier, Value>> otherUsages() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this method? It's not used anywhere.

return otherUsages;
}

public String pretty() {
StringBuilder sb = new StringBuilder();
sb.append("leaf: ").append(pretty(condition));
return sb.toString();
}

private String pretty(Condition condition) {
return new StringBuilder()
.append(condition)
.append("(")
.append(condition.getSourceLocation().getFilename())
.append(":")
.append(condition.getSourceLocation().getLine())
.append(")")
.toString();
}

public String prettyWithPath(EndpointRuleSet ruleset) {
kstich marked this conversation as resolved.
Show resolved Hide resolved
PathFinder.Path path = PathFinder.findPath(ruleset, condition).orElseThrow(NoSuchElementException::new);
StringBuilder sb = new StringBuilder();
sb.append(pretty()).append("\n");
for (List<Condition> cond : path.negated()) {
kstich marked this conversation as resolved.
Show resolved Hide resolved
sb.append(StringUtils.indent(String.format("!%s", cond.toString()), 2));
}
for (Condition cond : path.positive()) {
sb.append(StringUtils.indent(cond.toString(), 2));
}
return sb.toString();
}
}
}
Loading