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

Revise FileSystemResource / FileSystemUtils / FileCopyUtils towards NIO.2 [SPR-15748] #20304

Closed
spring-issuemaster opened this issue Jul 8, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Jul 8, 2017

Qin Chuanqing opened SPR-15748 and commented

I do work in nokia, we do a much strict standard for code which analysis by sonar, with the plugin of called sonarLint which can install in IDEA, I can find some bad code smell, an example is in org.springframework.util.FileSystemUtils, we can find below in attachment, I think we should do something for it. and will never introduce any more.


Affects: 4.3.9

Attachments:

Issue Links:

  • #19822 StandardMultipartFile.transferTo should fall back to manual copy if Part.write doesn't support absolute locations (e.g. on Jetty)
  • #19262 Expose Channel on Resource
  • #20401 FileSystemUtils.deleteRecursively always returns false
  • #20881 PropertySourcesPlaceholderConfigurer can not ignore resource if not found
  • #21373 java.nio.file.Path support in FileSystemResource (with regular createRelative behavior, superseding PathResource)

Referenced from: commits 3714e7b, 12114a9

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 10, 2017

Juergen Hoeller commented

Those are arguably false positives since we do not care about the mkdir() and createNewFile() result there: We only want to make sure they exist, not that we just created them.

Also, our pattern of abstract utility classes without constructor declarations is all over the codebase: Such classes can't get instantiated due to the abstractness, and we ignore the fact that you could create subclasses with public constructors (if somebody has the urge to do, we let it happen).

That said, FileSystemUtils overall is outdated and not used anywhere in the codebase anymore, and now even superseded by NIO in the JDK itself. Let's therefore repurpose this ticket to deprecating FileSystemUtils in Spring Framework 5.0.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 13, 2017

Juergen Hoeller commented

Reopened since Spring Boot is using this, so we'll rather revise it instead of deprecate it.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 17, 2017

Juergen Hoeller commented

We're generally avoiding FileInputStream and FileOutputStream now, as well as file system operations on java.io.File, using the superior NIO.2 API (java.nio.file.Files) instead.

In addition, FileSystemUtils provides variants of deleteRecursively and copyRecursively with Path arguments now since there are no equivalents in the NIO.2 API itself... in contrast to simple copy operations along the lines of FileCopyUtils which do exist in java.nio.file.Files.

An interesting related article, in particular why FileInputStream and FileOutputStream should be avoided: https://arnaudroger.github.io/blog/2017/03/20/faster-reader-inpustream-in-java.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.