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

Use InputStream's new methods instead of StreamUtils #27702

Closed
wants to merge 2 commits into from

Conversation

yaboong
Copy link

@yaboong yaboong commented Nov 19, 2021

As Spring Framework 6 uses JDK17 for its baseline, we can make use of the methods transferTo(OutputStream out) and readAllBytes() supported by InputStream in JDK9 instead of StreamUtils.

@yaboong yaboong changed the title use InputStream's new methods instead of StreamUtils Use InputStream's new methods instead of StreamUtils Nov 19, 2021
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 19, 2021
@jhoeller jhoeller added this to the 6.0 M1 milestone Nov 19, 2021
* @return the number of bytes copied
* @throws IOException in case of I/O errors
*/
public static int copy(InputStream in, OutputStream out) throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't delete it. We have many legacy projects that still use this functionality. I propose to do this deprecate.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AntonLGVS Restored the method I deleted, thanks.
If it's still needed to support legacy projects, how about making this method use transferTo() in InputStream?
like below

public static int copy(InputStream in, OutputStream out) throws IOException {
    Assert.notNull(in, "No InputStream specified");
    Assert.notNull(out, "No OutputStream specified");

    int byteCount = (int) in.transferTo(out);
    out.flush();
    return byteCount;
}

Copy link
Contributor

@bananayong bananayong Nov 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure Spring should keep StreamUtils#copy method. For Utils such as copy method, It looks like more proper to use apache commons or guava libraries for legacy projects.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yaboong Thanks. It's a good idea to use transferTo() within the copy method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bananayong Why use an external library when Spring provides a basic set of methods. If you want to delete, you must first make a deprecate to give time for the transition. Otherwise, updating the spring package will break everything.

@snicoll snicoll added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 23, 2021
@jhoeller jhoeller modified the milestones: 6.0 M1, 6.0.x Dec 14, 2021
@bclozel bclozel self-assigned this Sep 9, 2022
@bclozel bclozel closed this in fa1f7f6 Sep 12, 2022
@bclozel bclozel modified the milestones: 6.0.x, 6.0.0-M6 Sep 12, 2022
@bclozel bclozel added the in: core Issues in core modules (aop, beans, core, context, expression) label Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants