Inconsitency with @visibility and the body resolution

[timotheeguerin]

Using @visibility("none")(or other name) in the goal to hide a property from http does result in some inconsitent behavior in the body resolution.

If a property is a metadataa prop like header it is not included in the body and if all properties were metadata props then the body is changed to be void instead of an empty object.

However the same logic doesn't apply for @visibility it is also inconsitent for request and response.

import "@typespec/http";

using TypeSpec.Http;

/**
 * Baseline 1: Having `@header` property only in request in response result in no body
 * Request body: void
 * Resonse body: void
 */
@route("/baseline")
@get op baseline( @header customHeader: string): { @header customHeader: string } ;

/**
 * Case 1: Adding `@visibility` to `@header` property behave differently for request and response.
 * Request body: {}
 * Resonse body: void
 */
@route("/case1")
op case1(@visibility("none") @header x: string): { @visibility("none") @header x: string };

/**
 * Case 2: Adding `@visibility` to non annotated property result in empty body
 * Request body: {}
 * Resonse body: {}
 */
@route("/case2")
op case2(@visibility("none") x: string): { @visibility("none") x: string };

Playground Link

Other inconsitency that apply to verisoning too

@route("1") op case1(): {}; // {}
@route("2") op case2(): {@header foo: string}; // void
@route("3") op case3(): {@visibility("none") @header foo: string}; // void
@route("4") op case4(): {@visibility("none") foo: string}; // {} now but would be void with fix
@route("5") op case5(): {@added(Versions.v2) foo: string}; // {} in v1
@route("6") op case6(): {@added(Versions.v2) foo: string, @header other: string}; // void in v1

Proposed fixes

If the parameters or response variant result in an empty model {} after removing properties due to visibility, metadata, etc. the resulkting body is void

The reason we had this inconsitency is when we had object as a type and op test(): object producing 204 nocontent was weird.

[bterlson]

Related discussion: #2511

[timotheeguerin]

There is a few problems:

• Azure recommendation is to always have a body, this could maybe solved by a decorator
• Why would @body behavior really be different. As it is more of a @bodyStartsHere having an empty object should really make void as well but then there is a bigger gap for actually returning {}

@route("2") op test2(
  /** @bodyStartsHere */
  @body _: {
    @header customHeader: string;
    @visibility("none") foo: string;
  },
): void;

[timotheeguerin]

Inconsistency with body

Proposal 1: Empty object after removing non-body properties get void

If the resulting model after removing all the non-body properties (Visibility, header, query, version, etc.) is an empty model ({}) then we have no body.

On top of that is this doesn't affect the status code. The status code will always be 200(for success) by default expect if the response body is explicitly void. #2542

op t1(): void; // 204 void
op t2(): {}; // 200 void
op t3(): {@header foo: string}; // 200 void
op t4(): {@visibility("gateway") foo: string}; // 200 void
op t5(): {@visibility("gateway") @header foo: string}; // 200 void
op t6(): {foo: string}; // 200 {foo: "abc"}

Now remains the problem of how do I actually get {} as a body. We have this issue where @body doesn't actually mean body and is very confusing https://github.com/Azure/typespec-azure/issues/3654

Proposal that would go with this would make @body the actual body. Any nested metadata properties inside would produce an error and an empty body would still result in {}
And as an alternative we would introduce @bodyRoot which has the current behavior of @body which means the body resolution start here(don't nest properties above) but it follows the same logic as when we resolve the body from the root of the response/parameters(Empty object means void).

op t7(): {@body _: {}} // 200 {}
op t8(): {@body _: {@visibility("gateway") data: string}} // 200 {}
op t9(): {@bodyRoot _: {}} // 200 void

op e1(): {@body _: {@header foo: string}} // Error: @body cannot contain metadata properties
op e2(): {@body _: {@query foo: string}} // Error: @body cannot contain metadata properties, or we just ignore and treat all those props as body prop so you can reuse the same model in different context?
op e3(): {@body _: {}, @bodyRoot root: {}} // Error: @body  cannot be used with @bodyRoot

Note that this part of the proposal could also be applied to proposal 2 as the main part is it is confusing to use @body today.

Proposal 2: Inverted logic

Always default to an empty object body {} unless the body is explicitly marked with void. This also means we don't auto choose the status code for you. Always 200(for success) unless void is the body type

This would be a bigger breaking change.

We could provide a template that helps

alias NoBody = { @body _: void};

op myOp(): {@header foo: string} & NoBody;

op t1(): void; // 204 void
op t2(): {}; // 200 {}
op t3(): {@header foo: string}; // 200 {}
op t4(): {@visibility("gateway") foo: string}; // 200 {}
op t5(): {@visibility("gateway") @header foo: string}; // 200 {}
op t6(): {foo: string}; // 200 {foo: "abc"}
op t7(): {@header foo: string} & NoBody; // 200 void

[timotheeguerin]

Use case examples

import "@typespec/http";

using TypeSpec.Http;

model T1 {
  @header eTag: string;
  @header createdAt: string;
}

@route("t1") op t1(): T1; // body: void

alias EmptyObjBody = {
  @body _: {};
};

model T2 {
  @header eTag: string;
  @header createdAt: string;
  ...EmptyObjBody;
}
@route("t2") op t2(): T2; // body: {}

model T3 {
  meta: {
    @header eTag: string;
    @header createdAt: string;
  };
  foo: string;
}
@route("t3") op t3(): T3; // body: {foo: ""}

playground

[timotheeguerin]

Use case for sdk

Case 1 model with only headers with body of void

model T1 {
  @header eTag: string;
  @header createdAt: string;
}

@route("t1") op t1(): T1; // body: void

Case 2: Only headers but with body of {}

model T2 {
  @header eTag: string;
  @header createdAt: string;
  ...EmptyObjBody;
}
@route("t2") op t2(): T2; // body: {}

Case 3: Headers are nested in a meta object

model T3 {
  meta: {
    @header eTag: string;
    @header createdAt: string;
  };
  foo: string;
}
@route("t3") op t3(): T3; // body: {foo: ""}

Example in a request

op req(@bodyRoot t3: T3, @header("if-modified-since") ifModifiedSince?: string ): void;

Here we seperated the logical model T3 from any "configuration" properties but headers can be everywhere.

[timotheeguerin]

Summary of current proposal

Empty object after removing non-body properties get void

If the resulting model after removing all the non-body properties (Visibility, header, query, version, etc.) is an empty model ({}) then we have no body.

On top of that is this doesn't affect the status code. The status code will always be 200(for success) by default expect if the response body is explicitly void. #2542

op t1(): void; // 204 void
op t2(): {}; // 200 void
op t3(): {@header foo: string}; // 200 void
op t4(): {@visibility("gateway") foo: string}; // 200 void
op t5(): {@visibility("gateway") @header foo: string}; // 200 void
op t6(): {foo: string}; // 200 {foo: "abc"}

Now remains the problem of how do I actually get {} as a body. We have this issue where @body doesn't actually mean body and is very confusing https://github.com/Azure/typespec-azure/issues/3654

Proposal that would go with this would make @body the actual body. Any nested metadata properties inside would produce an error and an empty body would still result in {}
And as an alternative we would introduce @bodyRoot which has the current behavior of @body which means the body resolution start here(don't nest properties above) but it follows the same logic as when we resolve the body from the root of the response/parameters(Empty object means void).

op t7(): {@body _: {}} // 200 {}
op t8(): {@body _: {@visibility("gateway") data: string}} // 200 {}
op t9(): {@bodyRoot _: {}} // 200 void

op e1(): {@body _: {@header foo: string}} // Error: @body cannot contain metadata properties
op e2(): {@body _: {@query foo: string}} // Error: @body cannot contain metadata properties, or we just ignore and treat all those props as body prop so you can reuse the same model in different context?
op e3(): {@body _: {}, @bodyRoot root: {}} // Error: @body  cannot be used with @bodyRoot

Note that this part of the proposal could also be applied to proposal 2 as the main part is it is confusing to use @body today.

Why not inverted logic (always mean {} unless explicitly void)

1. It would not be possible to represent a service that evolve from void => to returning a model or returning headers.(Changing from void to {@header @added(v2) foo: string}) would make v1 return {}
2. This is also a much bigger breaking change. A lot more spec will have to be explicit.

What about nested object

I think for those we cannot remove the property because it has nothing in it. The body root is this special case that is always consistent.

If we need to wrap lets say all headers under a headers property then we might need a @bodyIgnore

op test(): {
    @bodyIgnore headers: {
        @header `header-a`: string;
        @header `header-b`: string;
    },
    foo: string;
}

// This means:
// Headers:
//   header-a: "abc"
//   header-b: "def"
// Body:
//   {foo: "ghi"}

With visibility and versioning where the original inconsistency would happend with the body being void or {} we don't have this problem here as the headers property can also be annotated correctly.

Summary

1. Introduce new decorators and repurpose some

• @body => @bodyRoot in its current meaning
• @body => now means actual body. Nothing inside of this can be metadata(header, query, etc.)
• @bodyIgnore => provide a way to ignore a property from the body

2. If the resulting body is {} after removing all ignored properties then the body is void
3. Status code is always 200 unless the body is explicitly void then it is 204 (Or do we want to always make it 200)

[timotheeguerin]

Implementation #2868