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

[TCGC] design discussion, support options parameter in SDK #831

Open
4 tasks done
weidongxu-microsoft opened this issue May 14, 2024 · 0 comments
Open
4 tasks done
Labels
lib:tcgc Issues for @azure-tools/typespec-client-generator-core library needs-area

Comments

@weidongxu-microsoft
Copy link
Member

weidongxu-microsoft commented May 14, 2024

Clear and concise description of the problem

Requirement

route can be

@resource("resource-collection")
model Resource {
  @key name: string;
  prop1: string;
}
model RequiredClientOptions {
  prop1: string;
}
model OptionalClientOptions {
  @header headerParam?: string;
  @query queryParam?: string;
  prop2?: string;
}
@trait
model RequestPropertiessTrait {
  @traitContext(TraitContext.Action)
  request: {
    @traitLocation(TraitLocation.Parameters)
    properties: {
      prop3: string
    };
  };
}
model Response {
}

op action is ResourceOperations.ResourceAction<
  Resource,
  {@header headerParam2?: string, ...RequiredClientOptions, ...OptionalClientOptions},
  Response,
  RequestPropertiessTrait>;

The source of request parameters could be from operation, from (shared) model (explicit or implicit body property), from (shared) template, from (shared) trait. This is one of the difficulties, that dev is likely not able to write an operation with a single model that contains all request parameters and body properties, as they uses Azure.Core template and trait.

A typical situation in SDK is that the parameters of the API is too long/complex, and SDK would like to group them into an options parameter pattern, e.g.

public Response action(ActionOptions actionOptions);

public class ActionOptions {
  String name; // path
  String prop1; // body
  String prop2; // body
  String prop3; // body
  String queryParam; // query
  String headerParam; // header
  String headerParam2; // header
}

(some properties in ActionOptions is required)

A slightly different API could be

public Response action(String name, ActionOptions actionOptions);

Alternatives on Design

One major difficulty is to have the mapping of the properties in (if we define this options parameter model in client.tsp)

model ActionOptions {
  name: Resource.name;
  prop1: RequiredClientOptions.prop1;
  prop2?: OptionalClientOptions.prop2;
  prop3: string; // not able to ref
  queryParam?: OptionalClientOptions.queryParam;
  headerParam?: OptionalClientOptions.headerParam;
  headerParam2?: string; // not able to ref
}

to the parameters of the operation

op action is ResourceOperations.ResourceAction<
  Resource,
  {@header headerParam2?: string, ...RequiredClientOptions, ...OptionalClientOptions},
  Response,
  RequestPropertiessTrait>;

Candidate design: provide a model name for "options parameter"

Provide a decorator that only provide the name for "options parameter".

@@optionsParameter(Service.action, "ActionOptions");

Emitter would put all the path/header/query/property of the API into this options parameter.
This solution could avoid the difficulty of mapping the properties.

For a POST action already have a model as request body, we can ask dev to use an alias/spread to avoid the model be visible in SDK.

This may also solve the problem that Python want the alias/spread (i.e., all parameters and body properties on method signature), and other language want the options parameter -- via the scope of @optionsParameter.

This design is not able to support API with finer structure like public Response action(String name, ActionOptions actionOptions).

Candidate design: TOM pattern

model ActionOptions {
  @sourceProperty(RequiredClientOptions.prop1)
  prop1: RequiredClientOptions.prop1;

  @sourceProperty(OptionalClientOptions.prop2)
  prop2?: OptionalClientOptions.prop2;

  @sourceProperty(<what-to-do?>)
  prop3: string;

  @sourceProperty(OptionalClientOptions.queryParam)
  queryParam?: OptionalClientOptions.queryParam;

  @sourceProperty(OptionalClientOptions.headerParam)
  headerParam?: OptionalClientOptions.headerParam;

  @sourceProperty(<what-to-do?>)
  headerParam2?: string;
}

@serviceOperation(Service.action)
op action(name: Resource.name, actionOptions: ActionOptions);

This is a most flexible representation in client.tsp. However, it would involve the complexity of mapping the properties. It is hard to figure out that ActionOptions.prop3 is from trait RequestPropertiessTrait.reuqest.properties.prop3.
And it could be hard to maintain, if service evolve the API and add more (optional) parameters/properties.

Welcome suggestions on better design

Checklist

  • Follow our Code of Conduct
  • Check that this issue is about the Azure libraries for typespec. For feature request in the typespec language or core libraries file it in the TypeSpec repo
  • Read the docs.
  • Check that there isn't already an issue that request the same feature to avoid creating a duplicate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib:tcgc Issues for @azure-tools/typespec-client-generator-core library needs-area
Projects
None yet
Development

No branches or pull requests

2 participants