Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Select an option

  • Save weidongxu-microsoft/aa00910f86527abcd2f9435acdb58e52 to your computer and use it in GitHub Desktop.

Select an option

Save weidongxu-microsoft/aa00910f86527abcd2f9435acdb58e52 to your computer and use it in GitHub Desktop.
service_operation_signature_discussion

Generate client method signature which follow the serivce operation signature.

Different language may have different interpretation of the signature, based on best practise of that language.

Interprecate model as class

Model ClientOptions

@resource("resource-collection")
model Resource {
  @key name: string;
  prop1: string;
  prop2?: string;
}

model ClientOptions {
  @header headerParam?: string;
  ...Resource;
}

op put is ResourceCreateOrReplace<ClientOptions>;

Generates

Resource put(ClientOptions clientOptions);

While anonymous model

alias ClientOptions {
  @header headerParam?: string;
  ...Resource;
}

op put is ResourceCreateOrReplace<ClientOptions>;

Generates the usual param + body signature

Resource put(String name, Resource resource, String headerParam);
  • We can see some problem here. @key name in Resource also means the path parameter. Should it be in Resource class as well (instead of name in method argument)? And then think about @parentResource.
  • Other problem would be that some header would be from Traits, e.g. SupportsConditionalRequests related trait that supplies if-match headers.

Mix of model and anonymous model for arguments

@resource("resource-collection")
model Resource {
  @key name: string;
  prop1: string;
}

model RequiredClientOptions {
  ...Resource;
}

model OptionalClientOptions {
  @header headerParam?: string;
  prop2?: string;
}

op put is ResourceCreateOrReplace<{@header headerParam2?: string, ...RequiredClientOptions, ...OptionalClientOptions}>;

Generates

Resource put(RequiredClientOptions requiredClientOptions, OptionalClientOptions optionalClientOptions, String headerParam2);
  • {...RequiredClientOptions, ...OptionalClientOptions} part is already tricky.
  • And SDK still need to order optional argument to last. So it never able to follow the service operation exactly.

Personal opinion

Pro

  • The service operation could directly affect client method signature.
  • Language can make different interpretation that best suits its best practise (e.g. Python may avoid taking ClientOptions as class at all).

Con

  • It is hard to understand, especially when it coupled with template, trait, and resource (that would each contribute some path/query/header parameter).
  • Different interpretation could further degrade readability.
@haolingdong-msft
Copy link

Sorry I miss @body. What I want to express is: it is confusing to me that the service operation with spread or without spread generates the same code.

alias ClientOptions {
  @header headerParam?: string;
  ...Resource;
}
alias ClientOptions {
  @header headerParam?: string;
  @body resource: Resource;
}

@weidongxu-microsoft
Copy link
Author

weidongxu-microsoft commented Jan 12, 2024

Yes, it is.

And this is current behavior of Java and .NET (see Renhe's Loop). Hence, we are unlike to change the behavior (unless we have convincing reason to do this break).

And there is one reason we don't change the behavior, if we want to use the model to group the parameter/properties. In case

@resource("resource-collection")
model Resource {
  @key name: string;
  prop1: string;
}

model RequiredClientOptions {
  ...Resource;
}

model OptionalClientOptions {
  @header headerParam?: string;
  prop2?: string;
}

op put is ResourceCreateOrReplace<{@header headerParam2?: string, ...RequiredClientOptions, ...OptionalClientOptions}>;

If we generate put(prop1, prop2, headerParam2, ...) as a spread, you don't have a way to write typespec to say that we want client method put(RequiredClientOptions requiredClientOptions, OptionalClientOptions optionalClientOptions, ...), where these class is part of the body.
You have @body for a full body, but you don't have @partialBody for part of the body, or a mix of body property and header/query parameter.

@haolingdong-msft
Copy link

For your case, I tested in playground, code, seems typespec-autorest will spread the body properties. I guess our sdk code will also generate with the spread body?
Wonder if there is a way currently to let tsp say they want client method like put(RequiredClientOptions requiredClientOptions, OptionalClientOptions optionalClientOptions, ...)
image

@weidongxu-microsoft
Copy link
Author

weidongxu-microsoft commented Jan 15, 2024

No we don't. SDK emitter and typespec-autorest emitter IS DIFFERENT. Think of your Patch and the ResourceCreateOrUpdate.

For the create case, SDK definitely prefer the round-trip model, i.e., user send a Resource and get a Resource. Spread in this case is not welcome.

The point is so far I cannot think of one that is apparent. If you have the idea, let us know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment