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

Remove dependency on Commons FileUpload in ContentRequestMatchers #27742

Closed
wkwkhautbois opened this issue Nov 28, 2021 · 5 comments
Closed

Remove dependency on Commons FileUpload in ContentRequestMatchers #27742

wkwkhautbois opened this issue Nov 28, 2021 · 5 comments
Assignees
Labels
in: test Issues in the test module status: declined A suggestion or change that we don't feel we should currently apply

Comments

@wkwkhautbois
Copy link
Contributor

wkwkhautbois commented Nov 28, 2021

In Spring 5.3, multipartData() methods were added to org.springframework.test.web.client.match.ContentRequestMatchers, which are useful for testing a multipart request body.

As a slight limitation, to use this method with Spring 5.3, we need to add dependency on Commons-Fileupload in test scope.

The dependency was removed from Spring 6.0 in conjunction with #27423.

It would great if this change to ContentRequestMatchers would be backported to Spring 5.3.x.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 28, 2021
@sbrannen sbrannen added the in: test Issues in the test module label Nov 29, 2021
@sbrannen sbrannen changed the title Remove dependency to Commons FileUpload in ContentRequestMatchers Remove dependency on Commons FileUpload in ContentRequestMatchers Nov 29, 2021
@sbrannen
Copy link
Member

In any case, the Javadoc for org.springframework.test.web.client.match.ContentRequestMatchers.multipartData(MultiValueMap<String, ?>) needs to be updated to point out that it requires FileUpload support from tomcat-embed-core instead of from Commons FileUpload.

@jhoeller
Copy link
Contributor

That revision in 6.0 was not really about removing the dependency, rather about replacing it with the Commons FileUpload fork in Tomcat core. We are not going to backport that, we'd rather keep the original Commons FileUpload dependency in 5.3.x, otherwise we would force existing 5.3.x test setups to change their dependency declaration.

@sbrannen I'll take of updating the javadoc accordingly, good catch!

@jhoeller jhoeller added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 29, 2021
@jhoeller jhoeller self-assigned this Nov 29, 2021
@wkwkhautbois
Copy link
Contributor Author

Oh, I misunderstood the meaning of that merge request.
Now I see, thank you for your explanation.

@nightswimmings
Copy link

@jhoeller Just a little note, but as of today, the Javadoc is still not updated "Note: This method uses the Apache Commons FileUpload library to parse the multipart data and it must be on the test classpath."

@sdeleuze
Copy link
Contributor

sdeleuze commented Jan 9, 2024

I have created #31988 related issue (and will backport it to 6.0.x).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

6 participants