This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr-mcp.git
The following commit(s) were added to refs/heads/main by this push:
new 412a5350 fix: remove unreachable null checks and declare real
nullability (#267)
412a5350 is described below
commit 412a53505edee02da725c3e52a6031e7c4a2ce4c
Author: Aditya Parikh <[email protected]>
AuthorDate: Mon Oct 26 15:15:58 2026 -0400
fix: remove unreachable null checks and declare real nullability (#267)
IntelliJ reported conditions that can never be true and null flows that
the declared types did not allow. Each one was either a null check on an
input that cannot be null, or a type that claimed non-null for a value
that can be null. They are fixed in that order:
- Null checks on required @McpToolParam/@McpArg parameters are removed.
MCP SDK 2.0 validates tool arguments against the input schema before
dispatch, and CompleteRequest/CompleteArgument reject a null argument
or value in their constructors. The blank and empty checks stay.
- Values that really can be null keep their checks and get the type that
says so: optional prompt arguments (purpose, sample, sampleDocument),
PromptText.optionalCodeBlock's content, missing NamedList entries
(CollectionUtils getters take NamedList<?>), JSON nulls in
JsonResponseParser's containers, YAML front-matter nulls in
MarkdownDocumentCreator.addValue, and maxScore when scores were not
requested.
- SortClause.order is @Nullable. The search tool documents it as
optional, defaulting to asc, but the generated schema listed it as
required, so a clause without an order was rejected before reaching
the code that applies the default. Now only field is required.
Tests that passed null to now-required parameters are removed (10). The
two that pin a contract on purpose (the SDK rejecting a null completion
value, the binder passing null for a missing solr.url) keep the null and
suppress the warning. Other test fixes: @Nullable on captured system
properties, AssertJ isPositive() instead of unboxing nullable counters,
@AutoClose for the MCP client, assertions that were always true
(required() || !required(), size() >= 0, non-null span timestamps) now
check what their messages describe, and unused fields, mocks, throws
clauses and an empty else are gone.
Signed-off-by: Aditya Parikh <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
---
.../solr/mcp/server/collection/AliasService.java | 6 +-
.../mcp/server/collection/CollectionService.java | 29 ++++-----
.../mcp/server/collection/CollectionUtils.java | 6 +-
.../apache/solr/mcp/server/collection/Dtos.java | 2 +-
.../solr/mcp/server/config/JsonResponseParser.java | 14 ++---
.../solr/mcp/server/indexing/IndexingService.java | 8 +--
.../documentcreator/IndexingDocumentCreator.java | 4 +-
.../documentcreator/JsonDocumentCreator.java | 2 +-
.../documentcreator/MarkdownDocumentCreator.java | 2 +-
.../solr/mcp/server/schema/SchemaService.java | 7 ++-
.../solr/mcp/server/search/SearchResponse.java | 3 +-
.../solr/mcp/server/search/SearchService.java | 2 +-
.../apache/solr/mcp/server/search/SortClause.java | 3 +-
.../server/security/HttpSecurityConfiguration.java | 4 +-
.../apache/solr/mcp/server/util/PromptText.java | 5 +-
.../solr/mcp/server/BuildInfoReaderTest.java | 3 +-
.../solr/mcp/server/LoggingConfigurationTest.java | 3 +-
.../mcp/server/McpClientIntegrationTestBase.java | 43 ++++++--------
.../solr/mcp/server/McpToolRegistrationTest.java | 6 +-
.../solr/mcp/server/SampleDataRuntimeHints.java | 2 +-
.../mcp/server/collection/AliasServiceTest.java | 24 --------
.../CollectionServiceIntegrationTest.java | 68 ++++++++++++----------
.../server/collection/CollectionServiceTest.java | 32 ++--------
.../mcp/server/collection/CollectionUtilsTest.java | 21 ++++---
.../config/SolrConfigurationPropertiesTest.java | 1 +
.../DockerImageHttpIntegrationTest.java | 19 +++---
.../indexing/IndexingServiceIntegrationTest.java | 13 ++---
.../mcp/server/indexing/IndexingServiceTest.java | 9 ---
.../mcp/server/indexing/MarkdownIndexingTest.java | 22 +++----
.../documentcreator/JsonDocumentCreatorTest.java | 14 +----
.../observability/DistributedTracingTest.java | 8 +--
.../mcp/server/observability/TraceAssertions.java | 3 +-
.../schema/SchemaServiceIntegrationTest.java | 18 +-----
.../solr/mcp/server/schema/SchemaServiceTest.java | 37 ------------
.../search/SearchServiceIntegrationTest.java | 7 +--
.../solr/mcp/server/search/SearchServiceTest.java | 24 +++-----
.../solr/mcp/server/search/SortClauseTest.java | 2 -
37 files changed, 172 insertions(+), 304 deletions(-)
diff --git
a/src/main/java/org/apache/solr/mcp/server/collection/AliasService.java
b/src/main/java/org/apache/solr/mcp/server/collection/AliasService.java
index a930b70b..962124f9 100644
--- a/src/main/java/org/apache/solr/mcp/server/collection/AliasService.java
+++ b/src/main/java/org/apache/solr/mcp/server/collection/AliasService.java
@@ -130,10 +130,10 @@ public class AliasService {
String collections)
throws SolrServerException, IOException {
- if (aliasName == null || aliasName.isBlank()) {
+ if (aliasName.isBlank()) {
throw new IllegalArgumentException(BLANK_ALIAS_NAME_ERROR);
}
- if (collections == null || collections.isBlank()) {
+ if (collections.isBlank()) {
throw new IllegalArgumentException(BLANK_COLLECTIONS_ERROR);
}
@@ -176,7 +176,7 @@ public class AliasService {
@McpToolParam(description = "Name of the alias to delete") String
aliasName)
throws SolrServerException, IOException {
- if (aliasName == null || aliasName.isBlank()) {
+ if (aliasName.isBlank()) {
throw new IllegalArgumentException(BLANK_ALIAS_NAME_ERROR);
}
diff --git
a/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java
b/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java
index 057ede94..aac8d277 100644
--- a/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java
+++ b/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java
@@ -135,9 +135,6 @@ public class CollectionService {
/** Universal Solr query pattern to match all documents in a collection */
private static final String ALL_DOCUMENTS_QUERY = "*:*";
- /** Suffix pattern used to identify shard names in SolrCloud deployments */
- private static final String SHARD_SUFFIX = "_shard";
-
/**
* Matches a SolrCloud shard/replica suffix at the end of a core name, e.g.
{@code _shard1} or
* {@code _shard1_replica_n1}. Anchored so that collection names merely
containing "_shard" are
@@ -338,13 +335,13 @@ public class CollectionService {
@PreAuthorize("isAuthenticated()")
@McpComplete(uri = "solr://{collection}/schema")
public List<String> completeCollection(CompleteRequest.CompleteArgument
argument) {
- if (argument == null || !"collection".equals(argument.name())) {
+ if (!"collection".equals(argument.name())) {
return List.of();
}
- String prefix = argument.value() == null ? "" :
argument.value().toLowerCase(Locale.ROOT);
+ String prefix = argument.value().toLowerCase(Locale.ROOT);
try {
return listCollections().stream()
- .filter(c -> c != null &&
c.toLowerCase(Locale.ROOT).startsWith(prefix))
+ .filter(c -> c.toLowerCase(Locale.ROOT).startsWith(prefix))
.sorted()
.limit(MAX_COMPLETION_RESULTS)
.toList();
@@ -675,7 +672,7 @@ public class CollectionService {
}
@SuppressWarnings("unchecked")
- private @Nullable CacheInfo extractSingleCacheInfo(NamedList<Object>
coreMetrics, String key) {
+ private @Nullable CacheInfo extractSingleCacheInfo(NamedList<?> coreMetrics,
String key) {
NamedList<Object> cache = (NamedList<Object>) coreMetrics.get(key);
if (cache == null) {
return null;
@@ -752,10 +749,10 @@ public class CollectionService {
* collection is not enabled.
*
* @param stats the handler statistics to evaluate
- * @return true if the stats are null or all handler types are null
+ * @return true if all handler types are null
*/
private boolean isHandlerStatsEmpty(HandlerStats stats) {
- return stats == null || (stats.selectHandler() == null &&
stats.updateHandler() == null);
+ return stats.selectHandler() == null && stats.updateHandler() == null;
}
/**
@@ -776,14 +773,14 @@ public class CollectionService {
GenericSolrRequest request =
new GenericSolrRequest(SolrRequest.METHOD.GET, ADMIN_METRICS_PATH,
params);
- NamedList<Object> response;
+ NamedList<?> response;
try {
response = solrClient.request(request);
} catch (SolrServerException | IOException | SolrException e) {
logger.debug("Cache/handler metrics unavailable for collection: {}",
collection, e);
return null;
}
- NamedList<Object> metrics = (NamedList<Object>) response.get(METRICS_KEY);
+ NamedList<?> metrics = (NamedList<?>) response.get(METRICS_KEY);
if (metrics == null || metrics.size() == 0) {
return null;
}
@@ -833,7 +830,7 @@ public class CollectionService {
* <p><strong>Extraction Logic:</strong>
*
* <ul>
- * <li>Detects shard patterns containing the {@value #SHARD_SUFFIX} suffix
+ * <li>Detects a trailing {@code _shard<N>} or {@code
_shard<N>_replica...} suffix
* <li>Returns the substring before the shard identifier
* <li>Returns the original string if no shard pattern is detected
* </ul>
@@ -850,10 +847,6 @@ public class CollectionService {
* @return the extracted collection name, or the original string if no shard
pattern found
*/
String extractCollectionName(String collectionOrShard) {
- if (collectionOrShard == null || collectionOrShard.isEmpty()) {
- return collectionOrShard;
- }
-
// Strip only a real SolrCloud shard/replica suffix. Matching a bare
// "_shard" anywhere would truncate legitimate collection names such as
// "orders_shard_archive" down to "orders".
@@ -965,7 +958,7 @@ public class CollectionService {
@Nullable Integer replicationFactor)
throws SolrServerException, IOException {
- if (name == null || name.isBlank()) {
+ if (name.isBlank()) {
throw new IllegalArgumentException(BLANK_COLLECTION_NAME_ERROR);
}
@@ -1045,7 +1038,7 @@ public class CollectionService {
description =
"Optional one-line description of what the collection is for
(used only to ground the conversation).",
required = false)
- String purpose) {
+ @Nullable String purpose) {
String purposeLine =
(purpose == null || purpose.isBlank()) ? "" : "\nPurpose:
%s\n".formatted(purpose.strip());
return """
diff --git
a/src/main/java/org/apache/solr/mcp/server/collection/CollectionUtils.java
b/src/main/java/org/apache/solr/mcp/server/collection/CollectionUtils.java
index 2635633b..19128668 100644
--- a/src/main/java/org/apache/solr/mcp/server/collection/CollectionUtils.java
+++ b/src/main/java/org/apache/solr/mcp/server/collection/CollectionUtils.java
@@ -93,7 +93,7 @@ public class CollectionUtils {
* @see Number#longValue()
* @see Long#parseLong(String)
*/
- public static @Nullable Long getLong(NamedList<Object> response, String key)
{
+ public static @Nullable Long getLong(NamedList<?> response, String key) {
Object value = response.get(key);
if (value == null) return null;
@@ -139,7 +139,7 @@ public class CollectionUtils {
* @return the Float value if found and convertible, {@code null} otherwise
* @see Number#floatValue()
*/
- public static @Nullable Float getFloat(NamedList<Object> stats, String key) {
+ public static @Nullable Float getFloat(NamedList<?> stats, String key) {
Object value = stats.get(key);
if (value == null) return null;
@@ -200,7 +200,7 @@ public class CollectionUtils {
* @see Integer#parseInt(String)
* @see #getLong(NamedList, String)
*/
- public static @Nullable Integer getInteger(NamedList<Object> response,
String key) {
+ public static @Nullable Integer getInteger(NamedList<?> response, String
key) {
Object value = response.get(key);
if (value == null) return null;
diff --git a/src/main/java/org/apache/solr/mcp/server/collection/Dtos.java
b/src/main/java/org/apache/solr/mcp/server/collection/Dtos.java
index dddc3fd5..3c389089 100644
--- a/src/main/java/org/apache/solr/mcp/server/collection/Dtos.java
+++ b/src/main/java/org/apache/solr/mcp/server/collection/Dtos.java
@@ -157,7 +157,7 @@ record QueryStats(
Long start,
/** Highest relevance score among the returned documents */
- Float maxScore) {}
+ @Nullable Float maxScore) {}
/**
* Solr cache utilization statistics across all cache types.
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 49f77f0b..79c440c4 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
@@ -90,7 +90,7 @@ class JsonResponseParser extends ResponseParser {
private static final String FACET_FIELDS_PATH = "facet_counts/facet_fields";
@Override
- public NamedList<Object> processResponse(InputStream body, String encoding) {
+ public NamedList<@Nullable Object> processResponse(InputStream body, String
encoding) {
try {
return toNamedList(mapper.readTree(body), "");
} catch (JacksonException e) {
@@ -99,8 +99,8 @@ class JsonResponseParser extends ResponseParser {
}
}
- private SimpleOrderedMap<Object> toNamedList(JsonNode objectNode, String
path) {
- SimpleOrderedMap<Object> result = new SimpleOrderedMap<>();
+ private SimpleOrderedMap<@Nullable Object> toNamedList(JsonNode objectNode,
String path) {
+ SimpleOrderedMap<@Nullable Object> result = new SimpleOrderedMap<>();
objectNode
.properties()
.forEach(
@@ -150,7 +150,7 @@ class JsonResponseParser extends ResponseParser {
if (isFlatNamedList(arrayNode)) {
return flatArrayToNamedList(arrayNode);
}
- List<Object> list = new ArrayList<>(arrayNode.size());
+ List<@Nullable Object> list = new ArrayList<>(arrayNode.size());
arrayNode.forEach(element -> list.add(convertValue(element, path)));
return list;
}
@@ -177,8 +177,8 @@ class JsonResponseParser extends ResponseParser {
return true;
}
- private SimpleOrderedMap<Object> flatArrayToNamedList(JsonNode arrayNode) {
- SimpleOrderedMap<Object> result = new SimpleOrderedMap<>();
+ private SimpleOrderedMap<@Nullable Object> flatArrayToNamedList(JsonNode
arrayNode) {
+ SimpleOrderedMap<@Nullable Object> result = new SimpleOrderedMap<>();
for (int i = 0; i < arrayNode.size(); i += 2) {
// Values here are facet counts (scalars), never nested facet arrays.
result.add(arrayNode.get(i).textValue(), convertValue(arrayNode.get(i +
1), ""));
@@ -210,7 +210,7 @@ class JsonResponseParser extends ResponseParser {
JsonNode val = entry.getValue();
if (val.isArray()) {
// Multi-valued field — always a plain list, never a flat
NamedList
- List<Object> values = new ArrayList<>(val.size());
+ List<@Nullable Object> values = new ArrayList<>(val.size());
val.forEach(v -> values.add(convertValue(v, "")));
doc.setField(entry.getKey(), values);
} else {
diff --git
a/src/main/java/org/apache/solr/mcp/server/indexing/IndexingService.java
b/src/main/java/org/apache/solr/mcp/server/indexing/IndexingService.java
index 5fd8c483..699f643a 100644
--- a/src/main/java/org/apache/solr/mcp/server/indexing/IndexingService.java
+++ b/src/main/java/org/apache/solr/mcp/server/indexing/IndexingService.java
@@ -37,6 +37,7 @@ import org.apache.solr.common.util.ContentStreamBase;
import
org.apache.solr.mcp.server.indexing.documentcreator.IndexingDocumentCreator;
import org.apache.solr.mcp.server.util.PromptNames;
import org.apache.solr.mcp.server.util.PromptText;
+import org.jspecify.annotations.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.ai.mcp.annotation.McpArg;
@@ -355,9 +356,6 @@ public class IndexingService {
"Markdown documents to index, one string per document, each
optionally starting with YAML front matter")
List<String> documents)
throws IOException, SolrServerException {
- if (documents == null) {
- throw new IllegalArgumentException("documents cannot be null");
- }
List<SolrInputDocument> schemalessDoc = new ArrayList<>();
for (String markdown : documents) {
schemalessDoc.addAll(indexingDocumentCreator.createSchemalessDocumentsFromMarkdown(markdown));
@@ -461,7 +459,7 @@ public class IndexingService {
private record IndexTool(String format, String name, String paramName,
String payload) {}
private static IndexTool resolveIndexTool(String format) {
- String normalized = (format == null) ? "" : format.trim().toLowerCase();
+ String normalized = format.trim().toLowerCase();
return switch (normalized) {
case "json" ->
new IndexTool(
@@ -512,7 +510,7 @@ public class IndexingService {
description =
"Optional small sample of the input document(s) to ground
field-shape decisions",
required = false)
- String sample) {
+ @Nullable String sample) {
IndexTool indexTool = resolveIndexTool(format);
String sampleSection =
PromptText.optionalCodeBlock(
diff --git
a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/IndexingDocumentCreator.java
b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/IndexingDocumentCreator.java
index b0652e0c..5f66b075 100644
---
a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/IndexingDocumentCreator.java
+++
b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/IndexingDocumentCreator.java
@@ -105,8 +105,8 @@ public class IndexingDocumentCreator {
throws DocumentProcessingException {
// Input validation
- if (markdown == null || markdown.trim().isEmpty()) {
- throw new DocumentProcessingException("Markdown input cannot be null or
empty");
+ if (markdown.trim().isEmpty()) {
+ throw new DocumentProcessingException("Markdown input cannot be empty");
}
return markdownDocumentCreator.create(markdown);
diff --git
a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/JsonDocumentCreator.java
b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/JsonDocumentCreator.java
index 5754ff38..62080b8c 100644
---
a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/JsonDocumentCreator.java
+++
b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/JsonDocumentCreator.java
@@ -120,7 +120,7 @@ public class JsonDocumentCreator implements
SolrDocumentCreator {
*/
public List<SolrInputDocument> create(List<Map<String, Object>> documents)
throws DocumentProcessingException {
- if (documents == null || documents.isEmpty()) {
+ if (documents.isEmpty()) {
throw new DocumentProcessingException("JSON input cannot be empty");
}
return flatten(objectMapper.valueToTree(documents));
diff --git
a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/MarkdownDocumentCreator.java
b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/MarkdownDocumentCreator.java
index 3e84db54..bff112b7 100644
---
a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/MarkdownDocumentCreator.java
+++
b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/MarkdownDocumentCreator.java
@@ -309,7 +309,7 @@ public class MarkdownDocumentCreator implements
SolrDocumentCreator {
block.unlink();
}
- private static void addValue(SolrInputDocument doc, String fieldName, Object
value) {
+ private static void addValue(SolrInputDocument doc, String fieldName,
@Nullable Object value) {
switch (value) {
case null -> {}
case Map<?, ?> nested ->
diff --git a/src/main/java/org/apache/solr/mcp/server/schema/SchemaService.java
b/src/main/java/org/apache/solr/mcp/server/schema/SchemaService.java
index 7e738aa7..cda33227 100644
--- a/src/main/java/org/apache/solr/mcp/server/schema/SchemaService.java
+++ b/src/main/java/org/apache/solr/mcp/server/schema/SchemaService.java
@@ -32,6 +32,7 @@ import
org.apache.solr.client.solrj.request.schema.FieldTypeDefinition;
import org.apache.solr.client.solrj.request.schema.SchemaRequest;
import org.apache.solr.client.solrj.response.schema.SchemaRepresentation;
import org.apache.solr.mcp.server.util.PromptNames;
+import org.jspecify.annotations.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.ai.mcp.annotation.McpArg;
@@ -429,13 +430,13 @@ public class SchemaService {
}
private static void requireCollection(String collection) {
- if (collection == null || collection.isBlank()) {
+ if (collection.isBlank()) {
throw new IllegalArgumentException("Collection name must not be blank");
}
}
private static void requireNonEmpty(List<?> list, String name) {
- if (list == null || list.isEmpty()) {
+ if (list.isEmpty()) {
throw new IllegalArgumentException(name + " must not be empty");
}
}
@@ -521,7 +522,7 @@ public class SchemaService {
name = "sampleDocument",
description = "Optional single document in JSON to ground field
inference",
required = false)
- String sampleDocument) {
+ @Nullable String sampleDocument) {
String sampleSection =
optionalCodeBlock(
sampleDocument,
diff --git
a/src/main/java/org/apache/solr/mcp/server/search/SearchResponse.java
b/src/main/java/org/apache/solr/mcp/server/search/SearchResponse.java
index 13f95302..e05277fd 100644
--- a/src/main/java/org/apache/solr/mcp/server/search/SearchResponse.java
+++ b/src/main/java/org/apache/solr/mcp/server/search/SearchResponse.java
@@ -18,6 +18,7 @@ package org.apache.solr.mcp.server.search;
import java.util.List;
import java.util.Map;
+import org.jspecify.annotations.Nullable;
/**
* Immutable record representing a structured search response from Apache Solr
operations.
@@ -109,6 +110,6 @@ import java.util.Map;
public record SearchResponse(
long numFound,
long start,
- Float maxScore,
+ @Nullable Float maxScore,
List<Map<String, Object>> documents,
Map<String, Map<String, Long>> facets) {}
diff --git a/src/main/java/org/apache/solr/mcp/server/search/SearchService.java
b/src/main/java/org/apache/solr/mcp/server/search/SearchService.java
index 849ede71..9fdf61ff 100644
--- a/src/main/java/org/apache/solr/mcp/server/search/SearchService.java
+++ b/src/main/java/org/apache/solr/mcp/server/search/SearchService.java
@@ -295,7 +295,7 @@ public class SearchService {
// sorting
if (!CollectionUtils.isEmpty(sortClauses)) {
sortClauses.stream()
- .filter(clause -> clause != null &&
StringUtils.hasText(clause.field()))
+ .filter(clause -> StringUtils.hasText(clause.field()))
.map(SortClause::toSolrSortClause)
.forEach(solrQuery::addSort);
}
diff --git a/src/main/java/org/apache/solr/mcp/server/search/SortClause.java
b/src/main/java/org/apache/solr/mcp/server/search/SortClause.java
index 3cb70ef8..c5f889e4 100644
--- a/src/main/java/org/apache/solr/mcp/server/search/SortClause.java
+++ b/src/main/java/org/apache/solr/mcp/server/search/SortClause.java
@@ -18,6 +18,7 @@ package org.apache.solr.mcp.server.search;
import java.util.Locale;
import org.apache.solr.client.solrj.request.SolrQuery;
+import org.jspecify.annotations.Nullable;
import org.springframework.util.StringUtils;
/**
@@ -31,7 +32,7 @@ import org.springframework.util.StringUtils;
* @param order the sort direction, {@code asc} or {@code desc} (case
insensitive); defaults to
* {@code asc} when omitted
*/
-public record SortClause(String field, String order) {
+public record SortClause(String field, @Nullable String order) {
/** Default sort order applied when {@code order} is omitted. */
private static final String DEFAULT_ORDER = "asc";
diff --git
a/src/main/java/org/apache/solr/mcp/server/security/HttpSecurityConfiguration.java
b/src/main/java/org/apache/solr/mcp/server/security/HttpSecurityConfiguration.java
index 890da624..ca43fa78 100644
---
a/src/main/java/org/apache/solr/mcp/server/security/HttpSecurityConfiguration.java
+++
b/src/main/java/org/apache/solr/mcp/server/security/HttpSecurityConfiguration.java
@@ -48,7 +48,7 @@ class HttpSecurityConfiguration {
name = "http.security.enabled",
havingValue = "true",
matchIfMissing = true)
- SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
+ SecurityFilterChain securityFilterChain(HttpSecurity http) {
http.authorizeHttpRequests(
auth -> {
// Liveness/readiness probes need anonymous access for load
@@ -104,7 +104,7 @@ class HttpSecurityConfiguration {
@Bean
@ConditionalOnProperty(name = "http.security.enabled", havingValue = "false")
- SecurityFilterChain unsecured(HttpSecurity http) throws Exception {
+ SecurityFilterChain unsecured(HttpSecurity http) {
return http.authorizeHttpRequests(auth -> auth.anyRequest().permitAll())
// MCP inspector
.cors(cors -> cors.configurationSource(corsConfigurationSource()))
diff --git a/src/main/java/org/apache/solr/mcp/server/util/PromptText.java
b/src/main/java/org/apache/solr/mcp/server/util/PromptText.java
index c53f1e9c..350b6d93 100644
--- a/src/main/java/org/apache/solr/mcp/server/util/PromptText.java
+++ b/src/main/java/org/apache/solr/mcp/server/util/PromptText.java
@@ -16,6 +16,8 @@
*/
package org.apache.solr.mcp.server.util;
+import org.jspecify.annotations.Nullable;
+
/** Shared text-shaping helpers for {@code @McpPrompt} method bodies. */
public final class PromptText {
@@ -33,7 +35,8 @@ public final class PromptText {
* @param absentLine the full bullet line for the "no content" case
* @return a string ready to be interpolated into a Java text block
*/
- public static String optionalCodeBlock(String content, String presentLead,
String absentLine) {
+ public static String optionalCodeBlock(
+ @Nullable String content, String presentLead, String absentLine) {
if (content == null || content.isBlank()) {
return " - " + absentLine;
}
diff --git a/src/test/java/org/apache/solr/mcp/server/BuildInfoReaderTest.java
b/src/test/java/org/apache/solr/mcp/server/BuildInfoReaderTest.java
index dd57e2f1..e1049d2a 100644
--- a/src/test/java/org/apache/solr/mcp/server/BuildInfoReaderTest.java
+++ b/src/test/java/org/apache/solr/mcp/server/BuildInfoReaderTest.java
@@ -18,6 +18,7 @@ package org.apache.solr.mcp.server;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import org.jspecify.annotations.Nullable;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -34,7 +35,7 @@ class BuildInfoReaderTest {
private static final String SUFFIX_PROPERTY =
"solr.mcp.docker.image.tag.suffix";
- private String previousSuffix;
+ private @Nullable String previousSuffix;
@BeforeEach
void captureSuffixProperty() {
diff --git
a/src/test/java/org/apache/solr/mcp/server/LoggingConfigurationTest.java
b/src/test/java/org/apache/solr/mcp/server/LoggingConfigurationTest.java
index 149d0810..97fb97bd 100644
--- a/src/test/java/org/apache/solr/mcp/server/LoggingConfigurationTest.java
+++ b/src/test/java/org/apache/solr/mcp/server/LoggingConfigurationTest.java
@@ -26,6 +26,7 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.UncheckedIOException;
import java.util.Properties;
+import org.jspecify.annotations.Nullable;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -63,7 +64,7 @@ class LoggingConfigurationTest {
private static final String SPRING_VARIANT = "logback-spring.xml";
- private String previousStatusListenerClass;
+ private @Nullable String previousStatusListenerClass;
@BeforeEach
void rememberStatusListenerProperty() {
diff --git
a/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
b/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
index 912a4bca..bacd8506 100644
--- a/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
+++ b/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
@@ -38,7 +38,7 @@ import java.util.Map;
import java.util.Objects;
import java.util.function.Function;
import java.util.stream.Collectors;
-import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AutoClose;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
@@ -71,6 +71,8 @@ public abstract class McpClientIntegrationTestBase {
protected static final ObjectMapper OBJECT_MAPPER =
JsonMapper.builder().build();
+ @AutoClose
+ @SuppressWarnings("NotNullFieldNotInitialized") // set by beforeAll()
protected McpSyncClient mcpClient;
protected abstract McpSyncClient createClient() throws Exception;
@@ -81,13 +83,6 @@ public abstract class McpClientIntegrationTestBase {
mcpClient.initialize();
}
- @AfterAll
- void afterAll() {
- if (mcpClient != null) {
- mcpClient.close();
- }
- }
-
@Test
@Order(1)
void pingServer() {
@@ -234,7 +229,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(7)
- void searchAllDocuments() throws Exception {
+ void searchAllDocuments() {
CallToolResult result =
mcpClient.callTool(
new CallToolRequest(
@@ -250,7 +245,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(8)
- void searchWithFilterQuery() throws Exception {
+ void searchWithFilterQuery() {
CallToolResult result =
mcpClient.callTool(
new CallToolRequest(
@@ -273,7 +268,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(9)
- void searchWithKeyword() throws Exception {
+ void searchWithKeyword() {
CallToolResult result =
mcpClient.callTool(
new CallToolRequest("search", Map.of("collection", COLLECTION,
"query", "title:Solr")));
@@ -289,7 +284,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(10)
- void searchWithPagination() throws Exception {
+ void searchWithPagination() {
CallToolResult page1 =
mcpClient.callTool(
new CallToolRequest(
@@ -342,7 +337,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(13)
- void searchWithFacets() throws Exception {
+ void searchWithFacets() {
CallToolResult result =
mcpClient.callTool(
new CallToolRequest(
@@ -389,7 +384,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(15)
- void searchFindsAllDocumentsAfterCsvIndexing() throws Exception {
+ void searchFindsAllDocumentsAfterCsvIndexing() {
CallToolResult result =
mcpClient.callTool(
new CallToolRequest(
@@ -402,7 +397,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(16)
- void addFieldsToTestCollection() throws Exception {
+ void addFieldsToTestCollection() {
List<Map<String, Object>> fields =
List.of(
Map.of(
@@ -476,7 +471,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(18)
- void searchWithNewFieldFilters() throws Exception {
+ void searchWithNewFieldFilters() {
CallToolResult byPlatform =
mcpClient.callTool(
new CallToolRequest(
@@ -512,7 +507,7 @@ public abstract class McpClientIntegrationTestBase {
// (the markdown document carries none of the fields its filter queries
touch).
@Test
@Order(18)
- void indexMarkdownDocumentAndFindItById() throws Exception {
+ void indexMarkdownDocumentAndFindItById() {
String markdown =
"""
---
@@ -756,7 +751,7 @@ public abstract class McpClientIntegrationTestBase {
"Result should mention at least one shows-specific field that was
added: " + text);
}
- private java.util.Set<String> fetchExistingFieldNames(String collection)
throws Exception {
+ private java.util.Set<String> fetchExistingFieldNames(String collection) {
CallToolResult schemaResult =
mcpClient.callTool(new CallToolRequest("get-schema",
Map.of("collection", collection)));
assertNotError(schemaResult);
@@ -796,7 +791,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(22)
- void searchAllShowsAndAssertCount() throws Exception {
+ void searchAllShowsAndAssertCount() {
CallToolResult result =
mcpClient.callTool(
new CallToolRequest(
@@ -814,7 +809,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(23)
- void searchShowsWithFacetByPlatform() throws Exception {
+ void searchShowsWithFacetByPlatform() {
CallToolResult result =
mcpClient.callTool(
new CallToolRequest(
@@ -863,7 +858,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(24)
- void searchShowsWithFilterAndKeyword() throws Exception {
+ void searchShowsWithFilterAndKeyword() {
// Filter to one platform, then full-text search the description field.
CallToolResult result =
mcpClient.callTool(
@@ -892,7 +887,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(25)
- void searchShowsByMultiValuedGenre() throws Exception {
+ void searchShowsByMultiValuedGenre() {
// Multi-valued strings field: filter on a single genre value.
CallToolResult result =
mcpClient.callTool(
@@ -945,7 +940,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(27)
- void getShowsCollectionStats() throws Exception {
+ void getShowsCollectionStats() {
CallToolResult result =
mcpClient.callTool(
new CallToolRequest("get-collection-stats", Map.of("collection",
SHOWS_COLLECTION)));
@@ -1189,7 +1184,7 @@ public abstract class McpClientIntegrationTestBase {
@Test
@Order(39)
- void blankOptionalSearchEntriesAreIgnoredThroughMcp() throws Exception {
+ void blankOptionalSearchEntriesAreIgnoredThroughMcp() {
var result =
mcpClient.callTool(
new CallToolRequest(
diff --git
a/src/test/java/org/apache/solr/mcp/server/McpToolRegistrationTest.java
b/src/test/java/org/apache/solr/mcp/server/McpToolRegistrationTest.java
index be3da514..1de409fd 100644
--- a/src/test/java/org/apache/solr/mcp/server/McpToolRegistrationTest.java
+++ b/src/test/java/org/apache/solr/mcp/server/McpToolRegistrationTest.java
@@ -213,14 +213,12 @@ class McpToolRegistrationTest {
// Verify first parameter (collection) is required
McpToolParam firstParam = parameters[0].getAnnotation(McpToolParam.class);
- assertTrue(
- firstParam.required() || !firstParam.required(),
- "First parameter annotation should specify required status");
+ assertTrue(firstParam.required(), "The collection parameter should be
required");
// Verify optional parameters have required=false
for (int i = 1; i < parameters.length; i++) {
McpToolParam param = parameters[i].getAnnotation(McpToolParam.class);
- // Optional parameters should be marked as such in description or
required flag
+ assertFalse(param.required(), parameters[i].getName() + " should be
optional");
assertNotNull(
param.description(), "Parameter should have description indicating
if it's optional");
}
diff --git
a/src/test/java/org/apache/solr/mcp/server/SampleDataRuntimeHints.java
b/src/test/java/org/apache/solr/mcp/server/SampleDataRuntimeHints.java
index faf0e86e..25baf758 100644
--- a/src/test/java/org/apache/solr/mcp/server/SampleDataRuntimeHints.java
+++ b/src/test/java/org/apache/solr/mcp/server/SampleDataRuntimeHints.java
@@ -38,7 +38,7 @@ import
org.springframework.test.context.aot.TestRuntimeHintsRegistrar;
* It lives in test sources on purpose: {@code SolrNativeHints} is the
production equivalent and
* must not ship test fixture names in the application image.
*
- * @see org.apache.solr.mcp.server.indexing.ShowsSampleDataIntegrationTest
+ * <p>Used by {@code indexing.ShowsSampleDataIntegrationTest}.
*/
public class SampleDataRuntimeHints implements TestRuntimeHintsRegistrar {
diff --git
a/src/test/java/org/apache/solr/mcp/server/collection/AliasServiceTest.java
b/src/test/java/org/apache/solr/mcp/server/collection/AliasServiceTest.java
index 94cdc741..a86e0b19 100644
--- a/src/test/java/org/apache/solr/mcp/server/collection/AliasServiceTest.java
+++ b/src/test/java/org/apache/solr/mcp/server/collection/AliasServiceTest.java
@@ -113,14 +113,6 @@ class AliasServiceTest {
.hasMessage("Alias name must not be blank");
}
- @Test
- @DisplayName("throws IllegalArgumentException when alias name is null")
- void throwsWhenAliasNameNull() {
- assertThatThrownBy(() -> aliasService.createAlias(null, "ORDERS_V2"))
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessage("Alias name must not be blank");
- }
-
@Test
@DisplayName("throws IllegalArgumentException when collections is blank")
void throwsWhenCollectionsBlank() {
@@ -128,14 +120,6 @@ class AliasServiceTest {
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Collections must not be blank");
}
-
- @Test
- @DisplayName("throws IllegalArgumentException when collections is null")
- void throwsWhenCollectionsNull() {
- assertThatThrownBy(() -> aliasService.createAlias("ORDERS", null))
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessage("Collections must not be blank");
- }
}
@Nested
@@ -168,13 +152,5 @@ class AliasServiceTest {
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Alias name must not be blank");
}
-
- @Test
- @DisplayName("throws IllegalArgumentException when alias name is null")
- void throwsWhenAliasNameNull() {
- assertThatThrownBy(() -> aliasService.deleteAlias(null))
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessage("Alias name must not be blank");
- }
}
}
diff --git
a/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceIntegrationTest.java
b/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceIntegrationTest.java
index 3cb17f4a..8573b12a 100644
---
a/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceIntegrationTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceIntegrationTest.java
@@ -16,6 +16,7 @@
*/
package org.apache.solr.mcp.server.collection;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -138,9 +139,7 @@ class CollectionServiceIntegrationTest {
assertNotNull(queryStats);
assertNotNull(queryStats.queryTime());
assertEquals(
- (long) DOC_COUNT,
- queryStats.totalResults(),
- "totalResults should match indexed document count");
+ DOC_COUNT, queryStats.totalResults(), "totalResults should match
indexed document count");
assertEquals(0L, queryStats.start());
if (!MBEANS_AVAILABLE) {
@@ -153,29 +152,29 @@ class CollectionServiceIntegrationTest {
assertNotNull(metrics.cacheStats(), "Cache stats should not be null after
queries ran");
CacheStats cacheStats = metrics.cacheStats();
assertNotNull(cacheStats.queryResultCache());
- assertTrue(
- cacheStats.queryResultCache().lookups() > 0,
- "Query result cache should have lookups after queries");
+ assertThat(cacheStats.queryResultCache().lookups())
+ .as("Query result cache should have lookups after queries")
+ .isPositive();
assertNotNull(cacheStats.documentCache());
- assertTrue(
- cacheStats.documentCache().lookups() > 0,
- "Document cache should have lookups after queries");
+ assertThat(cacheStats.documentCache().lookups())
+ .as("Document cache should have lookups after queries")
+ .isPositive();
assertNotNull(cacheStats.filterCache());
- assertTrue(
- cacheStats.filterCache().lookups() > 0,
- "Filter cache should have lookups after filter queries");
+ assertThat(cacheStats.filterCache().lookups())
+ .as("Filter cache should have lookups after filter queries")
+ .isPositive();
// Handler stats should reflect actual request counts
assertNotNull(metrics.handlerStats(), "Handler stats should not be null
after queries ran");
HandlerStats handlerStats = metrics.handlerStats();
assertNotNull(handlerStats.selectHandler());
- assertTrue(
- handlerStats.selectHandler().requests() > 0,
- "Select handler should have processed requests");
+ assertThat(handlerStats.selectHandler().requests())
+ .as("Select handler should have processed requests")
+ .isPositive();
assertNotNull(handlerStats.updateHandler());
- assertTrue(
- handlerStats.updateHandler().requests() > 0,
- "Update handler should have processed requests from indexing");
+ assertThat(handlerStats.updateHandler().requests())
+ .as("Update handler should have processed requests from indexing")
+ .isPositive();
}
@Test
@@ -206,9 +205,7 @@ class CollectionServiceIntegrationTest {
assertTrue(status.responseTime() >= 0);
assertEquals(
- (long) DOC_COUNT,
- status.totalDocuments(),
- "Health check should report indexed document count");
+ DOC_COUNT, status.totalDocuments(), "Health check should report
indexed document count");
assertNotNull(status.lastChecked());
assertTrue(
@@ -235,12 +232,11 @@ class CollectionServiceIntegrationTest {
assertEquals(TEST_COLLECTION,
collectionService.extractCollectionName(TEST_COLLECTION));
assertEquals("films",
collectionService.extractCollectionName("films_shard1_replica_n1"));
assertEquals("products",
collectionService.extractCollectionName("products_shard2_replica_n3"));
- assertNull(collectionService.extractCollectionName(null));
assertEquals("", collectionService.extractCollectionName(""));
}
@Test
- void testGetCacheMetrics_afterQueries() throws Exception {
+ void testGetCacheMetrics_afterQueries() {
CacheStats cacheStats = collectionService.getCacheMetrics(TEST_COLLECTION);
if (!MBEANS_AVAILABLE) {
@@ -253,23 +249,29 @@ class CollectionServiceIntegrationTest {
// Query result cache: warm-up queries should have generated lookups
CacheInfo qrc = cacheStats.queryResultCache();
assertNotNull(qrc);
- assertTrue(qrc.lookups() > 0, "queryResultCache lookups should be positive
after queries");
+ assertThat(qrc.lookups())
+ .as("queryResultCache lookups should be positive after queries")
+ .isPositive();
assertNotNull(qrc.hitratio());
assertNotNull(qrc.size());
// Document cache: reading documents populates this cache
CacheInfo dc = cacheStats.documentCache();
assertNotNull(dc);
- assertTrue(dc.lookups() > 0, "documentCache lookups should be positive
after queries");
+ assertThat(dc.lookups())
+ .as("documentCache lookups should be positive after queries")
+ .isPositive();
// Filter cache: the filter queries we ran should generate lookups
CacheInfo fc = cacheStats.filterCache();
assertNotNull(fc);
- assertTrue(fc.lookups() > 0, "filterCache lookups should be positive after
filter queries");
+ assertThat(fc.lookups())
+ .as("filterCache lookups should be positive after filter queries")
+ .isPositive();
}
@Test
- void testGetHandlerMetrics_afterQueriesAndIndexing() throws Exception {
+ void testGetHandlerMetrics_afterQueriesAndIndexing() {
HandlerStats handlerStats =
collectionService.getHandlerMetrics(TEST_COLLECTION);
if (!MBEANS_AVAILABLE) {
@@ -282,7 +284,9 @@ class CollectionServiceIntegrationTest {
// Select handler: warm-up queries should have driven request counts > 0
HandlerInfo select = handlerStats.selectHandler();
assertNotNull(select);
- assertTrue(select.requests() > 0, "Select handler requests should be
positive after queries");
+ assertThat(select.requests())
+ .as("Select handler requests should be positive after queries")
+ .isPositive();
// Solr may omit these counters entirely or report an explicit 0 - both
mean
// "nothing went wrong". Requiring null made the test depend on which.
assertTrue(
@@ -294,16 +298,18 @@ class CollectionServiceIntegrationTest {
// Update handler: indexing 50 docs should have driven request counts > 0
HandlerInfo update = handlerStats.updateHandler();
assertNotNull(update);
- assertTrue(update.requests() > 0, "Update handler requests should be
positive after indexing");
+ assertThat(update.requests())
+ .as("Update handler requests should be positive after indexing")
+ .isPositive();
}
@Test
- void testGetCacheMetrics_nonExistentCollection() throws Exception {
+ void testGetCacheMetrics_nonExistentCollection() {
assertNull(collectionService.getCacheMetrics("non_existent_collection"));
}
@Test
- void testGetHandlerMetrics_nonExistentCollection() throws Exception {
+ void testGetHandlerMetrics_nonExistentCollection() {
assertNull(collectionService.getHandlerMetrics("non_existent_collection"));
}
diff --git
a/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceTest.java
b/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceTest.java
index 1410c304..cbb22a1e 100644
---
a/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceTest.java
@@ -39,6 +39,7 @@ import org.apache.solr.client.solrj.response.SolrPingResponse;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.util.NamedList;
+import org.jspecify.annotations.Nullable;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledInNativeImage;
@@ -118,15 +119,6 @@ class CollectionServiceTest {
assertEquals("simple_collection", result);
}
- @Test
- void extractCollectionName_WithNullInput_ShouldReturnNull() {
- // When
- String result = collectionService.extractCollectionName(null);
-
- // Then
- assertNull(result);
- }
-
@Test
void extractCollectionName_WithEmptyString_ShouldReturnEmptyString() {
// When
@@ -297,7 +289,6 @@ class CollectionServiceTest {
assertNotNull(result);
assertTrue(result.isHealthy());
assertEquals(5000, result.responseTime());
- assertTrue(result.responseTime() > 1000, "Should capture slow response
time");
}
@Test
@@ -307,6 +298,7 @@ class CollectionServiceTest {
SolrHealthStatus result =
collectionService.checkHealth("error_collection");
assertFalse(result.isHealthy());
+ assertNotNull(result.errorMessage());
assertTrue(result.errorMessage().contains("Network error"));
assertNull(result.responseTime());
}
@@ -706,7 +698,6 @@ class CollectionServiceTest {
HandlerStats emptyStats = new HandlerStats(null, null);
assertTrue((boolean) method.invoke(collectionService, emptyStats));
- assertTrue((boolean) method.invoke(collectionService, (HandlerStats)
null));
HandlerStats nonEmptyStats =
new HandlerStats(new HandlerInfo(100L, null, null, null, null, null),
null);
@@ -731,7 +722,7 @@ class CollectionServiceTest {
@Test
void listCollections_NullCollections() throws Exception {
- NamedList<Object> response = new NamedList<>();
+ NamedList<@Nullable Object> response = new NamedList<>();
response.add("collections", null);
when(solrClient.request(any(), any())).thenReturn(response);
@@ -835,10 +826,6 @@ class CollectionServiceTest {
return wrapInMetricsResponse(createSelectHandlerCoreMetrics(), collection);
}
- private NamedList<Object> createMockUpdateHandlerData() {
- return wrapInMetricsResponse(createUpdateHandlerCoreMetrics());
- }
-
private NamedList<Object> wrapInMetricsResponse(NamedList<Object>
coreMetrics) {
return wrapInMetricsResponse(coreMetrics, "test_collection");
}
@@ -975,11 +962,11 @@ class CollectionServiceTest {
}
@Test
+ @SuppressWarnings("DataFlowIssue") // passes null on purpose to pin the SDK
contract
void completeCollection_NullValueRejectedAtSdkBoundary() {
// Since MCP Java SDK 2.0, CompleteArgument's constructor rejects a null
- // value (1.x and earlier accept it), so the
- // CollectionService#completeCollection null-value branch is unreachable
- // from a real MCP client. Document the SDK contract here.
+ // value (1.x and earlier accept it), so
CollectionService#completeCollection
+ // does not null-check it. Pin the SDK contract here.
assertThrows(
IllegalArgumentException.class,
() -> new CompleteRequest.CompleteArgument("collection", null));
@@ -1019,13 +1006,6 @@ class CollectionServiceTest {
verify(spyService, never()).listCollections();
}
- @Test
- void completeCollection_WithNullArgument_ReturnsEmpty() {
- List<String> result = collectionService.completeCollection(null);
-
- assertTrue(result.isEmpty());
- }
-
@Test
void completeCollection_CapsResultsAtMax() throws Exception {
CollectionService spyService = spy(collectionService);
diff --git
a/src/test/java/org/apache/solr/mcp/server/collection/CollectionUtilsTest.java
b/src/test/java/org/apache/solr/mcp/server/collection/CollectionUtilsTest.java
index 5a3c7dd1..a98f5a45 100644
---
a/src/test/java/org/apache/solr/mcp/server/collection/CollectionUtilsTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/collection/CollectionUtilsTest.java
@@ -16,12 +16,15 @@
*/
package org.apache.solr.mcp.server.collection;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.within;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import java.math.BigDecimal;
import java.math.BigInteger;
import org.apache.solr.common.util.NamedList;
+import org.jspecify.annotations.Nullable;
import org.junit.jupiter.api.Test;
/**
@@ -32,7 +35,7 @@ class CollectionUtilsTest {
@Test
void testGetLong_withNullValue() {
- NamedList<Object> namedList = new NamedList<>();
+ NamedList<@Nullable Object> namedList = new NamedList<>();
namedList.add("nullKey", null);
assertNull(CollectionUtils.getLong(namedList, "nullKey"));
@@ -119,7 +122,7 @@ class CollectionUtilsTest {
@Test
void testGetFloat_withNullValue() {
- NamedList<Object> namedList = new NamedList<>();
+ NamedList<@Nullable Object> namedList = new NamedList<>();
namedList.add("nullKey", null);
assertNull(CollectionUtils.getFloat(namedList, "nullKey"));
@@ -145,7 +148,7 @@ class CollectionUtilsTest {
NamedList<Object> namedList = new NamedList<>();
namedList.add("floatKey", 123.45f);
- assertEquals(123.45f, CollectionUtils.getFloat(namedList, "floatKey"),
0.001f);
+ assertThat(CollectionUtils.getFloat(namedList,
"floatKey")).isCloseTo(123.45f, within(0.001f));
}
@Test
@@ -153,7 +156,8 @@ class CollectionUtilsTest {
NamedList<Object> namedList = new NamedList<>();
namedList.add("doubleKey", 123.456789);
- assertEquals(123.456789f, CollectionUtils.getFloat(namedList,
"doubleKey"), 0.001f);
+ assertThat(CollectionUtils.getFloat(namedList, "doubleKey"))
+ .isCloseTo(123.456789f, within(0.001f));
}
@Test
@@ -169,7 +173,7 @@ class CollectionUtilsTest {
NamedList<Object> namedList = new NamedList<>();
namedList.add("bigDecKey", new BigDecimal("123.45"));
- assertEquals(123.45f, CollectionUtils.getFloat(namedList, "bigDecKey"),
0.001f);
+ assertThat(CollectionUtils.getFloat(namedList,
"bigDecKey")).isCloseTo(123.45f, within(0.001f));
}
@Test
@@ -177,7 +181,7 @@ class CollectionUtilsTest {
NamedList<Object> namedList = new NamedList<>();
namedList.add("stringKey", "123.45");
- assertEquals(123.45f, CollectionUtils.getFloat(namedList, "stringKey"),
0.001f);
+ assertThat(CollectionUtils.getFloat(namedList,
"stringKey")).isCloseTo(123.45f, within(0.001f));
}
@Test
@@ -198,7 +202,7 @@ class CollectionUtilsTest {
@Test
void testGetInteger_withNullValue() {
- NamedList<Object> namedList = new NamedList<>();
+ NamedList<@Nullable Object> namedList = new NamedList<>();
namedList.add("nullKey", null);
assertNull(CollectionUtils.getInteger(namedList, "nullKey"));
@@ -298,7 +302,8 @@ class CollectionUtilsTest {
NamedList<Object> namedList = new NamedList<>();
namedList.add("negativeKey", -123.45f);
- assertEquals(-123.45f, CollectionUtils.getFloat(namedList, "negativeKey"),
0.001f);
+ assertThat(CollectionUtils.getFloat(namedList, "negativeKey"))
+ .isCloseTo(-123.45f, within(0.001f));
}
@Test
diff --git
a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigurationPropertiesTest.java
b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigurationPropertiesTest.java
index 9741442c..6565c040 100644
---
a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigurationPropertiesTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigurationPropertiesTest.java
@@ -74,6 +74,7 @@ class SolrConfigurationPropertiesTest {
/** The binder passes null when the property is absent altogether. */
@Test
+ @SuppressWarnings("DataFlowIssue") // passes null on purpose, as the binder
does
void rejectsAMissingUrl() {
assertThatThrownBy(() -> new SolrConfigurationProperties(null, null, null))
.isInstanceOf(IllegalArgumentException.class)
diff --git
a/src/test/java/org/apache/solr/mcp/server/containerization/DockerImageHttpIntegrationTest.java
b/src/test/java/org/apache/solr/mcp/server/containerization/DockerImageHttpIntegrationTest.java
index 43850bae..883fb849 100644
---
a/src/test/java/org/apache/solr/mcp/server/containerization/DockerImageHttpIntegrationTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/containerization/DockerImageHttpIntegrationTest.java
@@ -131,22 +131,19 @@ class DockerImageHttpIntegrationTest {
.forPort(HTTP_PORT)
.withStartupTimeout(Duration.ofSeconds(60)));
- private static HttpClient httpClient;
- private static String baseUrl;
+ private static final HttpClient httpClient =
+ HttpClient.newBuilder().connectTimeout(Duration.ofSeconds(10)).build();
@BeforeAll
static void setup() {
log.info("Solr container started. Internal URL: http://solr:8983/solr/";);
log.info("MCP Server container started in HTTP mode");
+ log.info("MCP Server HTTP endpoint available at: {}", baseUrl());
+ }
- // Initialize HTTP client
- httpClient =
HttpClient.newBuilder().connectTimeout(Duration.ofSeconds(10)).build();
-
- // Get the mapped port for accessing the container from the host
- Integer mappedPort = mcpServerContainer.getMappedPort(HTTP_PORT);
- baseUrl = "http://localhost:"; + mappedPort;
-
- log.info("MCP Server HTTP endpoint available at: {}", baseUrl);
+ /** The MCP server's HTTP endpoint, through the port the container is mapped
to on the host. */
+ private static String baseUrl() {
+ return "http://localhost:"; + mcpServerContainer.getMappedPort(HTTP_PORT);
}
@Test
@@ -202,7 +199,7 @@ class DockerImageHttpIntegrationTest {
// Test that the HTTP endpoint is accessible
HttpRequest request =
HttpRequest.newBuilder()
- .uri(URI.create(baseUrl + "/actuator/health"))
+ .uri(URI.create(baseUrl() + "/actuator/health"))
.timeout(Duration.ofSeconds(10))
.GET()
.build();
diff --git
a/src/test/java/org/apache/solr/mcp/server/indexing/IndexingServiceIntegrationTest.java
b/src/test/java/org/apache/solr/mcp/server/indexing/IndexingServiceIntegrationTest.java
index 8ba64a1e..c7ea1ce4 100644
---
a/src/test/java/org/apache/solr/mcp/server/indexing/IndexingServiceIntegrationTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/indexing/IndexingServiceIntegrationTest.java
@@ -40,7 +40,6 @@ import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.context.annotation.Import;
-import org.testcontainers.containers.SolrContainer;
import org.testcontainers.junit.jupiter.Testcontainers;
/** Integration tests for IndexingService using a real Solr instance via
Testcontainers. */
@@ -52,7 +51,6 @@ class IndexingServiceIntegrationTest {
private static final String COLLECTION_NAME = "indexing_test_" +
System.currentTimeMillis();
private static boolean initialized = false;
- @Autowired private SolrContainer solrContainer;
@Autowired private IndexingDocumentCreator indexingDocumentCreator;
@Autowired private IndexingService indexingService;
@Autowired private SearchService searchService;
@@ -71,7 +69,7 @@ class IndexingServiceIntegrationTest {
}
@Test
- void testCreateSchemalessDocumentsFromJson() throws Exception {
+ void testCreateSchemalessDocumentsFromJson() {
// Test JSON string
String json =
"""
@@ -124,9 +122,6 @@ class IndexingServiceIntegrationTest {
} else {
assertEquals(true, inStockValue);
}
- } else {
- // If inStock is not present in the document, we'll skip this assertion
- // Removed debug print statement
}
Object authorValue = doc.getFieldValue("author");
@@ -642,7 +637,7 @@ class IndexingServiceIntegrationTest {
}
@Test
- void testNonArrayJsonInput() throws Exception {
+ void testNonArrayJsonInput() {
// Test JSON string that is not an array but a single object
String json =
"""
@@ -667,7 +662,7 @@ class IndexingServiceIntegrationTest {
}
@Test
- void testConvertJsonValueTypes() throws Exception {
+ void testConvertJsonValueTypes() {
// Test JSON with different value types
String json =
"""
@@ -703,7 +698,7 @@ class IndexingServiceIntegrationTest {
}
@Test
- void testDirectFieldNamesAreUsedAsGiven() throws Exception {
+ void testDirectFieldNamesAreUsedAsGiven() {
String json =
"""
[
diff --git
a/src/test/java/org/apache/solr/mcp/server/indexing/IndexingServiceTest.java
b/src/test/java/org/apache/solr/mcp/server/indexing/IndexingServiceTest.java
index 983ead72..0e31b95c 100644
--- a/src/test/java/org/apache/solr/mcp/server/indexing/IndexingServiceTest.java
+++ b/src/test/java/org/apache/solr/mcp/server/indexing/IndexingServiceTest.java
@@ -387,15 +387,6 @@ class IndexingServiceTest {
assertEquals(2, captor.getValue().getDocuments().size());
}
- @Test
- void
indexMarkdownDocuments_WithNullDocuments_ThrowsIllegalArgumentException() {
- IllegalArgumentException ex =
- assertThrows(
- IllegalArgumentException.class,
- () -> indexingService.indexMarkdownDocuments("test_collection",
null));
- assertEquals("documents cannot be null", ex.getMessage());
- }
-
@Test
void indexDataPrompt_jsonPath_referencesIndexJsonDocuments() {
String sample =
diff --git
a/src/test/java/org/apache/solr/mcp/server/indexing/MarkdownIndexingTest.java
b/src/test/java/org/apache/solr/mcp/server/indexing/MarkdownIndexingTest.java
index 12035dd9..eac84750 100644
---
a/src/test/java/org/apache/solr/mcp/server/indexing/MarkdownIndexingTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/indexing/MarkdownIndexingTest.java
@@ -42,7 +42,7 @@ class MarkdownIndexingTest {
@Autowired private IndexingDocumentCreator indexingDocumentCreator;
@Test
- void testCreateSchemalessDocumentsFromMarkdownWithFrontMatter() throws
Exception {
+ void testCreateSchemalessDocumentsFromMarkdownWithFrontMatter() {
// Given
String markdown =
"""
@@ -89,7 +89,7 @@ class MarkdownIndexingTest {
}
@Test
- void testFrontMatterTitleWinsOverFirstHeading() throws Exception {
+ void testFrontMatterTitleWinsOverFirstHeading() {
// Given
String markdown =
"""
@@ -111,7 +111,7 @@ class MarkdownIndexingTest {
}
@Test
- void testCreateSchemalessDocumentsFromMarkdownWithoutFrontMatter() throws
Exception {
+ void testCreateSchemalessDocumentsFromMarkdownWithoutFrontMatter() {
// Given
String markdown =
"""
@@ -141,7 +141,7 @@ class MarkdownIndexingTest {
}
@Test
- void testPlainTextMarkdownWithoutHeadings() throws Exception {
+ void testPlainTextMarkdownWithoutHeadings() {
// Given
String markdown = "Just a plain paragraph of text without any structure.";
@@ -160,7 +160,7 @@ class MarkdownIndexingTest {
}
@Test
- void testFrontMatterFieldNamesAreUsedAsGiven() throws Exception {
+ void testFrontMatterFieldNamesAreUsedAsGiven() {
// Given
String markdown =
"""
@@ -186,7 +186,7 @@ class MarkdownIndexingTest {
}
@Test
- void testFrontMatterListValuesBecomeMultiValuedFields() throws Exception {
+ void testFrontMatterListValuesBecomeMultiValuedFields() {
// Given
String markdown =
"""
@@ -209,7 +209,7 @@ class MarkdownIndexingTest {
}
@Test
- void testFrontMatterBlockListValuesBecomeMultiValuedFields() throws
Exception {
+ void testFrontMatterBlockListValuesBecomeMultiValuedFields() {
// Given
String markdown =
"""
@@ -233,7 +233,7 @@ class MarkdownIndexingTest {
}
@Test
- void testFrontMatterIdIsUsedAsDocumentId() throws Exception {
+ void testFrontMatterIdIsUsedAsDocumentId() {
// Given
String markdown =
"""
@@ -255,7 +255,7 @@ class MarkdownIndexingTest {
}
@Test
- void testGeneratedIdIsStableForSameContent() throws Exception {
+ void testGeneratedIdIsStableForSameContent() {
// Given
String markdown = "# Stable\n\nSame content, same id.";
@@ -279,11 +279,11 @@ class MarkdownIndexingTest {
void testEmptyMarkdownThrowsException() {
assertThatThrownBy(() ->
indexingDocumentCreator.createSchemalessDocumentsFromMarkdown(""))
.isInstanceOf(DocumentProcessingException.class)
- .hasMessageContaining("cannot be null or empty");
+ .hasMessageContaining("cannot be empty");
}
@Test
- void testMarkdownFormattingIsStrippedFromContent() throws Exception {
+ void testMarkdownFormattingIsStrippedFromContent() {
// Given
String markdown =
"""
diff --git
a/src/test/java/org/apache/solr/mcp/server/indexing/documentcreator/JsonDocumentCreatorTest.java
b/src/test/java/org/apache/solr/mcp/server/indexing/documentcreator/JsonDocumentCreatorTest.java
index 2937f4c9..bcd3024f 100644
---
a/src/test/java/org/apache/solr/mcp/server/indexing/documentcreator/JsonDocumentCreatorTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/indexing/documentcreator/JsonDocumentCreatorTest.java
@@ -42,7 +42,7 @@ class JsonDocumentCreatorTest {
private final JsonDocumentCreator creator = new
JsonDocumentCreator(objectMapper);
@Test
- void objectsAndStringProduceTheSameDocuments() throws Exception {
+ void objectsAndStringProduceTheSameDocuments() {
List<Map<String, Object>> objects =
List.of(
show("netflix-001", "Stranger Things", 8.7, true, "Sci-Fi",
"Horror"),
@@ -79,18 +79,6 @@ class JsonDocumentCreatorTest {
.hasMessage("JSON input cannot be empty");
}
- /**
- * A missing {@code documents} argument arrives as null. The creator owns
the "nothing to index"
- * policy for both entry points, so it reports null the same way it reports
empty rather than
- * letting the service NPE.
- */
- @Test
- void nullListIsRejected() {
- assertThatThrownBy(() -> creator.create((List<Map<String, Object>>) null))
- .isInstanceOf(DocumentProcessingException.class)
- .hasMessage("JSON input cannot be empty");
- }
-
private static Map<String, Object> show(
String id, String title, double rating, boolean ongoing, String...
genres) {
Map<String, Object> show = new LinkedHashMap<>();
diff --git
a/src/test/java/org/apache/solr/mcp/server/observability/DistributedTracingTest.java
b/src/test/java/org/apache/solr/mcp/server/observability/DistributedTracingTest.java
index 13b3929e..4e887e9d 100644
---
a/src/test/java/org/apache/solr/mcp/server/observability/DistributedTracingTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/observability/DistributedTracingTest.java
@@ -20,8 +20,8 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;
import io.micrometer.tracing.test.simple.SimpleTracer;
+import java.time.Instant;
import java.util.concurrent.TimeUnit;
-import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.mcp.server.TestcontainersConfiguration;
import org.apache.solr.mcp.server.search.SearchService;
import org.junit.jupiter.api.AfterEach;
@@ -72,8 +72,6 @@ class DistributedTracingTest {
@Autowired private SearchService searchService;
- @Autowired private SolrClient solrClient;
-
@Autowired private SimpleTracer tracer;
@BeforeEach
@@ -218,7 +216,9 @@ class DistributedTracingTest {
assertThat(spans)
.as("All spans should have start and end times")
.allMatch(
- span -> span.getStartTimestamp() != null &&
span.getEndTimestamp() != null);
+ span ->
+ span.getStartTimestamp().isAfter(Instant.EPOCH)
+ &&
!span.getEndTimestamp().isBefore(span.getStartTimestamp()));
});
}
}
diff --git
a/src/test/java/org/apache/solr/mcp/server/observability/TraceAssertions.java
b/src/test/java/org/apache/solr/mcp/server/observability/TraceAssertions.java
index ea77b906..fd93118f 100644
---
a/src/test/java/org/apache/solr/mcp/server/observability/TraceAssertions.java
+++
b/src/test/java/org/apache/solr/mcp/server/observability/TraceAssertions.java
@@ -23,6 +23,7 @@ import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.sdk.trace.data.SpanData;
import java.util.List;
import java.util.function.Predicate;
+import org.jspecify.annotations.Nullable;
/**
* Helper utilities for asserting on distributed traces in tests.
@@ -165,7 +166,7 @@ public class TraceAssertions {
* @param spanName the span name to search for
* @return the first matching span, or null if not found
*/
- public static SpanData findSpan(List<SpanData> spans, String spanName) {
+ public static @Nullable SpanData findSpan(List<SpanData> spans, String
spanName) {
return spans.stream()
.filter(span -> span.getName().contains(spanName))
.findFirst()
diff --git
a/src/test/java/org/apache/solr/mcp/server/schema/SchemaServiceIntegrationTest.java
b/src/test/java/org/apache/solr/mcp/server/schema/SchemaServiceIntegrationTest.java
index d0958a50..387a647c 100644
---
a/src/test/java/org/apache/solr/mcp/server/schema/SchemaServiceIntegrationTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/schema/SchemaServiceIntegrationTest.java
@@ -101,17 +101,6 @@ class SchemaServiceIntegrationTest {
"Getting schema for non-existent collection should throw exception");
}
- @Test
- void testGetSchema_NullCollection() {
- // When/Then
- assertThrows(
- Exception.class,
- () -> {
- schemaService.getSchema(null);
- },
- "Getting schema with null collection should throw exception");
- }
-
@Test
void testGetSchema_EmptyCollection() {
// When/Then
@@ -160,8 +149,8 @@ class SchemaServiceIntegrationTest {
// Then - verify dynamic fields are accessible
assertNotNull(schema.getDynamicFields(), "Dynamic fields should not be
null");
- // Most Solr schemas have some dynamic fields by default
- assertTrue(schema.getDynamicFields().size() >= 0, "Dynamic fields should
be a valid list");
+ // The _default configset defines dynamic fields such as *_s and *_i
+ assertFalse(schema.getDynamicFields().isEmpty(), "Dynamic fields should
not be empty");
// Check for common dynamic field patterns
boolean hasStringDynamicField =
@@ -182,9 +171,6 @@ class SchemaServiceIntegrationTest {
// Then - verify copy fields are accessible
assertNotNull(schema.getCopyFields(), "Copy fields should not be null");
-
- // Copy fields list can be empty, that's valid
- assertTrue(schema.getCopyFields().size() >= 0, "Copy fields should be a
valid list");
}
@Test
diff --git
a/src/test/java/org/apache/solr/mcp/server/schema/SchemaServiceTest.java
b/src/test/java/org/apache/solr/mcp/server/schema/SchemaServiceTest.java
index 8156e95f..d36670ef 100644
--- a/src/test/java/org/apache/solr/mcp/server/schema/SchemaServiceTest.java
+++ b/src/test/java/org/apache/solr/mcp/server/schema/SchemaServiceTest.java
@@ -30,8 +30,6 @@ import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.request.schema.SchemaRequest;
import org.apache.solr.client.solrj.request.schema.SchemaRequest.MultiUpdate;
-import org.apache.solr.client.solrj.response.schema.SchemaRepresentation;
-import org.apache.solr.client.solrj.response.schema.SchemaResponse;
import org.apache.solr.common.util.NamedList;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -54,10 +52,6 @@ class SchemaServiceTest {
@Mock private ObjectMapper objectMapper;
- @Mock private SchemaResponse schemaResponse;
-
- @Mock private SchemaRepresentation schemaRepresentation;
-
private SchemaService schemaService;
@BeforeEach
@@ -125,18 +119,6 @@ class SchemaServiceTest {
});
}
- @Test
- void testGetSchema_WithNullCollection() {
- // Given a null collection name
- // Then should throw an exception (NullPointerException or
- // IllegalArgumentException)
- assertThrows(
- Exception.class,
- () -> {
- schemaService.getSchema(null);
- });
- }
-
@Test
void testGetSchema_WithEmptyCollection() {
// Given an empty collection name
@@ -155,20 +137,8 @@ class SchemaServiceTest {
assertNotNull(service);
}
- @Test
- void testConstructor_WithNullClient() {
- // Test constructor with null client
- assertDoesNotThrow(
- () -> {
- new SchemaService(null, objectMapper);
- });
- }
-
@Test
void addFields_blankCollection_throws() {
- assertThrows(
- IllegalArgumentException.class,
- () -> schemaService.addFields(null, List.of(Map.of("name", "x",
"type", "string"))));
assertThrows(
IllegalArgumentException.class,
() -> schemaService.addFields("", List.of(Map.of("name", "x", "type",
"string"))));
@@ -179,7 +149,6 @@ class SchemaServiceTest {
@Test
void addFields_emptyList_throws() {
- assertThrows(IllegalArgumentException.class, () ->
schemaService.addFields("col", null));
assertThrows(IllegalArgumentException.class, () ->
schemaService.addFields("col", List.of()));
}
@@ -224,11 +193,6 @@ class SchemaServiceTest {
@Test
void addFieldTypes_blankCollection_throws() {
- assertThrows(
- IllegalArgumentException.class,
- () ->
- schemaService.addFieldTypes(
- null, List.of(Map.of("name", "x", "class", "solr.StrField"))));
assertThrows(
IllegalArgumentException.class,
() ->
@@ -243,7 +207,6 @@ class SchemaServiceTest {
@Test
void addFieldTypes_emptyList_throws() {
- assertThrows(IllegalArgumentException.class, () ->
schemaService.addFieldTypes("col", null));
assertThrows(
IllegalArgumentException.class, () ->
schemaService.addFieldTypes("col", List.of()));
}
diff --git
a/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
index c462e790..1adb6de3 100644
---
a/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
@@ -194,10 +194,9 @@ class SearchServiceIntegrationTest {
* to one, so a plain list surfaces as {@code ClassCastException: ArrayList
cannot be cast to
* NamedList} instead of an empty result.
*
- * <p>{@link org.apache.solr.mcp.server.config.JsonResponseParserTest} pins
the same behaviour at
- * the parser boundary against a hand-written payload. This test is the
end-to-end counterpart: it
- * proves a real Solr actually emits {@code []} for a zero-hit facet, which
is the premise the
- * unit tests assume.
+ * <p>{@code config.JsonResponseParserTest} pins the same behaviour at the
parser boundary against
+ * a hand-written payload. This test is the end-to-end counterpart: it
proves a real Solr actually
+ * emits {@code []} for a zero-hit facet, which is the premise the unit
tests assume.
*/
@Test
void facetingAQueryThatMatchesNothingReturnsEmptyFacets()
diff --git
a/src/test/java/org/apache/solr/mcp/server/search/SearchServiceTest.java
b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceTest.java
index 23f9ed83..53124cf0 100644
--- a/src/test/java/org/apache/solr/mcp/server/search/SearchServiceTest.java
+++ b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceTest.java
@@ -30,7 +30,6 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.io.IOException;
-import java.util.Arrays;
import java.util.List;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrServerException;
@@ -71,13 +70,7 @@ class SearchServiceTest {
SearchService localService = new SearchService(mockClient);
assertNotNull(
localService.search(
- "test_collection",
- " \t",
- Arrays.asList("", " \t", null),
- Arrays.asList(null, "", " \n"),
- null,
- null,
- null));
+ "test_collection", " \t", List.of("", " \t"), List.of("", " \n"),
null, null, null));
}
@Test
@@ -99,7 +92,7 @@ class SearchServiceTest {
null,
null,
null,
- Arrays.asList(new SortClause("", ""), new SortClause(" \t",
"asc"), null),
+ List.of(new SortClause("", ""), new SortClause(" \t", "asc")),
null,
null));
}
@@ -129,10 +122,9 @@ class SearchServiceTest {
localService.search(
"test_collection",
"{!edismax qf='name author_ss'}george martin",
- Arrays.asList("", filter, null, " \n", "price:[0 TO 10]"),
- Arrays.asList("\t", facet, "", null, "author_ss"),
- Arrays.asList(
- null,
+ List.of("", filter, " \n", "price:[0 TO 10]"),
+ List.of("\t", facet, "", "author_ss"),
+ List.of(
new SortClause("", "asc"),
new SortClause("price", "DESC"),
new SortClause(" \n", ""),
@@ -414,8 +406,7 @@ class SearchServiceTest {
SolrDocumentList mockDocuments = createMockDocumentList();
when(mockResponse.getResults()).thenReturn(mockDocuments);
when(mockResponse.getFacetFields()).thenReturn(createMockFacetFields());
- when(mockClient.query(eq("test_collection"), any(SolrQuery.class)))
- .thenAnswer(invocation -> mockResponse);
+ when(mockClient.query(eq("test_collection"),
any(SolrQuery.class))).thenReturn(mockResponse);
SearchService localService = new SearchService(mockClient);
SearchResponse result =
localService.search("test_collection", null, null, facetFields, null,
null, null);
@@ -432,8 +423,7 @@ class SearchServiceTest {
SolrDocumentList mockDocuments = createMockDocumentList();
when(mockResponse.getResults()).thenReturn(mockDocuments);
when(mockResponse.getFacetFields()).thenReturn(null);
- when(mockClient.query(eq("test_collection"), any(SolrQuery.class)))
- .thenAnswer(invocation -> mockResponse);
+ when(mockClient.query(eq("test_collection"),
any(SolrQuery.class))).thenReturn(mockResponse);
SearchService localService = new SearchService(mockClient);
SearchResponse result =
localService.search("test_collection", null, null, null, sortClauses,
null, null);
diff --git
a/src/test/java/org/apache/solr/mcp/server/search/SortClauseTest.java
b/src/test/java/org/apache/solr/mcp/server/search/SortClauseTest.java
index 76c8a745..f3a64a5b 100644
--- a/src/test/java/org/apache/solr/mcp/server/search/SortClauseTest.java
+++ b/src/test/java/org/apache/solr/mcp/server/search/SortClauseTest.java
@@ -57,8 +57,6 @@ class SortClauseTest {
@Test
void rejectsMissingField() {
- assertThrows(
- IllegalArgumentException.class, () -> new SortClause(null,
"asc").toSolrSortClause());
assertThrows(
IllegalArgumentException.class, () -> new SortClause(" ",
"asc").toSolrSortClause());
}
