-
Notifications
You must be signed in to change notification settings - Fork 219
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
Refactor smithy-rules-engine #1855
Merged
Merged
+15,051
−10,657
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jul 13, 2023
JordonPhillips
approved these changes
Jul 19, 2023
hpmellema
approved these changes
Jul 19, 2023
kstich
force-pushed
the
endpoints_refactor
branch
from
August 8, 2023 16:18
92af1be
to
ce42033
Compare
hpmellema
approved these changes
Aug 8, 2023
JordonPhillips
approved these changes
Aug 14, 2023
kstich
force-pushed
the
endpoints_refactor
branch
from
September 5, 2023 18:24
e71e029
to
976082a
Compare
This commit collects many changes to limit the public interface, align with Java standards, align with idioms from the rest of the Smithy codebase, and generally clean up the codebase. Several packages, classes, and methods have been removed, renamed, moved, or changed visibility. Use of `assert` has been removed and the usage of `stream` has been reduced. Classes with multiple subtypes have had their subtype names adjusted for compatibility with Java and been broken out to their own packages and classes.
This commit removes the custom validation system that required code to validate a ruleset and its tests, integrating the validation with Smithy's built in ModelValidation system. Several bugs in validators and test cases were fixed. Test cases were moved to using the standard errorfiles based setup where applicable. Some related refactoring was also performed.
This commit adds validation that for every endpoint test case that contains operationInputs, the operation input entries match their specified operation's input structure. Co-authored-by: Russell Cohen <[email protected]>
This commit collects many changes to limit the public interface, align with Java standards, align with idioms from the rest of the Smithy codebase, and generally clean up the codebase. Several packages, classes, and methods have been removed, renamed, moved, or changed visibility. Direct access to inner mutable types has been removed. The internal complexity of the CoverageChecker has been significantly reduced while maintaining used functionality.
This commit refactors the configuration and inheritance hierarchy of functions in the rules engine. Two intermediate abstract classes have been eliminated, SingleArgFunction and Function. All functions are now supplied through an SPI on FunctionDefinition, and each implementation now supplies a FunctionDefinition. Special cases for methods when generating from FunctionNode instances have also been removed, aligning all implementations to the same construction. No behavior has changed in the ExpressionVisitor to reduce the impact of migration. Unused, or minimally test-case used, convenience functions have been removed. Several bugs were fixed, including dropping context information from certain sets of RulesError generation.
This commit pulls the AWS specific rules engine functions into a new package, smithy-aws-rules-engine. This includes parseArn, partition, and isVirtualHostableS3Bucket. The partition loading has also moved to this package, including its data types. Relevant tests were migrated. Behavior that was only possible to validate using the parseArn function have been moved, even if the behavior is not AWS specific. Tests that used AWS functions but did not require it were updated to be agnostic.
This commit updates the endpoint rule set SPI to provide several components to the implementation, built-in parameters and auth scheme validators in addition to library functions. AWS specific components have been extracted to their own package, and core components are provided through their own extension.
Add the `disableNormalizePath` property as an allowed property for sigv4 and sigv4a auth schemes. This parameter is required for some services (eg: S3) to correctly sign requests. Specifying it in endpoint parameters will allow SDKs to potentially remove more S3 customizations. Co-authored-by: Alex Woods <[email protected]>
This commit adds a ModelTransformerPlugin implementation that removes test case entries that rely on operations being removed from a model. If all test cases are removed, the trait is also removed.
Updates the loading of the EndpointRuleSetExtension SPI to use a lazy instance holder that is loaded when referenced instead of a lock-based static initializer. The APIs that access the holder are labeled internal, as the access patterns may change if the content needs to vary by instance of the EndpointRuleSet.
kstich
force-pushed
the
endpoints_refactor
branch
from
September 6, 2023 06:56
976082a
to
40358a9
Compare
* Add defaultGlobalRegion to partitions output * Rename default to implicit * Remove optional
This commit adds a `SyntaxElement` abstract class that contains several methods intended for use in constructing rule-sets in code. This class implements two new interfaces (`ToCondition` and `ToExpression`) to support converting between types of syntax elements that compose into rule-sets. The `Condition`, `Parameter`, and `Expression` classes extend this, allowing all types of ruleset syntax to be composed this way.
kstich
force-pushed
the
endpoints_refactor
branch
from
September 6, 2023 15:52
489cd28
to
5575572
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replaces #1821
Significant refactoring of smithy-rules-engine
This commit collects many changes to limit the public interface,
align with Java standards, align with idioms from the rest of the
Smithy codebase, and generally clean up the codebase.
Several packages, classes, and methods have been removed, renamed,
moved, or changed visibility. Use of
assert
has been removed andthe usage of
stream
has been reduced. Classes with multiple subtypeshave had their subtype names adjusted for compatibility with Java and
been broken out to their own packages and classes.
Enable RuleSet and Test validation by default
This commit removes the custom validation system that required code
to validate a ruleset and its tests, integrating the validation with
Smithy's built in ModelValidation system. Several bugs in validators
and test cases were fixed. Test cases were moved to using the standard
errorfiles based setup where applicable.
Some related refactoring was also performed.
Add validation of endpoint test operation inputs
This commit adds validation that for every endpoint test case that
contains operationInputs, the operation input entries match their
specified operation's input structure.
Co-authored-by: Russell Cohen [email protected]
Continued refactoring of smithy-rules-engine
This commit collects many changes to limit the public interface,
align with Java standards, align with idioms from the rest of the
Smithy codebase, and generally clean up the codebase.
Several packages, classes, and methods have been removed, renamed,
moved, or changed visibility. Direct access to inner mutable types
has been removed. The internal complexity of the CoverageChecker
has been significantly reduced while maintaining used functionality.
Refactor rules engine functions
This commit refactors the configuration and inheritance hierarchy
of functions in the rules engine. Two intermediate abstract classes
have been eliminated, SingleArgFunction and Function.
All functions are now supplied through an SPI on FunctionDefinition,
and each implementation now supplies a FunctionDefinition. Special
cases for methods when generating from FunctionNode instances have
also been removed, aligning all implementations to the same
construction. No behavior has changed in the ExpressionVisitor to
reduce the impact of migration.
Unused, or minimally test-case used, convenience functions have been
removed.
Several bugs were fixed, including dropping context information from
certain sets of RulesError generation.
Extract AWS specific rules engine functions
This commit pulls the AWS specific rules engine functions into a new
package, smithy-aws-rules-engine. This includes parseArn, partition,
and isVirtualHostableS3Bucket. The partition loading has also moved
to this package, including its data types.
Relevant tests were migrated. Behavior that was only possible to
validate using the parseArn function have been moved, even if the
behavior is not AWS specific. Tests that used AWS functions but did
not require it were updated to be agnostic.
Update license headers
Add javadoc and clean up public interfaces
Address CR Feedback
Extract AWS built-ins and auth validation
This commit updates the endpoint rule set SPI to provide several
components to the implementation, built-in parameters and auth
scheme validators in addition to library functions. AWS specific
components have been extracted to their own package, and core
components are provided through their own extension.
Address more CR feedback
Add rules-engine specifications
Improve in-code function composition
Allow disableNormalizePath for sigv4/sigv4a
Add the
disableNormalizePath
property as an allowed property forsigv4 and sigv4a auth schemes. This parameter is required for some
services (eg: S3) to correctly sign requests. Specifying it in
endpoint parameters will allow SDKs to potentially remove more S3
customizations.
Co-authored-by: Alex Woods [email protected]
Add ModelTransformerPlugin for endpoint test cases
This commit adds a
ModelTransformerPlugin
implementation that removestest case entries that rely on operations being removed from a model.
If all test cases are removed, the trait is also removed.
Refactor EndpointRuleSetExtension SPI loading
Updates the loading of the EndpointRuleSetExtension SPI to use a lazy
instance holder that is loaded when referenced instead of a lock-based
static initializer. The APIs that access the holder are labeled internal,
as the access patterns may change if the content needs to vary by
instance of the EndpointRuleSet.
Move AWS content to smithy-aws-endpoints
Fix issues validating rule/test params
Add ImplicitGlobalRegion to Partition output
Improve in-code rule-set composition
This commit adds a
SyntaxElement
abstract class that contains severalmethods intended for use in constructing rule-sets in code. This class
implements two new interfaces (
ToCondition
andToExpression
) to supportconverting between types of syntax elements that compose into rule-sets.
The
Condition
,Parameter
, andExpression
classes extend this,allowing all types of ruleset syntax to be composed this way.
Remove deprecated NumberNode method use
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.