Skip to content

Commit

Permalink
Update to Body consistency in http request (#2945)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
timotheeguerin and markcowl authored Apr 17, 2024
1 parent fec9990 commit 1265330
Show file tree
Hide file tree
Showing 31 changed files with 1,215 additions and 446 deletions.
23 changes: 23 additions & 0 deletions .chronus/changes/body-consitency-2024-2-27-18-35-44-1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: breaking
packages:
- "@typespec/http"
---

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}};
```
18 changes: 18 additions & 0 deletions .chronus/changes/body-consitency-2024-2-27-18-35-44-2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: breaking
packages:
- "@typespec/http"
---

Implicit 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
```
22 changes: 22 additions & 0 deletions .chronus/changes/body-consitency-2024-2-27-18-35-44.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: breaking
packages:
- "@typespec/http"
---

`@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} };
```

9 changes: 9 additions & 0 deletions .chronus/changes/body-consitency-2024-3-2-15-21-10.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: feature
packages:
- "@typespec/openapi3"
- "@typespec/rest"
---

Add supoort for new `@bodyRoot` and `@body` distinction
18 changes: 18 additions & 0 deletions .chronus/changes/body-consitency-2024-3-2-15-21-9.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: breaking
packages:
- "@typespec/http"
---

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: {}}
```
68 changes: 67 additions & 1 deletion docs/libraries/http/reference/decorators.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ toc_max_heading_level: 3

### `@body` {#@TypeSpec.Http.body}

Explicitly specify that this property is to be set as the body
Explicitly specify that this property type will be exactly the HTTP body.

This means that any properties under `@body` cannot be marked as headers, query parameters, or path parameters.
If wanting to change the resolution of the body but still mix parameters, use `@bodyRoot`.

```typespec
@TypeSpec.Http.body
Expand All @@ -33,6 +36,69 @@ op download(): {
};
```

### `@bodyIgnore` {#@TypeSpec.Http.bodyIgnore}

Specify that this property shouldn't be included in the HTTP body.
This can be useful when bundling metadata together that would result in an empty property to be included in the body.

```typespec
@TypeSpec.Http.bodyIgnore
```

#### Target

`ModelProperty`

#### Parameters

None

#### Examples

```typespec
op upload(
name: string,
@bodyIgnore headers: {
@header id: string;
},
): void;
```

### `@bodyRoot` {#@TypeSpec.Http.bodyRoot}

Specify that the body resolution should be resolved from that property.
By default the body is resolved by including all properties in the operation request/response that are not metadata.
This allows to nest the body in a property while still allowing to use headers, query parameters, and path parameters in the same model.

```typespec
@TypeSpec.Http.bodyRoot
```

#### Target

`ModelProperty`

#### Parameters

None

#### Examples

```typespec
op upload(
@bodyRoot user: {
name: string;
@header id: string;
},
): void;
op download(): {
@bodyRoot user: {
name: string;
@header id: string;
};
};
```

### `@delete` {#@TypeSpec.Http.delete}

Specify the HTTP verb for the target operation to be `DELETE`.
Expand Down
2 changes: 2 additions & 0 deletions docs/libraries/http/reference/index.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ npm install --save-peer @typespec/http
### Decorators

- [`@body`](./decorators.md#@TypeSpec.Http.body)
- [`@bodyIgnore`](./decorators.md#@TypeSpec.Http.bodyIgnore)
- [`@bodyRoot`](./decorators.md#@TypeSpec.Http.bodyRoot)
- [`@delete`](./decorators.md#@TypeSpec.Http.delete)
- [`@get`](./decorators.md#@TypeSpec.Http.get)
- [`@head`](./decorators.md#@TypeSpec.Http.head)
Expand Down
70 changes: 69 additions & 1 deletion packages/http/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ Available ruleSets:
### TypeSpec.Http

- [`@body`](#@body)
- [`@bodyIgnore`](#@bodyignore)
- [`@bodyRoot`](#@bodyroot)
- [`@delete`](#@delete)
- [`@get`](#@get)
- [`@head`](#@head)
Expand All @@ -55,7 +57,10 @@ Available ruleSets:

#### `@body`

Explicitly specify that this property is to be set as the body
Explicitly specify that this property type will be exactly the HTTP body.

This means that any properties under `@body` cannot be marked as headers, query parameters, or path parameters.
If wanting to change the resolution of the body but still mix parameters, use `@bodyRoot`.

```typespec
@TypeSpec.Http.body
Expand All @@ -78,6 +83,69 @@ op download(): {
};
```

#### `@bodyIgnore`

Specify that this property shouldn't be included in the HTTP body.
This can be useful when bundling metadata together that would result in an empty property to be included in the body.

```typespec
@TypeSpec.Http.bodyIgnore
```

##### Target

`ModelProperty`

##### Parameters

None

##### Examples

```typespec
op upload(
name: string,
@bodyIgnore headers: {
@header id: string;
},
): void;
```

#### `@bodyRoot`

Specify that the body resolution should be resolved from that property.
By default the body is resolved by including all properties in the operation request/response that are not metadata.
This allows to nest the body in a property while still allowing to use headers, query parameters, and path parameters in the same model.

```typespec
@TypeSpec.Http.bodyRoot
```

##### Target

`ModelProperty`

##### Parameters

None

##### Examples

```typespec
op upload(
@bodyRoot user: {
name: string;
@header id: string;
},
): void;
op download(): {
@bodyRoot user: {
name: string;
@header id: string;
};
};
```

#### `@delete`

Specify the HTTP verb for the target operation to be `DELETE`.
Expand Down
29 changes: 28 additions & 1 deletion packages/http/generated-defs/TypeSpec.Http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ import type {
export type StatusCodeDecorator = (context: DecoratorContext, target: ModelProperty) => void;

/**
* Explicitly specify that this property is to be set as the body
* Explicitly specify that this property type will be exactly the HTTP body.
*
* This means that any properties under `@body` cannot be marked as headers, query parameters, or path parameters.
* If wanting to change the resolution of the body but still mix parameters, use `@bodyRoot`.
*
* @example
* ```typespec
Expand Down Expand Up @@ -84,6 +87,30 @@ export type PathDecorator = (
paramName?: string
) => void;

/**
* Specify that the body resolution should be resolved from that property.
* By default the body is resolved by including all properties in the operation request/response that are not metadata.
* This allows to nest the body in a property while still allowing to use headers, query parameters, and path parameters in the same model.
*
* @example
* ```typespec
* op upload(@bodyRoot user: {name: string, @header id: string}): void;
* op download(): {@bodyRoot user: {name: string, @header id: string}};
* ```
*/
export type BodyRootDecorator = (context: DecoratorContext, target: ModelProperty) => void;

/**
* Specify that this property shouldn't be included in the HTTP body.
* This can be useful when bundling metadata together that would result in an empty property to be included in the body.
*
* @example
* ```typespec
* op upload(name: string, @bodyIgnore headers: {@header id: string}): void;
* ```
*/
export type BodyIgnoreDecorator = (context: DecoratorContext, target: ModelProperty) => void;

/**
* Specify the HTTP verb for the target operation to be `GET`.
*
Expand Down
8 changes: 8 additions & 0 deletions packages/http/generated-defs/TypeSpec.Http.ts-test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/** An error here would mean that the decorator is not exported or doesn't have the right name. */
import {
$body,
$bodyIgnore,
$bodyRoot,
$delete,
$get,
$head,
Expand All @@ -19,6 +21,8 @@ import {
} from "@typespec/http";
import {
BodyDecorator,
BodyIgnoreDecorator,
BodyRootDecorator,
DeleteDecorator,
GetDecorator,
HeadDecorator,
Expand All @@ -42,6 +46,8 @@ type Decorators = {
$header: HeaderDecorator;
$query: QueryDecorator;
$path: PathDecorator;
$bodyRoot: BodyRootDecorator;
$bodyIgnore: BodyIgnoreDecorator;
$get: GetDecorator;
$put: PutDecorator;
$post: PostDecorator;
Expand All @@ -62,6 +68,8 @@ const _: Decorators = {
$header,
$query,
$path,
$bodyRoot,
$bodyIgnore,
$get,
$put,
$post,
Expand Down
30 changes: 29 additions & 1 deletion packages/http/lib/http-decorators.tsp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ extern dec query(target: ModelProperty, queryNameOrOptions?: string | QueryOptio
extern dec path(target: ModelProperty, paramName?: valueof string);

/**
* Explicitly specify that this property is to be set as the body
* Explicitly specify that this property type will be exactly the HTTP body.
*
* This means that any properties under `@body` cannot be marked as headers, query parameters, or path parameters.
* If wanting to change the resolution of the body but still mix parameters, use `@bodyRoot`.
*
* @example
*
Expand All @@ -94,6 +97,31 @@ extern dec path(target: ModelProperty, paramName?: valueof string);
*/
extern dec body(target: ModelProperty);

/**
* Specify that the body resolution should be resolved from that property.
* By default the body is resolved by including all properties in the operation request/response that are not metadata.
* This allows to nest the body in a property while still allowing to use headers, query parameters, and path parameters in the same model.
*
* @example
*
* ```typespec
* op upload(@bodyRoot user: {name: string, @header id: string}): void;
* op download(): {@bodyRoot user: {name: string, @header id: string}};
* ```
*/
extern dec bodyRoot(target: ModelProperty);
/**
* Specify that this property shouldn't be included in the HTTP body.
* This can be useful when bundling metadata together that would result in an empty property to be included in the body.
*
* @example
*
* ```typespec
* op upload(name: string, @bodyIgnore headers: {@header id: string}): void;
* ```
*/
extern dec bodyIgnore(target: ModelProperty);

/**
* Specify the status code for this response. Property type must be a status code integer or a union of status code integer.
*
Expand Down
Loading

0 comments on commit 1265330

Please sign in to comment.