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

Conditionally set Content-Type header #1456

Merged
merged 2 commits into from
Oct 29, 2024
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
33 changes: 24 additions & 9 deletions packages/codegen.go/src/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -736,12 +736,21 @@ function createProtocolRequest(azureARM: boolean, client: go.Client, method: go.
headerParams.push(param);
}
}

let contentType: string | undefined;
for (const param of headerParams.sort((a: go.HeaderParameter, b: go.HeaderParameter) => { return helpers.sortAscending(a.headerName, b.headerName);})) {
if (param.headerName.match(/^content-type$/)) {
// canonicalize content-type as req.SetBody checks for it via its canonicalized name :(
param.headerName = 'Content-Type';
}
if (go.isRequiredParameter(param) || go.isLiteralParameter(param) || go.isClientSideDefault(param.kind)) {

if (param.headerName === 'Content-Type' && param.kind === 'literal') {
// the content-type header will be set as part of emitSetBodyWithErrCheck
// to handle cases where the body param is optional. we don't want to set
// the content-type if the body is nil.
// we do it like this as tsp specifies content-type while swagger does not.
contentType = helpers.formatParamValue(param, imports);
} else if (go.isRequiredParameter(param) || go.isLiteralParameter(param) || go.isClientSideDefault(param.kind)) {
text += emitHeaderSet(param, '\t');
} else if (param.location === 'client' && !param.group) {
// global optional param
Expand All @@ -757,8 +766,12 @@ function createProtocolRequest(azureARM: boolean, client: go.Client, method: go.

const partialBodyParams = values(method.parameters).where((param: go.Parameter) => { return go.isPartialBodyParameter(param); }).toArray();
const bodyParam = <go.BodyParameter | undefined>values(method.parameters).where((each: go.Parameter) => { return go.isBodyParameter(each) || go.isFormBodyParameter(each) || go.isMultipartFormBodyParameter(each); }).first();
const emitSetBodyWithErrCheck = function(setBodyParam: string): string {
return `if err := ${setBodyParam}; err != nil {\n\treturn nil, err\n}\n`;
const emitSetBodyWithErrCheck = function(setBodyParam: string, contentType?: string): string {
let content = `if err := ${setBodyParam}; err != nil {\n\treturn nil, err\n}\n;`;
if (contentType) {
content = `req.Raw().Header["Content-Type"] = []string{${contentType}}\n` + content;
}
return content;
};

if (partialBodyParams.length > 0) {
Expand All @@ -783,6 +796,8 @@ function createProtocolRequest(azureARM: boolean, client: go.Client, method: go.
text += `\t\tbody.${capitalize(partialBodyParam.serializedName)} = options.${capitalize(partialBodyParam.name)}\n\t}\n`;
}
}
// TODO: spread params are JSON only https://github.com/Azure/autorest.go/issues/1455
text += '\treq.Raw().Header["Content-Type"] = []string{"application/json"}\n';
text += '\tif err := runtime.MarshalAsJSON(req, body); err != nil {\n\t\treturn nil, err\n\t}\n';
text += '\treturn req, nil\n';
} else if (!bodyParam) {
Expand Down Expand Up @@ -844,22 +859,22 @@ function createProtocolRequest(azureARM: boolean, client: go.Client, method: go.
setBody = `req.SetBody(streaming.NopCloser(bytes.NewReader(${body})), "application/${bodyParam.bodyFormat.toLowerCase()}")`;
}
if (go.isRequiredParameter(bodyParam) || go.isLiteralParameter(bodyParam)) {
text += `\t${emitSetBodyWithErrCheck(setBody)}`;
text += `\t${emitSetBodyWithErrCheck(setBody, contentType)}`;
text += '\treturn req, nil\n';
} else {
text += emitParamGroupCheck(bodyParam);
text += `\t${emitSetBodyWithErrCheck(setBody)}`;
text += `\t${emitSetBodyWithErrCheck(setBody, contentType)}`;
text += '\t\treturn req, nil\n';
text += '\t}\n';
text += '\treturn req, nil\n';
}
} else if (bodyParam.bodyFormat === 'binary') {
if (go.isRequiredParameter(bodyParam)) {
text += `\t${emitSetBodyWithErrCheck(`req.SetBody(${bodyParam.name}, ${bodyParam.contentType})`)}`;
text += `\t${emitSetBodyWithErrCheck(`req.SetBody(${bodyParam.name}, ${bodyParam.contentType})`, contentType)}`;
text += '\treturn req, nil\n';
} else {
text += emitParamGroupCheck(bodyParam);
text += `\t${emitSetBodyWithErrCheck(`req.SetBody(${helpers.getParamName(bodyParam)}, ${bodyParam.contentType})`)}`;
text += `\t${emitSetBodyWithErrCheck(`req.SetBody(${helpers.getParamName(bodyParam)}, ${bodyParam.contentType})`, contentType)}`;
text += '\treturn req, nil\n';
text += '\t}\n';
text += '\treturn req, nil\n';
Expand All @@ -870,12 +885,12 @@ function createProtocolRequest(azureARM: boolean, client: go.Client, method: go.
const bodyParam = <go.BodyParameter>values(method.parameters).where((each: go.Parameter) => { return go.isBodyParameter(each); }).first();
if (go.isRequiredParameter(bodyParam)) {
text += `\tbody := streaming.NopCloser(strings.NewReader(${bodyParam.name}))\n`;
text += `\t${emitSetBodyWithErrCheck(`req.SetBody(body, ${bodyParam.contentType})`)}`;
text += `\t${emitSetBodyWithErrCheck(`req.SetBody(body, ${bodyParam.contentType})`, contentType)}`;
text += '\treturn req, nil\n';
} else {
text += emitParamGroupCheck(bodyParam);
text += `\tbody := streaming.NopCloser(strings.NewReader(${helpers.getParamName(bodyParam)}))\n`;
text += `\t${emitSetBodyWithErrCheck(`req.SetBody(body, ${bodyParam.contentType})`)}`;
text += `\t${emitSetBodyWithErrCheck(`req.SetBody(body, ${bodyParam.contentType})`, contentType)}`;
text += '\treturn req, nil\n';
text += '\t}\n';
text += '\treturn req, nil\n';
Expand Down
1 change: 1 addition & 0 deletions packages/typespec-go/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

* Fake servers will honor the caller's context in the `*http.Request`.
* Add missing error check when parsing multipart/form content in fakes.
* Optional request bodies will only set the `Content-Type` header when a body is specified.

### Other Fixes

Expand Down
10 changes: 9 additions & 1 deletion packages/typespec-go/src/tcgcadapter/clients.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,15 @@ export class clientAdapter {
throw new Error(`didn't find operation parameter for method ${sdkMethod.name} parameter ${param.name}`);
}

if (opParam.kind === 'header' && opParam.serializedName.match(/^content-type$/i) && param.type.kind === 'constant') {
// if the body param is optional, then the content-type param is also optional.
// for optional constants, this has the side effect of the param being treated like
// a flag which isn't what we want. so, we mark it as required. we ONLY do this
// if the content-type is a constant (i.e. literal value).
// the content-type will be conditionally set based on the requiredness of the body.
opParam.optional = false;
}

let adaptedParam: go.Parameter;
if (opParam.kind === 'body' && opParam.type.kind === 'model' && opParam.type.kind !== param.type.kind) {
const paramKind = this.adaptParameterKind(param);
Expand Down Expand Up @@ -687,7 +696,6 @@ export class clientAdapter {
private adaptParameterKind(param: tcgc.SdkBodyParameter | tcgc.SdkEndpointParameter | tcgc.SdkHeaderParameter | tcgc.SdkMethodParameter | tcgc.SdkPathParameter | tcgc.SdkQueryParameter): go.ParameterKind {
// NOTE: must check for constant type first as it will also set clientDefaultValue
if (param.type.kind === 'constant') {
// TODO: https://github.com/Azure/autorest.go/issues/1444
if (param.optional) {
return 'flag';
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 0 additions & 16 deletions packages/typespec-go/test/armlargeinstance/fake/zz_internal.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions packages/typespec-go/test/armlargeinstance/zz_options.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading