From 590631356e6a990215b68edee558cbb47144023a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20H=C3=B8ydahl?= Date: Fri, 21 Aug 2026 16:27:46 +0200 Subject: [PATCH 1/2] SOLR-18394: Simplify JettyConfig construction The private constructor now reads its fields directly from the Builder, and the copy factory returns a clone of the builder snapshot kept by the built config. This removes the 10-argument constructor and the manual field-by-field copy logic, so adding a new Jetty option only requires a field and setter in the Builder plus the public final field. --- .../org/apache/solr/embedded/JettyConfig.java | 75 +++++++------------ 1 file changed, 29 insertions(+), 46 deletions(-) diff --git a/solr/test-framework/src/java/org/apache/solr/embedded/JettyConfig.java b/solr/test-framework/src/java/org/apache/solr/embedded/JettyConfig.java index 601c6b5f2b58..34ac99afd8b5 100644 --- a/solr/test-framework/src/java/org/apache/solr/embedded/JettyConfig.java +++ b/solr/test-framework/src/java/org/apache/solr/embedded/JettyConfig.java @@ -37,27 +37,21 @@ public class JettyConfig { public final boolean enableV2; public final boolean enableGracefulShutdown; - private JettyConfig( - boolean onlyHttp1, - int port, - int portRetryTime, - boolean stopAtShutdown, - Long waitForLoadingCoresToFinishMs, - Map extraServlets, - Map, String> extraFilters, - SSLConfig sslConfig, - boolean enableV2, - boolean enableGracefulShutdown) { - this.onlyHttp1 = onlyHttp1; - this.port = port; - this.portRetryTime = portRetryTime; - this.stopAtShutdown = stopAtShutdown; - this.waitForLoadingCoresToFinishMs = waitForLoadingCoresToFinishMs; - this.extraServlets = extraServlets; - this.extraFilters = extraFilters; - this.sslConfig = sslConfig; - this.enableV2 = enableV2; - this.enableGracefulShutdown = enableGracefulShutdown; + /** Snapshot of the builder that built this config; enables {@link #builder(JettyConfig)}. */ + private final Builder builder; + + private JettyConfig(Builder builder) { + this.builder = builder; + this.onlyHttp1 = builder.onlyHttp1; + this.port = builder.port; + this.portRetryTime = builder.portRetryTime; + this.stopAtShutdown = builder.stopAtShutdown; + this.waitForLoadingCoresToFinishMs = builder.waitForLoadingCoresToFinishMs; + this.extraServlets = builder.extraServlets; + this.extraFilters = builder.extraFilters; + this.sslConfig = builder.sslConfig; + this.enableV2 = builder.enableV2; + this.enableGracefulShutdown = builder.enableGracefulShutdown; } public static Builder builder() { @@ -65,22 +59,10 @@ public static Builder builder() { } public static Builder builder(JettyConfig other) { - Builder builder = new Builder(); - - builder.onlyHttp1 = other.onlyHttp1; - builder.port = other.port; - builder.portRetryTime = other.portRetryTime; - builder.stopAtShutdown = other.stopAtShutdown; - builder.waitForLoadingCoresToFinishMs = other.waitForLoadingCoresToFinishMs; - builder.extraServlets = other.extraServlets; - builder.extraFilters = other.extraFilters; - builder.sslConfig = other.sslConfig; - builder.enableV2 = other.enableV2; - builder.enableGracefulShutdown = other.enableGracefulShutdown; - return builder; + return other.builder.clone(); } - public static class Builder { + public static class Builder implements Cloneable { boolean onlyHttp1 = false; int port = 0; @@ -154,18 +136,19 @@ public Builder withPortRetryTime(int portRetryTime) { return this; } + /** Shallow copy; maps are shared with the original, matching historic copy semantics. */ + @Override + public Builder clone() { + try { + return (Builder) super.clone(); + } catch (CloneNotSupportedException e) { + throw new AssertionError(e); + } + } + public JettyConfig build() { - return new JettyConfig( - onlyHttp1, - port, - portRetryTime, - stopAtShutdown, - waitForLoadingCoresToFinishMs, - extraServlets, - extraFilters, - sslConfig, - enableV2, - enableGracefulShutdown); + // clone so later mutations of this builder don't leak into the built config's snapshot + return new JettyConfig(clone()); } } } From 9e78bb08ae7f66dd00991ad05716742f7c6a35ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20H=C3=B8ydahl?= Date: Fri, 21 Aug 2026 19:34:21 +0200 Subject: [PATCH 2/2] SOLR-18394: Deep-clone builder maps and make built config maps unmodifiable Addresses review feedback: the shallow clone left extraServlets/extraFilters aliased between the builder, built configs, and copies from builder(JettyConfig), so a second build() or a mutated copy could silently change an existing config. The clone now copies both maps, and the built config exposes unmodifiable views. --- .../java/org/apache/solr/embedded/JettyConfig.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/solr/test-framework/src/java/org/apache/solr/embedded/JettyConfig.java b/solr/test-framework/src/java/org/apache/solr/embedded/JettyConfig.java index 34ac99afd8b5..9733c0082a87 100644 --- a/solr/test-framework/src/java/org/apache/solr/embedded/JettyConfig.java +++ b/solr/test-framework/src/java/org/apache/solr/embedded/JettyConfig.java @@ -17,6 +17,7 @@ package org.apache.solr.embedded; import jakarta.servlet.Filter; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; import java.util.TreeMap; @@ -47,8 +48,8 @@ private JettyConfig(Builder builder) { this.portRetryTime = builder.portRetryTime; this.stopAtShutdown = builder.stopAtShutdown; this.waitForLoadingCoresToFinishMs = builder.waitForLoadingCoresToFinishMs; - this.extraServlets = builder.extraServlets; - this.extraFilters = builder.extraFilters; + this.extraServlets = Collections.unmodifiableMap(builder.extraServlets); + this.extraFilters = Collections.unmodifiableMap(builder.extraFilters); this.sslConfig = builder.sslConfig; this.enableV2 = builder.enableV2; this.enableGracefulShutdown = builder.enableGracefulShutdown; @@ -136,11 +137,14 @@ public Builder withPortRetryTime(int portRetryTime) { return this; } - /** Shallow copy; maps are shared with the original, matching historic copy semantics. */ + /** Copies the maps too, so the clone is fully independent; the SSLConfig is shared. */ @Override public Builder clone() { try { - return (Builder) super.clone(); + Builder clone = (Builder) super.clone(); + clone.extraServlets = new TreeMap<>(extraServlets); + clone.extraFilters = new LinkedHashMap<>(extraFilters); + return clone; } catch (CloneNotSupportedException e) { throw new AssertionError(e); }