microsoft / typespec

https://typespec.io/
MIT License
4.55k stars 221 forks source link

.NET Client Patterns: Service description for bytes upload should generate client following unbranded .NET client patterns #4909

Open annelo-msft opened 3 weeks ago

annelo-msft commented 3 weeks ago

TypeSpec file:

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

@service({
    title: "PutBytes",
  })

  namespace PutBytes;

  using TypeSpec.Http;
  using TypeSpec.Rest;

  interface PutBytesClient {
    @put @route("upload") Upload(document: bytes) : void;
  }

Expected client output

A diff of expected generator output compared to current generator output is here: https://github.com/annelo-msft/typespec/pull/2

JoshLove-msft commented 3 weeks ago

Currently in order to generate the content type and a binary upload method that doesn't serialize a model, the tsp would need to look like this - note we add the contentType header and body decorator:

  interface PutBytesClient {
    @put @route("upload") Upload(@header contentType: "application/octet-stream", @body document: bytes) : void;
  }
JoshLove-msft commented 3 weeks ago

/cc @schaabs for thoughts on the tsp requirements mentioned in previous comment.

annelo-msft commented 3 weeks ago

Adding the context that @KrzysztofCwalina raised the concern with me directly that those decorators would need to be added to obtain a client that used the headers and didn't write those models.

I am happy to defer to @schaabs and @KrzysztofCwalina regarding whether that is the TypeSpec we would like customers/partners to need to write in order to have our .NET generator write a client that can upload application/octet-stream bytes to a service. For me personally, I would wonder the following about:

@put @route("upload") Upload(document: bytes) : void;
  1. When does it make sense that this TypeSpec would use application/json ContentType header? Is it common?
    • My thinking with this question is -- ideally the most common desired outcome would map to the simplest TypeSpec. It would be preferable if TypeSpec authors needed to write more TypeSpec to satisfy uncommon scenarios, rather than the inverse
  2. Does it make sense that bytes would need a model to support it for the most common case?
    • My thinking here is that we have .NET types (in BCL, Azure.Core, and SCM) that map to bytes. Ideally, these would be used to represent the input parameters of the client method for this service operation in the most common case. Model types make sense when more than this is needed (to provide what is essentially a POCO to group primitives and other BCL/Core types that need to be assembled to write the content of a request to send to the service). If a TypeSpec type has a corresponding .NET BCL/Core type, I am not sure I see the benefit of adding a model to the public API for that -- or having an internal type to wrap it, since that would add performance overhead in terms of allocations and possibly CPU time as well.
JoshLove-msft commented 3 weeks ago

As part of determining the tsp -> C# story, we should keep in mind what the experience would look like across other language generators. It would be odd if a single tsp generated semantically different clients across the languages that we support, which makes me wonder if these type of inferences would be better to make further upstream in TCGC so that all the language emitters get it for free.