From d7bca27521f442fc6688ad94e106ec97a0c854fd Mon Sep 17 00:00:00 2001 From: adityamparikh Date: Fri, 24 Apr 2026 14:13:11 -0400 Subject: [PATCH] fix(search): add JSpecify @Nullable annotations to SearchService parameters Add @Nullable annotations from JSpecify to optional parameters in SearchService.search() (query, filterQueries, facetFields, sortClauses, start, rows) and to SearchResponse.maxScore. This correctly expresses the nullability contract for MCP tool parameters that AI clients may omit. Also add @NullMarked package-info.java for the search subpackage so NullAway enforces null safety, and fix test code to use null-safe patterns (assertNotNull before dereference) for Map.get() results. Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: adityamparikh --- .../mcp/server/search/SearchResponse.java | 3 ++- .../solr/mcp/server/search/SearchService.java | 13 ++++++------ .../solr/mcp/server/search/package-info.java | 20 +++++++++++++++++++ .../search/SearchServiceIntegrationTest.java | 18 +++++++++++------ 4 files changed, 41 insertions(+), 13 deletions(-) create mode 100644 src/main/java/org/apache/solr/mcp/server/search/package-info.java 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 3e8d914f..029d8ff1 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 @@ import java.util.List; import java.util.Map; +import org.jspecify.annotations.Nullable; /** * Immutable record representing a structured search response from Apache Solr @@ -134,6 +135,6 @@ * @see org.apache.solr.client.solrj.response.QueryResponse * @see org.apache.solr.common.SolrDocumentList */ -public record SearchResponse(long numFound, long start, Float maxScore, List> documents, +public record SearchResponse(long numFound, long start, @Nullable Float maxScore, List> documents, Map> 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 c29ccb6a..8ba6cc09 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 @@ -30,6 +30,7 @@ import org.apache.solr.common.SolrDocument; import org.apache.solr.common.SolrDocumentList; import org.apache.solr.common.params.FacetParams; +import org.jspecify.annotations.Nullable; import org.springaicommunity.mcp.annotation.McpTool; import org.springaicommunity.mcp.annotation.McpToolParam; import org.springframework.security.access.prepost.PreAuthorize; @@ -242,12 +243,12 @@ private static Map> getFacets(QueryResponse queryRespo } """) public SearchResponse search(@McpToolParam(description = "Solr collection to query") String collection, - @McpToolParam(description = "Solr q parameter. If none specified defaults to \"*:*\"", required = false) String query, - @McpToolParam(description = "Solr fq parameter", required = false) List filterQueries, - @McpToolParam(description = "Solr facet fields", required = false) List facetFields, - @McpToolParam(description = "Solr sort parameter", required = false) List> sortClauses, - @McpToolParam(description = "Starting offset for pagination", required = false) Integer start, - @McpToolParam(description = "Number of rows to return", required = false) Integer rows) + @McpToolParam(description = "Solr q parameter. If none specified defaults to \"*:*\"", required = false) @Nullable String query, + @McpToolParam(description = "Solr fq parameter", required = false) @Nullable List filterQueries, + @McpToolParam(description = "Solr facet fields", required = false) @Nullable List facetFields, + @McpToolParam(description = "Solr sort parameter", required = false) @Nullable List> sortClauses, + @McpToolParam(description = "Starting offset for pagination", required = false) @Nullable Integer start, + @McpToolParam(description = "Number of rows to return", required = false) @Nullable Integer rows) throws SolrServerException, IOException { // query diff --git a/src/main/java/org/apache/solr/mcp/server/search/package-info.java b/src/main/java/org/apache/solr/mcp/server/search/package-info.java new file mode 100644 index 00000000..08a30417 --- /dev/null +++ b/src/main/java/org/apache/solr/mcp/server/search/package-info.java @@ -0,0 +1,20 @@ +/* + * 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. + */ +@NullMarked +package org.apache.solr.mcp.server.search; + +import org.jspecify.annotations.NullMarked; 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 9f5ff900..4fce0189 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,7 +194,9 @@ void testSearchWithQuery() throws SolrServerException, IOException { List> documents = result.documents(); assertEquals(1, documents.size()); Map book = documents.getFirst(); - assertEquals("A Game of Thrones", ((List) book.get("name")).getFirst()); + List nameField = (List) book.get("name"); + assertNotNull(nameField); + assertEquals("A Game of Thrones", nameField.getFirst()); } @Test @@ -205,7 +207,9 @@ void testSearchReturnsAuthor() throws Exception { List> documents = result.documents(); assertEquals(3, documents.size()); Map book = documents.getFirst(); - assertEquals("George R.R. Martin", ((List) book.get("author_ss")).getFirst()); + List authorField = (List) book.get("author_ss"); + assertNotNull(authorField); + assertEquals("George R.R. Martin", authorField.getFirst()); } @Test @@ -224,9 +228,9 @@ void testSearchWithPrice() throws Exception { List> documents = result.documents(); assertFalse(documents.isEmpty()); Map book = documents.getFirst(); - double currentPrice = ((List) book.get("price")).isEmpty() - ? 0.0 - : ((Number) ((List) book.get("price")).getFirst()).doubleValue(); + List priceField = (List) book.get("price"); + assertNotNull(priceField); + double currentPrice = priceField.isEmpty() ? 0.0 : ((Number) priceField.getFirst()).doubleValue(); assertTrue(currentPrice > 0); } @@ -277,7 +281,9 @@ void testSortBySequence() throws Exception { assertFalse(documents.isEmpty()); int previousSequence = 0; for (Map book : documents) { - int currentSequence = ((Number) book.get("sequence_i")).intValue(); + Number sequenceValue = (Number) book.get("sequence_i"); + assertNotNull(sequenceValue); + int currentSequence = sequenceValue.intValue(); assertTrue(currentSequence >= previousSequence, "Books should be sorted by sequence_i in ascending order"); previousSequence = currentSequence; }