From 487f09087d1c5a4668ff2b8a880f03ad02b1b5bb Mon Sep 17 00:00:00 2001 From: adityamparikh Date: Sat, 28 Feb 2026 23:38:36 +0000 Subject: [PATCH 1/7] feat(config): switch Solr wire format from JavaBin to JSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the default BinaryResponseParser (wt=javabin) with a custom JsonResponseParser (wt=json) for future-proofing and improved debuggability. The JsonResponseParser converts Solr's JSON response envelope into the NamedList tree that SolrJ typed response classes expect: - JSON objects → SimpleOrderedMap (extends NamedList, implements Map, satisfying both QueryResponse's NamedList casts and SchemaResponse's Map cast) - JSON objects with numFound+docs → SolrDocumentList - Flat alternating arrays [String, non-String, ...] → SimpleOrderedMap (Solr's json.nl=flat encoding for facet counts) - All other arrays → List - Decimal numbers → Float (matching JavaBin's float type, required by SchemaResponse's (Float) version cast) - Small integers → Integer, large integers → Long Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: adityamparikh --- .../mcp/server/config/JsonResponseParser.java | 207 ++++++++++++++++++ .../solr/mcp/server/config/SolrConfig.java | 5 +- 2 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/apache/solr/mcp/server/config/JsonResponseParser.java diff --git a/src/main/java/org/apache/solr/mcp/server/config/JsonResponseParser.java b/src/main/java/org/apache/solr/mcp/server/config/JsonResponseParser.java new file mode 100644 index 0000000..97f5053 --- /dev/null +++ b/src/main/java/org/apache/solr/mcp/server/config/JsonResponseParser.java @@ -0,0 +1,207 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.config; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.IOException; +import java.io.InputStream; +import java.io.Reader; +import java.util.ArrayList; +import java.util.List; +import org.apache.solr.client.solrj.ResponseParser; +import org.apache.solr.common.SolrDocument; +import org.apache.solr.common.SolrDocumentList; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; + +/** + * SolrJ {@link ResponseParser} that requests JSON wire format ({@code wt=json}) + * and converts the Solr JSON response into the {@link NamedList} object tree + * that SolrJ's typed response classes + * ({@link org.apache.solr.client.solrj.response.QueryResponse}, + * {@link org.apache.solr.client.solrj.response.LukeResponse}, etc.) expect + * internally. + * + *

+ * This allows the application to keep all existing SolrJ response handling + * unchanged while moving off the JavaBin ({@code wt=javabin}) wire format. + * + *

+ * Structural conversions: + *

    + *
  • JSON objects → {@link NamedList} (preserving key order)
    • + *
  • JSON objects containing {@code numFound} + {@code docs} → + * {@link SolrDocumentList}
    • + *
  • JSON arrays with alternating {@code [String, non-String, ...]} pairs → + * {@link NamedList} (Solr's {@code json.nl=flat} facet encoding)
    • + *
  • All other JSON arrays → {@link List}
    • + *
  • JSON integers → {@link Integer} or {@link Long} (by value size)
    • + *
  • JSON decimals → {@link Double}
    • + *
  • JSON booleans → {@link Boolean}
    • + *
  • JSON strings → {@link String}
    • + *
+ * + *

+ * Flat NamedList detection: Solr serializes facet counts as + * flat arrays using {@code json.nl=flat} (the default). An array is treated as + * a flat NamedList when every even-indexed element is a {@link String} and + * every odd-indexed element is a non-{@link String} value. This reliably + * distinguishes {@code ["term", 5, "term2", 3]} (facet NamedList) from + * {@code ["col1", "col2"]} (plain string list). + */ +class JsonResponseParser extends ResponseParser { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + @Override + public String getWriterType() { + return "json"; + } + + @Override + public String getContentType() { + return "application/json; charset=UTF-8"; + } + + @Override + public NamedList processResponse(InputStream body, String encoding) { + try { + return toNamedList(MAPPER.readTree(body)); + } catch (IOException e) { + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed to parse Solr JSON response", e); + } + } + + @Override + public NamedList processResponse(Reader reader) { + try { + return toNamedList(MAPPER.readTree(reader)); + } catch (IOException e) { + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed to parse Solr JSON response", e); + } + } + + private SimpleOrderedMap toNamedList(JsonNode objectNode) { + SimpleOrderedMap result = new SimpleOrderedMap<>(); + objectNode.fields().forEachRemaining(entry -> result.add(entry.getKey(), convertValue(entry.getValue()))); + return result; + } + + private Object convertValue(JsonNode node) { + if (node.isNull()) + return null; + if (node.isBoolean()) + return node.booleanValue(); + if (node.isTextual()) + return node.textValue(); + if (node.isInt()) + return node.intValue(); + if (node.isLong()) + return node.longValue(); + if (node.isDouble() || node.isFloat()) + return node.floatValue(); + if (node.isObject()) + return convertObject(node); + if (node.isArray()) + return convertArray(node); + return node.asText(); + } + + private Object convertObject(JsonNode node) { + // Detect a Solr query result set by the presence of numFound + docs + if (node.has("numFound") && node.has("docs")) { + return toSolrDocumentList(node); + } + return toNamedList(node); + } + + private Object convertArray(JsonNode arrayNode) { + // Detect Solr's flat NamedList encoding: [String, non-String, String, + // non-String, ...] + // Used for facet counts (json.nl=flat default). Distinguished from plain string + // arrays + // by requiring odd-indexed elements to be non-string values. + if (isFlatNamedList(arrayNode)) { + return flatArrayToNamedList(arrayNode); + } + List list = new ArrayList<>(arrayNode.size()); + arrayNode.forEach(element -> list.add(convertValue(element))); + return list; + } + + /** + * Returns true when the array has even length, every even-indexed element is a + * string (the key), and every odd-indexed element is NOT a string (the value). + * This heuristic correctly identifies Solr facet data like + * {@code ["fantasy", 10, "scifi", 5]} while rejecting plain string arrays like + * {@code ["col1", "col2"]}. + */ + private boolean isFlatNamedList(JsonNode arrayNode) { + int size = arrayNode.size(); + if (size == 0 || size % 2 != 0) + return false; + for (int i = 0; i < size; i += 2) { + if (!arrayNode.get(i).isTextual()) + return false; // key must be string + if (arrayNode.get(i + 1).isTextual()) + return false; // value must not be string + } + return true; + } + + private SimpleOrderedMap flatArrayToNamedList(JsonNode arrayNode) { + SimpleOrderedMap result = new SimpleOrderedMap<>(); + for (int i = 0; i < arrayNode.size(); i += 2) { + result.add(arrayNode.get(i).textValue(), convertValue(arrayNode.get(i + 1))); + } + return result; + } + + private SolrDocumentList toSolrDocumentList(JsonNode node) { + SolrDocumentList list = new SolrDocumentList(); + list.setNumFound(node.get("numFound").longValue()); + list.setStart(node.path("start").longValue()); + JsonNode numFoundExact = node.get("numFoundExact"); + if (numFoundExact != null && !numFoundExact.isNull()) { + list.setNumFoundExact(numFoundExact.booleanValue()); + } + JsonNode maxScore = node.get("maxScore"); + if (maxScore != null && !maxScore.isNull()) { + list.setMaxScore(maxScore.floatValue()); + } + node.get("docs").forEach(doc -> list.add(toSolrDocument(doc))); + return list; + } + + private SolrDocument toSolrDocument(JsonNode node) { + SolrDocument doc = new SolrDocument(); + node.fields().forEachRemaining(entry -> { + JsonNode val = entry.getValue(); + if (val.isArray()) { + // Multi-valued field — always a plain list, never a flat NamedList + List values = new ArrayList<>(val.size()); + val.forEach(v -> values.add(convertValue(v))); + doc.setField(entry.getKey(), values); + } else { + doc.setField(entry.getKey(), convertValue(val)); + } + }); + return doc; + } +} diff --git a/src/main/java/org/apache/solr/mcp/server/config/SolrConfig.java b/src/main/java/org/apache/solr/mcp/server/config/SolrConfig.java index 7359ccf..cefa735 100644 --- a/src/main/java/org/apache/solr/mcp/server/config/SolrConfig.java +++ b/src/main/java/org/apache/solr/mcp/server/config/SolrConfig.java @@ -186,8 +186,9 @@ SolrClient solrClient(SolrConfigurationProperties properties) { } } - // Use with explicit base URL + // Use with explicit base URL; JSON wire format replaces the JavaBin default return new Http2SolrClient.Builder(url).withConnectionTimeout(CONNECTION_TIMEOUT_MS, TimeUnit.MILLISECONDS) - .withIdleTimeout(SOCKET_TIMEOUT_MS, TimeUnit.MILLISECONDS).build(); + .withIdleTimeout(SOCKET_TIMEOUT_MS, TimeUnit.MILLISECONDS).withResponseParser(new JsonResponseParser()) + .build(); } } From a387b19f00d91d752d16594cef1fd937cb346924 Mon Sep 17 00:00:00 2001 From: adityamparikh Date: Thu, 5 Mar 2026 23:25:03 -0500 Subject: [PATCH 2/7] refactor(config): make JsonResponseParser a Spring Bean Extract JsonResponseParser instantiation into a dedicated @Bean method so it can be injected as a dependency into solrClient(), making the wiring explicit and enabling overriding in tests. Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: adityamparikh --- .../org/apache/solr/mcp/server/config/SolrConfig.java | 9 +++++++-- .../apache/solr/mcp/server/config/SolrConfigTest.java | 10 +++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/apache/solr/mcp/server/config/SolrConfig.java b/src/main/java/org/apache/solr/mcp/server/config/SolrConfig.java index cefa735..95f8104 100644 --- a/src/main/java/org/apache/solr/mcp/server/config/SolrConfig.java +++ b/src/main/java/org/apache/solr/mcp/server/config/SolrConfig.java @@ -168,7 +168,12 @@ public class SolrConfig { * @see SolrConfigurationProperties#url() */ @Bean - SolrClient solrClient(SolrConfigurationProperties properties) { + JsonResponseParser jsonResponseParser() { + return new JsonResponseParser(); + } + + @Bean + SolrClient solrClient(SolrConfigurationProperties properties, JsonResponseParser jsonResponseParser) { String url = properties.url(); // Ensure URL is properly formatted for Solr @@ -188,7 +193,7 @@ SolrClient solrClient(SolrConfigurationProperties properties) { // Use with explicit base URL; JSON wire format replaces the JavaBin default return new Http2SolrClient.Builder(url).withConnectionTimeout(CONNECTION_TIMEOUT_MS, TimeUnit.MILLISECONDS) - .withIdleTimeout(SOCKET_TIMEOUT_MS, TimeUnit.MILLISECONDS).withResponseParser(new JsonResponseParser()) + .withIdleTimeout(SOCKET_TIMEOUT_MS, TimeUnit.MILLISECONDS).withResponseParser(jsonResponseParser) .build(); } } diff --git a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java index 342ddd0..e14c1fb 100644 --- a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java +++ b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java @@ -81,7 +81,7 @@ void testUrlNormalization(String inputUrl, String expectedUrl) { SolrConfig solrConfig = new SolrConfig(); // Test URL normalization - SolrClient client = solrConfig.solrClient(testProperties); + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser()); assertNotNull(client); var httpClient = assertInstanceOf(Http2SolrClient.class, client); @@ -101,7 +101,7 @@ void testUrlWithoutTrailingSlash() { SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983"); SolrConfig solrConfig = new SolrConfig(); - SolrClient client = solrConfig.solrClient(testProperties); + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser()); Http2SolrClient httpClient = (Http2SolrClient) client; // Should add trailing slash and solr path @@ -120,7 +120,7 @@ void testUrlWithTrailingSlashButNoSolrPath() { SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/"); SolrConfig solrConfig = new SolrConfig(); - SolrClient client = solrConfig.solrClient(testProperties); + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser()); Http2SolrClient httpClient = (Http2SolrClient) client; // Should add solr path to existing trailing slash @@ -139,7 +139,7 @@ void testUrlWithSolrPathButNoTrailingSlash() { SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/solr"); SolrConfig solrConfig = new SolrConfig(); - SolrClient client = solrConfig.solrClient(testProperties); + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser()); Http2SolrClient httpClient = (Http2SolrClient) client; // Should add trailing slash @@ -158,7 +158,7 @@ void testUrlAlreadyProperlyFormatted() { SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/solr/"); SolrConfig solrConfig = new SolrConfig(); - SolrClient client = solrConfig.solrClient(testProperties); + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser()); Http2SolrClient httpClient = (Http2SolrClient) client; // Should remain unchanged From ca805998f4778f742c35bd9d7660ea345645661a Mon Sep 17 00:00:00 2001 From: adityamparikh Date: Fri, 6 Mar 2026 19:27:46 -0500 Subject: [PATCH 3/7] refactor(config): inject Spring's ObjectMapper into JsonResponseParser Replace the static new ObjectMapper() with Spring's auto-configured ObjectMapper bean injected via constructor. Use MediaType.APPLICATION_JSON_VALUE for the content type constant instead of a raw string literal. Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: adityamparikh --- .../solr/mcp/server/config/JsonResponseParser.java | 13 +++++++++---- .../apache/solr/mcp/server/config/SolrConfig.java | 5 +++-- .../solr/mcp/server/config/SolrConfigTest.java | 11 ++++++----- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/apache/solr/mcp/server/config/JsonResponseParser.java b/src/main/java/org/apache/solr/mcp/server/config/JsonResponseParser.java index 97f5053..b57daf8 100644 --- a/src/main/java/org/apache/solr/mcp/server/config/JsonResponseParser.java +++ b/src/main/java/org/apache/solr/mcp/server/config/JsonResponseParser.java @@ -29,6 +29,7 @@ import org.apache.solr.common.SolrException; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.SimpleOrderedMap; +import org.springframework.http.MediaType; /** * SolrJ {@link ResponseParser} that requests JSON wire format ({@code wt=json}) @@ -67,7 +68,11 @@ */ class JsonResponseParser extends ResponseParser { - private static final ObjectMapper MAPPER = new ObjectMapper(); + private final ObjectMapper mapper; + + JsonResponseParser(ObjectMapper mapper) { + this.mapper = mapper; + } @Override public String getWriterType() { @@ -76,13 +81,13 @@ public String getWriterType() { @Override public String getContentType() { - return "application/json; charset=UTF-8"; + return MediaType.APPLICATION_JSON_VALUE; } @Override public NamedList processResponse(InputStream body, String encoding) { try { - return toNamedList(MAPPER.readTree(body)); + return toNamedList(mapper.readTree(body)); } catch (IOException e) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed to parse Solr JSON response", e); } @@ -91,7 +96,7 @@ public NamedList processResponse(InputStream body, String encoding) { @Override public NamedList processResponse(Reader reader) { try { - return toNamedList(MAPPER.readTree(reader)); + return toNamedList(mapper.readTree(reader)); } catch (IOException e) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed to parse Solr JSON response", e); } diff --git a/src/main/java/org/apache/solr/mcp/server/config/SolrConfig.java b/src/main/java/org/apache/solr/mcp/server/config/SolrConfig.java index 95f8104..ed2cf88 100644 --- a/src/main/java/org/apache/solr/mcp/server/config/SolrConfig.java +++ b/src/main/java/org/apache/solr/mcp/server/config/SolrConfig.java @@ -16,6 +16,7 @@ */ package org.apache.solr.mcp.server.config; +import com.fasterxml.jackson.databind.ObjectMapper; import java.util.concurrent.TimeUnit; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.Http2SolrClient; @@ -168,8 +169,8 @@ public class SolrConfig { * @see SolrConfigurationProperties#url() */ @Bean - JsonResponseParser jsonResponseParser() { - return new JsonResponseParser(); + JsonResponseParser jsonResponseParser(ObjectMapper objectMapper) { + return new JsonResponseParser(objectMapper); } @Bean diff --git a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java index e14c1fb..086c459 100644 --- a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java +++ b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.*; +import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.apache.solr.mcp.server.TestcontainersConfiguration; @@ -81,7 +82,7 @@ void testUrlNormalization(String inputUrl, String expectedUrl) { SolrConfig solrConfig = new SolrConfig(); // Test URL normalization - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser()); + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(new ObjectMapper())); assertNotNull(client); var httpClient = assertInstanceOf(Http2SolrClient.class, client); @@ -101,7 +102,7 @@ void testUrlWithoutTrailingSlash() { SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983"); SolrConfig solrConfig = new SolrConfig(); - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser()); + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(new ObjectMapper())); Http2SolrClient httpClient = (Http2SolrClient) client; // Should add trailing slash and solr path @@ -120,7 +121,7 @@ void testUrlWithTrailingSlashButNoSolrPath() { SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/"); SolrConfig solrConfig = new SolrConfig(); - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser()); + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(new ObjectMapper())); Http2SolrClient httpClient = (Http2SolrClient) client; // Should add solr path to existing trailing slash @@ -139,7 +140,7 @@ void testUrlWithSolrPathButNoTrailingSlash() { SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/solr"); SolrConfig solrConfig = new SolrConfig(); - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser()); + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(new ObjectMapper())); Http2SolrClient httpClient = (Http2SolrClient) client; // Should add trailing slash @@ -158,7 +159,7 @@ void testUrlAlreadyProperlyFormatted() { SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/solr/"); SolrConfig solrConfig = new SolrConfig(); - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser()); + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(new ObjectMapper())); Http2SolrClient httpClient = (Http2SolrClient) client; // Should remain unchanged From 4b08938fb3edeb075e520d53c15fc46df913d107 Mon Sep 17 00:00:00 2001 From: adityamparikh Date: Fri, 6 Mar 2026 19:57:42 -0500 Subject: [PATCH 4/7] test(config): use @JsonTest for URL normalization tests to get Spring's ObjectMapper Extract URL normalization tests from SolrConfigTest into a dedicated SolrConfigUrlNormalizationTest annotated with @JsonTest, so Spring's auto-configured ObjectMapper is injected rather than using new ObjectMapper(). Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: adityamparikh --- .../mcp/server/config/SolrConfigTest.java | 106 --------------- .../SolrConfigUrlNormalizationTest.java | 126 ++++++++++++++++++ 2 files changed, 126 insertions(+), 106 deletions(-) create mode 100644 src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java diff --git a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java index 086c459..ce109c6 100644 --- a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java +++ b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigTest.java @@ -18,13 +18,10 @@ import static org.junit.jupiter.api.Assertions.*; -import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.apache.solr.mcp.server.TestcontainersConfiguration; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.CsvSource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.context.annotation.Import; @@ -68,107 +65,4 @@ void testSolrConfigurationProperties() { properties.url()); } - @ParameterizedTest - @CsvSource({"http://localhost:8983, http://localhost:8983/solr", - "http://localhost:8983/, http://localhost:8983/solr", - "http://localhost:8983/solr, http://localhost:8983/solr", - "http://localhost:8983/solr/, http://localhost:8983/solr", - "http://localhost:8983/custom/solr/, http://localhost:8983/custom/solr"}) - void testUrlNormalization(String inputUrl, String expectedUrl) { - // Create a test properties object - SolrConfigurationProperties testProperties = new SolrConfigurationProperties(inputUrl); - - // Create SolrConfig instance - SolrConfig solrConfig = new SolrConfig(); - - // Test URL normalization - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(new ObjectMapper())); - assertNotNull(client); - - var httpClient = assertInstanceOf(Http2SolrClient.class, client); - assertEquals(expectedUrl, httpClient.getBaseURL()); - - // Clean up - try { - client.close(); - } catch (Exception e) { - // Ignore close errors in test - } - } - - @Test - void testUrlWithoutTrailingSlash() { - // Test URL without trailing slash branch - SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983"); - SolrConfig solrConfig = new SolrConfig(); - - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(new ObjectMapper())); - Http2SolrClient httpClient = (Http2SolrClient) client; - - // Should add trailing slash and solr path - assertEquals("http://localhost:8983/solr", httpClient.getBaseURL()); - - try { - client.close(); - } catch (Exception e) { - // Ignore close errors in test - } - } - - @Test - void testUrlWithTrailingSlashButNoSolrPath() { - // Test URL with trailing slash but no solr path branch - SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/"); - SolrConfig solrConfig = new SolrConfig(); - - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(new ObjectMapper())); - Http2SolrClient httpClient = (Http2SolrClient) client; - - // Should add solr path to existing trailing slash - assertEquals("http://localhost:8983/solr", httpClient.getBaseURL()); - - try { - client.close(); - } catch (Exception e) { - // Ignore close errors in test - } - } - - @Test - void testUrlWithSolrPathButNoTrailingSlash() { - // Test URL with solr path but no trailing slash - SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/solr"); - SolrConfig solrConfig = new SolrConfig(); - - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(new ObjectMapper())); - Http2SolrClient httpClient = (Http2SolrClient) client; - - // Should add trailing slash - assertEquals("http://localhost:8983/solr", httpClient.getBaseURL()); - - try { - client.close(); - } catch (Exception e) { - // Ignore close errors in test - } - } - - @Test - void testUrlAlreadyProperlyFormatted() { - // Test URL that's already properly formatted - SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/solr/"); - SolrConfig solrConfig = new SolrConfig(); - - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(new ObjectMapper())); - Http2SolrClient httpClient = (Http2SolrClient) client; - - // Should remain unchanged - assertEquals("http://localhost:8983/solr", httpClient.getBaseURL()); - - try { - client.close(); - } catch (Exception e) { - // Ignore close errors in test - } - } } diff --git a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java new file mode 100644 index 0000000..f90b449 --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java @@ -0,0 +1,126 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.config; + +import static org.junit.jupiter.api.Assertions.*; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.impl.Http2SolrClient; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.json.JsonTest; + +@JsonTest +class SolrConfigUrlNormalizationTest { + + @Autowired + private ObjectMapper objectMapper; + + @ParameterizedTest + @CsvSource({"http://localhost:8983, http://localhost:8983/solr", + "http://localhost:8983/, http://localhost:8983/solr", + "http://localhost:8983/solr, http://localhost:8983/solr", + "http://localhost:8983/solr/, http://localhost:8983/solr", + "http://localhost:8983/custom/solr/, http://localhost:8983/custom/solr"}) + void testUrlNormalization(String inputUrl, String expectedUrl) { + SolrConfigurationProperties testProperties = new SolrConfigurationProperties(inputUrl); + SolrConfig solrConfig = new SolrConfig(); + + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(objectMapper)); + assertNotNull(client); + + var httpClient = assertInstanceOf(Http2SolrClient.class, client); + assertEquals(expectedUrl, httpClient.getBaseURL()); + + try { + client.close(); + } catch (Exception e) { + // Ignore close errors in test + } + } + + @Test + void testUrlWithoutTrailingSlash() { + SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983"); + SolrConfig solrConfig = new SolrConfig(); + + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(objectMapper)); + Http2SolrClient httpClient = (Http2SolrClient) client; + + assertEquals("http://localhost:8983/solr", httpClient.getBaseURL()); + + try { + client.close(); + } catch (Exception e) { + // Ignore close errors in test + } + } + + @Test + void testUrlWithTrailingSlashButNoSolrPath() { + SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/"); + SolrConfig solrConfig = new SolrConfig(); + + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(objectMapper)); + Http2SolrClient httpClient = (Http2SolrClient) client; + + assertEquals("http://localhost:8983/solr", httpClient.getBaseURL()); + + try { + client.close(); + } catch (Exception e) { + // Ignore close errors in test + } + } + + @Test + void testUrlWithSolrPathButNoTrailingSlash() { + SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/solr"); + SolrConfig solrConfig = new SolrConfig(); + + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(objectMapper)); + Http2SolrClient httpClient = (Http2SolrClient) client; + + assertEquals("http://localhost:8983/solr", httpClient.getBaseURL()); + + try { + client.close(); + } catch (Exception e) { + // Ignore close errors in test + } + } + + @Test + void testUrlAlreadyProperlyFormatted() { + SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/solr/"); + SolrConfig solrConfig = new SolrConfig(); + + SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(objectMapper)); + Http2SolrClient httpClient = (Http2SolrClient) client; + + assertEquals("http://localhost:8983/solr", httpClient.getBaseURL()); + + try { + client.close(); + } catch (Exception e) { + // Ignore close errors in test + } + } +} From bcfe508babcc362d7d811a635de53f3271421c49 Mon Sep 17 00:00:00 2001 From: adityamparikh Date: Sun, 8 Mar 2026 20:08:38 -0400 Subject: [PATCH 5/7] fix(quality): resolve SonarQube violations Replace unused catch variable with underscore unnamed pattern (S7467) in SolrConfigUrlNormalizationTest - Java 25 feature. Signed-off-by: Aditya Parikh Co-Authored-By: Claude Opus 4.6 Signed-off-by: adityamparikh --- .../server/config/SolrConfigUrlNormalizationTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java index f90b449..072a785 100644 --- a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java +++ b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java @@ -51,7 +51,7 @@ void testUrlNormalization(String inputUrl, String expectedUrl) { try { client.close(); - } catch (Exception e) { + } catch (Exception _) { // Ignore close errors in test } } @@ -68,7 +68,7 @@ void testUrlWithoutTrailingSlash() { try { client.close(); - } catch (Exception e) { + } catch (Exception _) { // Ignore close errors in test } } @@ -85,7 +85,7 @@ void testUrlWithTrailingSlashButNoSolrPath() { try { client.close(); - } catch (Exception e) { + } catch (Exception _) { // Ignore close errors in test } } @@ -102,7 +102,7 @@ void testUrlWithSolrPathButNoTrailingSlash() { try { client.close(); - } catch (Exception e) { + } catch (Exception _) { // Ignore close errors in test } } @@ -119,7 +119,7 @@ void testUrlAlreadyProperlyFormatted() { try { client.close(); - } catch (Exception e) { + } catch (Exception _) { // Ignore close errors in test } } From 9e3fac07b8b05cefa3e75fd6837995c43f80ad88 Mon Sep 17 00:00:00 2001 From: adityamparikh Date: Tue, 24 Mar 2026 14:28:54 -0400 Subject: [PATCH 6/7] refactor(test): remove duplicate URL normalization tests The individual @Test methods were exact duplicates of cases already covered by the @ParameterizedTest with @CsvSource. Co-Authored-By: Claude Opus 4.6 Signed-off-by: adityamparikh --- .../SolrConfigUrlNormalizationTest.java | 69 ------------------- 1 file changed, 69 deletions(-) diff --git a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java index 072a785..cbb9864 100644 --- a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java +++ b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java @@ -21,7 +21,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.Http2SolrClient; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.springframework.beans.factory.annotation.Autowired; @@ -55,72 +54,4 @@ void testUrlNormalization(String inputUrl, String expectedUrl) { // Ignore close errors in test } } - - @Test - void testUrlWithoutTrailingSlash() { - SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983"); - SolrConfig solrConfig = new SolrConfig(); - - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(objectMapper)); - Http2SolrClient httpClient = (Http2SolrClient) client; - - assertEquals("http://localhost:8983/solr", httpClient.getBaseURL()); - - try { - client.close(); - } catch (Exception _) { - // Ignore close errors in test - } - } - - @Test - void testUrlWithTrailingSlashButNoSolrPath() { - SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/"); - SolrConfig solrConfig = new SolrConfig(); - - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(objectMapper)); - Http2SolrClient httpClient = (Http2SolrClient) client; - - assertEquals("http://localhost:8983/solr", httpClient.getBaseURL()); - - try { - client.close(); - } catch (Exception _) { - // Ignore close errors in test - } - } - - @Test - void testUrlWithSolrPathButNoTrailingSlash() { - SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/solr"); - SolrConfig solrConfig = new SolrConfig(); - - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(objectMapper)); - Http2SolrClient httpClient = (Http2SolrClient) client; - - assertEquals("http://localhost:8983/solr", httpClient.getBaseURL()); - - try { - client.close(); - } catch (Exception _) { - // Ignore close errors in test - } - } - - @Test - void testUrlAlreadyProperlyFormatted() { - SolrConfigurationProperties testProperties = new SolrConfigurationProperties("http://localhost:8983/solr/"); - SolrConfig solrConfig = new SolrConfig(); - - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(objectMapper)); - Http2SolrClient httpClient = (Http2SolrClient) client; - - assertEquals("http://localhost:8983/solr", httpClient.getBaseURL()); - - try { - client.close(); - } catch (Exception _) { - // Ignore close errors in test - } - } } From a349e8430de50f58344f0ae746031ec163d2b177 Mon Sep 17 00:00:00 2001 From: adityamparikh Date: Tue, 24 Mar 2026 14:33:25 -0400 Subject: [PATCH 7/7] refactor(test): use try-with-resources for SolrClient in URL normalization test Co-Authored-By: Claude Opus 4.6 Signed-off-by: adityamparikh --- .../config/SolrConfigUrlNormalizationTest.java | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java index cbb9864..bca9219 100644 --- a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java +++ b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigUrlNormalizationTest.java @@ -38,20 +38,15 @@ class SolrConfigUrlNormalizationTest { "http://localhost:8983/solr, http://localhost:8983/solr", "http://localhost:8983/solr/, http://localhost:8983/solr", "http://localhost:8983/custom/solr/, http://localhost:8983/custom/solr"}) - void testUrlNormalization(String inputUrl, String expectedUrl) { + void testUrlNormalization(String inputUrl, String expectedUrl) throws Exception { SolrConfigurationProperties testProperties = new SolrConfigurationProperties(inputUrl); SolrConfig solrConfig = new SolrConfig(); - SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(objectMapper)); - assertNotNull(client); + try (SolrClient client = solrConfig.solrClient(testProperties, new JsonResponseParser(objectMapper))) { + assertNotNull(client); - var httpClient = assertInstanceOf(Http2SolrClient.class, client); - assertEquals(expectedUrl, httpClient.getBaseURL()); - - try { - client.close(); - } catch (Exception _) { - // Ignore close errors in test + var httpClient = assertInstanceOf(Http2SolrClient.class, client); + assertEquals(expectedUrl, httpClient.getBaseURL()); } } }