Skip to content

CIMD: single-pass Cache-Control parsing and URI verification de-duplication#1481

Draft
Copilot wants to merge 3 commits intoISSUE-45284from
copilot/review-issue-45284-performance
Draft

CIMD: single-pass Cache-Control parsing and URI verification de-duplication#1481
Copilot wants to merge 3 commits intoISSUE-45284from
copilot/review-issue-45284-performance

Conversation

Copy link

Copilot AI commented Feb 28, 2026

ISSUE-45284 requested a review focused on runtime efficiency and maintainability in the CIMD path. This PR tightens hot-path parsing work and removes duplicated validation logic while preserving behavior.

  • Performance

    • Reworked ClientMetadataCacheControl parsing to a single-pass directive scan.
    • Eliminated repeated stream traversals for no-cache, no-store, max-age, and s-maxage.
    • Preserved existing directive semantics (only a single max-age/s-maxage is effective; duplicates invalidate that directive).
  • Maintainability

    • Consolidated repeated metadata URI verification blocks in verifyClientMetadata(...) into verifyClientMetadataUri(fieldName, uriValue).
    • Kept mandatory vs optional field handling explicit at call sites, reducing copy-paste risk without changing validation outcomes.
  • Focused coverage

    • Added ClientMetadataCacheControlTest for:
      • max-age clamping to configured bounds,
      • duplicate max-age handling,
      • combined no-cache + s-maxage directive parsing.
// Before: multiple passes over same directives list
if (sList.stream().filter(i -> i.startsWith("max-age=")).count() == 1) { ... }
if (sList.stream().filter(i -> i.startsWith("s-maxage=")).count() == 1) { ... }

// After: single pass + counters
for (String directive : sList) {
    if ("no-cache".equals(directive)) { ... }
    else if (directive.startsWith("max-age=")) { ... }
    else if (directive.startsWith("s-maxage=")) { ... }
}

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:

  • repo.gradle.org
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.12/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.12/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.12 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.12/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/keycloak/keycloak org.codehaus.plexus.classworlds.launcher.Launcher -pl services -DskipTests compile -q (dns block)
  • repository.jboss.org
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.12/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.12/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.12 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.12/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/keycloak/keycloak org.codehaus.plexus.classworlds.launcher.Launcher -pl services -DskipTests compile -q (dns block)
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.12/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.12/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.12 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.12/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/keycloak/keycloak org.codehaus.plexus.classworlds.launcher.Launcher -pl services -am -DskipTests compile -q (dns block)
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.12/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.12/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.12 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.12/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/keycloak/keycloak org.codehaus.plexus.classworlds.launcher.Launcher -pl services -am -Dtest=ClientMetadataCacheControlTest -Dsurefire.failIfNoSpecifiedTests=false test -q (dns block)

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


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits February 28, 2026 14:45
Co-authored-by: tnorimat <25092005+tnorimat@users.noreply.github.com>
Co-authored-by: tnorimat <25092005+tnorimat@users.noreply.github.com>
Copilot AI changed the title [WIP] Review performance and maintainability of branch ISSUE-45284 CIMD: single-pass Cache-Control parsing and URI verification de-duplication Feb 28, 2026
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.

2 participants