From fa236cc89ccf7c3be9601075fd00907cab99a1b4 Mon Sep 17 00:00:00 2001 From: Aditya Parikh Date: Sat, 12 Sep 2026 12:39:21 -0400 Subject: [PATCH 1/3] feat(indexing)!: fold per-format inline tools into index-documents Replace index-json-documents, index-csv-documents, index-xml-documents and index-markdown-documents with a single index-documents tool that takes collection, content and a required format (json, csv, xml, markdown or the md alias; case-insensitive). The four per-format methods remain as Java entry points used by tests and by the new tool's dispatch. Why one tool: it mirrors the shape of file ingestion (collection, payload, format) so clients learn one calling convention, keeps a single home for indexing guidance, and removes three near-identical schemas from every session's tool catalog. The format is an explicit argument rather than sniffed: inline payloads have no filename, and CSV and Markdown are both plain text with no safe distinguishing prefix. The index-data prompt now instructs the model to call index-documents with the matching format. README, THREAT_MODEL and the observability test README are updated; the MCP client, sample client, registration and unit tests cover the new tool and assert the old names are gone. BREAKING CHANGE: MCP clients and prompts that call the four per-format indexing tools must call index-documents with a format argument. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019zJ9WA8nNyA5Yxb8ueM7vV Signed-off-by: Aditya Parikh --- README.md | 7 +- THREAT_MODEL.md | 2 +- .../mcp/server/indexing/IndexingService.java | 155 ++++++++++-------- .../server/McpClientIntegrationTestBase.java | 36 ++-- .../mcp/server/McpToolRegistrationTest.java | 31 ++++ .../apache/solr/mcp/server/SampleClient.java | 18 +- .../server/indexing/IndexingServiceTest.java | 84 +++++++++- .../solr/mcp/server/observability/README.md | 4 +- 8 files changed, 227 insertions(+), 110 deletions(-) diff --git a/README.md b/README.md index 0d0c1f7c..3f78a5ce 100644 --- a/README.md +++ b/README.md @@ -96,10 +96,7 @@ Using a different client, or want STDIO/HTTP/Docker options? See the per-client | Tool | Description | |------|-------------| | `search` | Full-text search with filtering, faceting, sorting, and pagination | -| `index-json-documents` | Index documents from a JSON string into a collection | -| `index-csv-documents` | Index documents from a CSV string into a collection | -| `index-xml-documents` | Index documents from an XML string into a collection | -| `index-markdown-documents` | Index a markdown document into a collection, extracting front matter, title, headings, and body text | +| `index-documents` | Index documents supplied inline into a collection; `format` selects `json`, `csv`, `xml` or `markdown` (front matter, title, headings and body extracted) | | `create-collection` | Create a collection (configSet, numShards, replicationFactor optional — default `_default`, `1`, `1`) | | `list-collections` | List all available Solr collections | | `get-collection-stats` | Get statistics and metrics for a collection | @@ -127,7 +124,7 @@ Slash-command-style workflow templates that walk the assistant through a canonic | `setup-collection` | `name`, `purpose` (optional) | Pick configset / shards / replication factor, create the collection, verify it | | `view-schema` | `collection` | Read-only schema walkthrough | | `design-schema` | `collection`, `datasetDescription`, `sampleDocument` (optional) | Choose field types and apply additive schema changes | -| `index-data` | `collection`, `format` (`json` / `csv` / `xml`), `sample` (optional) | Pick the right indexing tool and confirm the result | +| `index-data` | `collection`, `format` (`json` / `csv` / `xml` / `markdown`), `sample` (optional) | Verify the schema, call `index-documents` with the right format, and confirm the result | | `search-collection` | `collection`, `question` | Translate a natural-language question into a Solr query | ### Completions diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index f230fe5c..a088e012 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -315,7 +315,7 @@ Two adversaries are in scope; several are explicitly not. 6. **XML indexing is XXE-hardened.** `XmlDocumentCreator` builds a `DocumentBuilderFactory` with secure processing on, DOCTYPE disallowed, external general/parameter entities off, XInclude off, entity-expansion off. - *Violation:* an XXE/entity-expansion payload in an `index-xml-documents` body + *Violation:* an XXE/entity-expansion payload in an `index-documents` body with `format=xml` reads a local file or hangs the parser. *Severity:* high. *(documented — `XmlDocumentCreator.createSecureDocumentBuilderFactory`.)* 7. **Tool behaviour hints are advertised honestly.** Every tool carries MCP 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 13504967..b4552706 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 @@ -19,6 +19,7 @@ import io.micrometer.observation.annotation.Observed; import java.io.IOException; import java.util.List; +import java.util.Locale; import java.util.Set; import java.util.TreeSet; import java.util.stream.Collectors; @@ -148,14 +149,68 @@ public IndexingService(SolrClient solrClient, IndexingDocumentCreator indexingDo this.indexingDocumentCreator = indexingDocumentCreator; } + /** + * Indexes documents supplied inline as a string, selecting the parser by + * {@code format}. This is the single inline indexing MCP tool; it mirrors the + * shape of file ingestion (collection, payload, format) so clients learn one + * calling convention, and it keeps one home for indexing guidance instead of + * four near-identical tool schemas in every session's catalog. + * + *

+ * The format is an explicit argument rather than sniffed from the content: + * inline payloads have no filename, and CSV and Markdown are both plain text + * with no safe distinguishing prefix. + * + * @param collection + * the name of the Solr collection to index into + * @param content + * the documents, as a string in the given format + * @param format + * one of {@code json}, {@code csv}, {@code xml}, {@code markdown} + * (alias {@code md}); case-insensitive + * @return a summary of how many documents were indexed and the field names as + * indexed + * @throws IllegalArgumentException + * if the format is missing or not one of the accepted values + * @throws IOException + * if there are I/O errors during Solr communication + * @throws SolrServerException + * if there are Solr-specific errors during indexing + * @throws ParserConfigurationException + * if the XML parser cannot be configured + * @throws SAXException + * if the XML content is malformed + */ + @PreAuthorize("isAuthenticated()") + @McpTool( + name = "index-documents", + annotations = @McpTool.McpAnnotations(idempotentHint = true), + description = "Index documents supplied inline into a Solr collection. Set format to json (array of objects or a single object), " + + "csv (first row is the header), xml (Solr or generic elements), or markdown (one document; front matter, " + + "title, headings and body are extracted; supply a stable 'id' in the YAML front matter). " + + "Only convert source content to markdown when it is not already JSON, CSV or XML. " + + "Field names are sanitized for Solr compatibility (lowercased, special characters replaced with underscores); " + + "the response lists the field names as indexed.") + public String indexDocuments(@McpToolParam(description = "Solr collection to index into") String collection, + @McpToolParam(description = "The documents, as a string in the given format") String content, + @McpToolParam(description = "Format of content: json, csv, xml or markdown (alias md)") String format) + throws IOException, SolrServerException, ParserConfigurationException, SAXException { + return switch (normalizeFormat(format)) { + case "json" -> indexJsonDocuments(collection, content); + case "csv" -> indexCsvDocuments(collection, content); + case "xml" -> indexXmlDocuments(collection, content); + default -> indexMarkdownDocuments(collection, content); + }; + } + /** * Indexes documents from a JSON string into a specified Solr collection. * *

* This method serves as the primary entry point for document indexing - * operations and is exposed as an MCP tool for AI client interactions. It - * processes JSON data containing document arrays and indexes them using a - * schema-less approach. + * operations from Java; MCP clients use the {@code index-documents} tool with + * the matching {@code format}. It processes JSON data containing document + * arrays and indexes them using a schema-less approach. * *

* Supported JSON Formats: @@ -210,15 +265,7 @@ public IndexingService(SolrClient solrClient, IndexingDocumentCreator indexingDo * @see #indexDocuments(String, List) */ @PreAuthorize("isAuthenticated()") - @McpTool( - name = "index-json-documents", - annotations = @McpTool.McpAnnotations(idempotentHint = true), - description = "Index documents from json String into Solr collection. Field names are" - + " sanitized for Solr compatibility (lowercased, special characters replaced" - + " with underscores); the response lists the field names as indexed") - public String indexJsonDocuments(@McpToolParam(description = "Solr collection to index into") String collection, - @McpToolParam(description = "JSON string containing documents to index") String json) - throws IOException, SolrServerException { + public String indexJsonDocuments(String collection, String json) throws IOException, SolrServerException { List schemalessDoc = indexingDocumentCreator.createSchemalessDocumentsFromJson(json); int successCount = indexDocuments(collection, schemalessDoc); return "Successfully indexed " + successCount + " of " + schemalessDoc.size() + " documents into collection '" @@ -230,9 +277,9 @@ public String indexJsonDocuments(@McpToolParam(description = "Solr collection to * *

* This method serves as the primary entry point for CSV document indexing - * operations and is exposed as an MCP tool for AI client interactions. It - * processes CSV data with headers and indexes them using a schema-less - * approach. + * operations from Java; MCP clients use the {@code index-documents} tool with + * the matching {@code format}. It processes CSV data with headers and indexes + * them using a schema-less approach. * *

* Supported CSV Formats: @@ -285,15 +332,7 @@ public String indexJsonDocuments(@McpToolParam(description = "Solr collection to * @see #indexDocuments(String, List) */ @PreAuthorize("isAuthenticated()") - @McpTool( - name = "index-csv-documents", - annotations = @McpTool.McpAnnotations(idempotentHint = true), - description = "Index documents from CSV string into Solr collection. Column names are" - + " sanitized for Solr compatibility (lowercased, special characters replaced" - + " with underscores); the response lists the field names as indexed") - public String indexCsvDocuments(@McpToolParam(description = "Solr collection to index into") String collection, - @McpToolParam(description = "CSV string containing documents to index") String csv) - throws IOException, SolrServerException { + public String indexCsvDocuments(String collection, String csv) throws IOException, SolrServerException { List schemalessDoc = indexingDocumentCreator.createSchemalessDocumentsFromCsv(csv); int successCount = indexDocuments(collection, schemalessDoc); return "Successfully indexed " + successCount + " of " + schemalessDoc.size() + " documents into collection '" @@ -305,9 +344,9 @@ public String indexCsvDocuments(@McpToolParam(description = "Solr collection to * *

* This method serves as the primary entry point for XML document indexing - * operations and is exposed as an MCP tool for AI client interactions. It - * processes XML data with nested elements and attributes, indexing them using a - * schema-less approach. + * operations from Java; MCP clients use the {@code index-documents} tool with + * the matching {@code format}. It processes XML data with nested elements and + * attributes, indexing them using a schema-less approach. * *

* Supported XML Formats: @@ -384,14 +423,7 @@ public String indexCsvDocuments(@McpToolParam(description = "Solr collection to * @see #indexDocuments(String, List) */ @PreAuthorize("isAuthenticated()") - @McpTool( - name = "index-xml-documents", - annotations = @McpTool.McpAnnotations(idempotentHint = true), - description = "Index documents from XML string into Solr collection. Element names are" - + " sanitized for Solr compatibility (lowercased, special characters replaced" - + " with underscores); the response lists the field names as indexed") - public String indexXmlDocuments(@McpToolParam(description = "Solr collection to index into") String collection, - @McpToolParam(description = "XML string containing documents to index") String xml) + public String indexXmlDocuments(String collection, String xml) throws ParserConfigurationException, SAXException, IOException, SolrServerException { List schemalessDoc = indexingDocumentCreator.createSchemalessDocumentsFromXml(xml); int successCount = indexDocuments(collection, schemalessDoc); @@ -404,9 +436,10 @@ public String indexXmlDocuments(@McpToolParam(description = "Solr collection to * *

* This method serves as the primary entry point for markdown document indexing - * operations and is exposed as an MCP tool for AI client interactions. Unlike - * the structured formats (JSON, CSV, XML), markdown is a prose format, so - * searchable structure is extracted from the document content itself. + * operations from Java; MCP clients use the {@code index-documents} tool with + * {@code format=markdown}. Unlike the structured formats (JSON, CSV, XML), + * markdown is a prose format, so searchable structure is extracted from the + * document content itself. * *

* Field Extraction: @@ -458,16 +491,7 @@ public String indexXmlDocuments(@McpToolParam(description = "Solr collection to * @see #indexDocuments(String, List) */ @PreAuthorize("isAuthenticated()") - @McpTool( - name = "index-markdown-documents", - annotations = @McpTool.McpAnnotations(idempotentHint = true), - description = "Index a document from markdown String into Solr collection, extracting front matter, title, headings, and body text. " - + "Do NOT use for JSON/CSV/XML input; use index-json-documents, index-csv-documents, or index-xml-documents instead. " - + "Only convert source content to markdown when there is no dedicated tool for the source format, and supply a stable 'id' in the YAML front matter when doing so.") - public String indexMarkdownDocuments(@McpToolParam(description = "Solr collection to index into") String collection, - @McpToolParam( - description = "Markdown string to index, optionally starting with YAML front matter") String markdown) - throws IOException, SolrServerException { + public String indexMarkdownDocuments(String collection, String markdown) throws IOException, SolrServerException { List schemalessDoc = indexingDocumentCreator.createSchemalessDocumentsFromMarkdown(markdown); int successCount = indexDocuments(collection, schemalessDoc); return "Successfully indexed " + successCount + " of " + schemalessDoc.size() + " documents into collection '" @@ -608,19 +632,21 @@ public int indexDocuments(String collection, List documents) } /** - * Maps an input-format keyword to the MCP tool and payload parameter for that - * format. + * Normalizes a user-supplied format keyword to the canonical value accepted by + * {@code index-documents}. + * + * @param format + * {@code json}, {@code csv}, {@code xml}, {@code markdown} or + * {@code md}, in any case, with surrounding whitespace ignored + * @return {@code json}, {@code csv}, {@code xml} or {@code markdown} + * @throws IllegalArgumentException + * if the format is null, blank or unrecognized */ - private record IndexTool(String name, String paramName) { - } - - private static IndexTool resolveIndexTool(String format) { - String normalized = (format == null) ? "" : format.trim().toLowerCase(); + static String normalizeFormat(String format) { + String normalized = (format == null) ? "" : format.trim().toLowerCase(Locale.ROOT); return switch (normalized) { - case "json" -> new IndexTool("index-json-documents", "json"); - case "csv" -> new IndexTool("index-csv-documents", "csv"); - case "xml" -> new IndexTool("index-xml-documents", "xml"); - case "markdown", "md" -> new IndexTool("index-markdown-documents", "markdown"); + case "json", "csv", "xml" -> normalized; + case "markdown", "md" -> "markdown"; default -> throw new IllegalArgumentException("format must be one of json/csv/xml/markdown, got: " + format); }; @@ -659,7 +685,7 @@ public String indexDataPrompt( name = "sample", description = "Optional small sample of the input document(s) to ground field-shape decisions", required = false) String sample) { - IndexTool indexTool = resolveIndexTool(format); + String normalizedFormat = normalizeFormat(format); String sampleSection = PromptText.optionalCodeBlock(sample, "Sample input:", "No sample was provided. If the user has not pasted the documents yet, ask for them (or a representative subset) before indexing."); return """ @@ -677,12 +703,13 @@ public String indexDataPrompt( %s 3. Index the documents. - - Call `%s` with `collection=%s` and `%s=`. + - Call `index-documents` with `collection=%s`, `format=%s` and + `content=`. - The tool batches internally and commits at the end. The return value is the count of successfully indexed documents. - On error, read the message carefully: an "unknown field" error means the schema is missing a field — go back to step 1 and run `design-schema`. A parse error means - the input format does not match the chosen tool — fix the payload and retry. + the input does not match `format` — fix the payload or the format and retry. 4. Verify the count. - Call `check-health` on `%s` and confirm the reported doc count increased by the @@ -691,7 +718,7 @@ public String indexDataPrompt( Next step suggestion: once data is indexed, the `search-collection` prompt drives searching it. - """.formatted(indexTool.paramName(), collection, collection, sampleSection, indexTool.name(), - collection, indexTool.paramName(), collection); + """.formatted(normalizedFormat, collection, collection, sampleSection, collection, normalizedFormat, + collection); } } 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 3b9f2302..60d4d4ff 100644 --- a/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java +++ b/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java @@ -103,8 +103,9 @@ void listToolsReturnsExpectedTools() { List toolNames = toolsResult.tools().stream().map(t -> t.name()).toList(); assertTrue(toolNames.contains("create-collection"), "Should have create-collection tool"); - assertTrue(toolNames.contains("index-json-documents"), "Should have index-json-documents tool"); - assertTrue(toolNames.contains("index-markdown-documents"), "Should have index-markdown-documents tool"); + assertTrue(toolNames.contains("index-documents"), "Should have index-documents tool"); + assertFalse(toolNames.contains("index-json-documents"), + "Per-format indexing tools were folded into index-documents"); assertTrue(toolNames.contains("search"), "Should have search tool"); assertTrue(toolNames.contains("list-collections"), "Should have list-collections tool"); assertTrue(toolNames.contains("check-health"), "Should have check-health tool"); @@ -132,11 +133,8 @@ void toolsExposeBehaviorHints() { assertHint(tools, "create-collection", /* readOnly */ false, /* destructive */ false, /* idempotent */ false); // Indexing: destructive (Solr overwrites by uniqueKey) but idempotent — - // posting the same JSON/CSV/XML twice leaves the index in the same state. - assertHint(tools, "index-json-documents", false, true, true); - assertHint(tools, "index-csv-documents", false, true, true); - assertHint(tools, "index-xml-documents", false, true, true); - assertHint(tools, "index-markdown-documents", false, true, true); + // posting the same payload twice leaves the index in the same state. + assertHint(tools, "index-documents", false, true, true); } private static void assertReadOnly(Map tools, String name) { @@ -192,8 +190,8 @@ void indexJsonDocuments() { ] """; - CallToolResult result = mcpClient - .callTool(new CallToolRequest("index-json-documents", Map.of("collection", COLLECTION, "json", json))); + CallToolResult result = mcpClient.callTool(new CallToolRequest("index-documents", + Map.of("collection", COLLECTION, "content", json, "format", "json"))); assertNotNull(result); assertNotError(result); @@ -329,8 +327,8 @@ void indexCsvDocuments() { 7,CSV Document Two,Frank,csv-test """; - CallToolResult result = mcpClient - .callTool(new CallToolRequest("index-csv-documents", Map.of("collection", COLLECTION, "csv", csv))); + CallToolResult result = mcpClient.callTool(new CallToolRequest("index-documents", + Map.of("collection", COLLECTION, "content", csv, "format", "csv"))); assertNotNull(result); assertNotError(result); @@ -377,8 +375,8 @@ void indexDocumentWithNewFields() { ] """; - CallToolResult result = mcpClient - .callTool(new CallToolRequest("index-json-documents", Map.of("collection", COLLECTION, "json", json))); + CallToolResult result = mcpClient.callTool(new CallToolRequest("index-documents", + Map.of("collection", COLLECTION, "content", json, "format", "json"))); assertNotNull(result); assertNotError(result); @@ -417,8 +415,8 @@ void indexMarkdownDocumentAndFindItById() throws Exception { Index markdown documents through the MCP server. """; - CallToolResult indexResult = mcpClient.callTool(new CallToolRequest("index-markdown-documents", - Map.of("collection", COLLECTION, "markdown", markdown))); + CallToolResult indexResult = mcpClient.callTool(new CallToolRequest("index-documents", + Map.of("collection", COLLECTION, "content", markdown, "format", "markdown"))); assertNotNull(indexResult); assertNotError(indexResult); @@ -539,8 +537,8 @@ void indexShowsFromClasspathResource() throws Exception { String showsJson = loadClasspathResource("/shows.json"); assertFalse(showsJson.isBlank(), "shows.json resource must not be blank"); - CallToolResult result = mcpClient.callTool( - new CallToolRequest("index-json-documents", Map.of("collection", SHOWS_COLLECTION, "json", showsJson))); + CallToolResult result = mcpClient.callTool(new CallToolRequest("index-documents", + Map.of("collection", SHOWS_COLLECTION, "content", showsJson, "format", "json"))); assertNotNull(result); assertNotError(result); @@ -736,8 +734,8 @@ void getIndexDataPromptReturnsGuidance() { new GetPromptRequest("index-data", Map.of("collection", SHOWS_COLLECTION, "format", "json"))); String text = extractFirstMessageText(result); - assertTrue(text.contains("index-json-documents"), - "Prompt body should select index-json-documents for json format: " + text); + assertTrue(text.contains("index-documents") && text.contains("format=json"), + "Prompt body should select index-documents with format=json: " + text); assertTrue(text.contains("get-schema"), "Prompt body should reference get-schema verification: " + text); } 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 3813675f..3a1f5915 100644 --- a/src/test/java/org/apache/solr/mcp/server/McpToolRegistrationTest.java +++ b/src/test/java/org/apache/solr/mcp/server/McpToolRegistrationTest.java @@ -221,6 +221,37 @@ void testCollectionCompletionsCoverSchemaResourceAndCollectionTakingPrompts() { } } + /** + * Inline indexing is one tool with a {@code format} argument rather than one + * tool per format: it mirrors the file-ingestion tool's shape, keeps a single + * home for indexing guidance, and removes three near-identical schemas from + * every session's tool catalog. + */ + @Test + void inlineIndexingIsExposedAsASingleTool() { + List indexingTools = Arrays.stream(IndexingService.class.getDeclaredMethods()) + .filter(m -> m.isAnnotationPresent(McpTool.class)).map(m -> m.getAnnotation(McpTool.class).name()) + .filter(name -> name.startsWith("index-")).sorted().toList(); + + assertEquals(List.of("index-documents"), indexingTools); + } + + @Test + void indexDocumentsToolDeclaresCollectionContentAndFormat() throws NoSuchMethodException { + Method method = IndexingService.class.getMethod("indexDocuments", String.class, String.class, String.class); + McpTool tool = method.getAnnotation(McpTool.class); + assertEquals("index-documents", tool.name()); + + List params = Arrays.stream(method.getParameters()).map(p -> p.getAnnotation(McpToolParam.class)) + .map(a -> a.required() ? "required" : "optional").toList(); + assertEquals(List.of("required", "required", "required"), params, + "collection, content and format must all be required"); + assertTrue( + tool.description().contains("json") && tool.description().contains("csv") + && tool.description().contains("xml") && tool.description().contains("markdown"), + "Description should name every accepted format: " + tool.description()); + } + /** * Invariant: every public MCP entry point — tool, resource, prompt, or * completion — must carry {@code @PreAuthorize}. Annotating a shared helper is diff --git a/src/test/java/org/apache/solr/mcp/server/SampleClient.java b/src/test/java/org/apache/solr/mcp/server/SampleClient.java index f9b5f0f2..a3c28eb4 100644 --- a/src/test/java/org/apache/solr/mcp/server/SampleClient.java +++ b/src/test/java/org/apache/solr/mcp/server/SampleClient.java @@ -156,9 +156,9 @@ public void run() { // bound plus per-name checks so adding a tool does not break this // client - an exact count went stale as soon as create-collection, // add-fields and add-field-types were added. - Set expectedToolNames = Set.of("index-json-documents", "index-csv-documents", - "get-collection-stats", "search", "list-collections", "check-health", "index-xml-documents", - "get-schema", "create-collection", "add-fields", "add-field-types"); + Set expectedToolNames = Set.of("index-documents", "get-collection-stats", "search", + "list-collections", "check-health", "get-schema", "create-collection", "add-fields", + "add-field-types"); assertTrue(toolsList.tools().size() >= expectedToolNames.size(), "Expected at least " + expectedToolNames.size() + " tools, got " + toolsList.tools().size()); @@ -181,13 +181,11 @@ public void run() { // Validate specific tools based on expected behavior switch (tool.name()) { - case "index-json-documents" : - assertTrue(tool.description().toLowerCase().contains("json"), - "JSON indexing tool should mention JSON in" + " description"); - break; - case "index-csv-documents" : - assertTrue(tool.description().toLowerCase().contains("csv"), - "CSV indexing tool should mention CSV in" + " description"); + case "index-documents" : + assertTrue( + tool.description().toLowerCase().contains("json") + && tool.description().toLowerCase().contains("csv"), + "Indexing tool should name the accepted formats in its description"); break; case "search" : // single word, no hyphen needed assertTrue(tool.description().toLowerCase().contains("search"), 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 b1491d16..33c69e14 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 @@ -324,6 +324,75 @@ private List createMockDocuments(int count) { return docs; } + @Test + void indexDocuments_JsonFormat_ParsesWithJsonCreator() throws Exception { + String json = "[{\"id\":\"1\",\"title\":\"Test\"}]"; + when(indexingDocumentCreator.createSchemalessDocumentsFromJson(json)).thenReturn(createMockDocuments(1)); + when(solrClient.add(eq("test_collection"), any(Collection.class))).thenReturn(null); + when(solrClient.commit("test_collection")).thenReturn(null); + + String result = indexingService.indexDocuments("test_collection", json, "json"); + + assertTrue(result.contains("1 of 1"), result); + verify(indexingDocumentCreator).createSchemalessDocumentsFromJson(json); + } + + @Test + void indexDocuments_CsvFormat_ParsesWithCsvCreator() throws Exception { + String csv = "id,title\n1,Test"; + when(indexingDocumentCreator.createSchemalessDocumentsFromCsv(csv)).thenReturn(createMockDocuments(1)); + when(solrClient.add(eq("test_collection"), any(Collection.class))).thenReturn(null); + when(solrClient.commit("test_collection")).thenReturn(null); + + indexingService.indexDocuments("test_collection", csv, "CSV"); + + verify(indexingDocumentCreator).createSchemalessDocumentsFromCsv(csv); + } + + @Test + void indexDocuments_XmlFormat_ParsesWithXmlCreator() throws Exception { + String xml = "1"; + when(indexingDocumentCreator.createSchemalessDocumentsFromXml(xml)).thenReturn(createMockDocuments(1)); + when(solrClient.add(eq("test_collection"), any(Collection.class))).thenReturn(null); + when(solrClient.commit("test_collection")).thenReturn(null); + + indexingService.indexDocuments("test_collection", xml, " xml "); + + verify(indexingDocumentCreator).createSchemalessDocumentsFromXml(xml); + } + + @Test + void indexDocuments_MdAlias_ParsesWithMarkdownCreator() throws Exception { + String markdown = "---\nid: doc-1\n---\n# Title"; + when(indexingDocumentCreator.createSchemalessDocumentsFromMarkdown(markdown)) + .thenReturn(createMockDocuments(1)); + when(solrClient.add(eq("test_collection"), any(Collection.class))).thenReturn(null); + when(solrClient.commit("test_collection")).thenReturn(null); + + indexingService.indexDocuments("test_collection", markdown, "md"); + + verify(indexingDocumentCreator).createSchemalessDocumentsFromMarkdown(markdown); + } + + @Test + void indexDocuments_UnknownFormat_RejectsBeforeTouchingSolr() { + IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, + () -> indexingService.indexDocuments("test_collection", "{}", "yaml")); + + assertTrue(ex.getMessage().contains("json"), ex.getMessage()); + assertTrue(ex.getMessage().contains("markdown"), ex.getMessage()); + verifyNoInteractions(solrClient); + } + + @Test + void indexDocuments_MissingFormat_RejectsBeforeTouchingSolr() { + assertThrows(IllegalArgumentException.class, + () -> indexingService.indexDocuments("test_collection", "{}", null)); + assertThrows(IllegalArgumentException.class, + () -> indexingService.indexDocuments("test_collection", "{}", " ")); + verifyNoInteractions(solrClient); + } + @Test void indexDataPrompt_jsonPath_referencesIndexJsonDocuments() { String sample = """ @@ -332,7 +401,8 @@ void indexDataPrompt_jsonPath_referencesIndexJsonDocuments() { String body = indexingService.indexDataPrompt("library", "json", sample); assertTrue(body.contains("library"), "Prompt should mention the target collection name"); - assertTrue(body.contains("index-json-documents"), "JSON path should reference index-json-documents tool"); + assertTrue(body.contains("index-documents"), "Prompt should reference the index-documents tool"); + assertTrue(body.contains("format=json"), "JSON path should pass format=json: " + body); assertTrue(body.contains("get-schema"), "Prompt should reference get-schema for verification"); assertTrue(body.contains("design-schema"), "Prompt should reference design-schema as fallback when fields are missing"); @@ -343,31 +413,29 @@ void indexDataPrompt_jsonPath_referencesIndexJsonDocuments() { void indexDataPrompt_csvPath_referencesIndexCsvDocuments() { String body = indexingService.indexDataPrompt("library", "csv", null); - assertTrue(body.contains("index-csv-documents"), "CSV path should reference index-csv-documents tool"); - assertFalse(body.contains("index-json-documents"), "CSV path should not reference index-json-documents tool"); + assertTrue(body.contains("format=csv"), "CSV path should pass format=csv: " + body); + assertFalse(body.contains("format=json"), "CSV path should not pass format=json: " + body); } @Test void indexDataPrompt_xmlPath_referencesIndexXmlDocuments() { String body = indexingService.indexDataPrompt("library", "xml", null); - assertTrue(body.contains("index-xml-documents"), "XML path should reference index-xml-documents tool"); + assertTrue(body.contains("format=xml"), "XML path should pass format=xml: " + body); } @Test void indexDataPrompt_markdownPath_referencesIndexMarkdownDocuments() { String body = indexingService.indexDataPrompt("library", "markdown", null); - assertTrue(body.contains("index-markdown-documents"), - "Markdown path should reference index-markdown-documents tool"); + assertTrue(body.contains("format=markdown"), "Markdown path should pass format=markdown: " + body); } @Test void indexDataPrompt_mdAliasResolvesToMarkdownTool() { String body = indexingService.indexDataPrompt("library", "md", null); - assertTrue(body.contains("index-markdown-documents"), - "'md' alias should reference index-markdown-documents tool"); + assertTrue(body.contains("format=markdown"), "'md' alias should resolve to format=markdown: " + body); } @Test diff --git a/src/test/java/org/apache/solr/mcp/server/observability/README.md b/src/test/java/org/apache/solr/mcp/server/observability/README.md index 7f597ec1..88b36559 100644 --- a/src/test/java/org/apache/solr/mcp/server/observability/README.md +++ b/src/test/java/org/apache/solr/mcp/server/observability/README.md @@ -142,9 +142,7 @@ For local development, you can verify tracing works by: All service methods annotated with `@Observed` automatically create spans: - **SearchService.search()** - Search operations -- **IndexingService.indexJsonDocuments()** - Document indexing -- **IndexingService.indexCsvDocuments()** - CSV indexing -- **IndexingService.indexXmlDocuments()** - XML indexing +- **IndexingService.indexDocuments()** - Inline document indexing (JSON, CSV, XML, Markdown) - **CollectionService.listCollections()** - Collection listing - **SchemaService.getSchema()** - Schema retrieval From 0ce479c74eb64d375828c2ff715e35ec42c64ec8 Mon Sep 17 00:00:00 2001 From: Aditya Parikh Date: Sat, 12 Sep 2026 13:51:00 -0400 Subject: [PATCH 2/3] style(indexing): write the index-documents description as a text block Match the text-block style SearchService already uses instead of a concatenated string. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019zJ9WA8nNyA5Yxb8ueM7vV Signed-off-by: Aditya Parikh --- .../mcp/server/indexing/IndexingService.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) 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 b4552706..5984b9a8 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 @@ -182,15 +182,14 @@ public IndexingService(SolrClient solrClient, IndexingDocumentCreator indexingDo * if the XML content is malformed */ @PreAuthorize("isAuthenticated()") - @McpTool( - name = "index-documents", - annotations = @McpTool.McpAnnotations(idempotentHint = true), - description = "Index documents supplied inline into a Solr collection. Set format to json (array of objects or a single object), " - + "csv (first row is the header), xml (Solr or generic elements), or markdown (one document; front matter, " - + "title, headings and body are extracted; supply a stable 'id' in the YAML front matter). " - + "Only convert source content to markdown when it is not already JSON, CSV or XML. " - + "Field names are sanitized for Solr compatibility (lowercased, special characters replaced with underscores); " - + "the response lists the field names as indexed.") + @McpTool(name = "index-documents", annotations = @McpTool.McpAnnotations(idempotentHint = true), description = """ + Index documents supplied inline into a Solr collection. Set format to json (an array of objects + or a single object), csv (first row is the header), xml (Solr or generic elements), + or markdown (one document; front matter, title, headings and body are extracted; supply a + stable 'id' in the YAML front matter). Only convert source content to markdown when it is not + already JSON, CSV or XML. Field names are sanitized for Solr compatibility (lowercased, special + characters replaced with underscores); the response lists the field names as indexed. + """) public String indexDocuments(@McpToolParam(description = "Solr collection to index into") String collection, @McpToolParam(description = "The documents, as a string in the given format") String content, @McpToolParam(description = "Format of content: json, csv, xml or markdown (alias md)") String format) From 6640ea4c9f5ff27acb5f706e8ecacd6e58b046b3 Mon Sep 17 00:00:00 2001 From: Aditya Parikh Date: Sat, 12 Sep 2026 23:10:46 -0400 Subject: [PATCH 3/3] refactor(indexing): drop dead PreAuthorize on the per-format helpers The four per-format methods are now only self-invoked from indexDocuments, so their @PreAuthorize never ran; the gate on the tool method is the real one. Also: no commit stubs in the new format tests (mocks return null anyway, and strict stubs would flag them once the commit overload changes), prompt tests renamed to match what they assert, one duplicated description assertion removed (SampleClient covers it), the tool javadoc halved, and THREAT_MODEL's tool counts updated for the single index tool. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01CiUHyyXLTo9ATdgg8eRFZJ Signed-off-by: Aditya Parikh --- THREAT_MODEL.md | 6 +++--- .../mcp/server/indexing/IndexingService.java | 17 ++++------------- .../mcp/server/McpToolRegistrationTest.java | 6 +----- .../server/indexing/IndexingServiceTest.java | 14 +++++--------- 4 files changed, 13 insertions(+), 30 deletions(-) diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index a088e012..ba0773f3 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -61,7 +61,7 @@ it speaks MCP (JSON-RPC) to an AI client over one of two transports — **STDIO* (streamable-HTTP; a network listener). On the other side it speaks SolrJ HTTP to **one** backend Solr instance whose location and credentials the operator fixes at startup via environment (`SOLR_URL`, optional `SOLR_USERNAME`/`SOLR_PASSWORD`). -It exposes eleven tools (search, three indexing formats, collection create/list/ +It exposes nine tools (search, inline indexing, collection create/list/ stats/health, schema get/add-fields/add-field-types), two resources (`solr://collections`, `solr://{collection}/schema`), and prompt/completion helpers. It translates natural-language requests — as structured by the calling @@ -245,7 +245,7 @@ trust table: | `search` | `collection` | **yes** | used only as a path segment against the fixed `SOLR_URL` base; **cannot redirect to another host**. What a path reaches *within* that Solr is the backend's authorization call. *(maintainer — Q-collection.)* | | `search` | `query` (`q`), `filterQueries` (`fq`) | **yes** | passed into `SolrQuery`; Solr query-parser semantics apply — Q-queryinj | | `search` | `facetFields`, `sortClauses`, `start`, `rows` | **yes** | forwarded to Solr; `rows` unbounded? — Q-resource | -| `index-*` | `collection`, `json`/`csv`/`xml` body | **yes** | parsed then written to index; XML parser is XXE-hardened *(documented)* | +| `index-documents` | `collection`, `content`, `format` | **yes** | parsed then written to index; XML parser is XXE-hardened *(documented)* | | `create-collection` | `name`, `configSet`, `numShards`, `replicationFactor` | **yes** | issues `CollectionAdminRequest.createCollection` to backend — Q-adminexposure | | `add-fields` / `add-field-types` | `collection`, field/type defs | **yes** | additive schema change (existing fields cannot be modified per README) | | config (startup only) | `SOLR_URL`, `SOLR_USERNAME`, `SOLR_PASSWORD` | **no — deployer config** | never wire from a tool argument *(documented)* | @@ -320,7 +320,7 @@ Two adversaries are in scope; several are explicitly not. `XmlDocumentCreator.createSecureDocumentBuilderFactory`.)* 7. **Tool behaviour hints are advertised honestly.** Every tool carries MCP annotations (`readOnlyHint` on the five read tools, `idempotentHint` on the - three index tools, `destructiveHint=false` on schema/create tools) so clients + index tool, `destructiveHint=false` on schema/create tools) so clients can build approval UX. *Violation:* a tool that mutates state advertises `readOnlyHint=true`. *Severity:* medium (client-UX safety). *(documented — README; `@McpTool.McpAnnotations` on each service method.)* 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 5984b9a8..581d6c68 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 @@ -151,15 +151,10 @@ public IndexingService(SolrClient solrClient, IndexingDocumentCreator indexingDo /** * Indexes documents supplied inline as a string, selecting the parser by - * {@code format}. This is the single inline indexing MCP tool; it mirrors the - * shape of file ingestion (collection, payload, format) so clients learn one - * calling convention, and it keeps one home for indexing guidance instead of - * four near-identical tool schemas in every session's catalog. - * - *

- * The format is an explicit argument rather than sniffed from the content: - * inline payloads have no filename, and CSV and Markdown are both plain text - * with no safe distinguishing prefix. + * {@code format}. One tool with a format argument replaces four near-identical + * tool schemas in every session's catalog. The format is explicit rather than + * sniffed: inline payloads have no filename, and CSV and Markdown are both + * plain text with no safe distinguishing prefix. * * @param collection * the name of the Solr collection to index into @@ -263,7 +258,6 @@ public String indexDocuments(@McpToolParam(description = "Solr collection to ind * @see IndexingDocumentCreator#createSchemalessDocumentsFromJson(String) * @see #indexDocuments(String, List) */ - @PreAuthorize("isAuthenticated()") public String indexJsonDocuments(String collection, String json) throws IOException, SolrServerException { List schemalessDoc = indexingDocumentCreator.createSchemalessDocumentsFromJson(json); int successCount = indexDocuments(collection, schemalessDoc); @@ -330,7 +324,6 @@ public String indexJsonDocuments(String collection, String json) throws IOExcept * @see IndexingDocumentCreator#createSchemalessDocumentsFromCsv(String) * @see #indexDocuments(String, List) */ - @PreAuthorize("isAuthenticated()") public String indexCsvDocuments(String collection, String csv) throws IOException, SolrServerException { List schemalessDoc = indexingDocumentCreator.createSchemalessDocumentsFromCsv(csv); int successCount = indexDocuments(collection, schemalessDoc); @@ -421,7 +414,6 @@ public String indexCsvDocuments(String collection, String csv) throws IOExceptio * @see IndexingDocumentCreator#createSchemalessDocumentsFromXml(String) * @see #indexDocuments(String, List) */ - @PreAuthorize("isAuthenticated()") public String indexXmlDocuments(String collection, String xml) throws ParserConfigurationException, SAXException, IOException, SolrServerException { List schemalessDoc = indexingDocumentCreator.createSchemalessDocumentsFromXml(xml); @@ -489,7 +481,6 @@ public String indexXmlDocuments(String collection, String xml) * @see IndexingDocumentCreator#createSchemalessDocumentsFromMarkdown(String) * @see #indexDocuments(String, List) */ - @PreAuthorize("isAuthenticated()") public String indexMarkdownDocuments(String collection, String markdown) throws IOException, SolrServerException { List schemalessDoc = indexingDocumentCreator.createSchemalessDocumentsFromMarkdown(markdown); int successCount = indexDocuments(collection, schemalessDoc); 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 3a1f5915..55ec38f5 100644 --- a/src/test/java/org/apache/solr/mcp/server/McpToolRegistrationTest.java +++ b/src/test/java/org/apache/solr/mcp/server/McpToolRegistrationTest.java @@ -237,7 +237,7 @@ void inlineIndexingIsExposedAsASingleTool() { } @Test - void indexDocumentsToolDeclaresCollectionContentAndFormat() throws NoSuchMethodException { + void indexDocumentsRequiresCollectionContentAndFormat() throws NoSuchMethodException { Method method = IndexingService.class.getMethod("indexDocuments", String.class, String.class, String.class); McpTool tool = method.getAnnotation(McpTool.class); assertEquals("index-documents", tool.name()); @@ -246,10 +246,6 @@ void indexDocumentsToolDeclaresCollectionContentAndFormat() throws NoSuchMethodE .map(a -> a.required() ? "required" : "optional").toList(); assertEquals(List.of("required", "required", "required"), params, "collection, content and format must all be required"); - assertTrue( - tool.description().contains("json") && tool.description().contains("csv") - && tool.description().contains("xml") && tool.description().contains("markdown"), - "Description should name every accepted format: " + tool.description()); } /** 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 33c69e14..05e7e3db 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 @@ -329,7 +329,6 @@ void indexDocuments_JsonFormat_ParsesWithJsonCreator() throws Exception { String json = "[{\"id\":\"1\",\"title\":\"Test\"}]"; when(indexingDocumentCreator.createSchemalessDocumentsFromJson(json)).thenReturn(createMockDocuments(1)); when(solrClient.add(eq("test_collection"), any(Collection.class))).thenReturn(null); - when(solrClient.commit("test_collection")).thenReturn(null); String result = indexingService.indexDocuments("test_collection", json, "json"); @@ -342,7 +341,6 @@ void indexDocuments_CsvFormat_ParsesWithCsvCreator() throws Exception { String csv = "id,title\n1,Test"; when(indexingDocumentCreator.createSchemalessDocumentsFromCsv(csv)).thenReturn(createMockDocuments(1)); when(solrClient.add(eq("test_collection"), any(Collection.class))).thenReturn(null); - when(solrClient.commit("test_collection")).thenReturn(null); indexingService.indexDocuments("test_collection", csv, "CSV"); @@ -354,7 +352,6 @@ void indexDocuments_XmlFormat_ParsesWithXmlCreator() throws Exception { String xml = "1"; when(indexingDocumentCreator.createSchemalessDocumentsFromXml(xml)).thenReturn(createMockDocuments(1)); when(solrClient.add(eq("test_collection"), any(Collection.class))).thenReturn(null); - when(solrClient.commit("test_collection")).thenReturn(null); indexingService.indexDocuments("test_collection", xml, " xml "); @@ -367,7 +364,6 @@ void indexDocuments_MdAlias_ParsesWithMarkdownCreator() throws Exception { when(indexingDocumentCreator.createSchemalessDocumentsFromMarkdown(markdown)) .thenReturn(createMockDocuments(1)); when(solrClient.add(eq("test_collection"), any(Collection.class))).thenReturn(null); - when(solrClient.commit("test_collection")).thenReturn(null); indexingService.indexDocuments("test_collection", markdown, "md"); @@ -394,7 +390,7 @@ void indexDocuments_MissingFormat_RejectsBeforeTouchingSolr() { } @Test - void indexDataPrompt_jsonPath_referencesIndexJsonDocuments() { + void indexDataPrompt_jsonPath_passesFormatJson() { String sample = """ [{"id":"1","title":"Test"}]"""; @@ -410,7 +406,7 @@ void indexDataPrompt_jsonPath_referencesIndexJsonDocuments() { } @Test - void indexDataPrompt_csvPath_referencesIndexCsvDocuments() { + void indexDataPrompt_csvPath_passesFormatCsv() { String body = indexingService.indexDataPrompt("library", "csv", null); assertTrue(body.contains("format=csv"), "CSV path should pass format=csv: " + body); @@ -418,21 +414,21 @@ void indexDataPrompt_csvPath_referencesIndexCsvDocuments() { } @Test - void indexDataPrompt_xmlPath_referencesIndexXmlDocuments() { + void indexDataPrompt_xmlPath_passesFormatXml() { String body = indexingService.indexDataPrompt("library", "xml", null); assertTrue(body.contains("format=xml"), "XML path should pass format=xml: " + body); } @Test - void indexDataPrompt_markdownPath_referencesIndexMarkdownDocuments() { + void indexDataPrompt_markdownPath_passesFormatMarkdown() { String body = indexingService.indexDataPrompt("library", "markdown", null); assertTrue(body.contains("format=markdown"), "Markdown path should pass format=markdown: " + body); } @Test - void indexDataPrompt_mdAliasResolvesToMarkdownTool() { + void indexDataPrompt_mdAlias_passesFormatMarkdown() { String body = indexingService.indexDataPrompt("library", "md", null); assertTrue(body.contains("format=markdown"), "'md' alias should resolve to format=markdown: " + body);