diff --git a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/CsvDocumentCreator.java b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/CsvDocumentCreator.java index 6c0a6642..037417b0 100644 --- a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/CsvDocumentCreator.java +++ b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/CsvDocumentCreator.java @@ -109,9 +109,8 @@ public CsvDocumentCreator() { * @see FieldNameSanitizer#sanitizeFieldName(String) */ public List create(String csv) throws DocumentProcessingException { - if (csv.isBlank()) { - throw new DocumentProcessingException("CSV input cannot be empty"); - } + SolrDocumentCreator.requireContent(csv, "CSV"); + if (csv.getBytes(StandardCharsets.UTF_8).length > MAX_INPUT_SIZE_BYTES) { throw new DocumentProcessingException( "Input too large: exceeds maximum size of " + MAX_INPUT_SIZE_BYTES + " bytes"); diff --git a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/IndexingDocumentCreator.java b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/IndexingDocumentCreator.java index 31e3b91f..01b4a3b9 100644 --- a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/IndexingDocumentCreator.java +++ b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/IndexingDocumentCreator.java @@ -131,10 +131,10 @@ public List createSchemalessDocumentsFromCsv(String csv) thro */ public List createSchemalessDocumentsFromXml(String xml) throws DocumentProcessingException { - // Input validation - if (xml == null || xml.trim().isEmpty()) { - throw new DocumentProcessingException("XML input cannot be null or empty"); - } + // Must run before the size check below, which would NPE on null input. + // XmlDocumentCreator.create repeats it so the contract holds for direct + // callers too; the shared helper keeps the message identical either way. + SolrDocumentCreator.requireContent(xml, "XML"); byte[] xmlBytes = xml.getBytes(StandardCharsets.UTF_8); if (xmlBytes.length > MAX_XML_SIZE_BYTES) { diff --git a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/JsonDocumentCreator.java b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/JsonDocumentCreator.java index 605e5204..ab16e5c2 100644 --- a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/JsonDocumentCreator.java +++ b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/JsonDocumentCreator.java @@ -116,9 +116,8 @@ public JsonDocumentCreator(ObjectMapper objectMapper) { * @see FieldNameSanitizer#sanitizeFieldName(String) */ public List create(String json) throws DocumentProcessingException { - if (json.isBlank()) { - throw new DocumentProcessingException("JSON input cannot be empty"); - } + SolrDocumentCreator.requireContent(json, "JSON"); + if (json.getBytes(StandardCharsets.UTF_8).length > MAX_INPUT_SIZE_BYTES) { throw new DocumentProcessingException( "Input too large: exceeds maximum size of " + MAX_INPUT_SIZE_BYTES + " bytes"); diff --git a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/SolrDocumentCreator.java b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/SolrDocumentCreator.java index 5761fed3..a0f8e728 100644 --- a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/SolrDocumentCreator.java +++ b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/SolrDocumentCreator.java @@ -46,7 +46,7 @@ * Implementation Guidelines: * *
    - *
  • Handle null or empty input gracefully + *
  • Reject null or empty input via {@link #requireContent(String, String)} *
  • Sanitize field names using {@link FieldNameSanitizer} *
  • Preserve original data types where possible *
  • Throw {@link DocumentProcessingException} for processing errors @@ -93,8 +93,9 @@ public interface SolrDocumentCreator { * Input Validation: * *
      - *
    • Null input should be handled gracefully (implementation-dependent) - *
    • Empty input should return empty list + *
    • Null or blank input throws DocumentProcessingException — see + * {@link #requireContent(String, String)} + *
    • Well-formed content that declares no documents returns an empty list *
    • Malformed content should throw DocumentProcessingException *
    * @@ -103,12 +104,35 @@ public interface SolrDocumentCreator { * objects. The format depends on the implementing class (JSON array, * CSV data, XML, etc.) * @return a list of SolrInputDocument objects created from the parsed content. - * Returns empty list if content is empty or contains no valid documents + * Returns empty list if the content contains no documents * @throws DocumentProcessingException - * if the content cannot be parsed or converted due to format - * errors, invalid structure, or processing failures - * @throws IllegalArgumentException - * if content is null (implementation-dependent) + * if the content is null or blank, or cannot be parsed or converted + * due to format errors, invalid structure, or processing failures */ List create(String content) throws DocumentProcessingException; + + /** + * Rejects null or blank input with a message consistent across every format. + * + *

    + * Implementations call this as the first statement of {@link #create(String)}. + * Although the {@code content} parameter is null-marked (see the package-level + * {@code @NullMarked}), that contract binds only callers the compiler can see. + * These creators are reached from {@code @McpTool} methods whose arguments are + * supplied reflectively from a client's JSON-RPC request, so a null can arrive + * at runtime regardless of the annotation. The check is a trust-boundary guard, + * not redundant defensive coding. + * + * @param content + * the raw content string supplied by the caller + * @param format + * the format label used in the error message (e.g. {@code "JSON"}) + * @throws DocumentProcessingException + * if {@code content} is null or contains only whitespace + */ + static void requireContent(String content, String format) throws DocumentProcessingException { + if (content == null || content.isBlank()) { + throw new DocumentProcessingException(format + " input cannot be null or empty"); + } + } } diff --git a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/XmlDocumentCreator.java b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/XmlDocumentCreator.java index 289c356f..9c2e47b3 100644 --- a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/XmlDocumentCreator.java +++ b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/XmlDocumentCreator.java @@ -72,6 +72,8 @@ public XmlDocumentCreator() { * errors occur */ public List create(String xml) throws DocumentProcessingException { + SolrDocumentCreator.requireContent(xml, "XML"); + try { Element rootElement = parseXmlDocument(xml); return processRootElement(rootElement); diff --git a/src/test/java/org/apache/solr/mcp/server/indexing/CsvIndexingTest.java b/src/test/java/org/apache/solr/mcp/server/indexing/CsvIndexingTest.java index 59fd8759..abec4727 100644 --- a/src/test/java/org/apache/solr/mcp/server/indexing/CsvIndexingTest.java +++ b/src/test/java/org/apache/solr/mcp/server/indexing/CsvIndexingTest.java @@ -17,9 +17,11 @@ package org.apache.solr.mcp.server.indexing; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.util.List; import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.mcp.server.indexing.documentcreator.DocumentProcessingException; import org.apache.solr.mcp.server.indexing.documentcreator.IndexingDocumentCreator; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -147,4 +149,34 @@ void testCreateSchemalessDocumentsFromCsvWithQuotedValues() throws Exception { assertThat(secondDoc.getFieldValue("name")).isEqualTo("Regular Name"); assertThat(secondDoc.getFieldValue("description")).isEqualTo("Regular description"); } + + @Test + void testCreateSchemalessDocumentsFromCsvWithNullInput() { + // Given + + // When/Then + assertThatThrownBy(() -> indexingDocumentCreator.createSchemalessDocumentsFromCsv(null)) + .isInstanceOf(DocumentProcessingException.class) + .hasMessageContaining("CSV input cannot be null or empty"); + } + + @Test + void testCreateSchemalessDocumentsFromCsvWithEmptyInput() { + // Given + + // When/Then + assertThatThrownBy(() -> indexingDocumentCreator.createSchemalessDocumentsFromCsv("")) + .isInstanceOf(DocumentProcessingException.class) + .hasMessageContaining("CSV input cannot be null or empty"); + } + + @Test + void testCreateSchemalessDocumentsFromCsvWithWhitespaceOnlyInput() { + // Given + + // When/Then + assertThatThrownBy(() -> indexingDocumentCreator.createSchemalessDocumentsFromCsv(" \n\t ")) + .isInstanceOf(DocumentProcessingException.class) + .hasMessageContaining("CSV input cannot be null or empty"); + } } diff --git a/src/test/java/org/apache/solr/mcp/server/indexing/JsonIndexingTest.java b/src/test/java/org/apache/solr/mcp/server/indexing/JsonIndexingTest.java new file mode 100644 index 00000000..378468ca --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/indexing/JsonIndexingTest.java @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.indexing; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.List; +import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.mcp.server.indexing.documentcreator.DocumentProcessingException; +import org.apache.solr.mcp.server.indexing.documentcreator.IndexingDocumentCreator; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.TestPropertySource; + +/** + * Test class for JSON indexing functionality in IndexingService. + * + *

    + * This test verifies that the IndexingService can correctly parse JSON data and + * convert it into SolrInputDocument objects using the schema-less approach, and + * that null or blank input is rejected with the same message shape used by the + * CSV and XML creators. + */ +@SpringBootTest +@TestPropertySource(locations = "classpath:application.properties") +class JsonIndexingTest { + + @Autowired + private IndexingDocumentCreator indexingDocumentCreator; + + @Test + void testCreateSchemalessDocumentsFromJson() throws Exception { + // Given + + String jsonData = """ + [ + {"id":"0553573403","name":"A Game of Thrones","price":7.99,"genre_s":"fantasy"}, + {"id":"0553293354","name":"Foundation","price":7.99,"genre_s":"scifi"} + ] + """; + + // When + List documents = indexingDocumentCreator.createSchemalessDocumentsFromJson(jsonData); + + // Then + assertThat(documents).hasSize(2); + + SolrInputDocument firstDoc = documents.getFirst(); + assertThat(firstDoc.getFieldValue("id")).isEqualTo("0553573403"); + assertThat(firstDoc.getFieldValue("name")).isEqualTo("A Game of Thrones"); + assertThat(firstDoc.getFieldValue("genre_s")).isEqualTo("fantasy"); + + SolrInputDocument secondDoc = documents.get(1); + assertThat(secondDoc.getFieldValue("id")).isEqualTo("0553293354"); + assertThat(secondDoc.getFieldValue("name")).isEqualTo("Foundation"); + assertThat(secondDoc.getFieldValue("genre_s")).isEqualTo("scifi"); + } + + @Test + void testCreateSchemalessDocumentsFromJsonWithNullInput() { + // Given + + // When/Then + assertThatThrownBy(() -> indexingDocumentCreator.createSchemalessDocumentsFromJson(null)) + .isInstanceOf(DocumentProcessingException.class) + .hasMessageContaining("JSON input cannot be null or empty"); + } + + @Test + void testCreateSchemalessDocumentsFromJsonWithEmptyInput() { + // Given + + // When/Then + assertThatThrownBy(() -> indexingDocumentCreator.createSchemalessDocumentsFromJson("")) + .isInstanceOf(DocumentProcessingException.class) + .hasMessageContaining("JSON input cannot be null or empty"); + } + + @Test + void testCreateSchemalessDocumentsFromJsonWithWhitespaceOnlyInput() { + // Given + + // When/Then + assertThatThrownBy(() -> indexingDocumentCreator.createSchemalessDocumentsFromJson(" \n\t ")) + .isInstanceOf(DocumentProcessingException.class) + .hasMessageContaining("JSON input cannot be null or empty"); + } +}