Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usage decorator (Input/Output/InputOutput) #4486

Open
bterlson opened this issue Sep 20, 2024 · 9 comments
Open

Usage decorator (Input/Output/InputOutput) #4486

bterlson opened this issue Sep 20, 2024 · 9 comments
Labels
compiler:core Issues for @typespec/compiler design:needed A design request has been raised that needs a proposal triaged:core
Milestone

Comments

@bterlson
Copy link
Member

bterlson commented Sep 20, 2024

Input and Output in TypeSpec

Various emitters and protocol would benefit from a way to differentiate between input and output.

  • GraphQL has different constraint for input and output types and providing a way to explicitly annoate a type as input or output would be useful.
  • Various Azure client emitters currently use Input and output to generate different client class(An outpout only model wouldn't have constructor generated)

Things to take into account:

  • We can clearly establish a what is used as input, output or both by looking at operation parameters and return type. We already provide an api to do so. function resolveUsages(container: Namespace | Interface | Operation): UsageTracker;
  • Usage could be affected by visibility(A readonly property being the only reference to a model wouldn't mark that model as an input). This means this might be different depending on the protocol.
  • Do we need a more generic usage system? Like tracking a type is used in a certain payload kind(e.g. Json vs Xml) - tcgc does this
  • We most likely want to error out when incompatible input or output are used.(e.g. an Input only model reference an output only model)
  • How does input vs output affect emitter that don't involve operations(e.g. json schema)
  • Could something be an input only in a protocol but output/input in another.

Single marker for input or output

In this case we could use in/out keyword which would result in a clean syntax.

@secret
in scalar password extends string;

It makes it quite simple to figure out that we have some incompatible reference here

out model UserRead { password: password }
^ error password is an input only type but is used in a output only type

It might become a bit more tricky when referenced from inout types. Do we automatically infer that properties, union variants that reference an input only or output only type should be omitted.

This now bleeds right into the visibility design where we expect properties/union variants to be explicitly annotated.

inout model User {
  id: UserId; // do we understand that this property is not part of the User input model
  password: password;
  firstName: string;
}

out model UserId {
  company: string;
  name: string;
}

We could also consider that an error. i.e. you can only reference type of the same usage kind(input only from input only, output only from output only and input/output from both).

This is however quite limiting when working with visibility and render this incompatible.

So why not just use visibility

Input/Output still differ from visibility. Input could have many different visibility(create, update, etc.)

Pass a protocol to a @usage decorator

Let suppose we have the following operations

@scope(Sql) // not a decorator today, just the idea
namespace DataBase;

op setUserFromDb(user: User): void;
op getUserFromDb(): User;

@scope(Http)
namespace FE;
op getUser(): User;
op setUser(user: User): void; // error user is set as input only for http protocol

We could define the following:

@usage(Usage.Input | Usage.Output, Sql)
@usage(Usage.Output, Http)
model User {
  firstName: string;
  @usage(Usage.Output, Http) id: UserId;
  // ^ error UserId is output only but used as input in sql

  @usage(Usage.Input, Http) password: string;
  // ^ error only specified as output for http
}


@usage(Usage.Output) model UserId { domain: string, name: string }

This looks a lot like visibility

This now looks awefully similar to visibility but with more limitation. Visibility is able to specify different kind for input or output (create, update,etc.).

So now if we take the example above and we just want to make password available in create

@usage(Usage.Input | Usage.Output, Sql)
@usage(Usage.Input, Http)
model User {
  firstName: string;
  @usage(Usage.Input, Http) @visibility(HttpVis.Create) password: string;
}

Now this is a bit tedious I need to specify things twice. We should be able to map visibility to input or output so those get applied automatically which would allow the following.

@usage(Usage.Input | Usage.Output, Sql)
@usage(Usage.Input, Http)
model User {
  firstName: string;
  @visibility(HttpVis.Create) password: string;
}

And then why do we not have @visibility on the model itself

@visibility(SqlVis)
@visibility(HttpVis.Create, HttpVis.Update)
model User {
  firstName: string;
  @visibility(HttpVis.Create) password: string;
}

At this point this just really feels like a more limited syntax/syntactic sugar for setting all the input vs output visibilities

Input/Output as a global flag and visibility to control protocol specificness

So this comes to the last option which is to keep visibility as the protocol specific decider and having in/out/inout keyword or @usage decorator but something that applies in all cases.

Taking the example above

out model UserId { domain: string, name: string }

inout model User {
  @visibility(HttpVis.read) id: UserId;
  @visibility(HttpVis.create) password: string;
}

With the code above we either wouldn't be able to validate which defeats a little the point of having those modifier or we'd need to duplicate things and have visibility just be a way to be more precise for a protocol

out model UserId { domain: string, name: string }

inout model User {
  out id: UserId;
  @visibility(HttpVis.create) in password: string;
  @visibility(HttpVis.read) in other: string;
  //  ^ error cannot apply a readonly visibility to a in property
}

How does this work with protocol agnostic emitters

One of the thing this could ideally solve is having the ability to mark readonly, writeonly in json schema for example.

In the case of using a global switch this seems quite straight forward, in/out can map to readonly, writeonly.
In the case of some protocol specific switch json schema would need to be able to choose which protocol to generate the schema from(or use the default value as the protocol was optional). This feels inline with the fact that it should probably also respect visibility(e.g. if generating json schema for models of an http service)

keyword vs decorator

I think a general thing we want for typespec is have the ability to use some decorator to specify some syntax sugar like @default or @optional, etc. So i believe it would be safer to start with a decorator and move to a syntax later if it is widely used. There is 2 options:

@in 
@out 
@inout
@usage(Usage.in)
@usage(Usage.out)
@usage(Usage.Input | Usage.Output)

The first one would be a cleaner syntax specially if it doesn't take a protocol

@markcowl markcowl added the design:needed A design request has been raised that needs a proposal label Sep 23, 2024
@markcowl markcowl added this to the [2024] November milestone Sep 23, 2024
@markcowl markcowl added triaged:core compiler:core Issues for @typespec/compiler labels Sep 23, 2024
@lmazuel lmazuel changed the title Access decorator (Input/Output/InputOutput) Usage decorator (Input/Output/InputOutput) Oct 8, 2024
@lmazuel
Copy link
Member

lmazuel commented Oct 8, 2024

@tadelesh
Copy link
Member

tadelesh commented Oct 9, 2024

Current usage logic in TCGC:

  • SdkModel, SdkEnum has usage attribute, the value is a bit map, and the default value is 0, while most common used values are:
    • None = 0
    • Input = 1 << 1
    • Output = 1 << 2
    • JsonMergePatch = 1 << 4
    • MultipartFormData = 1 << 5
    • Json = 1 << 8
    • Xml = 1 << 9
  • usage calculation from operation:
    • Referred in any convenient API's input: set Input and do propagation
    • Referred in any convenient API's output: set Output and do propagation
    • Referred in any convenient API's input or output and with json content type: set Json and do propagation
    • Referred in any convenient API's input or output and with xml content type: set Xml and do propagation
    • Referred in any convenient API's input and with json merge patch content type: set JsonMergePatch and do propagation
    • Referred in any convenient API's input and with multi-part form data content type: set MultipartFormData
  • User could use @usage in TCGC to override the default value, and the decorator could be added to namespace, model, enum, union (only work for union as enum)
  • @usage override calculation:
    • For all models and enums:
      • If @usage set on it or its namespace (the nearest), set override value to the types and do propagation
      • If any conflict happens in this process, a warning is added to diagnostics
        • Override value should not conflict with the existed value from operation or other propagation (for example, should not override Input | Output to Input)

Propagation logic:

  • For array or record, propagate to value type
  • For union, propagate to all union variants type
  • For enum value, propagate to its enum type
  • For nullable, propagate to its inner type
  • For model
    • Propagate to all properties type
    • Propagate to additional properties type
    • For discriminated base type, propagate to all its sub types
    • For sub type, propagate to its base type, but not propagate to any other siblings

One thing left is TCGC does not support override usage on operation, see issue here: Azure/typespec-azure#1301.

@tadelesh
Copy link
Member

tadelesh commented Oct 9, 2024

Some challenges of access calculation:

  1. Override base type with discriminator should propagate to all sub types, but override a sub type extend from a base type with discriminator, the propagation should stop on the base type.
  2. Do not propagate Input usage to read-only property.

@timotheeguerin
Copy link
Member

timotheeguerin commented Oct 10, 2024

  1. Do not propagate Input usage to read-only property.

This is a subset of the problem that input vs output might be protocol dependent and visibility dependent. This makes it probably quite hard to use in out keywords

@tadelesh
Copy link
Member

Another thing is scope, different language's client needs different usage override.

@timotheeguerin
Copy link
Member

how does that work? wouldn't a model only be used as input or outpur or both regardless of the client?

@tadelesh
Copy link
Member

tadelesh commented Oct 11, 2024

yeah. different usage for different client seems weird. and it seems all current usage in azure spec is wrong use case. the only scenario i could image is setting usage for model not referenced by any operation to generate them and different client need different orphan model.

@timotheeguerin
Copy link
Member

ok for the sake of this proposal I'll assume this one isn't maybe a real need as it might just make things incompehensible when combine with protocol/visibility

@timotheeguerin
Copy link
Member

Other uses case

  • for azure we'd want to be explicit

Concern with this for now:

  • tcgc had cases where it was not clear what the usage was, what are those??
  • any place decorator wouldn't work
  • inout can depends on the layer you are using and having tgo specify that layer in the decorator makes the spec look very hard to read

@steverice steverice moved this to Wishlist in GraphQL Emitter Nov 20, 2024
@markcowl markcowl modified the milestones: [2025] January, Backlog Jan 13, 2025
@timotheeguerin timotheeguerin removed their assignment Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:core Issues for @typespec/compiler design:needed A design request has been raised that needs a proposal triaged:core
Projects
None yet
Development

No branches or pull requests

5 participants