Skip to content

Commit

Permalink
Force request body params to be required for PATCH and PUT (#839)
Browse files Browse the repository at this point in the history
* Force request body params to be required for PATCH and PUT

Added honor-body-placement switch to opt out of this default beahvior.

* cleanup
  • Loading branch information
jhendrixMSFT committed Jun 13, 2022
1 parent 1515223 commit e0b4a68
Show file tree
Hide file tree
Showing 31 changed files with 402 additions and 578 deletions.
2 changes: 1 addition & 1 deletion .scripts/regeneration.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ for (namespace in goMappings) {
}

const blobStorage = 'https://raw.githubusercontent.com/Azure/azure-rest-api-specs/ee9bd6fe35eb7850ff0d1496c59259eb74f0d446/specification/storage/data-plane/Microsoft.BlobStorage/preview/2020-06-12/blob.json';
generate("azstorage", blobStorage, 'test/storage/2020-06-12/azblob', '--security=AzureKey --module="azstorage" --openapi-type="data-plane"');
generate("azstorage", blobStorage, 'test/storage/2020-06-12/azblob', '--security=AzureKey --module="azstorage" --openapi-type="data-plane" --honor-body-placement');

const network = 'https://raw.githubusercontent.com/Azure/azure-rest-api-specs/228cf296647f6e41182cee7d1a403990e6a8fe3c/specification/network/resource-manager/readme.md';
generateFromReadme("armnetwork", network, 'package-2020-03', 'test/network/2020-03-01/armnetwork', '--module=armnetwork --azure-arm=true');
Expand Down
3 changes: 3 additions & 0 deletions src/autorest-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,7 @@ help-content:
- key: stutter
type: string
description: Uses the specified value to remove stuttering from types and funcs instead of the built-in algorithm.
- key: honor-body-placement
type: boolean
description: When true, optional body parameters are treated as such for PATCH and PUT operations.
```
10 changes: 9 additions & 1 deletion src/transform/namer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { Session } from '@autorest/extension-base';
import { capitalize, uncapitalize } from '@azure-tools/codegen';
import { CodeModel, HttpHeader, Language } from '@autorest/codemodel';
import { CodeModel, HttpHeader, HttpMethod, Language } from '@autorest/codemodel';
import { visitor, clone, values } from '@azure-tools/linq';
import { CommonAcronyms, ReservedWords } from './mappings';
import { aggregateParameters, hasAdditionalProperties } from '../common/helpers';
Expand Down Expand Up @@ -87,6 +87,7 @@ export async function namer(session: Session<CodeModel>) {
model.language.go!.headAsBoolean = headAsBoolean;
const groupParameters = await session.getValue('group-parameters', true);
model.language.go!.groupParameters = groupParameters;
const honorBodyPlacement = await session.getValue('honor-body-placement', false);

// fix up type names
const structNames = new Set<string>();
Expand Down Expand Up @@ -190,6 +191,13 @@ export async function namer(session: Session<CodeModel>) {
param.language.go!.name = 'endpoint';
continue;
}
if (!honorBodyPlacement) {
const opMethod = op.requests![0].protocol.http!.method;
if (param.protocol.http?.in === 'body' && (opMethod === HttpMethod.Patch || opMethod === HttpMethod.Put)) {
// we enforce PATCH/PUT body parameters to be required. do this before fixing up the parameter name
param.required = true;
}
}
const inParamGroup = (param.extensions?.['x-ms-parameter-grouping'] && groupParameters) || param.required !== true;
const paramDetails = <Language>param.language.go;
// if this is part of a param group struct then don't apply param naming rules to it
Expand Down
8 changes: 4 additions & 4 deletions test/autorest/lrogroup/lroretrys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ func TestLRORetrysBeginPostAsyncRelativeRetrySucceeded(t *testing.T) {

func TestLRORetrysBeginPut201CreatingSucceeded200(t *testing.T) {
op := newLRORetrysClient()
poller, err := op.BeginPut201CreatingSucceeded200(context.Background(), nil)
poller, err := op.BeginPut201CreatingSucceeded200(context.Background(), Product{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPut201CreatingSucceeded200(context.Background(), &LRORetrysClientBeginPut201CreatingSucceeded200Options{
poller, err = op.BeginPut201CreatingSucceeded200(context.Background(), Product{}, &LRORetrysClientBeginPut201CreatingSucceeded200Options{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -129,11 +129,11 @@ func TestLRORetrysBeginPut201CreatingSucceeded200(t *testing.T) {

func TestLRORetrysBeginPutAsyncRelativeRetrySucceeded(t *testing.T) {
op := newLRORetrysClient()
poller, err := op.BeginPutAsyncRelativeRetrySucceeded(context.Background(), nil)
poller, err := op.BeginPutAsyncRelativeRetrySucceeded(context.Background(), Product{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPutAsyncRelativeRetrySucceeded(context.Background(), &LRORetrysClientBeginPutAsyncRelativeRetrySucceededOptions{
poller, err = op.BeginPutAsyncRelativeRetrySucceeded(context.Background(), Product{}, &LRORetrysClientBeginPutAsyncRelativeRetrySucceededOptions{
ResumeToken: rt,
})
require.NoError(t, err)
Expand Down
70 changes: 35 additions & 35 deletions test/autorest/lrogroup/lros_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func TestLROBeginDeleteProvisioning202Deletingcanceled200(t *testing.T) {

func TestLROBeginPatch201RetryWithAsyncHeader(t *testing.T) {
op := newLROSClient()
resp, err := op.BeginPatch201RetryWithAsyncHeader(context.Background(), nil)
resp, err := op.BeginPatch201RetryWithAsyncHeader(context.Background(), Product{}, nil)
require.NoError(t, err)
res, err := resp.PollUntilDone(context.Background(), &runtime.PollUntilDoneOptions{Frequency: time.Second})
require.NoError(t, err)
Expand All @@ -249,7 +249,7 @@ func TestLROBeginPatch201RetryWithAsyncHeader(t *testing.T) {
func TestLROBeginPatch202RetryWithAsyncAndLocationHeader(t *testing.T) {
t.Skip("https://github.com/Azure/autorest.testserver/pull/369")
op := newLROSClient()
resp, err := op.BeginPatch202RetryWithAsyncAndLocationHeader(context.Background(), nil)
resp, err := op.BeginPatch202RetryWithAsyncAndLocationHeader(context.Background(), Product{}, nil)
require.NoError(t, err)
res, err := resp.PollUntilDone(context.Background(), &runtime.PollUntilDoneOptions{Frequency: time.Second})
require.NoError(t, err)
Expand Down Expand Up @@ -469,11 +469,11 @@ func TestLROBeginPostDoubleHeadersFinalLocationGet(t *testing.T) {

func TestLROBeginPut200Acceptedcanceled200(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPut200Acceptedcanceled200(context.Background(), nil)
poller, err := op.BeginPut200Acceptedcanceled200(context.Background(), Product{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPut200Acceptedcanceled200(context.Background(), &LROsClientBeginPut200Acceptedcanceled200Options{
poller, err = op.BeginPut200Acceptedcanceled200(context.Background(), Product{}, &LROsClientBeginPut200Acceptedcanceled200Options{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -489,7 +489,7 @@ func TestLROBeginPut200Acceptedcanceled200(t *testing.T) {

func TestLROBeginPut200Succeeded(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPut200Succeeded(context.Background(), nil)
poller, err := op.BeginPut200Succeeded(context.Background(), Product{}, nil)
require.NoError(t, err)
_, err = poller.ResumeToken()
require.Error(t, err)
Expand All @@ -506,7 +506,7 @@ func TestLROBeginPut200Succeeded(t *testing.T) {

func TestLROBeginPut200SucceededNoState(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPut200SucceededNoState(context.Background(), nil)
poller, err := op.BeginPut200SucceededNoState(context.Background(), Product{}, nil)
require.NoError(t, err)
_, err = poller.ResumeToken()
require.Error(t, err)
Expand All @@ -521,11 +521,11 @@ func TestLROBeginPut200SucceededNoState(t *testing.T) {
// TODO check if this test should actually be returning a 200 or a 204
func TestLROBeginPut200UpdatingSucceeded204(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPut200UpdatingSucceeded204(context.Background(), nil)
poller, err := op.BeginPut200UpdatingSucceeded204(context.Background(), Product{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPut200UpdatingSucceeded204(context.Background(), &LROsClientBeginPut200UpdatingSucceeded204Options{
poller, err = op.BeginPut200UpdatingSucceeded204(context.Background(), Product{}, &LROsClientBeginPut200UpdatingSucceeded204Options{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -542,11 +542,11 @@ func TestLROBeginPut200UpdatingSucceeded204(t *testing.T) {

func TestLROBeginPut201CreatingFailed200(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPut201CreatingFailed200(context.Background(), nil)
poller, err := op.BeginPut201CreatingFailed200(context.Background(), Product{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPut201CreatingFailed200(context.Background(), &LROsClientBeginPut201CreatingFailed200Options{
poller, err = op.BeginPut201CreatingFailed200(context.Background(), Product{}, &LROsClientBeginPut201CreatingFailed200Options{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -562,11 +562,11 @@ func TestLROBeginPut201CreatingFailed200(t *testing.T) {

func TestLROBeginPut201CreatingSucceeded200(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPut201CreatingSucceeded200(context.Background(), &LROsClientBeginPut201CreatingSucceeded200Options{Product: &Product{}})
poller, err := op.BeginPut201CreatingSucceeded200(context.Background(), Product{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPut201CreatingSucceeded200(context.Background(), &LROsClientBeginPut201CreatingSucceeded200Options{
poller, err = op.BeginPut201CreatingSucceeded200(context.Background(), Product{}, &LROsClientBeginPut201CreatingSucceeded200Options{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -583,7 +583,7 @@ func TestLROBeginPut201CreatingSucceeded200(t *testing.T) {

func TestLROBeginPut201Succeeded(t *testing.T) {
op := newLROSClient()
resp, err := op.BeginPut201Succeeded(context.Background(), nil)
resp, err := op.BeginPut201Succeeded(context.Background(), Product{}, nil)
require.NoError(t, err)
res, err := resp.PollUntilDone(context.Background(), &runtime.PollUntilDoneOptions{Frequency: time.Second})
require.NoError(t, err)
Expand All @@ -598,11 +598,11 @@ func TestLROBeginPut201Succeeded(t *testing.T) {

func TestLROBeginPut202Retry200(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPut202Retry200(context.Background(), &LROsClientBeginPut202Retry200Options{Product: &Product{}})
poller, err := op.BeginPut202Retry200(context.Background(), Product{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPut202Retry200(context.Background(), &LROsClientBeginPut202Retry200Options{
poller, err = op.BeginPut202Retry200(context.Background(), Product{}, &LROsClientBeginPut202Retry200Options{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -616,11 +616,11 @@ func TestLROBeginPut202Retry200(t *testing.T) {

func TestLROBeginPutAsyncNoHeaderInRetry(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPutAsyncNoHeaderInRetry(context.Background(), &LROsClientBeginPutAsyncNoHeaderInRetryOptions{Product: &Product{}})
poller, err := op.BeginPutAsyncNoHeaderInRetry(context.Background(), Product{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPutAsyncNoHeaderInRetry(context.Background(), &LROsClientBeginPutAsyncNoHeaderInRetryOptions{
poller, err = op.BeginPutAsyncNoHeaderInRetry(context.Background(), Product{}, &LROsClientBeginPutAsyncNoHeaderInRetryOptions{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -637,11 +637,11 @@ func TestLROBeginPutAsyncNoHeaderInRetry(t *testing.T) {

func TestLROBeginPutAsyncNoRetrySucceeded(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPutAsyncNoRetrySucceeded(context.Background(), &LROsClientBeginPutAsyncNoRetrySucceededOptions{Product: &Product{}})
poller, err := op.BeginPutAsyncNoRetrySucceeded(context.Background(), Product{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPutAsyncNoRetrySucceeded(context.Background(), &LROsClientBeginPutAsyncNoRetrySucceededOptions{
poller, err = op.BeginPutAsyncNoRetrySucceeded(context.Background(), Product{}, &LROsClientBeginPutAsyncNoRetrySucceededOptions{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -658,11 +658,11 @@ func TestLROBeginPutAsyncNoRetrySucceeded(t *testing.T) {

func TestLROBeginPutAsyncNoRetrycanceled(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPutAsyncNoRetrycanceled(context.Background(), nil)
poller, err := op.BeginPutAsyncNoRetrycanceled(context.Background(), Product{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPutAsyncNoRetrycanceled(context.Background(), &LROsClientBeginPutAsyncNoRetrycanceledOptions{
poller, err = op.BeginPutAsyncNoRetrycanceled(context.Background(), Product{}, &LROsClientBeginPutAsyncNoRetrycanceledOptions{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -681,11 +681,11 @@ func TestLROBeginPutAsyncNoRetrycanceled(t *testing.T) {

func TestLROBeginPutAsyncNonResource(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPutAsyncNonResource(context.Background(), nil)
poller, err := op.BeginPutAsyncNonResource(context.Background(), SKU{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPutAsyncNonResource(context.Background(), &LROsClientBeginPutAsyncNonResourceOptions{
poller, err = op.BeginPutAsyncNonResource(context.Background(), SKU{}, &LROsClientBeginPutAsyncNonResourceOptions{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -699,11 +699,11 @@ func TestLROBeginPutAsyncNonResource(t *testing.T) {

func TestLROBeginPutAsyncRetryFailed(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPutAsyncRetryFailed(context.Background(), nil)
poller, err := op.BeginPutAsyncRetryFailed(context.Background(), Product{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPutAsyncRetryFailed(context.Background(), &LROsClientBeginPutAsyncRetryFailedOptions{
poller, err = op.BeginPutAsyncRetryFailed(context.Background(), Product{}, &LROsClientBeginPutAsyncRetryFailedOptions{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -722,11 +722,11 @@ func TestLROBeginPutAsyncRetryFailed(t *testing.T) {

func TestLROBeginPutAsyncRetrySucceeded(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPutAsyncRetrySucceeded(context.Background(), nil)
poller, err := op.BeginPutAsyncRetrySucceeded(context.Background(), Product{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPutAsyncRetrySucceeded(context.Background(), &LROsClientBeginPutAsyncRetrySucceededOptions{
poller, err = op.BeginPutAsyncRetrySucceeded(context.Background(), Product{}, &LROsClientBeginPutAsyncRetrySucceededOptions{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -743,11 +743,11 @@ func TestLROBeginPutAsyncRetrySucceeded(t *testing.T) {

func TestLROBeginPutAsyncSubResource(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPutAsyncSubResource(context.Background(), nil)
poller, err := op.BeginPutAsyncSubResource(context.Background(), SubProduct{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPutAsyncSubResource(context.Background(), &LROsClientBeginPutAsyncSubResourceOptions{
poller, err = op.BeginPutAsyncSubResource(context.Background(), SubProduct{}, &LROsClientBeginPutAsyncSubResourceOptions{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -763,11 +763,11 @@ func TestLROBeginPutAsyncSubResource(t *testing.T) {

func TestLROBeginPutNoHeaderInRetry(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPutNoHeaderInRetry(context.Background(), nil)
poller, err := op.BeginPutNoHeaderInRetry(context.Background(), Product{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPutNoHeaderInRetry(context.Background(), &LROsClientBeginPutNoHeaderInRetryOptions{
poller, err = op.BeginPutNoHeaderInRetry(context.Background(), Product{}, &LROsClientBeginPutNoHeaderInRetryOptions{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -784,11 +784,11 @@ func TestLROBeginPutNoHeaderInRetry(t *testing.T) {

func TestLROBeginPutNonResource(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPutNonResource(context.Background(), nil)
poller, err := op.BeginPutNonResource(context.Background(), SKU{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPutNonResource(context.Background(), &LROsClientBeginPutNonResourceOptions{
poller, err = op.BeginPutNonResource(context.Background(), SKU{}, &LROsClientBeginPutNonResourceOptions{
ResumeToken: rt,
})
require.NoError(t, err)
Expand All @@ -802,11 +802,11 @@ func TestLROBeginPutNonResource(t *testing.T) {

func TestLROBeginPutSubResource(t *testing.T) {
op := newLROSClient()
poller, err := op.BeginPutSubResource(context.Background(), nil)
poller, err := op.BeginPutSubResource(context.Background(), SubProduct{}, nil)
require.NoError(t, err)
rt, err := poller.ResumeToken()
require.NoError(t, err)
poller, err = op.BeginPutSubResource(context.Background(), &LROsClientBeginPutSubResourceOptions{
poller, err = op.BeginPutSubResource(context.Background(), SubProduct{}, &LROsClientBeginPutSubResourceOptions{
ResumeToken: rt,
})
require.NoError(t, err)
Expand Down
Loading

0 comments on commit e0b4a68

Please sign in to comment.