SOLR-17600 (Part 1/4): Migrate V2 API Payloads from MapSerializable#4463
SOLR-17600 (Part 1/4): Migrate V2 API Payloads from MapSerializable#4463isaric wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors multiple Solr handlers/tests to standardize conversion of Solr/MapWriter-style objects into map-like structures (favoring Utils.convertToMap(...) / SimpleOrderedMap) and updates one internal API adapter to a MapWriter#writeMap(...) implementation.
Changes:
- Replaced a number of
toMap(new HashMap<>())call sites withUtils.convertToMap(...)ornew SimpleOrderedMap<>(...). - Updated config/response building code paths to use
MapWriter/SimpleOrderedMaprather thanMapSerializable. - Adjusted
BaseHandlerApiSupportparameter adapter to implementwriteMap(...)and skip null-valued entries.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerAPI.java | Updates test param map construction to use SimpleOrderedMap. |
| solr/core/src/java/org/apache/solr/handler/export/ExportWriterStream.java | Uses SimpleOrderedMap when materializing MapWriter values. |
| solr/core/src/java/org/apache/solr/handler/designer/ManagedSchemaDiff.java | Uses Utils.convertToMap for diff map materialization. |
| solr/core/src/java/org/apache/solr/handler/admin/api/SplitShardAPI.java | Uses Utils.convertToMap for v2 payload → v1 params. |
| solr/core/src/java/org/apache/solr/handler/admin/api/SplitCoreAPI.java | Uses Utils.convertToMap for v2 payload → v1 params. |
| solr/core/src/java/org/apache/solr/handler/admin/api/RequestSyncShardAPI.java | Uses Utils.convertToMap for v2 payload → v1 params. |
| solr/core/src/java/org/apache/solr/handler/admin/api/RequestCoreRecoveryAPI.java | Uses Utils.convertToMap for v2 payload → v1 params. |
| solr/core/src/java/org/apache/solr/handler/admin/api/RequestBufferUpdatesAPI.java | Uses Utils.convertToMap for v2 payload → v1 params. |
| solr/core/src/java/org/apache/solr/handler/admin/api/RequestApplyCoreUpdatesAPI.java | Uses Utils.convertToMap for v2 payload → v1 params. |
| solr/core/src/java/org/apache/solr/handler/admin/api/RejoinLeaderElectionAPI.java | Uses Utils.convertToMap for v2 payload → v1 params. |
| solr/core/src/java/org/apache/solr/handler/admin/api/RebalanceLeadersAPI.java | Uses Utils.convertToMap for v2 payload → v1 params. |
| solr/core/src/java/org/apache/solr/handler/admin/api/PrepareCoreRecoveryAPI.java | Uses Utils.convertToMap for v2 payload → v1 params. |
| solr/core/src/java/org/apache/solr/handler/admin/api/OverseerOperationAPI.java | Builds v1 params using SimpleOrderedMap instead of toMap(HashMap). |
| solr/core/src/java/org/apache/solr/handler/admin/api/MoveReplicaAPI.java | Uses Utils.convertToMap for v2 payload → v1 params. |
| solr/core/src/java/org/apache/solr/handler/admin/api/ModifyCollectionAPI.java | Uses Utils.convertToMap for v2 payload → v1 params. |
| solr/core/src/java/org/apache/solr/handler/admin/api/MigrateDocsAPI.java | Uses Utils.convertToMap for v2 payload → v1 params. |
| solr/core/src/java/org/apache/solr/handler/admin/api/InstallShardData.java | Uses Utils.convertToMap before creating ZkNodeProps. |
| solr/core/src/java/org/apache/solr/handler/admin/api/CreateCore.java | Uses Utils.convertToMap to derive request body from v1 params. |
| solr/core/src/java/org/apache/solr/handler/admin/ZookeeperInfoHandler.java | Uses Utils.convertToMap for DocCollection serialization. |
| solr/core/src/java/org/apache/solr/handler/admin/IndexSizeEstimator.java | Uses SimpleOrderedMap when converting MapWriter results. |
| solr/core/src/java/org/apache/solr/handler/admin/BaseHandlerApiSupport.java | Switches adapter from toMap(...) to writeMap(...) and filters nulls. |
| solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java | Migrates plugin/config map building to MapWriter/SimpleOrderedMap and adds helper for component lookup. |
| solr/core/src/java/org/apache/solr/handler/ClusterAPI.java | Builds role command maps using SimpleOrderedMap. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| SolrParams solrParams = idFieldMapUpdated.toSolrParams(); | ||
| Map<String, Object> mapParams = solrParams.toMap(new HashMap<>()); | ||
| Map<String, Object> mapParams = new SimpleOrderedMap<>(solrParams); |
| public void joinOverseerLeaderElection(PayloadObj<OverseerOperationPayload> payload) | ||
| throws Exception { | ||
| final Map<String, Object> v1Params = payload.get().toMap(new HashMap<>()); | ||
| final Map<String, Object> v1Params = new SimpleOrderedMap<>(payload.get()); |
There was a problem hiding this comment.
Lets standardize on use of Utils.convertToMap in these places (classes with suffix API... on the request dispatch code-path)
| Map<String, Object> map = new SimpleOrderedMap<>(queue); | ||
| entry.setValue(map); | ||
| } else if (value instanceof MapWriterSummaryStatistics stats) { | ||
| Map<String, Object> map = new LinkedHashMap<>(); | ||
| stats.toMap(map); | ||
| Map<String, Object> map = new SimpleOrderedMap<>(stats); |
There was a problem hiding this comment.
SOM is good here; this is a response-structure
| m.put(ZNODEVER, params.getZnodeVersion()); | ||
| if (p != null) { | ||
| m.put(RequestParams.NAME, Map.of(parts.get(2), p.toMap(new LinkedHashMap<>()))); | ||
| m.put(RequestParams.NAME, Map.of(parts.get(2), new SimpleOrderedMap<>(p))); |
There was a problem hiding this comment.
SOM makes sense as it's a response structure here
| o instanceof MapSerializable | ||
| ? ((MapSerializable) o).toMap(new LinkedHashMap<>()) | ||
| o instanceof MapWriter | ||
| ? new SimpleOrderedMap<>((MapWriter) o) |
| String componentName = componentType == null ? null : req.getParams().get("componentName"); | ||
| boolean showParams = req.getParams().getBool("expandParams", false); | ||
| Map<String, Object> map = this.req.getCore().getSolrConfig().toMap(new LinkedHashMap<>()); | ||
| Map<String, Object> map = new SimpleOrderedMap<>(this.req.getCore().getSolrConfig()); |
| pluginInfo = (Map) plugin; | ||
| } else if (plugin instanceof PluginInfo) { | ||
| pluginInfo = ((PluginInfo) plugin).toMap(new LinkedHashMap<>()); | ||
| pluginInfo = new SimpleOrderedMap<>((PluginInfo) plugin); |
| return pluginInfo.containsKey("class") | ||
| ? pluginInfo | ||
| : (Map<String, Object>) pluginInfo.getOrDefault(componentName, new HashMap<>()); |
de2593f to
baae128
Compare
baae128 to
74d671e
Compare
dsmiley
left a comment
There was a problem hiding this comment.
Thanks for resuming this effort!
| public void joinOverseerLeaderElection(PayloadObj<OverseerOperationPayload> payload) | ||
| throws Exception { | ||
| final Map<String, Object> v1Params = payload.get().toMap(new HashMap<>()); | ||
| final Map<String, Object> v1Params = new SimpleOrderedMap<>(payload.get()); |
There was a problem hiding this comment.
Lets standardize on use of Utils.convertToMap in these places (classes with suffix API... on the request dispatch code-path)
| if (o == null) { | ||
| ew.put(param, null); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
can we skip putting 'null'? Curious, what test or issue brought this change about?
| Map<String, Object> map = new SimpleOrderedMap<>(queue); | ||
| entry.setValue(map); | ||
| } else if (value instanceof MapWriterSummaryStatistics stats) { | ||
| Map<String, Object> map = new LinkedHashMap<>(); | ||
| stats.toMap(map); | ||
| Map<String, Object> map = new SimpleOrderedMap<>(stats); |
There was a problem hiding this comment.
SOM is good here; this is a response-structure
| m.put(ZNODEVER, params.getZnodeVersion()); | ||
| if (p != null) { | ||
| m.put(RequestParams.NAME, Map.of(parts.get(2), p.toMap(new LinkedHashMap<>()))); | ||
| m.put(RequestParams.NAME, Map.of(parts.get(2), new SimpleOrderedMap<>(p))); |
There was a problem hiding this comment.
SOM makes sense as it's a response structure here
| o instanceof MapSerializable | ||
| ? ((MapSerializable) o).toMap(new LinkedHashMap<>()) | ||
| o instanceof MapWriter | ||
| ? new SimpleOrderedMap<>((MapWriter) o) |
This is Part 1 of 4 to deprecate and remove the
MapSerializableinterface as part of SOLR-17600.This PR migrates all V2 API payload and handler classes to use
MapWriter/Utils.convertToMap.Note: The work has been split into 4 PRs to make reviewing easier. The subsequent PRs are stacked on top of this one.