.NET Client Patterns: Simple service description should generate client following unbranded .NET client patterns

[annelo-msft]

TypeSpec file:

import "@typespec/http";
import "@typespec/rest";
import "@typespec/openapi3";

@service({
    title: "SimplePost",
  })
   
  namespace SimplePost;
   
  using TypeSpec.Http;
  using TypeSpec.Rest;
   
   
  interface SimplePostClient {
    @post @route("add") IncrementCount(@path addend: int32) : int32;
  }

Expected client output

• Client should have a public constructor that takes endpoint and client options
• Client constructor should validate arguments, assign endpoint, create options if needed, and create pipeline
• Convenience methods should take CancellationToken parameters
• Protocol methods should take a nullable RequestOptions parameter
• Rest file should only create instances of classifiers used by the client
• Rest file should not re-allocate a classifier that has already been allocated
• Request creation helpers should not add headers the service doesn't use
• Emitted internal helper files should not have methods that aren't used by the client
• Emitted internal helper files should be added if needed by the client
• Nullable annotations should be used
• No subclient factory method should be added when there are no subclients
• No internal constructors should be added when they are not called
• (would be nice) use file scoped namespace declarations

A diff of expected generator output compared to current generator output is here: annelo-msft#1

[jsquire]

//cc: @m-nash, @JoshLove-msft, @schaabs

[KrzysztofCwalina]

One additional thing: the client should treat 200 as success. Today, the mgc generator generates clients that always when service methods are called as no status codes are treated as success by default.

[annelo-msft]

@KrzysztofCwalina, can you provide more information about the problem you're seeing? In the output code I referenced in the issue, I am seeing that the response classifier is set to a "200 success" classifier:

message.ResponseClassifier = PipelineMessageClassifier200;

What code/behavior are you seeing that indicates clients aren't doing this today?

[JoshLove-msft]

Note that the reason the public constructor isn't generated and instead the subclient factory methods are generated is that we are defining an interface without also attributing with the client decorator. Interfaces are treated as subclients. /cc @schaabs if this is not the desired behavior.

If we instead inlined the operation, we would get the public constructor. Alternatively, if we did the following:

  @client
  interface SimplePostClient {
    @post @route("add") IncrementCount(@path addend: int32) : int32;
  }

we would also get a public constructor.

[annelo-msft]

Interesting - thanks for the clarification!

I have thoughts similar to what I posted here (#4909 (comment)) about this, but if it is as simple as adding a @client decorator, then my concerns are a bit smaller than the ones voiced in that comment.

I guess my question here would be: what is more common, generating clients or generating clients with subclients? Similar to my other comments, it seems like a better outcome for users if writing less code gets them the most common case. If subclients are less common, maybe it would be better to have a @subclient decorator than writing a subclient if a @client decorator is absent? I think a good end-goal here would be "if I write the least possible code I get the most likely outcome."

[JoshLove-msft]

Well it would be even less code to not include the interface at all, and then you would get the single client. I wonder if the interface is needed for the simple playground example.

[jsquire]

maybe it would be better to have a @subclient decorator than writing a subclient if a @client decorator is absent?

Is that something in our control or does that tie back to TCGC and/or TypeSpec cross-language behavior?

[annelo-msft]

Is that something in our control or does that tie back to TCGC and/or TypeSpec cross-language behavior?

I defer to @schaabs and others here. But it would be helpful if @KrzysztofCwalina could share his specific concerns here, and why he had originally expected the above TypeSpec to generate a different client than it does today.

[KrzysztofCwalina]

I expected playground examples to generate good client library.

The @client annotation is not in the rest library. It's Azure specific thing. And so we cannot use it for 3rd parties.

[JoshLove-msft]

I expected playground examples to generate good client library.

The @client annotation is not in the rest library. It's Azure specific thing. And so we cannot use it for 3rd parties.

I wonder if the playground example could be made simpler by not defining an interface.

[annelo-msft]

I see. I think there are two different considerations here: (1) can the generator write a client that follows guidelines for a given service API from some input TypeSpec ?, and (2) what do we think is the right TypeSpec to input to have the generator output a given client ?

I filed this issue with the intent to address (1). I understand from @jsquire that @schaabs is the correct person to work with for (2). I will loop back with @KrzysztofCwalina regarding his intentions around these asks -- whether it is to drive the former or the latter.

[annelo-msft]

Actually, I think there is a third question as well: (3) should the generator write a valid client from any valid TypeSpec ? If we decide the answer to (3) is no, it might be worth exploring at what point in the toolchain the inability to create a valid client is communicated to the human seeking to create the client -- is it at the TypeSpec layer, or somewhere in the generator layers? It is probably not an outcome we want for valid TypeSpecs to result in client code that doesn't compile and/or fails to create components like the pipeline that are required for the client to function.

[jsquire]

The answer to 3 is out of scope. It's a question wider than these efforts and needs to be explored in the cross-language working group. As a general rule, the answer to that is "no" - TypeSpec itself is not intended to be a blueprint for generating clients.

[annelo-msft]

As a general rule, the answer to that is "no" - TypeSpec itself is not intended to be a blueprint for generating clients.

Do you think that if the generator is unable to output a valid client, it might be helpful to the person invoking the generator to get an exception or some sort of error stating that the TypeSpec doesn't represent a valid client? It feels to me that writing client code that is invalid without some sort of explicit failure might lead people using the generator to believe that the output client should be valid.

[jsquire]

On the surface - yes, that would be helpful. I think it may be a difficult heuristic to define clearly and completely - what constitutes a "valid client?" The generator should already emit a message and refuse to generate when it encounters a pattern that it doesn't understand. It does not track over state for the entire emitted surface or a specific type and ask "does this entire thing make sense."

This is another thing that would need to be pushed up to the cross-language working group for discussion.

[annelo-msft]

what constitutes a "valid client?"

My thinking here was that a valid client might be defined as one that both compiles and can succeed at returning a value that is useful to an end-user that invokes it for service methods it exposes. I am happy to defer to the generator team regarding whether they want to raise a warning or not if such a client cannot be generated. It is likely that such an output would be discovered by the team creating the client at some point.

[JoshLove-msft]

It sounds like other languages are not generating subclients if the top-level namespace does not have any operations. So maybe we should do this.

[jsquire]

@schaabs: This one falls into your purview, as it has large potential to overlap with the cross-language working group and common agreements for TypeSpec => language conversion. Please take point on this one and work with our .NET Core/Generator/Architects team to come to a decision here.