reactiveui / refit

The automatic type-safe REST library for .NET Core, Xamarin and .NET. Heavily inspired by Square's Retrofit library, Refit turns your REST API into a live interface.
https://reactiveui.github.io/refit/
MIT License
8.27k stars 738 forks source link

Complex types as MultiPartItem #734

Closed candidodmv closed 3 years ago

candidodmv commented 4 years ago

Is your feature request related to a problem? Please describe.

I came across the necessity to upload a file with additional information, in the real world, would be something like a formulary where a user fill with several information, and at the end it's possible to attach a file (image, doc, etc..). In Refit, to do this acording the documentaion is:

    [Multipart]
    [Post("/users/{id}/photo")]
    Task UploadPhoto(int id, [AliasAs("myPhoto")] StreamPart stream);

but if i need to pass that information cited above the method signature become something like this:

    [Multipart]
    [Post("/users/{id}/photo")]
    Task UploadPhoto(int id,
          [AliasAs("name")] string info1,
          [AliasAs("email")] string info2,
          [AliasAs("tel")] string info3,
          [AliasAs("message")] string info4,
          [AliasAs("whatever1")] string info5,
          [AliasAs("whatever2")] string info6,
          [AliasAs("whatever3")] string info7,
          [AliasAs("whatever4")] string info8,
         ............
          [AliasAs("attachment")] StreamPart stream);

as it's possible to see at above sample the signature growth as follows the parameter needs.

Describe the solution you'd like

I believe that would be clearer if this scenario could be replaced with something like this:

    [Multipart]
    [Post("/users/{id}/photo")]
    Task UploadPhoto(int id,
          MyModel model,
          [AliasAs("attachment")] StreamPart stream);

where model implement something like IMultiPartItem with a method IEnumerable<HttpContent> GetContents and could be merged with the any other parmeters in the signature as currently there is.

Describe alternatives you've considered

I see the code and the common type between this MultipartItem but the reuse for this purpose not applied because of it's property:

public string FileName { get; }

In this scenario will not be a file, but a content, more specifc a StringContent that there's no "FileName " property.

Describe suggestions on how to achieve the feature

include another allowed type multipart methods support the following parameter types: currently: string (parameter name will be used as name and string value as value) byte array Stream FileInfo new: IMultiPartItem

and get it's content througth your method GetContents

Additional context

If the context that I have described could be achieved already there's , please, not consider my suggestion. Thanks

ch4m0n1x101 commented 4 years ago

Hi,

I think I am experiencing the same issue:

[Multipart] [Post("/api/job/AddImageBank")] Task AddImageBanks([Body] IEnumerable imageBanks, CancellationToken cancellationToken);

Where ImageBankDto is something like (more properties exist):

{ public int Id public string Description { get; set; } public StreamPart ImageFile { get; set; } }

Initially I get "Multipart requests may not contain a Body parameter"

If I try without the Body parameter i get "Unexpected parameter type in a Multipart request. Parameter is of type ImageBankDto, whereas allowed types are String, Stream, FileInfo, Byte array and anything that's JSON serializable"

Any ideas on a solution?

Thank in advance.

candidodmv commented 4 years ago

Hi @ledragon101 ! In your scenario, I try to follow the idea to proccess one by one item, because this approach of to send IEnumerable could have unexpected results. You could try to store the items that you want so sent locally and proccess and if succeed you mak as done, this can be useful too if happed fail. so, you do as follows I mentioned above, separate the properties of the model and sent one by one. I hope that help you.

ch4m0n1x101 commented 4 years ago

@candidodmv Hi Thanks for the reply. I do actually proecss the way you suggest by storing locally first and marking as sent upon success response. Originally I was converting images to base64 string but I'm having memory issues and garbage collection is slowing down the overall time of the request. This is why I thought I'd try sending as multipart - but that's another issue. I have tried sending one by one also but thought I'd have a go at sending all as one request to see if it was quicker as potentially there can be dozens of images to send as part of a single user action.

candidodmv commented 4 years ago

@ledragon101 your idea to try send all in one time to turn the process more quick can be a missconception, because the fact that you are sending one request with 10mb of payload is equal to send 100 requests of 100kb. You need consider that your final user can be using any kind of device, with less hardware or connection speed to low, and so on, all of this can make impossible to complete the operation in some groups of people. So, you must be aware this conditions to make you decision of what approach to use.

What I do is like this:

[Multipart]
    [Post("/users/{id}/photo")]
    Task UploadPhoto(int id,
          [AliasAs("name")] string info1,
          [AliasAs("email")] string info2,
          [AliasAs("tel")] string info3,
          [AliasAs("message")] string info4,
          [AliasAs("whatever1")] string info5,
          [AliasAs("whatever2")] string info6,
          [AliasAs("whatever3")] string info7,
          [AliasAs("whatever4")] string info8,
         ............
          [AliasAs("attachment")] StreamPart stream);

As you can see in the snippet above, my needs was to send one file ([AliasAs("attachment")] StreamPart stream)

ps. My scenario is mobile applications, if you aren't in the same situation reconsider my thoughts

natalie-o-perret commented 4 years ago

@benjaminhowarth1 Can I dev something about that?

clairernovotny commented 3 years ago

Closing due to age. Please try Refit v6 and reopen if still an issue.

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.