Copilot commented on code in PR #205:
URL: https://github.com/apache/solr-mcp/pull/205#discussion_r4027525574
##########
src/main/java/org/apache/solr/mcp/server/indexing/IndexingService.java:
##########
@@ -198,209 +201,124 @@ public IndexingService(SolrClient solrClient,
IndexingDocumentCreator indexingDo
*
* @param collection
* the name of the Solr collection to index documents into
- * @param documents
- * the documents to index, one map per document
+ * @param json
+ * JSON string containing an array of documents to index
* @return a human-readable summary reporting how many documents were
* successfully indexed
* @throws IOException
* if there are critical errors in JSON parsing or Solr
* communication
* @throws SolrServerException
* if Solr server encounters errors during indexing
- * @see IndexingDocumentCreator#createSchemalessDocumentsFromJson(List)
+ * @see
IndexingDocumentCreator#createSchemalessDocumentsFromJson(String)
* @see #indexDocuments(String, List)
*/
@PreAuthorize("isAuthenticated()")
@McpTool(
name = "index-json-documents",
annotations = @McpTool.McpAnnotations(idempotentHint =
true),
- description = "Index documents passed as a JSON array
of objects into Solr collection; one object"
- + " per document, multi-valued fields
as arrays, nested objects flattened with underscores."
- + " Pass the array itself, not a JSON
string. Field names are sanitized for Solr"
- + " compatibility (lowercased, special
characters replaced with underscores); the response"
- + " lists the field names as indexed")
+ 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 = "Documents to index: a
JSON array with one object per document") List<Map<String, Object>> documents)
+ @McpToolParam(description = "JSON string containing
documents to index") String json)
throws IOException, SolrServerException {
- List<SolrInputDocument> schemalessDoc =
indexingDocumentCreator.createSchemalessDocumentsFromJson(documents);
+ List<SolrInputDocument> schemalessDoc =
indexingDocumentCreator.createSchemalessDocumentsFromJson(json);
int successCount = indexDocuments(collection, schemalessDoc);
return "Successfully indexed " + successCount + " of " +
schemalessDoc.size() + " documents into collection '"
+ collection + "'" +
describeIndexedFields(schemalessDoc);
}
/**
- * Indexes documents from a CSV string into a specified Solr collection.
- *
- * <p>
- * 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.
- *
- * <p>
- * <strong>Supported CSV Formats:</strong>
- *
- * <ul>
- * <li><strong>Header Row Required</strong>: First row must contain
column names
- * <li><strong>Comma Delimited</strong>: Standard CSV format with comma
- * separators
- * <li><strong>Mixed Data Types</strong>: Automatic type detection by
Solr
- * </ul>
- *
- * <p>
- * <strong>Processing Workflow:</strong>
- *
- * <ol>
- * <li>Parse CSV string to extract headers and data rows
- * <li>Convert to schema-less SolrInputDocument objects
- * <li>Execute batch indexing with error handling
- * <li>Commit changes to make documents searchable
- * </ol>
- *
- * <p>
- * <strong>MCP Tool Usage:</strong>
- *
- * <p>
- * AI clients can invoke this method with natural language requests
like "index
- * this CSV data into my_collection" or "add these CSV records to the
search
- * index".
- *
- * <p>
- * <strong>Error Handling:</strong>
- *
- * <p>
- * If indexing fails, the method attempts individual document
processing to
- * maximize the number of successfully indexed documents. Detailed error
- * information is logged for troubleshooting purposes.
+ * Indexes CSV rows into a Solr collection by forwarding the payload,
as given,
+ * to Solr's own CSV update handler. Solr reads the header row for the
field
+ * names and parses the rows; the server does not inspect the payload.
A column
+ * name repeated in the header yields a multi-valued field, and empty
cells are
+ * skipped. Solr accepts or rejects the payload as a whole.
*
* @param collection
* the name of the Solr collection to index documents into
* @param csv
- * CSV string containing documents to index (first row must
be
- * headers)
- * @return a human-readable summary reporting how many documents were
- * successfully indexed
+ * CSV text with a header row
+ * @return a human-readable confirmation that Solr accepted and
committed the
+ * payload
* @throws IOException
- * if there are critical errors in CSV parsing or Solr
communication
+ * if communication with Solr fails
* @throws SolrServerException
- * if Solr server encounters errors during indexing
- * @see IndexingDocumentCreator#createSchemalessDocumentsFromCsv(String)
- * @see #indexDocuments(String, List)
+ * if Solr rejects the payload
*/
@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")
+ description = "Index documents from CSV string into
Solr collection via Solr's CSV handler. The first row"
+ + " is the header and its column names
are used as the field names, as given; repeat a column name"
+ + " to make that field multi-valued;
empty cells are skipped")
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 {
- List<SolrInputDocument> schemalessDoc =
indexingDocumentCreator.createSchemalessDocumentsFromCsv(csv);
- int successCount = indexDocuments(collection, schemalessDoc);
- return "Successfully indexed " + successCount + " of " +
schemalessDoc.size() + " documents into collection '"
- + collection + "'" +
describeIndexedFields(schemalessDoc);
+ ContentStreamUpdateRequest request = new
ContentStreamUpdateRequest("/update");
+ request.setParam("header", "true");
+ request.addContentStream(new
ContentStreamBase.StringStream(csv, "text/csv; charset=UTF-8"));
+ return forward(collection, request, "CSV payload");
}
/**
- * Indexes documents from an XML string into a specified Solr
collection.
- *
- * <p>
- * 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.
- *
- * <p>
- * <strong>Supported XML Formats:</strong>
- *
- * <ul>
- * <li><strong>Single Document</strong>: Root element treated as one
document
- * <li><strong>Multiple Documents</strong>: Child elements with 'doc',
'item',
- * or 'record' names treated as separate documents
- * <li><strong>Nested Elements</strong>: Automatically flattened with
underscore
- * notation
- * <li><strong>Attributes</strong>: Converted to fields with "_attr"
suffix
- * <li><strong>Mixed Data Types</strong>: Automatic type detection by
Solr
- * </ul>
- *
- * <p>
- * <strong>Processing Workflow:</strong>
- *
- * <ol>
- * <li>Parse XML string to extract elements and attributes
- * <li>Flatten nested structures using underscore notation
- * <li>Convert to schema-less SolrInputDocument objects
- * <li>Execute batch indexing with error handling
- * <li>Commit changes to make documents searchable
- * </ol>
- *
- * <p>
- * <strong>MCP Tool Usage:</strong>
- *
- * <p>
- * AI clients can invoke this method with natural language requests
like "index
- * this XML data into my_collection" or "add these XML records to the
search
- * index".
- *
- * <p>
- * <strong>Error Handling:</strong>
- *
- * <p>
- * If indexing fails, the method attempts individual document
processing to
- * maximize the number of successfully indexed documents. Detailed error
- * information is logged for troubleshooting purposes.
- *
- * <p>
- * <strong>Example XML Processing:</strong>
- *
- * <pre>{@code
- * Input:
- * <documents>
- * <document id="1">
- * <title>Sample</title>
- * <author>
- * <name>John Doe</name>
- * </author>
- * </document>
- * </documents>
- *
- * Result: {id_attr:"1", title:"Sample", author_name:"John Doe"}
- * }</pre>
+ * Indexes documents supplied in Solr's update XML format
+ * ({@code <add><doc><field name="...">...</field></doc></add>}) by
forwarding
+ * the payload to Solr's update handler. The only server-side step is
+ * {@link SolrUpdateXml}: the same grammar carries {@code <delete>} and
+ * {@code <commit>} commands that an indexing tool must not forward, so
the root
+ * element must be {@code <add>}. Solr parses the payload and accepts
or rejects
+ * it as a whole.
*
* @param collection
* the name of the Solr collection to index documents into
* @param xml
- * XML string containing documents to index
- * @return a human-readable summary reporting how many documents were
- * successfully indexed
- * @throws ParserConfigurationException
- * if XML parser configuration fails
- * @throws SAXException
- * if XML parsing fails due to malformed content
+ * a Solr {@code <add>} block
+ * @return a human-readable confirmation that Solr accepted and
committed the
+ * payload
* @throws IOException
- * if I/O errors occur during parsing or Solr communication
+ * if communication with Solr fails
* @throws SolrServerException
- * if Solr server encounters errors during indexing
- * @see IndexingDocumentCreator#createSchemalessDocumentsFromXml(String)
- * @see #indexDocuments(String, List)
+ * if Solr rejects the payload
*/
@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")
+ description = "Index documents from Solr update XML
into Solr collection: <add><doc><field name=\"id\">1</field>"
+ + "<field
name=\"genres\">a</field><field name=\"genres\">b</field></doc></add>; repeat
<field> for"
+ + " multi-valued fields. Only <add>
blocks are accepted; delete and commit commands are rejected."
+ + " Field names are used as given")
public String indexXmlDocuments(@McpToolParam(description = "Solr
collection to index into") String collection,
- @McpToolParam(description = "XML string containing
documents to index") String xml)
- throws ParserConfigurationException, SAXException,
IOException, SolrServerException {
- List<SolrInputDocument> schemalessDoc =
indexingDocumentCreator.createSchemalessDocumentsFromXml(xml);
- int successCount = indexDocuments(collection, schemalessDoc);
- return "Successfully indexed " + successCount + " of " +
schemalessDoc.size() + " documents into collection '"
- + collection + "'" +
describeIndexedFields(schemalessDoc);
+ @McpToolParam(description = "Solr update XML: an <add>
block of <doc> elements") String xml)
+ throws IOException, SolrServerException {
+ SolrUpdateXml.requireAddBlock(xml);
+ ContentStreamUpdateRequest request = new
ContentStreamUpdateRequest("/update");
+ request.addContentStream(new
ContentStreamBase.StringStream(xml, ClientUtils.TEXT_XML));
+ return forward(collection, request, "XML <add> block");
+ }
+
+ /**
+ * Sends a payload to a Solr update handler and reports Solr's answer.
The
+ * commit rides along on the same request rather than following as a
second
+ * round trip, so the status and query time reported here cover the
commit this
+ * message claims. Solr's update response carries no document count, so
none is
+ * claimed.
+ *
+ * <p>
+ * The commit is a soft one: {@code waitSearcher} keeps the documents
searchable
+ * the moment the tool returns, while the segment fsync is left to
Solr's
+ * {@code autoCommit}, which the {@code _default} configset enables at
15 s, so
+ * many small calls do not each force one.
+ */
+ private String forward(String collection, ContentStreamUpdateRequest
request, String payload)
+ throws IOException, SolrServerException {
+ request.setAction(AbstractUpdateRequest.ACTION.COMMIT, false,
true, true);
+ UpdateResponse response = request.process(solrClient,
collection);
+ return "Solr accepted the " + payload + " for collection '" +
collection + "' and committed it (status "
+ + response.getStatus() + ", " +
response.getQTime() + " ms)";
}
Review Comment:
This method unconditionally reports “Solr accepted … and committed it” even
if the update response indicates failure (non-zero status). To keep the message
accurate and avoid false positives, check `response.getStatus()` and throw an
exception (or return an error message) when the status is non-zero.
##########
src/test/java/org/apache/solr/mcp/server/indexing/IndexingServiceTest.java:
##########
@@ -296,19 +214,73 @@ void
indexJsonDocuments_WhenSolrClientThrowsException_ShouldPropagateException()
}
@Test
- void
indexCsvDocuments_WhenSolrClientThrowsIOException_ShouldPropagateException()
throws Exception {
- String csv = "id,title\n1,Test";
- List<SolrInputDocument> mockDocs = createMockDocuments(1);
-
when(indexingDocumentCreator.createSchemalessDocumentsFromCsv(csv)).thenReturn(mockDocs);
- when(solrClient.add(eq("test_collection"),
any(List.class))).thenThrow(new IOException("Network error"));
- when(solrClient.add(eq("test_collection"),
any(SolrInputDocument.class)))
- .thenThrow(new IOException("Network error"));
- when(solrClient.commit("test_collection", false, true,
true)).thenReturn(null);
+ void indexCsvDocuments_ForwardsPayloadAsGivenToSolrCsvHandler() throws
Exception {
+ when(solrClient.request(any(ContentStreamUpdateRequest.class),
eq("test_collection")))
+ .thenReturn(new NamedList<>());
+ String csv = "id,Show Title,genres,genres\n1,A,x,y\n2,B,z,\n";
+
+ String result =
indexingService.indexCsvDocuments("test_collection", csv);
+
+ ArgumentCaptor<ContentStreamUpdateRequest> captor =
ArgumentCaptor.forClass(ContentStreamUpdateRequest.class);
+ verify(solrClient).request(captor.capture(),
eq("test_collection"));
+ assertEquals("/update", captor.getValue().getPath());
+ assertEquals("true",
captor.getValue().getParams().get("header"));
+ assertNull(captor.getValue().getParams().get("fieldnames"),
"column names are Solr's to read, as given");
+ var stream =
captor.getValue().getContentStreams().iterator().next();
+ assertTrue(stream.getContentType().startsWith("text/csv"));
+ assertEquals(csv, new String(stream.getStream().readAllBytes(),
StandardCharsets.UTF_8));
+ assertEquals("true",
captor.getValue().getParams().get("commit"),
+ "the commit rides on the update request, not a
second round trip");
+ assertEquals("true",
captor.getValue().getParams().get("softCommit"),
+ "soft commit: searchable on return, the segment
fsync left to Solr's autoCommit");
+ verify(solrClient, never()).commit(anyString());
+ assertTrue(result.contains("Solr accepted the CSV payload"),
result);
+ assertTrue(result.contains("'test_collection'"), result);
+ assertFalse(result.contains(" of "), "no document count is
claimed: Solr does not report one: " + result);
+ }
- indexingService.indexCsvDocuments("test_collection", csv);
+ @Test
+ void indexXmlDocuments_ForwardsAddBlockToSolr() throws Exception {
+ when(solrClient.request(any(ContentStreamUpdateRequest.class),
eq("test_collection")))
+ .thenReturn(new NamedList<>());
+ String xml = "<add><doc><field name=\"id\">1</field><field
name=\"title\">T</field></doc></add>";
+
+ String result =
indexingService.indexXmlDocuments("test_collection", xml);
+
+ ArgumentCaptor<ContentStreamUpdateRequest> captor =
ArgumentCaptor.forClass(ContentStreamUpdateRequest.class);
+ verify(solrClient).request(captor.capture(),
eq("test_collection"));
+ assertTrue(
+
captor.getValue().getContentStreams().iterator().next().getContentType().startsWith("application/xml"));
+ assertEquals("true",
captor.getValue().getParams().get("commit"),
+ "the commit rides on the update request, not a
second round trip");
+ assertEquals("true",
captor.getValue().getParams().get("softCommit"),
+ "soft commit: searchable on return, the segment
fsync left to Solr's autoCommit");
+ verify(solrClient, never()).commit(anyString());
+ assertTrue(result.contains("Solr accepted the XML <add>
block"), result);
+ assertTrue(result.contains("'test_collection'"), result);
+ }
Review Comment:
The XML payload in this test is not well-formed (`<field name=\"title\">` is
never closed). This can let the test pass while exercising an impossible
success case (since Solr would reject malformed XML). Update the test string to
valid Solr update XML so the forwarding path is tested realistically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
