isaric commented on code in PR #4463:
URL: https://github.com/apache/solr/pull/4463#discussion_r3367288955
##########
solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerAPI.java:
##########
@@ -845,7 +844,7 @@ public void testSchemaDiffEndpoint() throws Exception {
idFieldMapUpdated.indexOf("omitTermFreqAndPositions", 0),
Boolean.FALSE);
SolrParams solrParams = idFieldMapUpdated.toSolrParams();
- Map<String, Object> mapParams = solrParams.toMap(new HashMap<>());
+ Map<String, Object> mapParams = new SimpleOrderedMap<>(solrParams);
Review Comment:
`SimpleOrderedMap<T> extends NamedList<T> implements Map<String, T>`
(SimpleOrderedMap.java:46). Assignment to `Map<String, Object>` compiles and
behaves correctly.
##########
solr/core/src/java/org/apache/solr/handler/admin/api/OverseerOperationAPI.java:
##########
@@ -59,7 +59,7 @@ public OverseerOperationAPI(CoreAdminHandler
coreAdminHandler) {
@Command(name = OVERSEER_OP_CMD)
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());
Review Comment:
`SimpleOrderedMap<T> extends NamedList<T> implements Map<String, T>`
(SimpleOrderedMap.java:46), so it is a `Map`. That said, this call site was
switched to `Utils.convertToMap(payload.get(), new HashMap<>())` per @dsmiley's
comment.
##########
solr/core/src/java/org/apache/solr/handler/admin/IndexSizeEstimator.java:
##########
@@ -277,12 +278,10 @@ private void convert(Map<String, Object> result) {
for (Map.Entry<String, Object> entry : result.entrySet()) {
Object value = entry.getValue();
if (value instanceof ItemPriorityQueue queue) {
- Map<String, Object> map = new LinkedHashMap<>();
- queue.toMap(map);
+ 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);
Review Comment:
`SimpleOrderedMap<T> extends NamedList<T> implements Map<String, T>`
(SimpleOrderedMap.java:46), so it is a `Map`. @dsmiley also confirmed SOM is
appropriate here as a response structure.
##########
solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java:
##########
@@ -204,7 +205,7 @@ private void handleGET() {
Map<String, Object> m = new LinkedHashMap<>();
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)));
Review Comment:
`SimpleOrderedMap` implements `Map<String, T>`, so the type is compatible.
This call site was further simplified — the `new SimpleOrderedMap<>(p)` wrapper
is dropped and `p` (a `MapWriter`) is passed directly.
##########
solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java:
##########
@@ -283,16 +284,16 @@ private void handleGET() {
Map pluginNameVsPluginInfo = (Map) val.get(parts.get(1));
if (pluginNameVsPluginInfo != null) {
Object o =
- pluginNameVsPluginInfo instanceof MapSerializable
+ pluginNameVsPluginInfo instanceof MapWriter
? pluginNameVsPluginInfo
: pluginNameVsPluginInfo.get(componentName);
Map<String, Object> pluginInfo =
- o instanceof MapSerializable
- ? ((MapSerializable) o).toMap(new LinkedHashMap<>())
+ o instanceof MapWriter
+ ? new SimpleOrderedMap<>((MapWriter) o)
Review Comment:
`SimpleOrderedMap` implements `Map<String, T>`, so the type is compatible.
The SOM stays here because the resulting map is mutated downstream
(`computeIfAbsent`, `put`).
##########
solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java:
##########
@@ -320,10 +323,18 @@ private void handleGET() {
}
}
+ @SuppressWarnings({"unchecked"})
+ private Map<String, Object> getComponentInfo(
+ Map<String, Object> pluginInfo, String componentName) {
+ return pluginInfo.containsKey("class")
+ ? pluginInfo
+ : (Map<String, Object>) pluginInfo.getOrDefault(componentName, new
HashMap<>());
+ }
+
private Map<String, Object> getConfigDetails(String componentType,
SolrQueryRequest req) {
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());
Review Comment:
`SimpleOrderedMap` implements `Map<String, T>`, so the type is compatible.
That said, this line was reverted to `.toMap(new LinkedHashMap<>())` because
`SolrConfig` does not implement `MapWriter` until Part 2 — the SOM constructor
call was premature.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]