Repository navigation
refactor(admin): drop commons-io by replacing two IOUtils.copy call sites (#7607) - #7887
Merged
Merged
Conversation
Aias00
approved these changes
Oct 21, 2026
Aias00
left a comment
Aias00
left a comment
Contributor
There was a problem hiding this comment.
Review: drop commons-io from shenyu-admin
Straightforward and complete. I verified the important part — that the removed dependency has no remaining callers — rather than assuming it:
org.apache.commons.ioappears in exactly two files across the whole repository on master (SandboxServiceImpl,ClusterForwardFilter), and both are converted here. So the direct dependency can safely go, andpr_buildconfirms it compiles.StreamUtils.copy(InputStream, OutputStream)has the same contract asIOUtils.copy: it returns the byte count, does not close either stream, and does not flush. Both call sites still flush explicitly (response.getOutputStream().flush()/response.flushBuffer()), so the response-writing semantics are unchanged.- Removing the version-pinned dependency block does not orphan
commons-io.versionin the root POM; other modules still use the property.
Non-blocking notes:
StreamUtils.BUFFER_SIZEis 4096 whileIOUtils.copydefaults to 8192, so proxied response bodies are now copied in half-size chunks. Irrelevant for admin-sized payloads, butClusterForwardFilterforwards arbitrary gateway responses — if you ever care,StreamUtils.copy(in, out, 8192)restores the old buffer.- This only removes the direct declaration.
commons-iois a very common transitive of test/proxy libraries, so the jar may still land on the admin classpath. If dropping it from the shaded distribution was the goal, confirm with./mvnw -pl shenyu-admin dependency:tree -Dincludes=commons-io:commons-iobefore claiming it is gone.
Nothing else to flag — the diff is minimal, imports are cleaned up, and the two conversions preserve behaviour.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7607.
Summary
The issue lists both
commons-ioandcommons-collections4for removal fromshenyu-admin. On master,commons-collections4is imported by ~65 files (not removable now), so this PR — as proposed in the comment above — only removescommons-io, whose entire admin usage is twoIOUtils.copycalls.Modifications
IOUtils.copy(...)call sites withorg.springframework.util.StreamUtils.copy(...)(ClusterForwardFilter,SandboxServiceImpl). Identical semantics: buffered copy with both streams left open for the caller.commons-iodependency fromshenyu-admin/pom.xmlonly. The${commons-io.version}property stays —shenyu-plugin-cryptorandshenyu-protocol-tcpstill reference it.Verifying this change
Targeted tests pass (
-pl shenyu-admin -am):Grep check passes:
Checkstyle passes:
./mvnw -B -ntp -pl shenyu-admin checkstyle:check # clean