SOLR-17600 (Part 3/4): Migrate SolrJ and Utilities from MapSerializable#4465
Open
isaric wants to merge 3 commits into
Open
SOLR-17600 (Part 3/4): Migrate SolrJ and Utilities from MapSerializable#4465isaric wants to merge 3 commits into
isaric wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR migrates a large set of Solr components from the legacy MapSerializable#toMap(...) pattern to the newer MapWriter#writeMap(...) model, often using SimpleOrderedMap / Utils.convertToMap(...) for materialization where a concrete map is required.
Changes:
- Convert multiple production classes and DTOs from
MapSerializabletoMapWriterand replacetoMap(...)implementations withwriteMap(...). - Replace many
toMap(new LinkedHashMap<>())call sites withnew SimpleOrderedMap<>(...)orUtils.convertToMap(...). - Remove some
MapSerializable-based serialization branches in writers/codecs and modernize a few helper conversions.
Reviewed changes
Copilot reviewed 49 out of 49 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| solr/test-framework/src/java/org/apache/solr/common/cloud/ClusterStateUtil.java | Switch debug state mapping to SimpleOrderedMap materialization. |
| solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleCborTest.java | Use SimpleOrderedMap when converting docs for CBOR test payloads. |
| solr/solrj/src/java/org/apache/solr/common/util/Utils.java | Adjust deep-copy and reflection mapping to rely on MapWriter + conversion helpers. |
| solr/solrj/src/java/org/apache/solr/common/util/TextWriter.java | Remove MapSerializable special-casing during text serialization. |
| solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java | Remove MapSerializable special-casing during javabin serialization. |
| solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java | Materialize MapWriter props via SimpleOrderedMap. |
| solr/solrj/src/java/org/apache/solr/common/MapWriterMap.java | Drop toMap(...) override now that MapWriter no longer extends MapSerializable. |
| solr/solrj/src/java/org/apache/solr/common/MapWriter.java | Remove MapSerializable inheritance and modernize EntryWriter#getBiConsumer. |
| solr/solrj/src/java/org/apache/solr/common/IteratorWriter.java | Convert nested MapWriter elements using SimpleOrderedMap. |
| solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamExplanation.java | Convert from toMap(...) to writeMap(...) implementation (child handling changed). |
| solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/expr/Explanation.java | Convert Explanation to implement MapWriter and emit fields via EntryWriter. |
| solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java | Copy init-args into a map via SimpleOrderedMap. |
| solr/core/src/test/org/apache/solr/update/SolrIndexConfigTest.java | Update tests to expect MapWriter and use SimpleOrderedMap for mapping. |
| solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerAPI.java | Convert params mapping to use SimpleOrderedMap. |
| solr/core/src/test/org/apache/solr/core/TestConfig.java | Update test mapping to SimpleOrderedMap. |
| solr/core/src/test/org/apache/solr/core/CacheConfigTest.java | Update cache config mapping in tests to SimpleOrderedMap. |
| solr/core/src/test/org/apache/solr/api/NodeConfigClusterPluginsSourceTest.java | Convert config mapping from toMap(...) into SimpleOrderedMap. |
| solr/core/src/java/org/apache/solr/update/SolrIndexConfig.java | Convert SolrIndexConfig to MapWriter and implement writeMap(...). |
| solr/core/src/java/org/apache/solr/update/IndexFingerprint.java | Convert to MapWriter and update toString() materialization. |
| solr/core/src/java/org/apache/solr/search/CacheConfig.java | Convert to MapWriter and emit args via a NamedList writer. |
| solr/core/src/java/org/apache/solr/schema/IndexSchema.java | Convert SchemaProps to MapWriter and return values via SimpleOrderedMap. |
| solr/core/src/java/org/apache/solr/handler/export/ExportWriterStream.java | Convert nested MapWriter values to SimpleOrderedMap when building tuples. |
| solr/core/src/java/org/apache/solr/handler/designer/ManagedSchemaDiff.java | Use Utils.convertToMap for diffing SimpleOrderedMap instances. |
| solr/core/src/java/org/apache/solr/handler/admin/api/SplitShardAPI.java | Use Utils.convertToMap for v2 payload → v1 params conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/api/SplitCoreAPI.java | Use Utils.convertToMap for v2 payload → v1 params conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/api/RequestSyncShardAPI.java | Use Utils.convertToMap for v2 payload → v1 params conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/api/RequestCoreRecoveryAPI.java | Use Utils.convertToMap for v2 payload → v1 params conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/api/RequestBufferUpdatesAPI.java | Use Utils.convertToMap for v2 payload → v1 params conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/api/RequestApplyCoreUpdatesAPI.java | Use Utils.convertToMap for v2 payload → v1 params conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/api/RejoinLeaderElectionAPI.java | Use Utils.convertToMap for v2 payload → v1 params conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/api/RebalanceLeadersAPI.java | Use Utils.convertToMap for v2 payload → v1 params conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/api/PrepareCoreRecoveryAPI.java | Use Utils.convertToMap for v2 payload → v1 params conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/api/OverseerOperationAPI.java | Use SimpleOrderedMap for payload → params conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/api/MoveReplicaAPI.java | Use Utils.convertToMap for v2 payload → v1 params conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/api/ModifyCollectionAPI.java | Use Utils.convertToMap for v2 payload → v1 params conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/api/MigrateDocsAPI.java | Use Utils.convertToMap for v2 payload → v1 params conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/api/InstallShardData.java | Use Utils.convertToMap before creating ZkNodeProps. |
| solr/core/src/java/org/apache/solr/handler/admin/api/CreateCore.java | Use Utils.convertToMap for v1 SolrParams → request-body conversion. |
| solr/core/src/java/org/apache/solr/handler/admin/ZookeeperInfoHandler.java | Use Utils.convertToMap when materializing DocCollection. |
| solr/core/src/java/org/apache/solr/handler/admin/IndexSizeEstimator.java | Replace ad-hoc map creation with SimpleOrderedMap materialization. |
| solr/core/src/java/org/apache/solr/handler/admin/BaseHandlerApiSupport.java | Switch request parameter export from toMap(...) to writeMap(...). |
| solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java | Update config/plugin info materialization to MapWriter + SimpleOrderedMap. |
| solr/core/src/java/org/apache/solr/handler/ClusterAPI.java | Use SimpleOrderedMap when converting RoleInfo to params. |
| solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java | Copy metadata into response properties via SimpleOrderedMap. |
| solr/core/src/java/org/apache/solr/core/SolrConfig.java | Convert core config mapping to MapWriter and restructure emitted maps via SimpleOrderedMap. |
| solr/core/src/java/org/apache/solr/core/RequestParams.java | Convert RequestParams/ParamSet to MapWriter and adjust map materialization call sites. |
| solr/core/src/java/org/apache/solr/core/PluginInfo.java | Convert PluginInfo to MapWriter and update serialization logic. |
| solr/core/src/java/org/apache/solr/core/ConfigOverlay.java | Convert overlay mapping to MapWriter. |
| solr/core/src/java/org/apache/solr/api/NodeConfigClusterPluginsSource.java | Simplify initArgs config mapping using NamedList#asMap. |
Comments suppressed due to low confidence (3)
solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamExplanation.java:1
- This no longer emits a
childrenfield at all. Previouslychildrenwas a list under the key\"children\"; now each child is written into the same map as the parent, which both changes the JSON shape and risks key collisions/overwrites. Restore thechildrenlist structure by collecting children into a list value (or emitting an array writer) and putting it once under\"children\".
solr/solrj/src/java/org/apache/solr/common/util/TextWriter.java:1 - This removes the
MapSerializableserialization path entirely. If any objects still implementMapSerializablebut notMapWriter(or are provided by external plugins), they will now fall through and may be rendered incorrectly or fail to serialize. IfMapSerializableis still supported/used, reintroduce a compatibility branch (or provide a bridging adapter toMapWriter) until allMapSerializableproducers are fully migrated.
solr/test-framework/src/java/org/apache/solr/common/cloud/ClusterStateUtil.java:1 - The comment claims JSON conversion requires "standard types like Map", but the code now stores a
SimpleOrderedMapas the value. IfSimpleOrderedMapis intentionally acceptable forUtils.toJSONString(...)(and/or is treated as a Map-like type by the JSON layer), the comment should be updated to avoid misleading future readers; otherwise, consider materializing to an actualMapto match the comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+207
to
209
| for (PluginInfo child : children) { | ||
| child.writeMap(ew); | ||
| } |
Comment on lines
+115
to
118
| public void writeMap(EntryWriter ew) throws IOException { | ||
| ew.put(ConfigOverlay.ZNODEVER, znodeVersion); | ||
| data.forEach(ew::putNoEx); | ||
| } |
Comment on lines
+189
to
190
| public void writeMap(EntryWriter ew) throws IOException { | ||
| for (Iterator<String> it = getParameterNamesIterator(); it.hasNext(); ) { |
| : map.get(key); | ||
| if (o == null) o = pathValues.get(key); | ||
| if (o == null && useRequestParams) o = origParams.getParams(key); | ||
| if (o == null) continue; |
a1a1a42 to
a01d5e0
Compare
a01d5e0 to
2995847
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is Part 3 of 4 to deprecate and remove the
MapSerializableinterface as part of SOLR-17600.This PR migrates SolrJ classes, test utilities, and stream explanations to use
MapWriter.Note: This PR is part of a stacked series and is built on top of Part 1 (#4463) and Part 2 (#4464). It currently displays the commits from Parts 1 & 2 as well, but they will automatically drop from this PR's diff once the previous parts are merged into
main.