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..04beefc1 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,7 @@ 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 d1801714..6f21e167 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 @@ -137,12 +137,6 @@ public List createSchemalessDocumentsFromCsv(String csv) thro * @see XmlDocumentCreator */ 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"); - } - byte[] xmlBytes = xml.getBytes(StandardCharsets.UTF_8); if (xmlBytes.length > MAX_XML_SIZE_BYTES) { throw new DocumentProcessingException( @@ -170,12 +164,6 @@ public List createSchemalessDocumentsFromXml(String xml) thro */ public List createSchemalessDocumentsFromMarkdown(String markdown) throws DocumentProcessingException { - - // Input validation - if (markdown == null || markdown.trim().isEmpty()) { - throw new DocumentProcessingException("Markdown input cannot be null or empty"); - } - return markdownDocumentCreator.create(markdown); } } 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..aab1204c 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,7 @@ 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/MarkdownDocumentCreator.java b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/MarkdownDocumentCreator.java index db560202..a108b087 100644 --- a/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/MarkdownDocumentCreator.java +++ b/src/main/java/org/apache/solr/mcp/server/indexing/documentcreator/MarkdownDocumentCreator.java @@ -125,22 +125,18 @@ public MarkdownDocumentCreator() { * * @param markdown * markdown string, optionally starting with YAML front matter - * @return a single-element list containing the created document, or an empty - * list if the input is blank + * @return a single-element list containing the created document * @throws DocumentProcessingException - * if the input exceeds the size limit or parsing fails + * if the input is blank, exceeds the size limit or fails to parse */ @Override public List create(String markdown) throws DocumentProcessingException { + SolrDocumentCreator.requireContent(markdown, "Markdown"); if (markdown.getBytes(StandardCharsets.UTF_8).length > MAX_INPUT_SIZE_BYTES) { throw new DocumentProcessingException( "Input too large: exceeds maximum size of " + MAX_INPUT_SIZE_BYTES + " bytes"); } - if (markdown.trim().isEmpty()) { - return List.of(); - } - Node document; try { document = parser.parse(markdown); 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..ca6b4410 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,8 @@ * Implementation Guidelines: * *
    - *
  • Handle null or empty input gracefully + *
  • Reject blank input via {@link #requireContent(String, String)} before + * parsing *
  • Sanitize field names using {@link FieldNameSanitizer} *
  • Preserve original data types where possible *
  • Throw {@link DocumentProcessingException} for processing errors @@ -93,22 +94,39 @@ public interface SolrDocumentCreator { * Input Validation: * *
      - *
    • Null input should be handled gracefully (implementation-dependent) - *
    • Empty input should return empty list - *
    • Malformed content should throw DocumentProcessingException + *
    • Blank content throws DocumentProcessingException (see + * {@link #requireContent(String, String)}) + *
    • Malformed content throws DocumentProcessingException *
    * * @param content * the content string to be parsed and converted to SolrInputDocument * 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 + * @return a list of SolrInputDocument objects created from the parsed content * @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 blank, or cannot be parsed or converted due to + * format errors, invalid structure, or processing failures */ List create(String content) throws DocumentProcessingException; + + /** + * Rejects blank content with one message shape shared by every format. + * + *

    + * Only blankness is checked. The creators are {@code @NullMarked}, so a null + * argument is a caller's contract violation, not an input to validate. + * + * @param content + * the raw input + * @param format + * the format name used in the message, for example {@code "JSON"} + * @throws DocumentProcessingException + * if {@code content} is empty or whitespace only + */ + static void requireContent(String content, String format) throws DocumentProcessingException { + if (content.isBlank()) { + throw new DocumentProcessingException(format + " input cannot be 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..c03cdaa6 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,7 @@ 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/MarkdownIndexingTest.java b/src/test/java/org/apache/solr/mcp/server/indexing/MarkdownIndexingTest.java index 2ec3a7c5..3791c7f7 100644 --- a/src/test/java/org/apache/solr/mcp/server/indexing/MarkdownIndexingTest.java +++ b/src/test/java/org/apache/solr/mcp/server/indexing/MarkdownIndexingTest.java @@ -259,7 +259,7 @@ void testGeneratedIdIsStableForSameContent() throws Exception { @Test void testEmptyMarkdownThrowsException() { assertThatThrownBy(() -> indexingDocumentCreator.createSchemalessDocumentsFromMarkdown("")) - .isInstanceOf(DocumentProcessingException.class).hasMessageContaining("cannot be null or empty"); + .isInstanceOf(DocumentProcessingException.class).hasMessage("Markdown input cannot be empty"); } @Test diff --git a/src/test/java/org/apache/solr/mcp/server/indexing/XmlIndexingTest.java b/src/test/java/org/apache/solr/mcp/server/indexing/XmlIndexingTest.java index 35129e8a..f697beaa 100644 --- a/src/test/java/org/apache/solr/mcp/server/indexing/XmlIndexingTest.java +++ b/src/test/java/org/apache/solr/mcp/server/indexing/XmlIndexingTest.java @@ -338,24 +338,13 @@ void testCreateSchemalessDocumentsFromXmlWithExternalEntity() { .isInstanceOf(RuntimeException.class); } - @Test - void testCreateSchemalessDocumentsFromXmlWithNullInput() { - // Given - - // When/Then - assertThatThrownBy(() -> indexingDocumentCreator.createSchemalessDocumentsFromXml(null)) - .isInstanceOf(DocumentProcessingException.class) - .hasMessageContaining("XML input cannot be null or empty"); - } - @Test void testCreateSchemalessDocumentsFromXmlWithEmptyInput() { // Given // When/Then assertThatThrownBy(() -> indexingDocumentCreator.createSchemalessDocumentsFromXml("")) - .isInstanceOf(DocumentProcessingException.class) - .hasMessageContaining("XML input cannot be null or empty"); + .isInstanceOf(DocumentProcessingException.class).hasMessage("XML input cannot be empty"); } @Test @@ -364,8 +353,7 @@ void testCreateSchemalessDocumentsFromXmlWithWhitespaceOnlyInput() { // When/Then assertThatThrownBy(() -> indexingDocumentCreator.createSchemalessDocumentsFromXml(" \n\t ")) - .isInstanceOf(DocumentProcessingException.class) - .hasMessageContaining("XML input cannot be null or empty"); + .isInstanceOf(DocumentProcessingException.class).hasMessage("XML input cannot be empty"); } @Test diff --git a/src/test/java/org/apache/solr/mcp/server/indexing/documentcreator/DocumentCreatorBlankInputTest.java b/src/test/java/org/apache/solr/mcp/server/indexing/documentcreator/DocumentCreatorBlankInputTest.java new file mode 100644 index 00000000..d2896698 --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/indexing/documentcreator/DocumentCreatorBlankInputTest.java @@ -0,0 +1,52 @@ +/* + * 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.documentcreator; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.fasterxml.jackson.databind.ObjectMapper; +import java.util.stream.Stream; +import org.junit.jupiter.api.Named; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +/** + * Every format rejects blank input the same way, in the creator itself, with + * one message shape. Null is not a case here: the creators are + * {@code @NullMarked}, so a null argument is a contract violation of the + * caller, not an input to be validated. + */ +class DocumentCreatorBlankInputTest { + + static Stream blankInputs() { + return Stream + .of(Arguments.of(Named.of("JSON", new JsonDocumentCreator(new ObjectMapper())), "JSON"), + Arguments.of(Named.of("CSV", new CsvDocumentCreator()), "CSV"), + Arguments.of(Named.of("XML", new XmlDocumentCreator()), "XML"), + Arguments.of(Named.of("Markdown", new MarkdownDocumentCreator()), "Markdown")) + .flatMap(creator -> Stream.of(Named.of("empty", ""), Named.of("whitespace", " \n\t ")) + .map(input -> Arguments.of(creator.get()[0], creator.get()[1], input))); + } + + @ParameterizedTest + @MethodSource("blankInputs") + void blankInputIsRejectedWithTheFormatName(SolrDocumentCreator creator, String format, String input) { + assertThatThrownBy(() -> creator.create(input)).isInstanceOf(DocumentProcessingException.class) + .hasMessage(format + " input cannot be empty"); + } +}