SOLR-17600 (Part 2/4): Migrate Core Configuration classes from MapSerializable by isaric · Pull Request #4464 · apache/solr

isaric · 2026-05-25T13:21:07Z

This is Part 2 of 4 to deprecate and remove the MapSerializable interface as part of SOLR-17600.

This PR migrates Core Configuration classes (e.g., SolrConfig, PluginInfo, ConfigOverlay) to use MapWriter.

Note: This PR is part of a stacked series and is built on top of Part 1 (#4463). It currently displays the commits from Part 1 as well, but they will automatically drop from this PR's diff once Part 1 is merged into main.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR migrates several Solr config/metadata classes and related call sites from the legacy MapSerializable#toMap(...) pattern to the newer MapWriter#writeMap(...) API, updating tests and various admin/handler code paths accordingly.

Changes:

Replace MapSerializable implementations with MapWriter (writeMap) across core config-related classes.
Update call sites to materialize map-like structures via SimpleOrderedMap or Utils.convertToMap(...) instead of toMap(new HashMap<>()).
Adjust tests/assertions to reflect the new MapWriter/SimpleOrderedMap based conversions.

Reviewed changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
solr/core/src/test/org/apache/solr/update/SolrIndexConfigTest.java	Update test to use `SimpleOrderedMap` and `MapWriter` assertions.
solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerAPI.java	Use `SimpleOrderedMap` instead of `SolrParams#toMap(...)`.
solr/core/src/test/org/apache/solr/core/TestConfig.java	Switch default mapping checks to `SimpleOrderedMap`.
solr/core/src/test/org/apache/solr/core/CacheConfigTest.java	Convert `CacheConfig` to map via `SimpleOrderedMap`.
solr/core/src/test/org/apache/solr/api/NodeConfigClusterPluginsSourceTest.java	Convert `MapWriter` config to map via `SimpleOrderedMap`.
solr/core/src/java/org/apache/solr/update/SolrIndexConfig.java	Migrate `SolrIndexConfig` from `MapSerializable` to `MapWriter`.
solr/core/src/java/org/apache/solr/update/IndexFingerprint.java	Migrate `IndexFingerprint` to `MapWriter`; update `toString()` materialization.
solr/core/src/java/org/apache/solr/search/CacheConfig.java	Migrate `CacheConfig` to `MapWriter`.
solr/core/src/java/org/apache/solr/schema/IndexSchema.java	Migrate `SchemaProps` to `MapWriter`; adjust property retrieval to `SimpleOrderedMap`.
solr/core/src/java/org/apache/solr/handler/export/ExportWriterStream.java	Materialize nested `MapWriter` values using `SimpleOrderedMap`.
solr/core/src/java/org/apache/solr/handler/designer/ManagedSchemaDiff.java	Convert `SimpleOrderedMap` to `Map` via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/SplitShardAPI.java	Convert request payload to v1 param map via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/SplitCoreAPI.java	Convert request payload to v1 param map via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/RequestSyncShardAPI.java	Convert request payload to v1 param map via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/RequestCoreRecoveryAPI.java	Convert request payload to v1 param map via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/RequestBufferUpdatesAPI.java	Convert request payload to v1 param map via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/RequestApplyCoreUpdatesAPI.java	Convert request payload to v1 param map via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/RejoinLeaderElectionAPI.java	Convert request payload to v1 param map via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/RebalanceLeadersAPI.java	Convert request payload to v1 param map via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/PrepareCoreRecoveryAPI.java	Convert request payload to v1 param map via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/OverseerOperationAPI.java	Convert request payload using `SimpleOrderedMap` instead of `toMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/MoveReplicaAPI.java	Convert request payload to v1 param map via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/ModifyCollectionAPI.java	Convert request payload to v1 param map via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/MigrateDocsAPI.java	Convert request payload to v1 param map via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/InstallShardData.java	Convert message payload into `ZkNodeProps` via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCore.java	Convert v1 params to request body via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/ZookeeperInfoHandler.java	Convert `DocCollection` to map via `Utils.convertToMap(...)`.
solr/core/src/java/org/apache/solr/handler/admin/IndexSizeEstimator.java	Materialize `MapWriter` values via `SimpleOrderedMap`.
solr/core/src/java/org/apache/solr/handler/admin/BaseHandlerApiSupport.java	Switch anonymous params object to `MapWriter#writeMap`.
solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java	Update config introspection logic to use `MapWriter`/`SimpleOrderedMap`.
solr/core/src/java/org/apache/solr/handler/ClusterAPI.java	Replace payload `toMap(...)` with `SimpleOrderedMap` conversion.
solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java	Copy metadata into unknown properties via `SimpleOrderedMap`.
solr/core/src/java/org/apache/solr/core/SolrConfig.java	Migrate `SolrConfig` and inner config holders to `MapWriter`.
solr/core/src/java/org/apache/solr/core/RequestParams.java	Migrate `RequestParams` and `ParamSet` to `MapWriter`.
solr/core/src/java/org/apache/solr/core/PluginInfo.java	Migrate `PluginInfo` to `MapWriter`.
solr/core/src/java/org/apache/solr/core/ConfigOverlay.java	Migrate `ConfigOverlay` to `MapWriter`.
solr/core/src/java/org/apache/solr/api/NodeConfigClusterPluginsSource.java	Use `NamedList#asMap(...)` for plugin initArgs config.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

isaric · 2026-06-09T14:52:41Z

+    new NamedList<>(attributes).writeMap(ew);
+    if (initArgs != null) {
+      new SimpleOrderedMap<>(initArgs).writeMap(ew);
+    }
+    if (children == null || children.isEmpty()) {
+      return;
+    }
+
+    for (PluginInfo child : children) {
+      child.writeMap(ew);
    }


Children grouping is preserved — the implementation collects them into a childrenGrouped LinkedHashMap keyed by child.name, aggregating duplicates into a List, then writes each entry. initArgs are written via initArgs.asMap(3).forEach(...) which preserves the multi-value flattening semantics. (Compile errors from the earlier Map.Entry<String, Object> and instanceof List<Object> list patterns were fixed in the fixup commit — NamedList.asMap returns a raw Map, and parameterized instanceof on Object is rejected by javac.)

isaric · 2026-06-09T14:52:43Z

+                  && !((List) o).isEmpty()
                  && ((List) o).get(0) instanceof String) {


Addressed — if (l.isEmpty() || l.get(0) instanceof String) (BaseHandlerApiSupport.java:211) treats empty lists as String[0], matching the prior invariant for multi-valued string params.

dsmiley · 2026-05-30T19:16:50Z

+      } else if (old instanceof List) {
+        ((List<Object>) old).add(child);


as it was before, can use Java 17 instanceof auto cast to 'list" here

Applied. Note: had to use raw List list rather than the parameterized List<Object> list — javac rejects the parameterized pattern as an unsafe cast from Object. Same shape as you suggested otherwise.

dsmiley · 2026-05-30T20:04:12Z

+      ew.put(IndexSchema.LUCENE_MATCH_VERSION_PARAM, luceneMatchVersion.toString());
+    }

+    ew.put("updateHandler", new SimpleOrderedMap<>(getUpdateHandlerInfo()));


I suspect you could place the MapWriter directly into the value of EntryWriter, without the SimpleOrderedMap copy. In the context of implementing writeMap (as is done here), this should be universally true, I think.

Done — getUpdateHandlerInfo() is now passed directly as a MapWriter value, no SOM copy. Same for indexConfig, httpCachingConfig, the cache configs via addCacheConfig, and the infos / infos.getFirst() plugin branches.

dsmiley · 2026-05-30T20:16:50Z

+            new SimpleOrderedMap<>(
+                m -> {
+                  new NamedList<>(items).writeMap(m);
+                }));


can't this be simply items I think (as it was) -- why the double conversion to NamedList and then to SimpleOrderedMap?

The double conversion is gone — the current block builds items as a LinkedHashMap (no intermediate NamedList or SOM) and writes it via a small MapWriter lambda. We need the explicit map because we merge two sources under the same name keys: PluginInfo entries from infos and Map<String,Object> entries from overlay.getNamedPlugins(plugin.tag). Open to dropping the lambda and just doing ew.put(tag, items) directly if you'd prefer — the response writers handle a mixed-value Map the same way.

dsmiley · 2026-05-30T20:19:12Z

-          for (PluginInfo info : infos) l.add(info);
-          result.put(tag, l);
+          ArrayList<MapWriter> writers = new ArrayList<>();
+          infos.forEach(info -> writers.add(new SimpleOrderedMap<>(info)));


use of SimpleOrderedMap copy constructor / conversion seems needless

And same for other side of the if condition

Dropped — both infos (multi-OK) and infos.getFirst() are now passed directly to ew.put(tag, ...). No SOM copy on either branch.

dsmiley · 2026-05-30T20:19:56Z

+          ew.put(tag, writers);
        } else {
-          result.put(tag, infos.get(0));
+          ew.put(tag, new SimpleOrderedMap<>(m -> infos.getFirst().writeMap(m)));


can't be as before?

Yes — restored. requestParsers is now Map.of("multipartUploadLimitKB", ..., "formUploadLimitKB", ...) without intermediate copies.

dsmiley · 2026-05-30T20:22:15Z

-    return result;
+    ew.put(
+        "requestDispatcher",
+        new SimpleOrderedMap<>(


The purpose of MapWriter is to write a map as efficiently as possible, ideally without an intermediary forced map (or NamedList / SimpleOrderedMap or other data structure conversions/copies). Prior to MapWriter, Solr used MapSerializable which forced the creation of maps.

I'll skip repeating this point but it applies to many changes in this commit.

Understood — applied across this PR. MapWriter instances flow into EntryWriter without an intermediate SimpleOrderedMap / NamedList / Map copy wherever the value isn't mutated afterwards. Where a real mutable Map was needed (e.g., SolrConfigHandler's response shaping that then calls computeIfAbsent / put / setValue), the SOM stays.

dsmiley · 2026-05-30T20:26:22Z


  public Map<String, Object> getNamedPropertyValues(String name, SolrParams params) {
-    return new SchemaProps(name, params, this).toMap(new LinkedHashMap<>());
+    return new SimpleOrderedMap<>(new SchemaProps(name, params, this));


Use LinkedHashMap here as this method isn't apparently isolated for serialization

Done — getNamedPropertyValues now returns Utils.convertToMap(new SchemaProps(...), new LinkedHashMap<>()).

dsmiley · 2026-05-30T20:27:45Z

-    // TODO: Should not create new HashMap?
-    return new HashMap<>(args);
+  public void writeMap(EntryWriter ew) throws IOException {
+    new NamedList<>(args).writeMap(ew);


surely we can do better (no NamedList creatoin). note the comment that was deleted -- same concept.

Done — CacheConfig.writeMap now iterates args.entrySet() directly. No NamedList allocation.

dsmiley · 2026-05-30T20:30:24Z

+        .putIfNotNull("mergeScheduler", mergeSchedulerInfo)
+        .putIfNotNull("metrics", metricsInfo)
+        .putIfNotNull("mergePolicyFactory", mergePolicyFactoryInfo)
+        .putIfNotNull("mergedSegmentWarmer", mergedSegmentWarmerInfo);


dsmiley · 2026-06-09T03:21:46Z

time to bring this branch up to date now that part 1 has merged

…ializable

- PluginInfo.writeMap: fix raw-type compile errors from NamedList.asMap() and unsafe parameterized instanceof check - SolrConfigHandler: switch the two MapWriter→Map conversions back to SimpleOrderedMap (now valid since SolrConfig/PluginInfo are MapWriter), preempting Part 3's removal of the toMap default

isaric · 2026-06-09T14:52:47Z

Rebased onto main and pushed a fixup commit (e9572c5267c):

PluginInfo.writeMap: fixed two pre-existing compile errors (NamedList.asMap() returns raw Map; parameterized instanceof on Object rejected by javac) — applied your Java-17 instanceof suggestion using raw List list.
SolrConfigHandler:339, 372: switched from .toMap(new LinkedHashMap<>()) back to new SimpleOrderedMap<>(...) now that SolrConfig / PluginInfo are MapWriter. This preempts Part 3 removing the toMap default from MapWriter.

Replied inline on the rest of the threads. Build is green.

Copilot AI review requested due to automatic review settings May 25, 2026 13:21

github-actions Bot added tests cat:search cat:index cat:api cat:schema cat:packagemanager labels May 25, 2026

This was referenced May 25, 2026

SOLR-17600 (Part 3/4): Migrate SolrJ and Utilities from MapSerializable #4465

Open

SOLR-17600 (Part 4/4): Delete MapSerializable interface #4466

Open

Copilot AI reviewed May 25, 2026

View reviewed changes

isaric force-pushed the SOLR-17600-2-core-config branch 2 times, most recently from 37d86fe to d0fa05e Compare May 26, 2026 05:55

dsmiley reviewed May 30, 2026

View reviewed changes

isaric force-pushed the SOLR-17600-2-core-config branch from d0fa05e to 7678939 Compare June 2, 2026 14:54

isaric added 2 commits June 9, 2026 16:41

SOLR-17600 (Part 2/4): Migrate Core Configuration classes from MapSer…

6aee7c3

…ializable

isaric force-pushed the SOLR-17600-2-core-config branch from 7678939 to e9572c5 Compare June 9, 2026 14:50

		&& !((List) o).isEmpty()
		&& ((List) o).get(0) instanceof String) {

		} else if (old instanceof List) {
		((List<Object>) old).add(child);

Conversation

isaric commented May 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsmiley commented Jun 9, 2026

Uh oh!

isaric commented Jun 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants