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: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<>()).

Had to walk this back in 11f40df6f5c — sorry. Deep materialization via Utils.convertToMap broke SchemaXmlWriter.writeFieldTypes: that method casts nested values to SimpleOrderedMap<Object> (e.g. analyzerProperties, perFieldSimilarityProperties). Utils.convertToMap recursively converts every nested MapWriter (including SimpleOrderedMap, which implements MapWriter) to a LinkedHashMap, breaking those downstream casts and causing 8 TestSolrConfigHandler failures on schema upgrade.

Restored new SimpleOrderedMap<>(new SchemaProps(name, params, this)); (the original Part 2 form). That does shallow conversion which preserves the inner SimpleOrderedMap shape SchemaXmlWriter relies on. Added a comment to explain the contract.

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-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.

dsmiley

just a couple little things. I'm eager to commit something to fix TestPackages from failing since p1.

dsmiley · 2026-06-09T20:02:23Z

-          m.put(child.name, l);
-        }
+  public void writeMap(EntryWriter ew) throws IOException {
+    new NamedList<>(attributes).writeMap(ew);


let's not create a new data structure just to write out our data structures. That's the overall theme of MapWritable -- don't do this.

The type here is simple so no multi-value concern.

Just need to call attributes.forEach(ew::putNoEx); here I think

Applied — attributes.forEach(ew::putNoEx); directly. NamedList allocation gone, SimpleOrderedMap import dropped. Pushed as 1ba1cffa447.

Had to walk this back in 11f40df6f5c — sorry. The attributes.forEach(ew::putNoEx) form fails at runtime with ClassCastException: LinkedHashMap cannot be cast to String. Root cause: attributes is declared Map<String, String>, but the PluginInfo(String, Map<String, Object>) constructor assigns it via raw types (unmodifiableMap((LinkedHashMap) m)), so values may be LinkedHashMap (subcomponent payloads). The synthetic bridge the compiler generates for BiConsumer<String, String> casts both args to String, which fails on those values.

Restored new NamedList<>(attributes).writeMap(ew); — that constructor iterates via entrySet() and stores everything as Object in its nvPairs ArrayList, so no String-cast is generated. Added a comment so this isn't lost again. Surfaces in TestSolrConfigHandler.testReqHandlerAPIs and testProperty.

dsmiley · 2026-06-09T20:17:46Z

    entryMetadata.timestamp = timestamp;
    if (details.getMetaData() != null) {
-      details.getMetaData().toMap(entryMetadata.unknownProperties());
+      entryMetadata.unknownProperties().putAll(new SimpleOrderedMap<>(details.getMetaData()));


why did this change not use details.getMetadata() as before? After all, we just checked it for not null so...

Switched to Utils.convertToMap(details.getMetaData(), entryMetadata.unknownProperties()); — same semantic as the original details.getMetaData().toMap(...), no SOM intermediate, future-proof against Part 3 removing the toMap default on MapWriter. Pushed as 1ba1cffa447.

- PluginInfo.writeMap: iterate attributes directly via attributes.forEach(ew::putNoEx) instead of wrapping in a NamedList - RequestParams.setParams: revert HashMap to LinkedHashMap to preserve insertion order - ClusterFileStore: use Utils.convertToMap to populate unknownProperties directly, dropping the intermediate SimpleOrderedMap

- SolrConfigHandler.getConfigDetails / expandUseParams: switch from new SimpleOrderedMap<>(...) to Utils.convertToMap(..., new LinkedHashMap<>()). SOM's MapWriter constructor does shallow conversion, leaving nested MapWriter lambdas (e.g. SolrConfig's "query" block) intact, which broke the downstream (Map) casts and caused TestPackages.testCoreReloadingPlugin to fail with CCE. Deep recursive materialization matches the original MapSerializable.toMap semantics this code path expects. - IndexSchema.getNamedPropertyValues(String, SolrParams): revert from Utils.convertToMap to new SimpleOrderedMap<>(new SchemaProps(...)). Deep materialization converted nested SimpleOrderedMaps to LinkedHashMaps, breaking SchemaXmlWriter.writeFieldTypes which casts values to SimpleOrderedMap. Comment added to prevent regressions. - PluginInfo.writeMap: revert attributes.forEach(ew::putNoEx) back to new NamedList<>(attributes).writeMap(ew). The forEach version's synthetic bridge method casts values to String, but the PluginInfo(String, Map<String, Object>) constructor assigns the field via raw types so values may not be Strings. Comment added.

isaric · 2026-06-10T14:12:46Z

TestPackages.testCoreReloadingPlugin is now fixed on this branch (11f40df6f5c). Root cause was a chain of two related bugs:

On main (introduced by part 1): SolrConfigHandler.handleGET was changed from instanceof MapSerializable to instanceof MapWriter. On post-part-1 main, classes like CacheConfig still only implement MapSerializable, so the check returns false and the code falls through to (Map<String, Object>) o — ClassCastException: CacheConfig cannot be cast to Map.
On part 2: My earlier fixup at SolrConfigHandler:339, 372 used new SimpleOrderedMap<>(...), which does only shallow conversion. SolrConfig.writeMap puts (MapWriter) m -> {...} lambdas as values for "query", "updateHandler", etc. Those lambdas survive the SOM construction unchanged, so downstream code casting to Map then blows up with SolrConfig$$Lambda cannot be cast to Map.

Fix: use Utils.convertToMap(..., new LinkedHashMap<>()) at both call sites — it recursively materializes nested MapWriter / IteratorWriter values into actual Map / List instances, matching the original MapSerializable.toMap semantics that this code path was designed around.

Two of your earlier suggestions had to be walked back as part of this — replies inline on those threads.

Verified passing on this branch:

TestPackages.testCoreReloadingPlugin
TestSolrConfigHandler (all 8 methods)

ecjLint flagged unused LinkedHashMap and Utils imports in IndexSchema after the SOM revert in 11f40df. spotlessJavaCheck also wanted SimpleOrderedMap re-sorted in TestConfig.

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

Merged

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

dsmiley reviewed Jun 9, 2026

View reviewed changes

isaric added 2 commits June 10, 2026 14:39

SOLR-17600: Remove unused imports and fix import order

a114c23

ecjLint flagged unused LinkedHashMap and Utils imports in IndexSchema after the SOM revert in 11f40df. spotlessJavaCheck also wanted SimpleOrderedMap re-sorted in TestConfig.

dsmiley added the no-changelog label Jun 11, 2026

dsmiley approved these changes Jun 11, 2026

View reviewed changes

dsmiley merged commit 22c37b3 into apache:main Jun 11, 2026
3 of 5 checks passed

		&& !((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!

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!

Uh oh!

dsmiley commented Jun 9, 2026

Uh oh!

isaric commented Jun 9, 2026

Uh oh!

dsmiley left a comment

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

isaric commented Jun 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants