Skip to content

Make ZIPCompressedMessage compression level instance-scoped while preserving (deprecated) legacy static API#2783

Open
Copilot wants to merge 6 commits into
masterfrom
copilot/fix-zipcompressedmessage-compression-level
Open

Make ZIPCompressedMessage compression level instance-scoped while preserving (deprecated) legacy static API#2783
Copilot wants to merge 6 commits into
masterfrom
copilot/fix-zipcompressedmessage-compression-level

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 17, 2026

ZIPCompressedMessage currently stores compression level in static state, so constructing/configuring one message can unintentionally affect others. This change scopes compression level to each message instance while keeping legacy behavior for older call sites and on-wire compatibility.

  • Compression level state model

    • Introduced per-instance compressionLevel.
    • Kept static setLevel(int) as a default-setting API for newly created messages.
    • Constructor overload with explicit level now sets only that instance.
  • Backward compatibility

    • Serialization/deserialization format is unchanged (ZIPSerializer behavior unchanged), so peers on older versions can still parse messages.
    • Existing uses of ZIPCompressedMessage.setLevel(...) continue to work as a global default for future instances.
  • API/docs cleanup

    • Added setCompressionLevel(int) for explicit instance-level configuration.
    • Deprecated static setLevel(int) and updated JavaDoc to direct users to setCompressionLevel(int).
    • Added getCompressionLevel() as the primary instance getter.
    • Kept getLevel() as a deprecated compatibility alias that forwards to getCompressionLevel(), with JavaDoc guidance to migrate.
    • Updated JavaDoc to reflect default-vs-instance semantics and corrected ZIP level wording (1 = fastest/least compression, 9 = slowest/best compression).
  • Focused regression coverage

    • Added tests to verify:
      • instance-specific level does not leak across messages
      • static default still applies only to subsequently created instances
      • deprecated getLevel() remains compatible and delegates consistently to getCompressionLevel()
ZIPCompressedMessage.setLevel(6); // default for new instances (deprecated API)

ZIPCompressedMessage a = new ZIPCompressedMessage(msgA);      // level 6
ZIPCompressedMessage b = new ZIPCompressedMessage(msgB, 1);   // level 1 (instance-only)
ZIPCompressedMessage c = new ZIPCompressedMessage(msgC);      // still level 6

int level = b.getCompressionLevel(); // preferred
int legacy = b.getLevel();           // deprecated alias

> [!WARNING]
>
>

Copilot AI changed the title [WIP] Fix ZIPCompressedMessage compression level to be non-static Make ZIPCompressedMessage compression level instance-scoped while preserving legacy static API May 17, 2026
Copilot AI requested a review from riccardobl May 17, 2026 14:53
@riccardobl riccardobl marked this pull request as ready for review May 17, 2026 17:29
*
* @param level The level.
*/
public static void setLevel(int level) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot deprecate this and suggest the use of the non-static method

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 17a995e: ZIPCompressedMessage.setLevel(int) is now deprecated and its JavaDoc points to the non-static setCompressionLevel(int) method.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 21, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dl.google.com
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED --add-opens=java.base/java.time=ALL-UNNAMED -XX:MaxMetaspaceSize=384m -XX:+HeapDumpOnOutOfMemoryError -Xms256m -Xmx512m -Dfile.encoding=UTF-8 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Make ZIPCompressedMessage compression level instance-scoped while preserving legacy static API Make ZIPCompressedMessage compression level instance-scoped while preserving (deprecated) legacy static API May 21, 2026
Copilot AI requested a review from riccardobl May 21, 2026 17:01
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot rename this to getCompressionLevel method, then create a new getLevel that is deprecated and points to getCompressionLevel. Mention in its javadoc to use getCompressionLevel

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Implemented in this PR: getCompressionLevel() is now the primary method, and deprecated getLevel() forwards to it with JavaDoc guidance. Added compatibility assertions in tests in aee9c45.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 21, 2026

@riccardobl I've opened a new pull request, #2794, to work on those changes. Once the pull request is ready, I'll request review from you.

@riccardobl
Copy link
Copy Markdown
Member

@riccardobl I've opened a new pull request, #2794, to work on those changes. Once the pull request is ready, I'll request review from you.

@copilot , just apply these patches to this pr...

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 21, 2026

@riccardobl I've opened a new pull request, #2794, to work on those changes. Once the pull request is ready, I'll request review from you.

@copilot , just apply these patches to this pr...

Applied directly to this PR as requested. Latest commits are d9133c2 and aee9c45.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dl.google.com
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED --add-opens=java.base/java.time=ALL-UNNAMED -XX:MaxMetaspaceSize=384m -XX:+HeapDumpOnOutOfMemoryError -Xms256m -Xmx512m -Dfile.encoding=UTF-8 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI requested a review from riccardobl May 21, 2026 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ZIPCompressedMessage compressionLevel should not be static

2 participants