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 d5dd1df..48ec56d 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 @@ -133,36 +133,39 @@ public class CollectionService { // Constants for API Parameters and Paths // ======================================== - /** Category parameter value for cache-related MBeans requests */ - private static final String CACHE_CATEGORY = "CACHE"; - - /** Category parameter value for query handler MBeans requests */ - private static final String QUERY_HANDLER_CATEGORY = "QUERYHANDLER"; - - /** - * Combined category parameter value for both query and update handler MBeans - * requests - */ - private static final String HANDLER_CATEGORIES = "QUERYHANDLER,UPDATEHANDLER"; - /** 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"; - /** Request parameter name for enabling statistics in MBeans requests */ - private static final String STATS_PARAM = "stats"; - - /** Request parameter name for specifying category filters in MBeans requests */ - private static final String CAT_PARAM = "cat"; - /** Request parameter name for specifying response writer type */ private static final String WT_PARAM = "wt"; /** JSON format specification for response writer type */ private static final String JSON_FORMAT = "json"; + /** URL path for Solr Metrics admin endpoint */ + private static final String ADMIN_METRICS_PATH = "/admin/metrics"; + + /** Request parameter name for specifying the metrics group */ + private static final String GROUP_PARAM = "group"; + + /** Request parameter name for filtering metrics by key prefix */ + private static final String PREFIX_PARAM = "prefix"; + + /** Metrics group for core-level metrics */ + private static final String CORE_GROUP = "core"; + + /** Prefix for cache metrics in the Metrics API */ + private static final String CACHE_METRIC_PREFIX = "CACHE.searcher"; + + /** Prefix for select handler metrics in the Metrics API */ + private static final String SELECT_HANDLER_METRIC_PREFIX = "QUERY./select"; + + /** Prefix for update handler metrics in the Metrics API */ + private static final String UPDATE_HANDLER_METRIC_PREFIX = "UPDATE./update"; + // ======================================== // Constants for Response Parsing // ======================================== @@ -173,30 +176,23 @@ public class CollectionService { /** Key name for segment count information in Luke response */ private static final String SEGMENT_COUNT_KEY = "segmentCount"; - /** Key name for query result cache in MBeans cache responses */ - private static final String QUERY_RESULT_CACHE_KEY = "queryResultCache"; - - /** Key name for document cache in MBeans cache responses */ - private static final String DOCUMENT_CACHE_KEY = "documentCache"; + /** Top-level key in Metrics API responses */ + private static final String METRICS_KEY = "metrics"; - /** Key name for filter cache in MBeans cache responses */ - private static final String FILTER_CACHE_KEY = "filterCache"; + /** Metrics API key for query result cache */ + private static final String QUERY_RESULT_CACHE_KEY = "CACHE.searcher.queryResultCache"; - /** Key name for statistics section in MBeans responses */ - private static final String STATS_KEY = "stats"; + /** Metrics API key for document cache */ + private static final String DOCUMENT_CACHE_KEY = "CACHE.searcher.documentCache"; - // ======================================== - // Constants for Handler Paths - // ======================================== + /** Metrics API key for filter cache */ + private static final String FILTER_CACHE_KEY = "CACHE.searcher.filterCache"; - /** URL path for Solr select (query) handler */ - private static final String SELECT_HANDLER_PATH = "/select"; + /** Flat metric key prefix for select handler stats */ + private static final String SELECT_HANDLER_KEY = "QUERY./select."; - /** URL path for Solr update handler */ - private static final String UPDATE_HANDLER_PATH = "/update"; - - /** URL path for Solr MBeans admin endpoint */ - private static final String ADMIN_MBEANS_PATH = "/admin/mbeans"; + /** Flat metric key prefix for update handler stats */ + private static final String UPDATE_HANDLER_KEY = "UPDATE./update."; // ======================================== // Constants for Statistics Field Names @@ -232,12 +228,6 @@ public class CollectionService { /** Field name for handler total processing time statistics */ private static final String TOTAL_TIME_FIELD = "totalTime"; - /** Field name for handler average time per request statistics */ - private static final String AVG_TIME_PER_REQUEST_FIELD = "avgTimePerRequest"; - - /** Field name for handler average requests per second statistics */ - private static final String AVG_REQUESTS_PER_SECOND_FIELD = "avgRequestsPerSecond"; - // ======================================== // Constants for Error Messages // ======================================== @@ -438,7 +428,7 @@ public SolrMetrics getCollectionStats( QueryResponse statsResponse = solrClient.query(actualCollection, new SolrQuery(ALL_DOCUMENTS_QUERY).setRows(0)); return new SolrMetrics(buildIndexStats(lukeResponse), buildQueryStats(statsResponse), - getCacheMetrics(actualCollection), getHandlerMetrics(actualCollection), new Date()); + fetchCacheMetrics(actualCollection), fetchHandlerMetrics(actualCollection), new Date()); } /** @@ -525,7 +515,7 @@ public QueryStats buildQueryStats(QueryResponse response) { * Retrieves cache performance metrics for all cache types in a Solr collection. * *

- * Collects detailed cache utilization statistics from Solr's MBeans endpoint, + * Collects detailed cache utilization statistics from Solr's Metrics API, * providing insights into cache effectiveness and memory usage patterns. Cache * performance directly impacts query response times and system efficiency. * @@ -567,38 +557,30 @@ public QueryStats buildQueryStats(QueryResponse response) { * @see #isCacheStatsEmpty(CacheStats) */ public CacheStats getCacheMetrics(String collection) { - try { - // Get MBeans for cache information - ModifiableSolrParams params = new ModifiableSolrParams(); - params.set(STATS_PARAM, "true"); - params.set(CAT_PARAM, CACHE_CATEGORY); - params.set(WT_PARAM, JSON_FORMAT); - - // Extract actual collection name from shard name if needed - String actualCollection = extractCollectionName(collection); - - // Validate collection exists first - if (!validateCollectionExists(actualCollection)) { - return null; // Return null instead of empty object - } - - String path = "/" + actualCollection + ADMIN_MBEANS_PATH; + String actualCollection = extractCollectionName(collection); - GenericSolrRequest request = new GenericSolrRequest(SolrRequest.METHOD.GET, path, params); + if (!validateCollectionExists(actualCollection)) { + return null; + } - NamedList response = solrClient.request(request); - CacheStats stats = extractCacheStats(response); + return fetchCacheMetrics(actualCollection); + } - // Return null if all cache stats are empty/null - if (isCacheStatsEmpty(stats)) { + /** + * Internal cache metrics fetch that assumes the collection has already been + * validated and the name has been extracted from any shard identifier. + */ + private CacheStats fetchCacheMetrics(String collection) { + try { + NamedList coreMetrics = fetchMetrics(collection, CACHE_METRIC_PREFIX); + if (coreMetrics == null) { return null; } - return stats; + CacheStats stats = extractCacheStats(coreMetrics); + return isCacheStatsEmpty(stats) ? null : stats; } catch (SolrServerException | IOException | RuntimeException _) { - // RuntimeException covers SolrException subclasses (e.g. RemoteSolrException) - // thrown when the /admin/mbeans endpoint is unavailable (removed in Solr 10). - return null; // Return null instead of empty object + return null; } } @@ -620,81 +602,26 @@ private boolean isCacheStatsEmpty(CacheStats stats) { } /** - * Extracts cache performance statistics from Solr MBeans response data. - * - *

- * Parses the raw MBeans response to extract structured cache performance - * metrics for all available cache types. Each cache type provides detailed - * statistics including hit ratios, eviction rates, and current utilization. - * - *

- * Parsed Cache Types: - * - *

    - *
  • queryResultCache - Complete query result caching - *
  • documentCache - Retrieved document data caching - *
  • filterCache - Filter query result caching - *
+ * Extracts cache performance statistics from Solr Metrics API response data. * - *

- * For each cache type, the following metrics are extracted: - * - *

    - *
  • lookups, hits, hitratio - Performance effectiveness - *
  • inserts, evictions - Memory management patterns - *
  • size - Current utilization - *
- * - * @param mbeans - * the raw MBeans response from Solr admin endpoint + * @param coreMetrics + * the core metrics from the Solr Metrics API * @return CacheStats object containing parsed metrics for all cache types - * @see CacheStats - * @see CacheInfo */ - private CacheStats extractCacheStats(NamedList mbeans) { - CacheInfo queryResultCacheInfo = null; - CacheInfo documentCacheInfo = null; - CacheInfo filterCacheInfo = null; - - @SuppressWarnings("unchecked") - NamedList caches = (NamedList) mbeans.get(CACHE_CATEGORY); - - if (caches != null) { - // Query result cache - @SuppressWarnings("unchecked") - NamedList queryResultCache = (NamedList) caches.get(QUERY_RESULT_CACHE_KEY); - if (queryResultCache != null) { - @SuppressWarnings("unchecked") - NamedList stats = (NamedList) queryResultCache.get(STATS_KEY); - queryResultCacheInfo = new CacheInfo(getLong(stats, LOOKUPS_FIELD), getLong(stats, HITS_FIELD), - getFloat(stats, HITRATIO_FIELD), getLong(stats, INSERTS_FIELD), getLong(stats, EVICTIONS_FIELD), - getLong(stats, SIZE_FIELD)); - } - - // Document cache - @SuppressWarnings("unchecked") - NamedList documentCache = (NamedList) caches.get(DOCUMENT_CACHE_KEY); - if (documentCache != null) { - @SuppressWarnings("unchecked") - NamedList stats = (NamedList) documentCache.get(STATS_KEY); - documentCacheInfo = new CacheInfo(getLong(stats, LOOKUPS_FIELD), getLong(stats, HITS_FIELD), - getFloat(stats, HITRATIO_FIELD), getLong(stats, INSERTS_FIELD), getLong(stats, EVICTIONS_FIELD), - getLong(stats, SIZE_FIELD)); - } + private CacheStats extractCacheStats(NamedList coreMetrics) { + return new CacheStats(extractSingleCacheInfo(coreMetrics, QUERY_RESULT_CACHE_KEY), + extractSingleCacheInfo(coreMetrics, DOCUMENT_CACHE_KEY), + extractSingleCacheInfo(coreMetrics, FILTER_CACHE_KEY)); + } - // Filter cache - @SuppressWarnings("unchecked") - NamedList filterCache = (NamedList) caches.get(FILTER_CACHE_KEY); - if (filterCache != null) { - @SuppressWarnings("unchecked") - NamedList stats = (NamedList) filterCache.get(STATS_KEY); - filterCacheInfo = new CacheInfo(getLong(stats, LOOKUPS_FIELD), getLong(stats, HITS_FIELD), - getFloat(stats, HITRATIO_FIELD), getLong(stats, INSERTS_FIELD), getLong(stats, EVICTIONS_FIELD), - getLong(stats, SIZE_FIELD)); - } + @SuppressWarnings("unchecked") + private CacheInfo extractSingleCacheInfo(NamedList coreMetrics, String key) { + NamedList cache = (NamedList) coreMetrics.get(key); + if (cache == null) { + return null; } - - return new CacheStats(queryResultCacheInfo, documentCacheInfo, filterCacheInfo); + return new CacheInfo(getLong(cache, LOOKUPS_FIELD), getLong(cache, HITS_FIELD), getFloat(cache, HITRATIO_FIELD), + getLong(cache, INSERTS_FIELD), getLong(cache, EVICTIONS_FIELD), getLong(cache, SIZE_FIELD)); } /** @@ -709,10 +636,10 @@ private CacheStats extractCacheStats(NamedList mbeans) { * Monitored Handlers: * *
    - *
  • Select Handler ({@value #SELECT_HANDLER_PATH}): - * Processes search and query requests - *
  • Update Handler ({@value #UPDATE_HANDLER_PATH}): - * Processes document indexing operations + *
  • Select Handler (/select): Processes search and query + * requests + *
  • Update Handler (/update): Processes document indexing + * operations *
* *

@@ -738,41 +665,36 @@ private CacheStats extractCacheStats(NamedList mbeans) { * null if unavailable * @see HandlerStats * @see HandlerInfo - * @see #extractHandlerStats(NamedList) + * @see #fetchFlatHandlerInfo(String, String, String) * @see #isHandlerStatsEmpty(HandlerStats) */ public HandlerStats getHandlerMetrics(String collection) { - try { - ModifiableSolrParams params = new ModifiableSolrParams(); - params.set(STATS_PARAM, "true"); - params.set(CAT_PARAM, HANDLER_CATEGORIES); - params.set(WT_PARAM, JSON_FORMAT); - - // Extract actual collection name from shard name if needed - String actualCollection = extractCollectionName(collection); - - // Validate collection exists first - if (!validateCollectionExists(actualCollection)) { - return null; // Return null instead of empty object - } - - String path = "/" + actualCollection + ADMIN_MBEANS_PATH; - - GenericSolrRequest request = new GenericSolrRequest(SolrRequest.METHOD.GET, path, params); + String actualCollection = extractCollectionName(collection); - NamedList response = solrClient.request(request); - HandlerStats stats = extractHandlerStats(response); + if (!validateCollectionExists(actualCollection)) { + return null; + } - // Return null if all handler stats are empty/null - if (isHandlerStatsEmpty(stats)) { - return null; - } + return fetchHandlerMetrics(actualCollection); + } - return stats; + /** + * Internal handler metrics fetch that assumes the collection has already been + * validated and the name has been extracted from any shard identifier. + */ + private HandlerStats fetchHandlerMetrics(String collection) { + try { + // Handler metrics are flat keys (e.g. QUERY./select.requests) so we + // fetch each handler prefix separately and reconstruct HandlerInfo + HandlerInfo selectHandler = fetchFlatHandlerInfo(collection, SELECT_HANDLER_METRIC_PREFIX, + SELECT_HANDLER_KEY); + HandlerInfo updateHandler = fetchFlatHandlerInfo(collection, UPDATE_HANDLER_METRIC_PREFIX, + UPDATE_HANDLER_KEY); + + HandlerStats stats = new HandlerStats(selectHandler, updateHandler); + return isHandlerStatsEmpty(stats) ? null : stats; } catch (SolrServerException | IOException | RuntimeException _) { - // RuntimeException covers SolrException subclasses (e.g. RemoteSolrException) - // thrown when the /admin/mbeans endpoint is unavailable (removed in Solr 10). - return null; // Return null instead of empty object + return null; } } @@ -793,69 +715,92 @@ private boolean isHandlerStatsEmpty(HandlerStats stats) { } /** - * Extracts request handler performance statistics from Solr MBeans response - * data. - * - *

- * Parses the raw MBeans response to extract structured handler performance - * metrics for query and update operations. Each handler provides detailed - * statistics about request processing including volume, errors, and timing. - * - *

- * Parsed Handler Types: - * - *

    - *
  • /select - Search and query request handler - *
  • /update - Document indexing request handler - *
- * - *

- * For each handler type, the following metrics are extracted: + * Fetches metrics from the Solr Metrics API for a given collection and prefix. * - *

    - *
  • requests, errors, timeouts - Volume and reliability - *
  • totalTime, avgTimePerRequest - Performance characteristics - *
  • avgRequestsPerSecond - Throughput capacity - *
- * - * @param mbeans - * the raw MBeans response from Solr admin endpoint - * @return HandlerStats object containing parsed metrics for all handler types - * @see HandlerStats - * @see HandlerInfo + * @param collection + * the collection name + * @param prefix + * the metric key prefix to filter (e.g. "CACHE.searcher", "HANDLER") + * @return the core-level metrics NamedList, or null if unavailable */ - private HandlerStats extractHandlerStats(NamedList mbeans) { - HandlerInfo selectHandlerInfo = null; - HandlerInfo updateHandlerInfo = null; - - @SuppressWarnings("unchecked") - NamedList queryHandlers = (NamedList) mbeans.get(QUERY_HANDLER_CATEGORY); + @SuppressWarnings("unchecked") + private NamedList fetchMetrics(String collection, String prefix) throws SolrServerException, IOException { + ModifiableSolrParams params = new ModifiableSolrParams(); + params.set(GROUP_PARAM, CORE_GROUP); + params.set(PREFIX_PARAM, prefix); + params.set(WT_PARAM, JSON_FORMAT); + + // Metrics API is a node-level endpoint, not per-collection + GenericSolrRequest request = new GenericSolrRequest(SolrRequest.METHOD.GET, ADMIN_METRICS_PATH, params); + + NamedList response = solrClient.request(request); + NamedList metrics = (NamedList) response.get(METRICS_KEY); + if (metrics == null || metrics.size() == 0) { + return null; + } - if (queryHandlers != null) { - // Select handler - @SuppressWarnings("unchecked") - NamedList selectHandler = (NamedList) queryHandlers.get(SELECT_HANDLER_PATH); - if (selectHandler != null) { - @SuppressWarnings("unchecked") - NamedList stats = (NamedList) selectHandler.get(STATS_KEY); - selectHandlerInfo = new HandlerInfo(getLong(stats, REQUESTS_FIELD), getLong(stats, ERRORS_FIELD), - getLong(stats, TIMEOUTS_FIELD), getLong(stats, TOTAL_TIME_FIELD), - getFloat(stats, AVG_TIME_PER_REQUEST_FIELD), getFloat(stats, AVG_REQUESTS_PER_SECOND_FIELD)); + // Find the core registry matching the requested collection + // Keys are like "solr.core..." + String corePrefix = "solr.core." + collection + "."; + for (int i = 0; i < metrics.size(); i++) { + String key = metrics.getName(i); + if (key != null && key.startsWith(corePrefix)) { + return (NamedList) metrics.getVal(i); } + } + return null; + } - // Update handler - @SuppressWarnings("unchecked") - NamedList updateHandler = (NamedList) queryHandlers.get(UPDATE_HANDLER_PATH); - if (updateHandler != null) { - @SuppressWarnings("unchecked") - NamedList stats = (NamedList) updateHandler.get(STATS_KEY); - updateHandlerInfo = new HandlerInfo(getLong(stats, REQUESTS_FIELD), getLong(stats, ERRORS_FIELD), - getLong(stats, TIMEOUTS_FIELD), getLong(stats, TOTAL_TIME_FIELD), - getFloat(stats, AVG_TIME_PER_REQUEST_FIELD), getFloat(stats, AVG_REQUESTS_PER_SECOND_FIELD)); - } + /** + * Fetches and extracts handler metrics from flat Solr Metrics API keys. + * + *

+ * Handler metrics in Solr are stored as flat keys (e.g. + * {@code QUERY./select.requests}) rather than nested objects. This method + * fetches core metrics filtered by the handler prefix and reconstructs a + * {@link HandlerInfo} from the individual flat keys. + * + * @param collection + * the collection name + * @param metricPrefix + * the prefix for the Metrics API filter (e.g. {@code QUERY./select}) + * @param keyPrefix + * the flat key prefix including trailing dot (e.g. + * {@code QUERY./select.}) + * @return HandlerInfo with stats, or null if unavailable + */ + private HandlerInfo fetchFlatHandlerInfo(String collection, String metricPrefix, String keyPrefix) + throws SolrServerException, IOException { + NamedList coreMetrics = fetchMetrics(collection, metricPrefix); + if (coreMetrics == null) { + return null; } + return extractFlatHandlerInfo(coreMetrics, keyPrefix); + } - return new HandlerStats(selectHandlerInfo, updateHandlerInfo); + /** + * Extracts a {@link HandlerInfo} from flat metric keys in core metrics. + * + * @param coreMetrics + * the core metrics NamedList with flat keys + * @param keyPrefix + * the flat key prefix including trailing dot (e.g. + * {@code QUERY./select.}) + * @return HandlerInfo reconstructed from flat keys, or null if no requests key + * found + */ + private HandlerInfo extractFlatHandlerInfo(NamedList coreMetrics, String keyPrefix) { + Long requests = getLong(coreMetrics, keyPrefix + REQUESTS_FIELD); + if (requests == null) { + return null; + } + Long errors = getLong(coreMetrics, keyPrefix + ERRORS_FIELD); + Long timeouts = getLong(coreMetrics, keyPrefix + TIMEOUTS_FIELD); + Long totalTime = getLong(coreMetrics, keyPrefix + TOTAL_TIME_FIELD); + // avgTimePerRequest and avgRequestsPerSecond are not available as flat metrics; + // compute avgTimePerRequest from totalTime/requests when possible + Float avgTimePerRequest = (requests > 0 && totalTime != null) ? (float) totalTime / requests : null; + return new HandlerInfo(requests, errors, timeouts, totalTime, avgTimePerRequest, null); } /** @@ -956,7 +901,7 @@ private boolean validateCollectionExists(String collection) { // shard // names) return collections.stream().anyMatch(c -> c.startsWith(collection + SHARD_SUFFIX)); - } catch (Exception _) { + } catch (Exception e) { return false; } } @@ -1012,18 +957,20 @@ private boolean validateCollectionExists(String collection) { */ @McpTool(name = "check-health", description = "Check health of a Solr collection") public SolrHealthStatus checkHealth(@McpToolParam(description = "Solr collection") String collection) { + String actualCollection = extractCollectionName(collection); try { // Ping Solr - SolrPingResponse pingResponse = solrClient.ping(collection); + SolrPingResponse pingResponse = solrClient.ping(actualCollection); // Get basic stats - QueryResponse statsResponse = solrClient.query(collection, new SolrQuery(ALL_DOCUMENTS_QUERY).setRows(0)); + QueryResponse statsResponse = solrClient.query(actualCollection, + new SolrQuery(ALL_DOCUMENTS_QUERY).setRows(0)); return new SolrHealthStatus(true, null, pingResponse.getElapsedTime(), - statsResponse.getResults().getNumFound(), new Date(), null, null, null); + statsResponse.getResults().getNumFound(), new Date(), actualCollection, null, null); } catch (Exception e) { - return new SolrHealthStatus(false, e.getMessage(), null, null, new Date(), null, null, null); + return new SolrHealthStatus(false, e.getMessage(), null, null, new Date(), actualCollection, null, null); } } 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 5c02fed..2df9309 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 @@ -64,6 +64,7 @@ public class CollectionUtils { private CollectionUtils() { + // Utility class — prevent instantiation } /** @@ -181,7 +182,18 @@ public static Long getLong(NamedList response, String key) { */ public static Float getFloat(NamedList stats, String key) { Object value = stats.get(key); - return value != null ? ((Number) value).floatValue() : 0.0f; + if (value == null) + return null; + + if (value instanceof Number number) { + return number.floatValue(); + } + + try { + return Float.parseFloat(value.toString()); + } catch (NumberFormatException _) { + return null; + } } /** 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 c520bd0..242ca84 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 @@ -18,12 +18,20 @@ import static org.junit.jupiter.api.Assertions.*; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; -import org.apache.solr.client.solrj.SolrClient; -import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import java.util.Map; import org.apache.solr.mcp.server.TestcontainersConfiguration; -import org.junit.jupiter.api.BeforeEach; +import org.apache.solr.mcp.server.indexing.IndexingService; +import org.apache.solr.mcp.server.search.SearchResponse; +import org.apache.solr.mcp.server.search.SearchService; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.context.annotation.Import; @@ -32,216 +40,210 @@ @SpringBootTest @Import(TestcontainersConfiguration.class) @Testcontainers(disabledWithoutDocker = true) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) class CollectionServiceIntegrationTest { + private static final Logger log = LoggerFactory.getLogger(CollectionServiceIntegrationTest.class); + private static final String TEST_COLLECTION = "test_collection"; + + private static final int DOC_COUNT = 50; + @Autowired private CollectionService collectionService; - @Autowired - private SolrClient solrClient; - private static boolean initialized = false; - - @BeforeEach - void setupCollection() throws Exception { - if (!initialized) { - // Create a test collection using the container's connection details - // Create a collection for testing - CollectionAdminRequest.Create createRequest = CollectionAdminRequest.createCollection(TEST_COLLECTION, - "_default", 1, 1); - createRequest.process(solrClient); + @Autowired + private IndexingService indexingService; - // Verify collection was created successfully - CollectionAdminRequest.List listRequest = new CollectionAdminRequest.List(); - listRequest.process(solrClient); + @Autowired + private SearchService searchService; - System.out.println("[DEBUG_LOG] Test collection created: " + TEST_COLLECTION); - initialized = true; + @Autowired + private ObjectMapper objectMapper; + + @BeforeAll + void setupCollectionWithData() throws Exception { + // 1. Create collection via CollectionService MCP tool + CollectionCreationResult created = collectionService.createCollection(TEST_COLLECTION, null, null, null); + assertTrue(created.success(), "Collection creation should succeed: " + created.message()); + log.debug("Test collection created: {}", TEST_COLLECTION); + + // 2. Index documents via IndexingService MCP tool + List> docs = new ArrayList<>(); + for (int i = 0; i < DOC_COUNT; i++) { + Map doc = new LinkedHashMap<>(); + doc.put("id", "doc-" + i); + doc.put("title_s", "Document " + i); + doc.put("category_s", (i % 2 == 0) ? "even" : "odd"); + doc.put("count_i", i); + docs.add(doc); + } + String json = objectMapper.writeValueAsString(docs); + indexingService.indexJsonDocuments(TEST_COLLECTION, json); + log.debug("Indexed {} documents via IndexingService", DOC_COUNT); + + // 3. Run searches via SearchService MCP tool to populate caches and handler + // stats + for (int i = 0; i < 5; i++) { + searchService.search(TEST_COLLECTION, "*:*", null, null, null, 0, 10); + searchService.search(TEST_COLLECTION, "title_s:Document", null, null, null, 0, 5); + searchService.search(TEST_COLLECTION, "*:*", List.of("category_s:even"), null, null, 0, 10); } + log.debug("Ran warm-up queries via SearchService"); } @Test void testListCollections() { - // Test listing collections List collections = collectionService.listCollections(); - // Print the collections for debugging - System.out.println("[DEBUG_LOG] Collections: " + collections); + log.debug("Collections: {}", collections); - // Enhanced assertions for collections list - assertNotNull(collections, "Collections list should not be null"); - assertFalse(collections.isEmpty(), "Collections list should not be empty"); + assertNotNull(collections); + assertFalse(collections.isEmpty()); - // Check if the test collection exists (either as exact name or as shard) boolean testCollectionExists = collections.contains(TEST_COLLECTION) || collections.stream().anyMatch(col -> col.startsWith(TEST_COLLECTION + "_shard")); assertTrue(testCollectionExists, - "Collections should contain the test collection: " + TEST_COLLECTION + " (found: " + collections + ")"); + "Collections should contain " + TEST_COLLECTION + " (found: " + collections + ")"); - // Verify collection names are not null or empty - for (String collection : collections) { - assertNotNull(collection, "Collection name should not be null"); - assertFalse(collection.trim().isEmpty(), "Collection name should not be empty"); - } - - // Verify expected collection characteristics assertEquals(collections.size(), collections.stream().distinct().count(), "Collection names should be unique"); - - // Verify that collections follow expected naming patterns - for (String collection : collections) { - // Collection names should either be simple names or shard names - assertTrue(collection.matches("^[a-zA-Z0-9_]+(_shard\\d+_replica_n\\d+)?$"), - "Collection name should follow expected pattern: " + collection); - } } @Test - void testGetCollectionStats() throws Exception { - // Test getting collection stats + void testGetCollectionStats_reflectsIndexedData() throws Exception { SolrMetrics metrics = collectionService.getCollectionStats(TEST_COLLECTION); - // Enhanced assertions for metrics - assertNotNull(metrics, "Collection stats should not be null"); - assertNotNull(metrics.timestamp(), "Timestamp should not be null"); + assertNotNull(metrics); + assertNotNull(metrics.timestamp()); - // Verify index stats - assertNotNull(metrics.indexStats(), "Index stats should not be null"); + // Index stats should reflect the documents we indexed IndexStats indexStats = metrics.indexStats(); - assertNotNull(indexStats.numDocs(), "Number of documents should not be null"); - assertTrue(indexStats.numDocs() >= 0, "Number of documents should be non-negative"); + assertNotNull(indexStats); + assertEquals(DOC_COUNT, indexStats.numDocs(), "numDocs should match indexed document count"); + assertNotNull(indexStats.segmentCount()); + assertTrue(indexStats.segmentCount() >= 1, "Should have at least one segment after indexing"); - // Verify query stats - assertNotNull(metrics.queryStats(), "Query stats should not be null"); + // Query stats come from the *:* probe query inside getCollectionStats QueryStats queryStats = metrics.queryStats(); - assertNotNull(queryStats.queryTime(), "Query time should not be null"); - assertTrue(queryStats.queryTime() >= 0, "Query time should be non-negative"); - assertNotNull(queryStats.totalResults(), "Total results should not be null"); - assertTrue(queryStats.totalResults() >= 0, "Total results should be non-negative"); - assertNotNull(queryStats.start(), "Start should not be null"); - assertTrue(queryStats.start() >= 0, "Start should be non-negative"); - - // Verify timestamp is recent (within last 10 seconds) - long currentTime = System.currentTimeMillis(); - long timestampTime = metrics.timestamp().getTime(); - assertTrue(currentTime - timestampTime < 10000, "Timestamp should be recent (within 10 seconds)"); - - // Verify optional stats (cache and handler stats may be null, which is - // acceptable) - if (metrics.cacheStats() != null) { - CacheStats cacheStats = metrics.cacheStats(); - // Verify at least one cache type exists if cache stats are present - assertTrue( - cacheStats.queryResultCache() != null || cacheStats.documentCache() != null - || cacheStats.filterCache() != null, - "At least one cache type should be present if cache stats exist"); - } - - if (metrics.handlerStats() != null) { - HandlerStats handlerStats = metrics.handlerStats(); - // Verify at least one handler type exists if handler stats are present - assertTrue(handlerStats.selectHandler() != null || handlerStats.updateHandler() != null, - "At least one handler type should be present if handler stats exist"); - } + assertNotNull(queryStats); + assertNotNull(queryStats.queryTime()); + assertEquals((long) DOC_COUNT, queryStats.totalResults(), "totalResults should match indexed document count"); + assertEquals(0L, queryStats.start()); + + // Cache stats should be present after warm-up queries + 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"); + assertNotNull(cacheStats.documentCache()); + assertTrue(cacheStats.documentCache().lookups() > 0, "Document cache should have lookups after queries"); + assertNotNull(cacheStats.filterCache()); + assertTrue(cacheStats.filterCache().lookups() > 0, "Filter cache should have lookups after filter queries"); + + // 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"); + assertNotNull(handlerStats.updateHandler()); + assertTrue(handlerStats.updateHandler().requests() > 0, + "Update handler should have processed requests from indexing"); } @Test - void testCheckHealthHealthy() { - // Test checking health of a valid collection + void testCheckHealth_healthy() { SolrHealthStatus status = collectionService.checkHealth(TEST_COLLECTION); - // Print the status for debugging - System.out.println("[DEBUG_LOG] Health status for valid collection: " + status); - - // Enhanced assertions for healthy collection - assertNotNull(status, "Health status should not be null"); - assertTrue(status.isHealthy(), "Collection should be healthy"); - - // Verify response time - assertNotNull(status.responseTime(), "Response time should not be null"); - assertTrue(status.responseTime() >= 0, "Response time should be non-negative"); - assertTrue(status.responseTime() < 30000, "Response time should be reasonable (< 30 seconds)"); - - // Verify document count - assertNotNull(status.totalDocuments(), "Total documents should not be null"); - assertTrue(status.totalDocuments() >= 0, "Total documents should be non-negative"); - - // Verify timestamp - assertNotNull(status.lastChecked(), "Last checked timestamp should not be null"); - long currentTime = System.currentTimeMillis(); - long lastCheckedTime = status.lastChecked().getTime(); - assertTrue(currentTime - lastCheckedTime < 5000, - "Last checked timestamp should be very recent (within 5 seconds)"); - - // Verify no error message for healthy collection - assertNull(status.errorMessage(), "Error message should be null for healthy collection"); - - // Verify string representation contains meaningful information - String statusString = status.toString(); - if (statusString != null) { - assertTrue(statusString.contains("healthy") || statusString.contains("true"), - "Status string should indicate healthy state"); - } + log.debug("Health status: {}", status); + + assertNotNull(status); + assertTrue(status.isHealthy()); + assertNull(status.errorMessage()); + assertEquals(TEST_COLLECTION, status.collection()); + + assertNotNull(status.responseTime()); + assertTrue(status.responseTime() >= 0); + + assertEquals((long) DOC_COUNT, status.totalDocuments(), "Health check should report indexed document count"); + + assertNotNull(status.lastChecked()); + assertTrue(System.currentTimeMillis() - status.lastChecked().getTime() < 5000); } @Test - void testCheckHealthUnhealthy() { - // Test checking health of an invalid collection - String nonExistentCollection = "non_existent_collection"; - SolrHealthStatus status = collectionService.checkHealth(nonExistentCollection); - - // Print the status for debugging - System.out.println("[DEBUG_LOG] Health status for invalid collection: " + status); - - // Enhanced assertions for unhealthy collection - assertNotNull(status, "Health status should not be null"); - assertFalse(status.isHealthy(), "Collection should not be healthy"); - - // Verify timestamp - assertNotNull(status.lastChecked(), "Last checked timestamp should not be null"); - long currentTime = System.currentTimeMillis(); - long lastCheckedTime = status.lastChecked().getTime(); - assertTrue(currentTime - lastCheckedTime < 5000, - "Last checked timestamp should be very recent (within 5 seconds)"); - - // Verify error message - assertNotNull(status.errorMessage(), "Error message should not be null for unhealthy collection"); - assertFalse(status.errorMessage().trim().isEmpty(), - "Error message should not be empty for unhealthy collection"); - - // Verify that performance metrics are null for unhealthy collection - assertNull(status.responseTime(), "Response time should be null for unhealthy collection"); - assertNull(status.totalDocuments(), "Total documents should be null for unhealthy collection"); - - // Verify error message contains meaningful information - String errorMessage = status.errorMessage().toLowerCase(); - assertTrue( - errorMessage.contains("collection") || errorMessage.contains("not found") - || errorMessage.contains("error") || errorMessage.contains("fail"), - "Error message should contain meaningful error information"); - - // Verify string representation indicates unhealthy state - String statusString = status.toString(); - if (statusString != null) { - assertTrue(statusString.contains("false") || statusString.contains("unhealthy") - || statusString.contains("error"), "Status string should indicate unhealthy state"); - } + void testCheckHealth_nonExistentCollection() { + String missing = "non_existent_collection"; + SolrHealthStatus status = collectionService.checkHealth(missing); + + assertNotNull(status); + assertFalse(status.isHealthy()); + assertNotNull(status.errorMessage()); + assertFalse(status.errorMessage().isBlank()); + assertEquals(missing, status.collection()); + assertNull(status.responseTime()); + assertNull(status.totalDocuments()); } @Test void testCollectionNameExtraction() { - // Test collection name extraction functionality - assertEquals(TEST_COLLECTION, collectionService.extractCollectionName(TEST_COLLECTION), - "Regular collection name should be returned as-is"); + 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("")); + } - assertEquals("films", collectionService.extractCollectionName("films_shard1_replica_n1"), - "Shard name should be extracted to base collection name"); + @Test + void testGetCacheMetrics_afterQueries() { + CacheStats cacheStats = collectionService.getCacheMetrics(TEST_COLLECTION); + + assertNotNull(cacheStats, "Cache stats should not be null after warm-up queries"); + + // 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"); + 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"); + + // 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"); + } - assertEquals("products", collectionService.extractCollectionName("products_shard2_replica_n3"), - "Complex shard name should be extracted correctly"); + @Test + void testGetHandlerMetrics_afterQueriesAndIndexing() { + HandlerStats handlerStats = collectionService.getHandlerMetrics(TEST_COLLECTION); + + assertNotNull(handlerStats, "Handler stats should not be null after activity"); + + // 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"); + assertNotNull(select.errors()); + assertNotNull(select.timeouts()); + + // 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"); + } - assertNull(collectionService.extractCollectionName(null), "Null input should return null"); + @Test + void testGetCacheMetrics_nonExistentCollection() { + assertNull(collectionService.getCacheMetrics("non_existent_collection")); + } - assertEquals("", collectionService.extractCollectionName(""), "Empty string should return empty string"); + @Test + void testGetHandlerMetrics_nonExistentCollection() { + assertNull(collectionService.getHandlerMetrics("non_existent_collection")); } @Test @@ -250,13 +252,26 @@ void createCollection_createsAndListable() throws Exception { CollectionCreationResult result = collectionService.createCollection(name, null, null, null); - assertTrue(result.success(), "Collection creation should succeed"); - assertEquals(name, result.name(), "Result should contain the collection name"); - assertNotNull(result.createdAt(), "Creation timestamp should be set"); + assertTrue(result.success()); + assertEquals(name, result.name()); + assertNotNull(result.createdAt()); List collections = collectionService.listCollections(); - boolean collectionExists = collections.contains(name) + boolean exists = collections.contains(name) || collections.stream().anyMatch(col -> col.startsWith(name + "_shard")); - assertTrue(collectionExists, "Newly created collection should appear in list (found: " + collections + ")"); + assertTrue(exists, "Newly created collection should appear in list (found: " + collections + ")"); + } + + @Test + void testSearchVerifiesIndexedDocuments() throws Exception { + // Verify the documents we indexed are actually searchable via SearchService + SearchResponse all = searchService.search(TEST_COLLECTION, "*:*", null, null, null, 0, DOC_COUNT); + assertEquals(DOC_COUNT, all.numFound(), "Should find all indexed documents"); + assertEquals(DOC_COUNT, all.documents().size(), "Should return all documents in single page"); + + // Filter search should return only even-category docs + SearchResponse evens = searchService.search(TEST_COLLECTION, "*:*", List.of("category_s:even"), null, null, 0, + DOC_COUNT); + assertEquals(DOC_COUNT / 2, evens.numFound(), "Should find 25 even-category documents"); } } 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 4f72424..81777d0 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 @@ -200,6 +200,7 @@ void checkHealth_WithHealthyCollection_ShouldReturnHealthyStatus() throws Except assertNull(result.errorMessage()); assertEquals(10L, result.responseTime()); assertEquals(100L, result.totalDocuments()); + assertEquals("test_collection", result.collection()); } @Test @@ -217,6 +218,7 @@ void checkHealth_WithUnhealthyCollection_ShouldReturnUnhealthyStatus() throws Ex assertTrue(result.errorMessage().contains("Connection failed")); assertNull(result.responseTime()); assertNull(result.totalDocuments()); + assertEquals("unhealthy_collection", result.collection()); } @Test @@ -460,9 +462,9 @@ void getCacheMetrics_EmptyStats() throws Exception { CollectionService spyService = spy(collectionService); doReturn(Arrays.asList("test_collection")).when(spyService).listCollections(); - NamedList mbeans = new NamedList<>(); - mbeans.add("CACHE", new NamedList<>()); - when(solrClient.request(any(SolrRequest.class))).thenReturn(mbeans); + // Metrics response with core metrics that contain no cache keys + NamedList response = wrapInMetricsResponse(new NamedList<>()); + when(solrClient.request(any(SolrRequest.class))).thenReturn(response); CacheStats result = spyService.getCacheMetrics("test_collection"); @@ -474,8 +476,8 @@ void getCacheMetrics_WithShardName() throws Exception { CollectionService spyService = spy(collectionService); doReturn(Arrays.asList("films_shard1_replica_n1")).when(spyService).listCollections(); - NamedList mbeans = createMockCacheData(); - when(solrClient.request(any(SolrRequest.class))).thenReturn(mbeans); + NamedList response = wrapInMetricsResponse(createCacheCoreMetrics(), "films"); + when(solrClient.request(any(SolrRequest.class))).thenReturn(response); CacheStats result = spyService.getCacheMetrics("films_shard1_replica_n1"); @@ -484,11 +486,11 @@ void getCacheMetrics_WithShardName() throws Exception { @Test void extractCacheStats() throws Exception { - NamedList mbeans = createMockCacheData(); + NamedList coreMetrics = createCacheCoreMetrics(); Method method = CollectionService.class.getDeclaredMethod("extractCacheStats", NamedList.class); method.setAccessible(true); - CacheStats result = (CacheStats) method.invoke(collectionService, mbeans); + CacheStats result = (CacheStats) method.invoke(collectionService, coreMetrics); assertNotNull(result.queryResultCache()); assertEquals(100L, result.queryResultCache().lookups()); @@ -497,11 +499,11 @@ void extractCacheStats() throws Exception { @Test void extractCacheStats_AllCacheTypes() throws Exception { - NamedList mbeans = createCompleteMockCacheData(); + NamedList coreMetrics = createCompleteCacheCoreMetrics(); Method method = CollectionService.class.getDeclaredMethod("extractCacheStats", NamedList.class); method.setAccessible(true); - CacheStats result = (CacheStats) method.invoke(collectionService, mbeans); + CacheStats result = (CacheStats) method.invoke(collectionService, coreMetrics); assertNotNull(result.queryResultCache()); assertNotNull(result.documentCache()); @@ -509,14 +511,14 @@ void extractCacheStats_AllCacheTypes() throws Exception { } @Test - void extractCacheStats_NullCacheCategory() throws Exception { - NamedList mbeans = new NamedList<>(); - mbeans.add("CACHE", null); + void extractCacheStats_NoCacheKeys() throws Exception { + // Core metrics with no cache keys at all + NamedList coreMetrics = new NamedList<>(); Method method = CollectionService.class.getDeclaredMethod("extractCacheStats", NamedList.class); method.setAccessible(true); - CacheStats result = (CacheStats) method.invoke(collectionService, mbeans); + CacheStats result = (CacheStats) method.invoke(collectionService, coreMetrics); assertNotNull(result); assertNull(result.queryResultCache()); @@ -552,13 +554,15 @@ void getHandlerMetrics_Success() throws Exception { CollectionService spyService = spy(collectionService); doReturn(Arrays.asList("test_collection")).when(spyService).listCollections(); - NamedList mbeans = createMockHandlerData(); - when(solrClient.request(any(SolrRequest.class))).thenReturn(mbeans); + // getHandlerMetrics makes two fetchMetrics calls (select then update); + // return select handler data for both calls (second has no update keys -> null) + when(solrClient.request(any(SolrRequest.class))).thenReturn(createMockSelectHandlerData()); HandlerStats result = spyService.getHandlerMetrics("test_collection"); assertNotNull(result); assertNotNull(result.selectHandler()); + assertEquals(500L, result.selectHandler().requests()); } @Test @@ -600,9 +604,9 @@ void getHandlerMetrics_EmptyStats() throws Exception { CollectionService spyService = spy(collectionService); doReturn(Arrays.asList("test_collection")).when(spyService).listCollections(); - NamedList mbeans = new NamedList<>(); - mbeans.add("QUERYHANDLER", new NamedList<>()); - when(solrClient.request(any(SolrRequest.class))).thenReturn(mbeans); + // Metrics response with core metrics that contain no handler keys + NamedList response = wrapInMetricsResponse(new NamedList<>()); + when(solrClient.request(any(SolrRequest.class))).thenReturn(response); HandlerStats result = spyService.getHandlerMetrics("test_collection"); @@ -614,8 +618,7 @@ void getHandlerMetrics_WithShardName() throws Exception { CollectionService spyService = spy(collectionService); doReturn(Arrays.asList("films_shard1_replica_n1")).when(spyService).listCollections(); - NamedList mbeans = createMockHandlerData(); - when(solrClient.request(any(SolrRequest.class))).thenReturn(mbeans); + when(solrClient.request(any(SolrRequest.class))).thenReturn(createMockSelectHandlerData("films")); HandlerStats result = spyService.getHandlerMetrics("films_shard1_replica_n1"); @@ -623,44 +626,47 @@ void getHandlerMetrics_WithShardName() throws Exception { } @Test - void extractHandlerStats() throws Exception { - NamedList mbeans = createMockHandlerData(); - Method method = CollectionService.class.getDeclaredMethod("extractHandlerStats", NamedList.class); + void extractFlatHandlerInfo_SelectHandler() throws Exception { + NamedList coreMetrics = createSelectHandlerCoreMetrics(); + Method method = CollectionService.class.getDeclaredMethod("extractFlatHandlerInfo", NamedList.class, + String.class); method.setAccessible(true); - HandlerStats result = (HandlerStats) method.invoke(collectionService, mbeans); + HandlerInfo result = (HandlerInfo) method.invoke(collectionService, coreMetrics, "QUERY./select."); - assertNotNull(result.selectHandler()); - assertEquals(500L, result.selectHandler().requests()); + assertNotNull(result); + assertEquals(500L, result.requests()); + assertEquals(5L, result.errors()); + assertEquals(2L, result.timeouts()); + assertEquals(10000L, result.totalTime()); + // avgTimePerRequest computed: 10000/500 = 20.0 + assertEquals(20.0f, result.avgTimePerRequest()); } @Test - void extractHandlerStats_BothHandlers() throws Exception { - NamedList mbeans = createCompleteHandlerData(); - Method method = CollectionService.class.getDeclaredMethod("extractHandlerStats", NamedList.class); + void extractFlatHandlerInfo_UpdateHandler() throws Exception { + NamedList coreMetrics = createUpdateHandlerCoreMetrics(); + Method method = CollectionService.class.getDeclaredMethod("extractFlatHandlerInfo", NamedList.class, + String.class); method.setAccessible(true); - HandlerStats result = (HandlerStats) method.invoke(collectionService, mbeans); + HandlerInfo result = (HandlerInfo) method.invoke(collectionService, coreMetrics, "UPDATE./update."); - assertNotNull(result.selectHandler()); - assertNotNull(result.updateHandler()); - assertEquals(500L, result.selectHandler().requests()); - assertEquals(250L, result.updateHandler().requests()); + assertNotNull(result); + assertEquals(250L, result.requests()); + assertEquals(2L, result.errors()); } @Test - void extractHandlerStats_NullHandlerCategory() throws Exception { - NamedList mbeans = new NamedList<>(); - mbeans.add("QUERYHANDLER", null); - - Method method = CollectionService.class.getDeclaredMethod("extractHandlerStats", NamedList.class); + void extractFlatHandlerInfo_NoHandlerKeys() throws Exception { + NamedList coreMetrics = new NamedList<>(); + Method method = CollectionService.class.getDeclaredMethod("extractFlatHandlerInfo", NamedList.class, + String.class); method.setAccessible(true); - HandlerStats result = (HandlerStats) method.invoke(collectionService, mbeans); + HandlerInfo result = (HandlerInfo) method.invoke(collectionService, coreMetrics, "QUERY./select."); - assertNotNull(result); - assertNull(result.selectHandler()); - assertNull(result.updateHandler()); + assertNull(result); } @Test @@ -725,121 +731,99 @@ void listCollections_IOError() throws Exception { assertTrue(result.isEmpty()); } - // Helper methods - private NamedList createMockCacheData() { - NamedList mbeans = new NamedList<>(); - NamedList cacheCategory = new NamedList<>(); - NamedList queryResultCache = new NamedList<>(); - NamedList queryStats = new NamedList<>(); + // Helper methods — mock the Solr Metrics API response format: + // response -> "metrics" -> "solr.core." -> "CACHE.searcher.xxx" / + // "HANDLER./xxx" + + // Core metrics builders (unwrapped — used by reflection tests for extract* + // methods) + private NamedList createCacheCoreMetrics() { + NamedList coreMetrics = new NamedList<>(); - queryStats.add("lookups", 100L); - queryStats.add("hits", 80L); - queryStats.add("hitratio", 0.8f); - queryStats.add("inserts", 20L); - queryStats.add("evictions", 5L); - queryStats.add("size", 100L); - queryResultCache.add("stats", queryStats); - cacheCategory.add("queryResultCache", queryResultCache); - mbeans.add("CACHE", cacheCategory); + NamedList queryResultCache = new NamedList<>(); + queryResultCache.add("lookups", 100L); + queryResultCache.add("hits", 80L); + queryResultCache.add("hitratio", 0.8f); + queryResultCache.add("inserts", 20L); + queryResultCache.add("evictions", 5L); + queryResultCache.add("size", 100L); + coreMetrics.add("CACHE.searcher.queryResultCache", queryResultCache); - return mbeans; + return coreMetrics; } - private NamedList createCompleteMockCacheData() { - NamedList mbeans = new NamedList<>(); - NamedList cacheCategory = new NamedList<>(); + private NamedList createCompleteCacheCoreMetrics() { + NamedList coreMetrics = createCacheCoreMetrics(); - // Query Result Cache - NamedList queryResultCache = new NamedList<>(); - NamedList queryStats = new NamedList<>(); - queryStats.add("lookups", 100L); - queryStats.add("hits", 80L); - queryStats.add("hitratio", 0.8f); - queryStats.add("inserts", 20L); - queryStats.add("evictions", 5L); - queryStats.add("size", 100L); - queryResultCache.add("stats", queryStats); - - // Document Cache NamedList documentCache = new NamedList<>(); - NamedList docStats = new NamedList<>(); - docStats.add("lookups", 200L); - docStats.add("hits", 150L); - docStats.add("hitratio", 0.75f); - docStats.add("inserts", 50L); - docStats.add("evictions", 10L); - docStats.add("size", 180L); - documentCache.add("stats", docStats); - - // Filter Cache + documentCache.add("lookups", 200L); + documentCache.add("hits", 150L); + documentCache.add("hitratio", 0.75f); + documentCache.add("inserts", 50L); + documentCache.add("evictions", 10L); + documentCache.add("size", 180L); + coreMetrics.add("CACHE.searcher.documentCache", documentCache); + NamedList filterCache = new NamedList<>(); - NamedList filterStats = new NamedList<>(); - filterStats.add("lookups", 150L); - filterStats.add("hits", 120L); - filterStats.add("hitratio", 0.8f); - filterStats.add("inserts", 30L); - filterStats.add("evictions", 8L); - filterStats.add("size", 140L); - filterCache.add("stats", filterStats); - - cacheCategory.add("queryResultCache", queryResultCache); - cacheCategory.add("documentCache", documentCache); - cacheCategory.add("filterCache", filterCache); - mbeans.add("CACHE", cacheCategory); - - return mbeans; - } - - private NamedList createMockHandlerData() { - NamedList mbeans = new NamedList<>(); - NamedList queryHandlerCategory = new NamedList<>(); - NamedList selectHandler = new NamedList<>(); - NamedList selectStats = new NamedList<>(); - - selectStats.add("requests", 500L); - selectStats.add("errors", 5L); - selectStats.add("timeouts", 2L); - selectStats.add("totalTime", 10000L); - selectStats.add("avgTimePerRequest", 20.0f); - selectStats.add("avgRequestsPerSecond", 25.0f); - selectHandler.add("stats", selectStats); - queryHandlerCategory.add("/select", selectHandler); - mbeans.add("QUERYHANDLER", queryHandlerCategory); - - return mbeans; - } - - private NamedList createCompleteHandlerData() { - NamedList mbeans = new NamedList<>(); - NamedList queryHandlerCategory = new NamedList<>(); - - // Select Handler - NamedList selectHandler = new NamedList<>(); - NamedList selectStats = new NamedList<>(); - selectStats.add("requests", 500L); - selectStats.add("errors", 5L); - selectStats.add("timeouts", 2L); - selectStats.add("totalTime", 10000L); - selectStats.add("avgTimePerRequest", 20.0f); - selectStats.add("avgRequestsPerSecond", 25.0f); - selectHandler.add("stats", selectStats); - - // Update Handler - NamedList updateHandler = new NamedList<>(); - NamedList updateStats = new NamedList<>(); - updateStats.add("requests", 250L); - updateStats.add("errors", 2L); - updateStats.add("timeouts", 1L); - updateStats.add("totalTime", 5000L); - updateStats.add("avgTimePerRequest", 20.0f); - updateStats.add("avgRequestsPerSecond", 50.0f); - updateHandler.add("stats", updateStats); - - queryHandlerCategory.add("/select", selectHandler); - queryHandlerCategory.add("/update", updateHandler); - mbeans.add("QUERYHANDLER", queryHandlerCategory); - - return mbeans; + filterCache.add("lookups", 150L); + filterCache.add("hits", 120L); + filterCache.add("hitratio", 0.8f); + filterCache.add("inserts", 30L); + filterCache.add("evictions", 8L); + filterCache.add("size", 140L); + coreMetrics.add("CACHE.searcher.filterCache", filterCache); + + return coreMetrics; + } + + // Handler metrics use flat keys in the Metrics API (e.g. + // QUERY./select.requests) + private NamedList createSelectHandlerCoreMetrics() { + NamedList coreMetrics = new NamedList<>(); + coreMetrics.add("QUERY./select.requests", 500L); + coreMetrics.add("QUERY./select.errors", 5L); + coreMetrics.add("QUERY./select.timeouts", 2L); + coreMetrics.add("QUERY./select.totalTime", 10000L); + return coreMetrics; + } + + private NamedList createUpdateHandlerCoreMetrics() { + NamedList coreMetrics = new NamedList<>(); + coreMetrics.add("UPDATE./update.requests", 250L); + coreMetrics.add("UPDATE./update.errors", 2L); + coreMetrics.add("UPDATE./update.timeouts", 1L); + coreMetrics.add("UPDATE./update.totalTime", 5000L); + return coreMetrics; + } + + // Wrapped response builders (used by tests that go through + // getCacheMetrics/getHandlerMetrics) + private NamedList createMockCacheData() { + return wrapInMetricsResponse(createCacheCoreMetrics()); + } + + private NamedList createMockSelectHandlerData() { + return wrapInMetricsResponse(createSelectHandlerCoreMetrics()); + } + + private NamedList createMockSelectHandlerData(String collection) { + return wrapInMetricsResponse(createSelectHandlerCoreMetrics(), collection); + } + + private NamedList createMockUpdateHandlerData() { + return wrapInMetricsResponse(createUpdateHandlerCoreMetrics()); + } + + private NamedList wrapInMetricsResponse(NamedList coreMetrics) { + return wrapInMetricsResponse(coreMetrics, "test_collection"); + } + + private NamedList wrapInMetricsResponse(NamedList coreMetrics, String collection) { + NamedList metrics = new NamedList<>(); + metrics.add("solr.core." + collection + ".shard1.replica_n1", coreMetrics); + NamedList response = new NamedList<>(); + response.add("metrics", metrics); + return response; } // createCollection tests 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 a4f9a7d..d4f0a89 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 @@ -123,14 +123,14 @@ void testGetFloat_withNullValue() { NamedList namedList = new NamedList<>(); namedList.add("nullKey", null); - assertEquals(0.0f, CollectionUtils.getFloat(namedList, "nullKey")); + assertNull(CollectionUtils.getFloat(namedList, "nullKey")); } @Test void testGetFloat_withMissingKey() { NamedList namedList = new NamedList<>(); - assertEquals(0.0f, CollectionUtils.getFloat(namedList, "missingKey")); + assertNull(CollectionUtils.getFloat(namedList, "missingKey")); } @Test @@ -173,6 +173,30 @@ void testGetFloat_withBigDecimalValue() { assertEquals(123.45f, CollectionUtils.getFloat(namedList, "bigDecKey"), 0.001f); } + @Test + void testGetFloat_withValidStringValue() { + NamedList namedList = new NamedList<>(); + namedList.add("stringKey", "123.45"); + + assertEquals(123.45f, CollectionUtils.getFloat(namedList, "stringKey"), 0.001f); + } + + @Test + void testGetFloat_withInvalidStringValue() { + NamedList namedList = new NamedList<>(); + namedList.add("invalidStringKey", "not_a_number"); + + assertNull(CollectionUtils.getFloat(namedList, "invalidStringKey")); + } + + @Test + void testGetFloat_withEmptyStringValue() { + NamedList namedList = new NamedList<>(); + namedList.add("emptyStringKey", ""); + + assertNull(CollectionUtils.getFloat(namedList, "emptyStringKey")); + } + @Test void testGetInteger_withNullValue() { NamedList namedList = new NamedList<>();