Skip to content

FilePart.transferTo should accept java.nio.file.Path [SPR-16925] #21464

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

Closed
spring-projects-issues opened this issue Jun 10, 2018 · 3 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jun 10, 2018

Tomasz Elendt opened SPR-16925 and commented

In Spring 5.0 http.codec.multipart.FilePart interface got introduced with
transferTo(File dest) method. I think it's very unfortunate that there's a public interface being added to Spring 5.x (that specify JDK 8+ in its minimum requirements) that uses java.io.File instead of java.nio.file.Path. Interestingly, the File object is completely not needed and the only implementation of that interface calls desc.toPath (and does not use any File-specific method).

For those not familiar with differences -- java.nio.file.Path is an interface representing a file, that has been added in Java 7. It has many advantages over old java.io.File, one of which that it might be associated with custom (non-default) filesystem.

A more concrete example -- if transferTo accepted java.nio.file.Path (and there were no path.toFile calls) one could use in-memory filesystem, like Jimfs in their controller tests.

I filed this bug report having specifically http.codec.multipart.FilePart API in mind, but I think that Spring should migrate to java.nio.file.Path in other places as well. Especially now, that support of older Java versions is dropped. I also think that no new code should be added that uses java.io.File - not sure if that can be verified by some static analysis tool.


Affects: 5.0 GA

Issue Links:

Referenced from: commits 1e5f8cc

@spring-projects-issues
Copy link
Collaborator Author

Tomasz Elendt commented

I think that a proper way to move forward would be to introduce a new method that accepts java.nio.file.Path (and marking the old one as deprecated):

Mono<Void> transferTo(Path dest);

@Deprecated
default Mono<Void> transferTo(File dest) {
    return transferTo(dest.toPath());
}

I can prepare PR for that.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

This comes from its Servlet sibling MultipartFile which also has transferTo(File). We'll introduce transferTo(Path) overloads in both of those places, and possibly others as well, for 5.1.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Rolled into FilePart and MultipartFile as well as ZeroCopyHttpOutputMessage, all with default methods in the interfaces. Since java.io.File is very common still and - e.g. for Servlet multipart handling - what the underlying target APIs are using, we'll rather keep the File variants of those methods in non-deprecated form for the time being, just providing Path variants right next to them for more flexibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants