Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Save weidongxu-microsoft/aa00910f86527abcd2f9435acdb58e52 to your computer and use it in GitHub Desktop.
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

https://gist.github.com/weidongxu-microsoft/aa00910f86527abcd2f9435acdb58e52#mix-of-alias-and-model-for-arguments
The case is Mix of alias and model for arguments, but I don't see alias definition.

@weidongxu-microsoft
Copy link
Author

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

alias does not necessary mean alias a = {}. {} is same.

@haolingdong-msft
Copy link

Emm, I thought {} is anonymous model.. I'm a bit confused between anonymous model and alias, when is {} treated as anonymous model, when alias?

@weidongxu-microsoft
Copy link
Author

OK, for all instances we actually mean "anonymous model".

Just we use alias too many times. alias just a syntax, it is fine to say alias A = B where B is a model or alias A = string.

@haolingdong-msft
Copy link

haolingdong-msft commented Jan 11, 2024

Clarify one thing: those tsp definitions are written in client.tsp or main.tsp? Seems this doc has nothing to do with client.tsp/TOM.
(This is discussing when service define an operation like above, what should we generate)

@weidongxu-microsoft
Copy link
Author

weidongxu-microsoft commented Jan 11, 2024

Yes, this design would all be in main.tsp. Emitter would infer about what to generate, depends on the model / anonymous model.

That is the reason why I say "It is hard to understand". It is hard for service/dev to know when to use model, when to use anonymous model, and how the choice affects emitter and SDK method signature.

(And our current spread https://github.com/Azure/cadl-ranch/tree/main/packages/cadl-ranch-specs/http/parameters/spread is kind of doing this. And Renhe's Loop "Spread Alias & Models in Method Signature" exists)

@haolingdong-msft
Copy link

haolingdong-msft commented Jan 12, 2024

I see, yes, service team does not know what will sdk look like when defining service operation currently, so it would be harder to let them describe how they want the client method look like. It's better if we could let service team see what does sdk look like when they write tsp, I guess having sdk emitters integrated to current playground could help. haha :) This maybe long-term though.

Back to the service operation, I do feel it is confusing, the behavior is different when using model and anonymous model, and for anonymous model. It would be even more confusing when using anonymous model together with spread.

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

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

op put is ResourceCreateOrReplace<ClientOptions>

Generates

Resource put(String name, Resource resource, String headerParam);

I don't quite understand why Resource is spread in ClientOptions, but in the generated method, the parameters are not spread, signature not Resource put(String name, String prop1, String prop2, String headerParam);

What will the generated method look like if tsp is define like below?

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

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

op put is ResourceCreateOrReplace<ClientOptions>

@weidongxu-microsoft
Copy link
Author

weidongxu-microsoft commented Jan 12, 2024

The ...Resource and resource: Resource is different on JSON.

Former have JSON {"prop1":"", "prop2":""}, latter be {"resource": {"prop1":"", "prop2":""}}

Unless you do @body, it is a body property. So resource is a property of request, not the request. (and typespec likely would give error, if you write this, for ResourceCreateOrReplace ask for a @resource the request)

@weidongxu-microsoft
Copy link
Author

weidongxu-microsoft commented Jan 12, 2024

What is equvalent is this

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

and

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

@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