From 0851b266231c79abc498ff6504fdc3ae4c3cef01 Mon Sep 17 00:00:00 2001 From: Aditya Parikh Date: Fri, 11 Sep 2026 11:34:08 -0400 Subject: [PATCH 1/2] fix(indexing): one blank-input rule for every document format The four creators disagreed on where and how blank input was rejected: JSON and CSV checked isBlank() in the creator, XML was checked only by the orchestrator (the creator itself failed with a parse error), and Markdown was checked in both places with different outcomes (orchestrator threw, creator returned an empty list). The messages differed too, and the interface javadoc promised three contracts none of them honoured. One SolrDocumentCreator.requireContent(content, format) helper now runs first in every create(); the orchestrator's two XML/Markdown checks are deleted. The helper checks blankness only. The creators are @NullMarked, so a null argument is a caller's contract violation rather than an input to validate; the null branches main still carried are removed along with the XML null test that pinned them. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Wh7SJkZhL1uuK7pYc3SLk8 Signed-off-by: Aditya Parikh --- .../documentcreator/CsvDocumentCreator.java | 4 +- .../IndexingDocumentCreator.java | 12 ---- .../documentcreator/JsonDocumentCreator.java | 4 +- .../MarkdownDocumentCreator.java | 5 +- .../documentcreator/SolrDocumentCreator.java | 38 +++++++++---- .../documentcreator/XmlDocumentCreator.java | 1 + .../server/indexing/MarkdownIndexingTest.java | 2 +- .../mcp/server/indexing/XmlIndexingTest.java | 16 +----- .../DocumentCreatorBlankInputTest.java | 56 +++++++++++++++++++ 9 files changed, 91 insertions(+), 47 deletions(-) create mode 100644 src/test/java/org/apache/solr/mcp/server/indexing/documentcreator/DocumentCreatorBlankInputTest.java 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..6f7f02be 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 @@ -132,15 +132,12 @@ public MarkdownDocumentCreator() { */ @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..62d7c165 --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/indexing/documentcreator/DocumentCreatorBlankInputTest.java @@ -0,0 +1,56 @@ +/* + * 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 creators() { + 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")); + } + + @ParameterizedTest + @MethodSource("creators") + void emptyInputIsRejectedWithTheFormatName(SolrDocumentCreator creator, String format) { + assertThatThrownBy(() -> creator.create("")).isInstanceOf(DocumentProcessingException.class) + .hasMessage(format + " input cannot be empty"); + } + + @ParameterizedTest + @MethodSource("creators") + void whitespaceOnlyInputIsRejectedWithTheFormatName(SolrDocumentCreator creator, String format) { + assertThatThrownBy(() -> creator.create(" \n\t ")).isInstanceOf(DocumentProcessingException.class) + .hasMessage(format + " input cannot be empty"); + } +} From ed4dca2e04718fec0129c9cabe6f19ebdf67f323 Mon Sep 17 00:00:00 2001 From: Aditya Parikh Date: Sat, 12 Sep 2026 23:10:46 -0400 Subject: [PATCH 2/2] fix(indexing): correct Markdown javadoc and fold the blank-input tests The create() javadoc still promised an empty list for blank input, which this change removes. The two parameterized tests differed only in the input literal, so they are one test over creators x blank inputs. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01CiUHyyXLTo9ATdgg8eRFZJ Signed-off-by: Aditya Parikh --- .../MarkdownDocumentCreator.java | 5 ++-- .../DocumentCreatorBlankInputTest.java | 26 ++++++++----------- 2 files changed, 13 insertions(+), 18 deletions(-) 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 6f7f02be..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,10 +125,9 @@ 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 { 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 index 62d7c165..d2896698 100644 --- 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 @@ -33,24 +33,20 @@ */ class DocumentCreatorBlankInputTest { - static Stream creators() { - 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")); + 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("creators") - void emptyInputIsRejectedWithTheFormatName(SolrDocumentCreator creator, String format) { - assertThatThrownBy(() -> creator.create("")).isInstanceOf(DocumentProcessingException.class) - .hasMessage(format + " input cannot be empty"); - } - - @ParameterizedTest - @MethodSource("creators") - void whitespaceOnlyInputIsRejectedWithTheFormatName(SolrDocumentCreator creator, String format) { - assertThatThrownBy(() -> creator.create(" \n\t ")).isInstanceOf(DocumentProcessingException.class) + @MethodSource("blankInputs") + void blankInputIsRejectedWithTheFormatName(SolrDocumentCreator creator, String format, String input) { + assertThatThrownBy(() -> creator.create(input)).isInstanceOf(DocumentProcessingException.class) .hasMessage(format + " input cannot be empty"); } }