Verify body consistency breaking change #27897
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Next Steps to MergeNext steps that must be taken to merge this PR:
|
Swagger Validation Report
|
Swagger Generation Artifacts
|
|
PR validation pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment. |
This was referenced Feb 23, 2024
|
PR validation pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts. |
timotheeguerin
added a commit
to microsoft/typespec
that referenced
this pull request
Apr 17, 2024
fix [#2868](#2868) - Change meaning of `@body` to mean this is the body and nothing will be excluded(show warning on containing metadata) - Add new `@bodyRoot` which has the same purpose as the old `@body`( Allows changing where the body is resolved from but allows mixing with metadata. - Add new `@bodyIgnore` which allows a property to be ignored from the body - Provide a new body resolution common function for request and response also fix #2075 ## Examples from original issue 1. [Inconsitency between request and response](https://cadlplayground.z22.web.core.windows.net/prs/2945/?c=aW1wb3J0ICJAdHlwZXNwZWMvaHR0cCI7Cgp1c2luZyBUeXBlU3BlYy5IdHRwOwoKLyoqCiAqIEJhc2VsaW5lIDE6IEhhdsQqYEBoZWFkZXJgIHByb3BlcnR5IG9ubHkgaW4gcmVxdWVzdMYLc3BvbnNlxAl1bMUTbm8gYm9kecRXUscpxBA6IHZvaWTGFnPFM80WLwpAcm91dGUoIi9i5wCNIikKQGdldCBvcCDIEygg5wCWIGN1c3RvbUjFDTogc3RyaW5nKToge90hIH0g6gD0Q2Fz5QDwQWRkxBtgQHZpc2liaWxpdHlgIHRv9AEBYmVoYXZlIGRpZmZlcmVudGx5IGZvcukBEGFuZOkBES7yAQB7ff8A%2FuUA%2FmNhc2UxIikKb3AgxQso6wCYKCJub%2BQBGOkA5XjuAPvfKsUqIH3vAQMy%2BgEDbm9uIGFubm90YXRlZOoBB%2BoB7GVtcOQBF%2FUB7%2FQA78UU7wDtMuoA7TL1AO3%2FAOXMIuUA3Q%3D%3D&e=%40typespec%2Fopenapi3&options=%7B%7D) 2. [Inconsitency between different ways](https://cadlplayground.z22.web.core.windows.net/prs/2945/?c=aW1wb3J0ICJAdHlwZXNwZWMvaHR0cCI7CtIZdmVyc2lvbmluZyI7Cgp1c2luZyBUeXBlU3BlYy5IdHRwO9AVVskyOwoKQHNlcnZpY2UKQMdJZWQoxyFzKQpuYW1lc3BhY2UgTXlTxik7CmVudW0gyCQgewogIHYxLMQGMiwKfQoKQHJvdXRlKCJ0MSIpIG9wIHQxKCk6IHZvaWQ7IC8vIDIwNMUNyigyxygyxCh7fccmMM8mM8cmM8UmQGhlYWRlciBmb286IHN0cmluZ9g5NMc5NMY5dmlzaWJpbGl0eSgiZ2F0ZXdheSIp30jFSDXHSDXcSP8AmMxQNsdQNsVQ1THGFiJhYmMifco5N8c5N8U5QGFkZOsBtC52MvMAzsR6IGluIHYxykc4x0c430fFRyzpANpvdGhlcthe&e=%40typespec%2Fopenapi3&options=%7B%7D) ## Breaking changes Azure spec PR showing scale of breaking changes Azure/azure-rest-api-specs#27897 ### `@body` means this is the body This change makes it that using `@body` will mean exactly this is the body and everything underneath will be included, including metadata properties. It will log a warning explaining that. ```tsp op a1(): {@Body _: {@Header foo: string, other: string} }; ^ warning header in a body, it will not be included as a header. ``` Solution use `@bodyRoot` as the goal is only to change where to resolve the body from. ```tsp op a1(): {@bodyRoot _: {@Header foo: string, other: string} }; ``` ### Empty model after removing metadata and visibility property result in void always This means the following case have changed from returning `{}` to no body ```tsp op b1(): {}; op b2(): {@visibility("none") prop: string}; op b3(): {@added(Versions.v2) prop: string}; ``` Workaround: Use explicit `@body` ```tsp op b1(): {@Body _: {}}; op b2(): {@Body _: {@visibility("none") prop: string}}; op b3(): {@Body _: {@added(Versions.v2) prop: string}}; ``` ### Status code always 200 except if response is explicitly `void` ```tsp op c1(): {@Header foo: string}; // status code 200 (used to be 204) ``` Solution: Add explicit `@statusCode` ```tsp op c1(): {@Header foo: string, @statuscode _: 204}; op c1(): {@Header foo: string, ...NoContent}; // or spread common model ``` ### Properties are not automatically omitted if everything was removed from metadata or visibility ```tsp op d1(): {headers: {@Header foo: string}}; // body will be {headers: {}} ``` Solution: use `@bodyIgnore` ```tsp op d1(): {@bodyIgnore headers: {@Header foo: string}}; // body will be {headers: {}} ``` --------- Co-authored-by: Mark Cowlishaw <[email protected]>
github-merge-queue bot
pushed a commit
to Azure/typespec-azure
that referenced
this pull request
Apr 17, 2024
Core PR microsoft/typespec#2945 Azure spec PR showing scale of breaking changes Azure/azure-rest-api-specs#27897
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR testing the breaking change in http library and see any effect
Core PR: microsoft/typespec#2945
TypeSpec Azure PR: Azure/typespec-azure#293