-
Notifications
You must be signed in to change notification settings - Fork 218
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
adds http-checksum trait support #741
Conversation
...model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator
Outdated
Show resolved
Hide resolved
...-model/src/main/resources/META-INF/services/software.amazon.smithy.model.traits.TraitService
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumTrait.java
Outdated
Show resolved
Hide resolved
d43be18
to
cd0433f
Compare
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumTrait.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumTrait.java
Outdated
Show resolved
Hide resolved
...c/test/resources/software/amazon/smithy/model/errorfiles/validators/http-checksum-trait.json
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumTrait.java
Show resolved
Hide resolved
smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
Outdated
Show resolved
Hide resolved
smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumTrait.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumProperties.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumProperties.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumProperties.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumProperties.java
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumProperties.java
Outdated
Show resolved
Hide resolved
...main/java/software/amazon/smithy/model/validation/validators/HttpChecksumTraitValidator.java
Outdated
Show resolved
Hide resolved
...main/java/software/amazon/smithy/model/validation/validators/HttpChecksumTraitValidator.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
Outdated
Show resolved
Hide resolved
smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
Outdated
Show resolved
Hide resolved
smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
Outdated
Show resolved
Hide resolved
smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
Outdated
Show resolved
Hide resolved
smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumTrait.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumProperties.java
Outdated
Show resolved
Hide resolved
...y-aws-traits/src/main/java/software/amazon/smithy/aws/traits/HttpChecksumTraitValidator.java
Outdated
Show resolved
Hide resolved
...y-aws-traits/src/main/java/software/amazon/smithy/aws/traits/HttpChecksumTraitValidator.java
Outdated
Show resolved
Hide resolved
...y-aws-traits/src/main/java/software/amazon/smithy/aws/traits/HttpChecksumTraitValidator.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumProperties.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumTrait.java
Show resolved
Hide resolved
...y-aws-traits/src/main/java/software/amazon/smithy/aws/traits/HttpChecksumTraitValidator.java
Outdated
Show resolved
Hide resolved
...s-traits/src/test/java/software/amazon/smithy/aws/traits/HttpChecksumTraitValidatorTest.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpChecksumProperties.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
Outdated
Show resolved
Hide resolved
smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
Outdated
Show resolved
Hide resolved
Adds loadsTrait test to verify round-trip behavior for trait. Adds validation to error when empty list of checksum algorithm is provided. Updates documentation.
for aws specific location support requirement. Updates to documentation and address feedback around java code.
…null properties for httpchecksum properties builder
…] is now set in build() function for HttpChecksumTraitProperty Builder. Also updates doc, and remove extra code for validator test
…ers and httpHeader trait
...main/java/software/amazon/smithy/model/validation/validators/HttpChecksumTraitValidator.java
Outdated
Show resolved
Hide resolved
...main/java/software/amazon/smithy/model/validation/validators/HttpChecksumTraitValidator.java
Outdated
Show resolved
Hide resolved
...main/java/software/amazon/smithy/model/validation/validators/HttpChecksumTraitValidator.java
Outdated
Show resolved
Hide resolved
...main/java/software/amazon/smithy/model/validation/validators/HttpChecksumTraitValidator.java
Outdated
Show resolved
Hide resolved
...main/java/software/amazon/smithy/model/validation/validators/HttpChecksumTraitValidator.java
Outdated
Show resolved
Hide resolved
...main/java/software/amazon/smithy/model/validation/validators/HttpChecksumTraitValidator.java
Outdated
Show resolved
Hide resolved
...main/java/software/amazon/smithy/model/validation/validators/HttpChecksumTraitValidator.java
Outdated
Show resolved
Hide resolved
|
||
.. code-tab:: smithy | ||
|
||
@httpChecksum( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we consider making these a little more explicit, customizable, and future proof by using a map similar to:
@httpChecksum(
request: {
sha256: {
header: "x-checksum-sha256"
},
crc32: {
header: "x-checksum-crc32"
}
}
)
Here, request
and response
are maps where each key is the algorithm, and each value is a union of possible locations (i.e., it can be set to header
or trailer
, and we could expand to query
in the future for requests if needed). The value of the union is the binding location name (i.e., header name or trailing header name).
This provides more flexibility than the current proposal because each algorithm can use whatever header or query string parameter is necessary. The header/query binding is explicit and does not need prefixes. The names can now support anything, including Content-MD5
if we ever wanted to support it (we probably don't but just an example).
Implementations don't need to perform any prefix logic to determine the header name either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea, have few concerns. Let's sync offline on this.
Replaced by #843 |
Status: Needs review
Description of changes:
[x] Adds
httpChecksum
trait to smithy.[x] Adds test cases and docs
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.