Projects that use java heavily rely on a clunky, verbose, and error prone method to access Resources for tests.
Adding the ability to request a file's contents as a string for testing REST or WEBMVC allows for cleaner test classes and more readable code.
Comment From: sbrannen
Thanks for the PR.
I'm not yet sure if we'll introduce functionality like this, but if we do, we'll need to take the following into account.
- Adding a
compile
dependency oncommons-io
is not an option. Instead, we'd likely want to use our ownFileCopyUtils
(potentially introducing new methods if necessary to allow for configurable encoding). - The code must adhere to the project's code style. Executing
./gradlew check
would be a good place to start to see Javadoc violations. - The name
getString()
is potentially misleading. A better name would be something likegetContentAsString()
. - Why would we want to limit this to
ClassPathResource
? Why not introduce it inResource
?
Comment From: mp911de
That's a neat optimization that sounds useful on the Resource
level.
Comment From: derrick-pericipio
All great feedback:
- Not sure why my local build didnt flag the code style violation.. my bad.
- I wondered what level to introduce this at so I started higher rather than lower.
-
I'll look at
FileCopyUtils
, can you elaborate on the desire to avoid commons? It's already on the class path at the parent project level. Ultimately it just makes the implementation cleaner, it's not necessary for functionality. -
Thoughts on throwing both
FileNotFoundException
andIOException
? I couldn't think of a more appropriate set when I put this together.
Comment From: sbrannen
As @jhoeller mentioned elsewhere, have you considered wrapping your ClassPathResource
in an EncodedResource
and then passing encodedResource.getReader()
to org.springframework.util.FileCopyUtils.copyToString(Reader)
?
Comment From: derrick-pericipio
@sbrannen
My ultimate goal was a developer quality of life improvement to bring this on par with the Kotlin experience. I will work on implementing this without Commons
Comment From: derrick-pericipio
I have pushed the implementation of the new function getContentAsString()
down to the Resource interface and provided it with a default behavior of toString()
to avoid having to override in both the Web and MVC modules.
The Concrete implementation was provided at the AbstractFileResolvingResource
level as any local or jar file should be able to be serialized. It no longer uses Apache Commons and uses the same base raw Java implementation as the Kotlin equivalent. This does rely on Java version being 7 or higher which I did not consider an issue as the minimum for the project is 8.
Ultimately I expect this to be utilized during Contract Driven and Test Driven Development processes to avoid having to use Raw String variables and to pull developers away from using ResourceUtils directly with either Commons or the overtly verbose Files.readBytes(File.path, Charset))
and having to handle the exceptions on their own (or forgetting)
This is a largely QOL enhancement to the Java developer experience as Kotlin provides a convenience function known as getResource(path).readText()
An alternative reasoning behind this enhancement is to close the need for access to ResourceUtils outside of the spring framework and allow for moving that Class to package private in a future change.
Comment From: derrick-pericipio
Please let me know if there is any other change desired for this enhancement.
If not, I look forward to potentially seeing this contribution join the Spring Ecosystem.
Comment From: rstoyanchev
Couldn't this be implemented for FileSystemResource
as well?
If the primary use case for this is in tests, I'm not sure I like seeing the methods directly on Resource
. Another option could be ResourceUtils
but even then the implementation looks so straight forward, we could just make it a test utility somewhere.
You mentioned for "testing REST or WEBMVC". Is there a specific place you've noticed where we could add this so resources are logged?
Comment From: derrick-anderson
Picking this back up on my new account. @rstoyanchev.
When i mentioned the use case it was in reference to situations when it made sense to serve static content from a jar file. Often i've seen it for sample json payloads for dummy rest endpoints (when users could consume a stub jar etc)
I can implement it for the FileSystemResource
if that makes sense. I pushed it down to Resource at @sbrannen 's reccomendation.
Where do we think this should belong?
Comment From: poutsma
Thank you for submitting a PR, and our apologies for taking this long to resolve it.
We made several changes to the PR, see https://github.com/spring-projects/spring-framework/commit/12d4dc1bae6e97dc8de43acac9e619a009207021 for more details.
Comment From: derrick-anderson
Thank you for the update! I'll review the updates this weekend.
Derrick Anderson (He/Him)