[
https://issues.apache.org/jira/browse/SOLR-18603?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18133578#comment-18133578
]
Ivan Saric commented on SOLR-18603:
-----------------------------------
Audit result (apache/solr main at 0524480aa, 2026-10-28). Searched all Java
main and test sources in every module plus Kotlin/Groovy, for single- and
multi-line {{extends}}, anonymous subclasses and Mockito mocks.
Subclasses of {{NamedList}} found:
# {{SimpleOrderedMap}} - expected. It has no subclasses of its own.
# {{CloudSolrClient.RouteResponse}} ({{org.apache.solr.client.solrj.impl}}) -
public nested class returned by {{CloudSolrClient.request()}} for routed
updates; callers cast the {{NamedList<Object>}} to it. Built only by
{{condenseResponse}}, which adds unique keys ("adds"/"deletes"/"deleteByQuery"
and "responseHeader"). Client-side only; never serialised to javabin. Nothing
overrides {{condenseResponse}}.
# {{NestableJsonFacetTest}} - twelve anonymous subclasses via double-brace
initialisation ({{new NamedList<>() \{\{ add(...); \}\}}}). Test-only.
No Mockito mocks of either class and no other subclasses.
Constraint: SolrJ has no {{module-info.java}}, so it compiles into the unnamed
module, where every permitted subclass of a sealed class must be in the *same
package*. {{RouteResponse}} is not in {{org.apache.solr.common.util}}, so it
cannot be listed in {{permits}} where it is.
Plan for the PR:
* {{NamedList}} becomes {{sealed}}, permitting only {{SimpleOrderedMap}}.
* {{SimpleOrderedMap}} becomes {{non-sealed}} (rather than {{final}}) so that
{{RouteResponse}} can keep being a {{NamedList}} through it.
* {{RouteResponse}} extends {{SimpleOrderedMap<Object>}} instead of
{{NamedList<Object>}}. Its keys are already unique, so behaviour is unchanged
and existing casts keep working.
* {{NestableJsonFacetTest}} rewritten to plain {{add()}} calls.
The trade-off is that the hierarchy is sealed at the {{NamedList}} level only;
{{SimpleOrderedMap}} stays open. The alternatives (refactor {{RouteResponse}}
to composition, which changes a public SolrJ return type; move it to
{{common.util}}; or add a {{module-info.java}} to SolrJ) all seemed out of
scope for a "minor" step. Happy to go another way if preferred.
> Seal the NamedList class hierarchy (SIP-22 step 4)
> --------------------------------------------------
>
> Key: SOLR-18603
> URL: https://issues.apache.org/jira/browse/SOLR-18603
> Project: Solr
> Issue Type: Improvement
> Components: SolrJ
> Reporter: Ivan Saric
> Priority: Minor
> Fix For: main(11.0)
>
>
> This is step 4 of [SIP-22: NamedList
> Reduction|https://cwiki.apache.org/confluence/spaces/SOLR/pages/340036504/SIP-22+NamedList+Reduction]:
> {quote}Minor: Use Java 17 "sealed classes" to insist the NamedList type
> hierarchy is exactly what we want; no surprises.{quote}
> Steps 1-3 of the SIP are done (SOLR-17623, SOLR-17635, SOLR-17625). This step
> was agreed as the next one on the dev list: [Follow-up work after
> SOLR-17600|https://lists.apache.org/thread/2o09q8yjmsx4l9b58mqqk5mgdbv7qf36].
> Per that thread, the SIP's "Solr 10" now means Solr 11 ({{main}}), since 10.0
> has shipped.
> h3. Proposal
> Declare {{org.apache.solr.common.util.NamedList}} as {{sealed}} with an
> explicit {{permits}} list, so that the set of NamedList subtypes is fixed and
> visible in one place. The expected outcome is that {{NamedList}} permits
> exactly {{SimpleOrderedMap}}; whether {{SimpleOrderedMap}} itself becomes
> {{final}} or stays open is to be decided from the audit below.
> Before writing any {{permits}} clause, audit every subclass of {{NamedList}}
> across all Gradle modules, including test sources, {{solr/modules/*}} and
> anonymous subclasses. Any subclass beyond {{SimpleOrderedMap}} is either
> removed or rewritten (if it is internal), or reported here as a finding
> before deciding whether to permit it.
> No behaviour changes and no public method signature changes. Those are later
> SIP-22 steps.
> h3. Compatibility
> {{NamedList}} is public SolrJ API, so sealing it is a plugin-compatibility
> change: a third-party subclass of {{NamedList}} will no longer compile. This
> is a {{main}} / 11.0-only change and will be called out in the changelog as
> an upgrade note. The SIP already anticipates plugin-compatibility impact.
> h3. Out of scope
> * Migrating {{new NamedList(...)}} instantiations to {{SimpleOrderedMap}}
> (SIP-22 step 5). That will be a separate Jira, per the SIP's "don't combine
> with anything else".
> * Changing public API signatures away from {{NamedList}} (SIP-22 step 7).
> One PR against {{main}}.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
