Async S3 Client Presigned Url Download API #5587
Conversation
|
| import java.util.Map; | ||
| import software.amazon.awssdk.codegen.model.service.Operation; | ||
|
|
||
| public class CustomSdkOperations { |
There was a problem hiding this comment.
Can you please mention the Json in Java doc of how this class looks like
| import software.amazon.awssdk.codegen.model.service.Operation; | ||
|
|
||
| public class CustomSdkOperations { | ||
| private Map<String, Operation> operations; |
There was a problem hiding this comment.
operations more sounds like List , can we rename it to something that conveys its a Map
| @@ -97,6 +98,11 @@ public TypeSpec poetSpec() { | |||
| .addMethod(generateTrySelectAuthScheme()) | |||
| .addMethod(generateGetIdentityMetric()) | |||
| .addMethod(putSelectedAuthSchemeMethodSpec()); | |||
|
|
|||
| if (endpointRulesSpecUtils.isS3()) { | |||
There was a problem hiding this comment.
Can we avoid Service specific info in the Core ? Can we get this from customization local to the specific service ?
| "PresignedCustomOperation":{ | ||
| "name":"PresignedCustomOperation", | ||
| "input":{ | ||
| "fqcn": "software.amazon.awssdk.codegen.poet.model.PresignedCustomOperation", |
There was a problem hiding this comment.
Instead of giving fqn can we generate the Requests from Json
The below is not accurate , but we can do something similar as below ?
{
"PresignedDownloadRequest":{
"type":"structure",
"required":["PresignedUrl"],
"members":{
"PresignedUrl":{
"shape":"URL",
"documentation":"<p>The presigned URL to download the object.</p>",
"locationName":"presignedUrl"
},
"StartByte":{
"shape":"Long",
"documentation":"<p>The byte at which to start downloading the object.</p>",
"locationName":"start"
},
"EndByte":{
"shape":"Long",
"documentation":"<p>The byte at which to stop downloading the object.</p>",
"locationName":"end"
},
"CustomHeaders":{
"shape":"CustomHeadersMap",
"documentation":"<p>Any custom headers to include in the request.</p>",
"locationName":"x-amz-custom-headers"
}
}
},
"URL":{"type":"string"},
"Long":{"type":"long"},
"CustomHeadersMap":{
"type":"map",
"key":{"shape":"String"},
"value":{"shape":"HeaderValues"}
},
"HeaderValues":{
"type":"list",
"member":{"shape":"String"}
},
"String":{"type":"string"}
}| @@ -93,6 +95,22 @@ public void setShapeName(String shapeName) { | |||
| this.shapeName = shapeName; | |||
| } | |||
|
|
|||
| public String getFqcn() { | |||
There was a problem hiding this comment.
I donot think we should directly gives Fqcn for operations , We already have
"customSdkShapes": {
"shapes":{
"SdkPartType":{
"type":"string",
"enum":[
"DEFAULT",
"LAST"
]
}
}
}Can we use it and pass the input and outputs as modeled structures ?
| @@ -71,6 +71,13 @@ private MethodSpec modifyHttpRequestMethod() { | |||
| .addStatement("return context.httpRequest()") | |||
| .endControlFlow().build(); | |||
|
|
|||
| if (endpointRulesSpecUtils.isS3()) { | |||
There was a problem hiding this comment.
why do we need this for this feature ?
Can we make a check to rely on customization specific to this feature and can we add to only to New Custom operation rather than all the APIs ?
| return intermediateModel.getShapes().values().stream().filter(s -> s.getC2jName().equals(shapeC2jName)).collect(toList()); | ||
| return intermediateModel.getShapes().values().stream() | ||
| .filter(s -> s.getC2jName() != null) | ||
| .filter(s -> s.getC2jName().equals(shapeC2jName)).collect(toList()); |
There was a problem hiding this comment.
we can do shapeC2jName.equals to avoid additional null check as below
intermediateModel.getShapes().values().stream()
.filter(s -> shapeC2jName.equals(s.getC2jName()))
.collect(toList());| @@ -303,6 +306,18 @@ public CodeBlock asyncExecutionHandler(IntermediateModel intermediateModel, Oper | |||
| return builder.build(); | |||
| } | |||
|
|
|||
| private void addMarshaller(CodeBlock.Builder builder, OperationModel opModel, IntermediateModel intermediateModel) { | |||
There was a problem hiding this comment.
If we model the request with Json as I mentioned in the customization.config we will not need this
|
Overall I was thinking we should try to be consistent with the model structure {
"operationModifiers": {
"PresignedDownload": {
"addNew": [
{
"name": "PresignedDownload",
"http": {
"method": "GET",
"requestUri": "/"
},
"input": "PresignedDownloadInput"
},
"output": {
"shape": "GetObjectOutput"
},
"errors": [
{
"shape": "InvalidObjectState"
}
],
"documentation": "Non-modeled operation for downloading S3 object with pre-signed URL"
}
]
}
}
}Define PresignedDownloadInput as mentioned in another comment |
Motivation and Context
Modifications
PresignedDownloadRequestand corresponding marshallerSdkInternalExecutionAttribute:PRESIGNED_URLCustomSdkOperationsandCustomSdkOperationsProcessorto add customPresignedDownloadoperation to S3 customization configfqcnandmarshallerFqcnExecutionAttributeAuthSchemeInterceptorSpecto useNoAuthAuthSchemeif presigned url operationRequestEndpointInterceptorSpecto skip setting endpoint if presigned url operationTesting
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License