Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -37,7 +37,11 @@
@SmithyInternalApi
public final class AddProtocolConfig implements TypeScriptIntegration {

public AddProtocolConfig() {
static {
init();
}

static void init() {
List<ShapeId> allowed = List.of(
AwsJson1_0Trait.ID,
AwsJson1_1Trait.ID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
import software.amazon.smithy.typescript.codegen.auth.http.integration.AddHttpSigningPlugin;
import software.amazon.smithy.typescript.codegen.integration.RuntimeClientPlugin;
import software.amazon.smithy.typescript.codegen.integration.TypeScriptIntegration;
import software.amazon.smithy.typescript.codegen.schema.SchemaGenerationAllowlist;
import software.amazon.smithy.utils.ListUtils;
import software.amazon.smithy.utils.MapUtils;
import software.amazon.smithy.utils.SetUtils;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ static boolean includeUnsignedPayloadSigV4Header(OperationShape operation) {
* @param visitor A ShapeVisitor that generates a serde function for shapes.
*/
static void generateDocumentBodyShapeSerde(
GenerationContext context,
Set<Shape> shapes,
ShapeVisitor<Void> visitor
GenerationContext context,
Set<Shape> shapes,
ShapeVisitor<Void> visitor
) {
// Walk all the shapes within those in the document and generate for them as well.
Walker shapeWalker = new Walker(NeighborProviderIndex.of(context.getModel()).getProvider());
Expand All @@ -123,7 +123,7 @@ static void generateJsonParseBodyWithQueryHeader(GenerationContext context) {
TypeScriptWriter writer = context.getWriter();
writer.addImport("HeaderBag", "__HeaderBag", TypeScriptDependency.SMITHY_TYPES);
writer.write(IoUtils.readUtf8Resource(
AwsProtocolUtils.class, "populate-body-with-query-compatibility-code-stub.ts"));
AwsProtocolUtils.class, "populate-body-with-query-compatibility-code-stub.ts"));
}

/**
Expand Down Expand Up @@ -176,9 +176,9 @@ static void generateBuildFormUrlencodedString(GenerationContext context) {
writer.addImport("extendedEncodeURIComponent", "__extendedEncodeURIComponent",
TypeScriptDependency.AWS_SMITHY_CLIENT);
writer.openBlock("const buildFormUrlencodedString = (formEntries: Record<string, string>): "
+ "string => Object.entries(formEntries).map(", ").join(\"&\");",
() -> writer.write("([key, value]) => __extendedEncodeURIComponent(key) + '=' + "
+ "__extendedEncodeURIComponent(value)"));
+ "string => Object.entries(formEntries).map(", ").join(\"&\");",
() -> writer.write("([key, value]) => __extendedEncodeURIComponent(key) + '=' + "
+ "__extendedEncodeURIComponent(value)"));
writer.write("");
}

Expand Down Expand Up @@ -251,7 +251,7 @@ static void writeIdempotencyAutofill(GenerationContext context, MemberShape memb
TypeScriptWriter writer = context.getWriter();
writer.addImport("v4", "generateIdempotencyToken", TypeScriptDependency.SMITHY_UUID);
writer.openBlock("if ($L === undefined) {", "}", inputLocation, () ->
writer.write("$L = generateIdempotencyToken();", inputLocation));
writer.write("$L = generateIdempotencyToken();", inputLocation));
}
}

Expand All @@ -266,10 +266,10 @@ static void writeIdempotencyAutofill(GenerationContext context, MemberShape memb
* @return A string representing the proper value provider for this timestamp.
*/
static String getInputTimestampValueProvider(
GenerationContext context,
MemberShape memberShape,
Format defaultFormat,
String inputLocation
GenerationContext context,
MemberShape memberShape,
Format defaultFormat,
String inputLocation
) {
HttpBindingIndex httpIndex = HttpBindingIndex.of(context.getModel());
TimestampFormatTrait.Format format = httpIndex.determineTimestampFormat(memberShape, DOCUMENT, defaultFormat);
Expand All @@ -278,9 +278,9 @@ static String getInputTimestampValueProvider(

static void generateProtocolTests(ProtocolGenerator generator, GenerationContext context) {
new HttpProtocolTestGenerator(context,
generator,
AwsProtocolUtils::filterProtocolTests,
AwsProtocolUtils::filterMalformedRequestTests).run();
generator,
AwsProtocolUtils::filterProtocolTests,
AwsProtocolUtils::filterMalformedRequestTests).run();
}

/**
Expand All @@ -307,28 +307,16 @@ static Map<String, TreeSet<String>> getErrorAliases(GenerationContext context,
}

private static boolean filterProtocolTests(
ServiceShape service,
OperationShape operation,
HttpMessageTestCase testCase,
TypeScriptSettings settings
ServiceShape service,
OperationShape operation,
HttpMessageTestCase testCase,
TypeScriptSettings settings
) {
// TODO: Remove when upstream tests update to serialize empty headers.
if (testCase.getId().contains("NullAndEmptyHeaders")) {
return true;
}

// TODO This test has an arbitrary root XML name NestedXmlMapWithXmlNameRequest
// TODO which doesn't match the input structure. We will need to update
// TODO the test comparator stub to ignore this if that is indeed intended.
if (testCase.getId().contains("NestedXmlMapWithXmlNameSerializes")) {
return true;
}

// TODO: remove when Glacier AccountID hyphen customization is implemented: SMITHY-2614
if (testCase.getId().equals("GlacierAccountId")) {
return true;
return settings.generateServerSdk();
}

// JSv3 does not populate default values on the client side
if (testCase.getTags().contains("defaults")) {
return true;
}
Expand All @@ -343,7 +331,7 @@ private static boolean filterProtocolTests(
return true;
}

// TODO: https://github.com/aws/aws-sdk-js-v3/issues/7169
// todo(schema): can be enabled when dropping the legacy codegen private client.
if (testCase.getId().equals("RestJsonHttpPayloadWithStructureAndEmptyResponseBody")) {
return true;
}
Expand All @@ -352,10 +340,10 @@ private static boolean filterProtocolTests(
}

private static boolean filterMalformedRequestTests(
ServiceShape service,
OperationShape operation,
HttpMalformedRequestTestCase testCase,
TypeScriptSettings settings
ServiceShape service,
OperationShape operation,
HttpMalformedRequestTestCase testCase,
TypeScriptSettings settings
) {
// Handling overflow/underflow of longs in JS is extraordinarily tricky.
// Numbers are actually all 62-bit floats, and so any integral number is
Expand All @@ -374,9 +362,8 @@ private static boolean filterMalformedRequestTests(
return true;
}

// TODO: fix in https://github.com/aws/aws-sdk-js-v3/issues/5545
if (testCase.getId().equals("RestJsonMalformedUnionUnknownMember")) {
return true;
return settings.generateServerSdk();
}

//TODO: reenable when the SSDK uses RE2 and not built-in regex for pattern constraints
Expand Down Expand Up @@ -405,15 +392,13 @@ private static boolean filterMalformedRequestTests(
return true;
}

// ToDo: https://github.com/aws/aws-sdk-js-v3/issues/6246
if (testCase.getId().equals("RestJsonStringPayloadNoContentType")
|| testCase.getId().equals("RestJsonWithBodyExpectsApplicationJsonContentTypeNoHeaders")) {
return true;
return settings.generateServerSdk();
}

// ToDo: https://github.com/aws/aws-sdk-js-v3/issues/6907
if (testCase.getId().equals("RestJsonWithoutBodyEmptyInputExpectsEmptyContentType")) {
return true;
return settings.generateServerSdk();
}

return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,5 +312,103 @@ describe(AwsRestJsonProtocol.name, () => {
"query-default-date": "1970-01-01T00:00:00Z",
});
});

it("inserts a default body only when the payload is a structure shape", async () => {
const operation = {
namespace: "ns",
name: "Operation",
traits: 0,
input: [
3,
"ns",
"Input",
0,
["payload", "name"],
[
[21 satisfies BlobSchema, { httpPayload: 1 }],
[0 satisfies StringSchema, { httpHeader: "name" }],
],
] satisfies StaticStructureSchema,
output: "unit" as const,
};

for (const body of ["", false, undefined, 0, new Uint8Array()]) {
const request = await protocol.serializeRequest(
operation,
{
name: "hi",
payload: body,
},
context
);
if (!body) {
expect(request.body).toBeFalsy();
}
expect(request.body).toEqual(body);
}

for (const body of [undefined, null]) {
const request = await protocol.serializeRequest(
{
...operation,
input: [
3,
"ns",
"Input",
0,
["payload", "name"],
[
[3, "ns", "PayloadStruct", { httpPayload: 1 }, ["a", "b"], [0, 0]] satisfies StaticStructureSchema,
[0 satisfies StringSchema, { httpHeader: "name" }],
],
] satisfies StaticStructureSchema,
},
{
payload: body,
},
context
);
expect(request.body).toEqual(`{}`);
}
});

it("should fill in the payload member of a response if the wire response was empty", async () => {
const operation = {
namespace: "ns",
name: "Operation",
traits: 0,
input: "unit" as const,
output: [
3,
"ns",
"Output",
0,
["name", "payload"],
[
[0, { httpHeader: "name" }],
[21, { httpPayload: 1 }],
],
] satisfies StaticStructureSchema,
};

const output = await protocol.deserializeResponse(
operation,
context,
new HttpResponse({
statusCode: 200,
headers: {},
})
);

expect(output).toEqual({
$metadata: {
cfId: undefined,
extendedRequestId: undefined,
httpStatusCode: 200,
requestId: undefined,
},
payload: null,
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {
HandlerExecutionContext,
HttpRequest,
HttpResponse,
MetadataBearer,
OperationSchema,
ResponseMetadata,
SerdeFunctions,
Expand Down Expand Up @@ -60,6 +61,9 @@ export class AwsRestJsonProtocol extends HttpBindingProtocol {
super.setSerdeContext(serdeContext);
}

/**
* @override
*/
public async serializeRequest<Input extends object>(
operationSchema: OperationSchema,
input: Input,
Expand All @@ -75,7 +79,8 @@ export class AwsRestJsonProtocol extends HttpBindingProtocol {
}
}

if (request.headers["content-type"] && !request.body) {
if (request.body == null && request.headers["content-type"] === this.getDefaultContentType()) {
// if content type is blob or string shape, we don't set a default body.
request.body = "{}";
}

Expand All @@ -84,6 +89,27 @@ export class AwsRestJsonProtocol extends HttpBindingProtocol {
return request;
}

/**
* @override
*/
public async deserializeResponse<Output extends MetadataBearer>(
operationSchema: OperationSchema,
context: HandlerExecutionContext & SerdeFunctions,
response: HttpResponse
): Promise<Output> {
const output: any & MetadataBearer = await super.deserializeResponse(operationSchema, context, response);
const outputSchema = NormalizedSchema.of(operationSchema.output);
for (const [name, member] of outputSchema.structIterator()) {
if (member.getMemberTraits().httpPayload && !(name in output)) {
output[name] = null;
}
}
return output;
}

/**
* @override
*/
protected async handleError(
operationSchema: OperationSchema,
context: HandlerExecutionContext & SerdeFunctions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export function accountIdDefaultMiddleware(): InitializeMiddleware<any, any> {
return <Output extends MetadataBearer>(next: InitializeHandler<any, Output>): InitializeHandler<any, Output> =>
async (args: InitializeHandlerArguments<any>): Promise<InitializeHandlerOutput<Output>> => {
const { input } = args;
if (input.accountId === undefined) {
if (!input.accountId) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

protocol tests say this should be inserted for both null and empty string, not only null.

input.accountId = "-";
}
return next({ ...args, input });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ it("GlacierChecksums:Request", async () => {
* hyphen (-) to indicate the current account. This should be default
* behavior if the customer provides a null or empty string.
*/
it.skip("GlacierAccountId:Request", async () => {
it("GlacierAccountId:Request", async () => {
const client = new GlacierClient({
...clientParams,
requestHandler: new RequestSerializationTestHandler(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ it("GlacierChecksums:Request", async () => {
* hyphen (-) to indicate the current account. This should be default
* behavior if the customer provides a null or empty string.
*/
it.skip("GlacierAccountId:Request", async () => {
it("GlacierAccountId:Request", async () => {
const client = new GlacierClient({
...clientParams,
requestHandler: new RequestSerializationTestHandler(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7030,7 +7030,7 @@ it("RestJsonNoInputAndOutputNoPayload:Response", async () => {
/**
* Do not send null values, but do send empty strings and empty lists over the wire in headers
*/
it.skip("RestJsonNullAndEmptyHeaders:Request", async () => {
it("RestJsonNullAndEmptyHeaders:Request", async () => {
const client = new RestJsonProtocolClient({
...clientParams,
requestHandler: new RequestSerializationTestHandler(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7030,7 +7030,7 @@ it("RestJsonNoInputAndOutputNoPayload:Response", async () => {
/**
* Do not send null values, but do send empty strings and empty lists over the wire in headers
*/
it.skip("RestJsonNullAndEmptyHeaders:Request", async () => {
it("RestJsonNullAndEmptyHeaders:Request", async () => {
const client = new RestJsonProtocolClient({
...clientParams,
requestHandler: new RequestSerializationTestHandler(),
Expand Down
Loading
Loading