Skip to content

Commit

Permalink
Conditionally set Content-Type header (#1456)
Browse files Browse the repository at this point in the history
* Conditionally set Content-Type header

When the body param is optional, the Content-Type header should be set
IFF the body param is not nil.
Don't expose optional Content-Type literal params as flags.

* refine changelog
  • Loading branch information
jhendrixMSFT authored Oct 29, 2024
1 parent 5a492ff commit cb77fd6
Show file tree
Hide file tree
Showing 18 changed files with 55 additions and 84 deletions.
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

0 comments on commit cb77fd6

Please sign in to comment.