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

[Ask] Do we allow mixed usage for file models among mfd and non-mfd operations? #5376

Open
MaryGao opened this issue Dec 16, 2024 · 5 comments
Assignees

Comments

@MaryGao
Copy link
Member

MaryGao commented Dec 16, 2024

Background

Below cases are relevant to multipart/form-data I've derived from todo spec and I'd like to confirm if we have any limitation on model sharing among mfd and non-mfd operations in typespec side?

I am asking because if the model is used in a mfd operation, emitters may handle them differently and generate different APIs for that model. And previously tcgc tried to add some limitations on model mixed usage and would report errors(tcgc diagnostic code) and I guess this is for Azure scope? Then how about unbranded?

playground here

Todo Spec Cases

Say we have models with TodoFileAttachment. The first question would be:

  • Do we allow File(spec) to be used as non-mfd operations for example op createJson(@header contentType: "application/json", contents: File): void;?
model TodoFileAttachment is File;

I would assume yes, since this is todo's pr demoed. Then we could have the model referenced by mfd and non-mfd ops.

  • For Case1, TodoFileAttachment is used in application/json operation.
  • For Case2, TodoFileAttachment is used as mfd body type directly.
  • For Case3, TodoFileAttachment is used as a property of mfd body type.

Here are the questions on how to intercept them:

  • For Case1, for bytes property contents in File, should we intercept as base64 string? How about it is used in other content type? Will we intercept bytes differently?
  • Can Case1 and Case2 co-exist in the same spec? Currently this is NOT allowed in tcgc and tcgc would report errors(tcgc diagnostic code).
  • Can Case1 and Case3 co-exist in the same spec? Currently tcgc would allow this case.
// Case1 - createJson: TodoFileAttachment is used as a normal operation
@post
@sharedRoute
op createJson(
  @header contentType: "application/json",
  item: TodoItem,
  attachments?: TodoFileAttachment[],
): TodoItem;

// Case2 - createForm: TodoFileAttachment is uesed as multipart body type
@post
op createForm(
  @header contentType: "multipart/form-data",
  @path itemId: TodoItem.id,
  @body contents: TodoFileAttachment,
): NoContentResponse | NotFoundResponse;

// Case3 - createFileAttachment: TodoUrlAttachment is used as multipart body property
@sharedRoute
@post
op createFileAttachment(
  @header contentType: "multipart/form-data",
  @multipartBody body: {
    item: HttpPart<TodoItem>;
    attachments?: HttpPart<TodoFileAttachment>[];
  },
): TodoItem;

Thinking in File

Option 1: Take File as a generic file concept

Currently File is a quite generic name and almost every languages have file relevant operations and in typespec we leverage model to represent File but without any limitation on that. Like in todo example both createForm and createJson want to upload files but one is file and the other is a record. This caused misleadings because we only take File as a real file object in multipart operation. In other situations it is just a model with three properties.

I was wondering if we could take File just as file in any cases, maybe another built-in scalar type, kind of bytes type in TypeSpec but having semantics to link with a file object.

@Private.httpFile
scalar File extends bytes;

Option 2: Take File only for a file part in MFD

Or we could limit the scope to multipart file, only allowed to use under @multipartBody. Then maybe it's better to rename as MultipartFile?

Not sure if we are allowed to extend with more properties or spread into another models.

@Private.httpFile
model MultipartFile{
  contentType?: string;
  filename?: string;
  contents: bytes;
}
@MaryGao MaryGao changed the title [Ask] Do we allow mixing usage for models among mulitpart and non-multipart operations? [Ask] Do we allow mixed usage for models among mfd and non-mfd operations? Dec 16, 2024
@msyyc
Copy link
Contributor

msyyc commented Dec 16, 2024

My understanding:

@markcowl
Copy link
Contributor

markcowl commented Dec 16, 2024

I think this is the intention of how bytes should be interpreted @bterlson @witemple-msft @chrisradek @AlitzelMendez feedback appreciated.

  • in a single part message body with a json content-type, like application/json, bytes should be treated as a base64 encoded string
  • in a single part message body with a binary content-type, like application/octet-stream, bytes should be treated as raw bytes
  • in a multipart message body
    • by default (no supplied media type), bytes should be treated as a stream of bytes
    • In a part with a json media type, bytes should be treated as a base64-encoded string
    • in a part with a binary media type, bytes should be treated as raw bytes

@MaryGao
Copy link
Member Author

MaryGao commented Dec 17, 2024

  • Can Case1 and Case2 co-exist in the same spec? Currently this is NOT allowed in tcgc and tcgc would report errors(tcgc diagnostic code).
  • Can Case1 and Case3 co-exist in the same spec? Currently tcgc would allow this case.

@markcowl Any idea if we allow to share the models among mfd and non-mfd operations for unbranded?

@qiaozha
Copy link
Member

qiaozha commented Dec 17, 2024

@markcowl I want to point out the previous bytes related discussion is still pending on typespec side bytes loop

@MaryGao MaryGao changed the title [Ask] Do we allow mixed usage for models among mfd and non-mfd operations? [Ask] Do we allow mixed usage for file models among mfd and non-mfd operations? Dec 27, 2024
@witemple-msft
Copy link
Member

For Case1, TodoFileAttachment is used in application/json operation.
For Case2, TodoFileAttachment is used as mfd body type directly.
For Case3, TodoFileAttachment is used as a property of mfd body type.

All three of these cases can coexist.

Just to clarify, in case 2, File cannot be the type of the multipart body itself, so I assume we are talking about when Http.File is the type of the body of a part in the request, not of the whole request.

Here are the cases:

// Payload: { filename?: string, contentType?: string, contents: base64 }
op case1(@header contentType: "application/json", @body data: Http.File);

// Payload: Multipart Form Data, array of files as parts with name `files`
// Part payload: byte stream
op case2(@header contentType: "multipart/form-data", @multipartBody: { files: HttpPart<Http.File>[] });

// Payload: Multipart Form Data, array of JSON objects as parts with name: `files`.
// Part Payload: { file: { filename?: string, contentType?: string, contents: base64 } }
op case3(@header contentType: "multipart/form-data", @multipartBody: { files: HttpPart<{ file: Http.File }>[] });

In general, when bytes appears in a JSON object, it has a default encoding that represents it as a base64 string. In other content-types, it may be treated differently.

In @typespec/http-server-js, I always represent File the same way with the following interface:

interface File {
  filename?: string;
  contentType?: string;
  contents: Uint8Array;
}

The serialization/deserialization of the type changes depending on the content-type it's referenced from, but it always ends up in that shape. If I had to generate a different representation of the type for multipart vs. JSON, then I'd add a suffix or prefix to the type names to distinguish them, i.e. JsonFile and MultipartFormDataFile.

@bterlson @markcowl I think there is probably a fourth case, where the content-type is unspecified and the body is exactly File, as in the following case:

op case4(@bodyRoot file: Http.File);

In this case we may want to get contentType from the request content-type header, but I'm not sure where we would get filename from.

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

No branches or pull requests

6 participants