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

Resteasy Reactive Client with multipart forms #22658

Closed
andlinger opened this issue Jan 5, 2022 · 11 comments · Fixed by #22857
Closed

Resteasy Reactive Client with multipart forms #22658

andlinger opened this issue Jan 5, 2022 · 11 comments · Fixed by #22857
Assignees
Labels
Milestone

Comments

@andlinger
Copy link
Contributor

Description

We use the Resteasy Rest Client with Multipart Forms. We would like to switch to Resteasy Reactive with the whole project. However, in our POJO form we use a multipart annotation for which there is no counterpart in Resteasy Reactive.

It would be great if the @PartFilename annotation could be implemented.

public class OurForm {
  @FormParam("files")
  @PartFilename("document.txt")
  @PartType(MediaType.TEXT_PLAIN)
  private String documentFile;
}

Implementation ideas

No response

@andlinger andlinger added the kind/enhancement New feature or request label Jan 5, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Jan 5, 2022

/cc @FroMage, @geoand, @stuartwdouglas

@geoand
Copy link
Contributor

geoand commented Jan 5, 2022

This is one for @michalszynkiewicz

Just to be clear, you are asking for @PartFilename on the server side? The idea being I assume that you control the saved file name?

@andlinger
Copy link
Contributor Author

I'm not quite sure what you mean by server side.

I have following rest client in my Quarkus backend:

@Path("/convert")
@RegisterRestClient
public interface Client {

    @POST
    @Consumes(MediaType.MULTIPART_FORM_DATA)
    @Produces("application/pdf")
    byte[] convert(@MultipartForm OurForm form) throws PdfGenerationException;
}

This client is used in our Quarkus backend to send a text file, which we have in our OurForm object, to another server and have this text file converted to a PDF.

@geoand
Copy link
Contributor

geoand commented Jan 5, 2022

Okay, so understood this wrong.

@michalszynkiewicz this is for you after all

@michalszynkiewicz
Copy link
Member

I added this to my todo list but I won't have time for it soon so I'm leaving it unassigned.

@geoand
Copy link
Contributor

geoand commented Jan 13, 2022

I'll take it as it seems like something we want for 2.7

@geoand geoand self-assigned this Jan 13, 2022
@geoand geoand removed the area/rest label Jan 13, 2022
geoand added a commit to geoand/quarkus that referenced this issue Jan 13, 2022
geoand added a commit to geoand/quarkus that referenced this issue Jan 13, 2022
gsmet added a commit that referenced this issue Jan 13, 2022
Allow specifying the filename of multipart requests in Reactive REST Client
@quarkus-bot quarkus-bot bot added this to the 2.7 - main milestone Jan 13, 2022
@andlinger
Copy link
Contributor Author

@geoand Are you sure this works like the non-reactive rest client? I just changed the dependencies to the Reactive Rest Client on Quarkus 2.7.0.CR1 and now it doesn't work anymore. The server, which is contacted with the REST client, no longer recognizes the transferred data.

Update:
The @PartFilename annotation only works when the annotated member is a java.io.File object. In the past this also worked if the member was of type String.

@geoand
Copy link
Contributor

geoand commented Jan 20, 2022

Right, we should probably only allow it on File, which is semantically more correct. WDYT @michalszynkiewicz ?

@andlinger
Copy link
Contributor Author

But in the JAX-RS client it also works with String and Buffer. Shouldn't it be the same in both implementations?

@geoand
Copy link
Contributor

geoand commented Jan 20, 2022

There is always a case to be made whether we should be perfectly compatible vs doing something that more semantically correct.

When it involves non-core features, I tend to prefer the latter. But in this case, it's ultimately up to @michalszynkiewicz

@andlinger
Copy link
Contributor Author

Is there a decision on this?

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

Successfully merging a pull request may close this issue.

3 participants