Skip to content

Improve performance and maintainability of CIMD client policy code#1480

Closed
Copilot wants to merge 2 commits intoISSUE-45284from
copilot/review-issue-45284-another-one
Closed

Improve performance and maintainability of CIMD client policy code#1480
Copilot wants to merge 2 commits intoISSUE-45284from
copilot/review-issue-45284-another-one

Conversation

Copy link

Copilot AI commented Feb 28, 2026

Review of the ISSUE-45284 branch identifying performance bottlenecks and maintainability issues in the CIMD (Client ID Metadata Document) client policy code.

Description

Performance

  • Eliminate double URI parsing in ClientIdUriSchemeCondition.applyPolicy()clientId was parsed to URI separately in both isUriSchemeMatched() and isTrustedDomainMatched(). Now parsed once.
  • Avoid repeated convertContentFilledList() in verifyUri() — Called up to 8+ times per verifyClientMetadata() invocation on the same config input. Now computed once and passed as parameter.
  • Single-pass Cache-Control parsingClientMetadataCacheControl constructor streamed the directive list 6 times. Replaced with one for-loop.
  • Single-pass header lookup — Cache-Control header found via two stream scans over headers[]. Replaced with one for-loop + early break.
  • Eliminate duplicate ClientMetadataCacheControl constructionfetchClientMetadata() built one, then OIDCClientRepresentationWithCacheControl built another from the raw string. Now accepts the already-parsed object.

Maintainability

  • Fix unused variablepaContext assigned then context re-cast on next line.
  • Fix swapped commentsCREATE block said "Update", UPDATE block said "Create".
  • Fix wrong HTTP status in javadoc307304 Not Modified.
  • Fix misleading config docs — Said "regex" but checkTrustedDomain() uses *. wildcard matching.
  • Fix typo — "mandagory" → "mandatory".
  • Extract verifyUriProperty/verifyUriPropertyIfPresent helpers — Replaces 8 repetitive lambda blocks in verifyClientMetadata():
// Before: repeated 8 times with slight variations
if (clientOIDC.getLogoUri() != null) {
    verifyUri(clientOIDC.getLogoUri(), (error, logMessageTemplate) -> {
        getLogger().warnv(logMessageTemplate, "logo_uri", clientOIDC.getLogoUri());
        throw invalidClientIdMetadata(error);
    });
}

// After
verifyUriPropertyIfPresent(clientOIDC.getLogoUri(), "logo_uri", trustedDomains);
  • Remove unused NameValuePair import.

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 compile -pl services -am -DskipTests -Dformat.skip=true -Denforcer.skip=true -Dcheckstyle.skip=true -Dimpsort.skip=true -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 compile -pl services -am -DskipTests -Dformat.skip=true -Denforcer.skip=true -Dcheckstyle.skip=true -Dimpsort.skip=true -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.

Performance:
- Parse URI once in ClientIdUriSchemeCondition.applyPolicy() instead of twice
- Compute trustedDomains list once and pass to verifyUri() instead of recomputing per call
- Single-pass Cache-Control header parsing (was 6 stream passes)
- Single-pass HTTP header lookup for Cache-Control (was 2 array scans)
- Avoid duplicate ClientMetadataCacheControl creation in fetchClientMetadata()

Maintainability:
- Fix unused paContext variable (use it instead of re-casting context)
- Fix swapped comments in process() (CREATE said "Update", UPDATE said "Create")
- Fix wrong HTTP status in Javadoc (307 → 304 Not Modified)
- Fix misleading "regex" in config descriptions (actually uses wildcard matching)
- Fix typo "mandagory" → "mandatory"
- Extract verifyUriProperty/verifyUriPropertyIfPresent helpers to reduce boilerplate
- Remove unused NameValuePair import

Co-authored-by: tnorimat <25092005+tnorimat@users.noreply.github.com>
Copilot AI changed the title [WIP] Review performance and maintainability improvements for ISSUE-45284 Improve performance and maintainability of CIMD client policy code Feb 28, 2026
@tnorimat tnorimat closed this Mar 2, 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