[
https://issues.apache.org/jira/browse/SOLR-18394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ASF GitHub Bot updated SOLR-18394:
----------------------------------
Labels: pull-request-available (was: )
> Simplify JettyConfig construction
> ---------------------------------
>
> Key: SOLR-18394
> URL: https://issues.apache.org/jira/browse/SOLR-18394
> Project: Solr
> Issue Type: Improvement
> Components: test-framework
> Reporter: Jan Høydahl
> Assignee: Jan Høydahl
> Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> {{JettyConfig}} (solr/test-framework) carries a private 11-argument
> constructor that
> {{Builder#build()}} is the only caller of, plus a {{builder(JettyConfig
> other)}} copy
> factory that re-assigns each of the same 11 fields by hand. Every new Jetty
> option
> therefore has to be added in four places: the field, the builder
> field/setter, the
> constructor parameter list, and the copy factory.
> As noted by [~dsmiley] in the review of SOLR-8474
> ([PR#4738|https://github.com/apache/solr/pull/4738]),
> where adding {{enableAdminUi}} grew the list to 11:
> {quote}This constructor has gotten out of control; only second to the worst
> offender --
> NodeConfig. It's only private so maybe we shouldn't care much, but maybe
> JettyConfig
> shouldn't be immutable, or should have a freeze().{quote}
> (For scale: {{NodeConfig}}'s private constructor takes ~37 parameters.)
> h2. Suggested approach
> * Have the private constructor take the {{Builder}} and read the fields from
> it, so
> adding an option touches only the builder — no parameter list to extend.
> * {{builder(JettyConfig other)}} can then copy from a single place as well,
> removing
> the second field-by-field list.
> * Keep {{JettyConfig}}'s public final fields as they are; {{JettySolrRunner}}
> and the
> test framework read them directly, so this stays a pure internal refactor
> with no
> call-site changes.
> * Alternatives discussed: dropping immutability, or adding a {{freeze()}}.
> Converting
> to a record is not viable while the fields are public and mutable maps are
> exposed.
> h2. Notes
> * Test-framework only — no production code, no back-compat concern.
> * {{NodeConfig}} deserves the same treatment but is a much larger change; out
> of scope here.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
