TIKA-4834 config comments

CHANGES.txt

@@ -1,5 +1,12 @@
 Release 4.1.0 - unreleased
 
+   * tika-server and tika-async-cli now start from a config that contains
+     // or /* */ comments, as the configuration docs have always said they
+     may. The main loader accepted them; the steps that re-read the user's
+     file to merge in server/CLI overrides (ConfigMerger, ensurePluginRoots)
+     used their own bare parser and refused the whole file; they now use the
+     shared TikaObjectMapperFactory mapper (TIKA-4834).
+
    * The Kafka pipes iterator no longer stops at the first empty poll. A newly
      subscribed consumer spends its first poll(s) joining the group and returns
      empty even when the topic has a backlog, so the iterator could enqueue zero

tika-pipes/tika-async-cli/src/main/java/org/apache/tika/async/cli/TikaAsyncCLI.java

@@ -38,6 +38,7 @@
 
 import org.apache.tika.config.EmbeddedLimits;
 import org.apache.tika.config.loader.TikaJsonConfig;
+import org.apache.tika.config.loader.TikaObjectMapperFactory;
 import org.apache.tika.exception.TikaConfigException;
 import org.apache.tika.parser.ParseContext;
 import org.apache.tika.pipes.api.FetchEmitTuple;
@@ -444,7 +445,8 @@ static String resolveDefaultPluginsDir() {
      * @return the config path to use (original if plugin-roots exists, or a new merged config)
      */
     static Path ensurePluginRoots(Path originalConfigPath, String pluginsDir) throws IOException {
-        ObjectMapper mapper = new ObjectMapper();
+        // The shared config mapper: same comment and strictness rules as the main loader.
+        ObjectMapper mapper = TikaObjectMapperFactory.getMapper();
         JsonNode rootNode = mapper.readTree(originalConfigPath.toFile());
 
         if (rootNode.has("plugin-roots")) {

tika-pipes/tika-async-cli/src/test/java/org/apache/tika/async/cli/AsyncCliParserTest.java

@@ -159,6 +159,23 @@ public void testFileListDefaultsOutputDir(@TempDir Path tmp) throws Exception {
     @TempDir
     Path tempDir;
 
+    /** TIKA-4834: the config docs permit comments. */
+    @Test
+    public void testEnsurePluginRootsAcceptsComments() throws Exception {
+        Path configPath = tempDir.resolve("config-comments.json");
+        Files.writeString(configPath, """
+            // leading comment
+            {
+              /* block */
+              "pipes": { "numClients": 3 } // trailing
+            }
+            """);
+        Path result = TikaAsyncCLI.ensurePluginRoots(configPath, null);
+        JsonNode root = new ObjectMapper().readTree(result.toFile());
+        assertTrue(root.has("plugin-roots"));
+        assertEquals(3, root.get("pipes").get("numClients").asInt());
+    }
+
     @Test
     public void testEnsurePluginRootsAddsDefault() throws Exception {
         // Create a config without plugin-roots

tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/config/ConfigMerger.java

@@ -27,13 +27,13 @@
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.SerializationFeature;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import org.apache.tika.config.TimeoutLimits;
+import org.apache.tika.config.loader.TikaObjectMapperFactory;
 import org.apache.tika.pipes.api.ComponentIds;
 
 /**
@@ -90,8 +90,8 @@ private ConfigMerger() {
      */
     public static MergeResult mergeOrCreate(Path existingConfig, ConfigOverrides overrides)
             throws IOException {
-        ObjectMapper mapper = new ObjectMapper();
-        mapper.enable(SerializationFeature.INDENT_OUTPUT);
+        // The shared config mapper: same comment and strictness rules as the main loader.
+        ObjectMapper mapper = TikaObjectMapperFactory.getMapper();
 
         ObjectNode root;
         if (existingConfig != null && Files.exists(existingConfig)) {

tika-pipes/tika-pipes-core/src/test/java/org/apache/tika/pipes/core/config/ConfigMergerTest.java

@@ -82,6 +82,40 @@ public void testCreateNewConfig() throws IOException {
         Files.deleteIfExists(result.configPath());
     }
 
+    /** TIKA-4834: the config docs permit comments; the merge step must not reject them. */
+    @Test
+    public void testMergeWithCommentedConfig() throws IOException {
+        String existingConfig = """
+                // leading line comment
+                {
+                    /* block comment */
+                    "fetchers": {
+                        "existing-fetcher": { // trailing comment
+                            "file-system-fetcher": {
+                                "basePath": "/existing/path"
+                            }
+                        }
+                    },
+                    "plugin-roots": "existing-plugins"
+                }
+                """;
+        Path existingPath = tempDir.resolve("commented-config.json");
+        Files.writeString(existingPath, existingConfig);
+
+        ConfigOverrides overrides = ConfigOverrides.builder()
+                .addFetcher("new-fetcher", "file-system-fetcher", Map.of("basePath", "/new/path"))
+                .build();
+        ConfigMerger.MergeResult result = ConfigMerger.mergeOrCreate(existingPath, overrides);
+
+        // The merged file is plain JSON: readable by a strict mapper, content intact.
+        JsonNode root = new ObjectMapper().readTree(result.configPath().toFile());
+        assertEquals("/existing/path", root.get("fetchers").get("existing-fetcher")
+                .get("file-system-fetcher").get("basePath").asText());
+        assertTrue(root.get("fetchers").has("new-fetcher"));
+        assertEquals("existing-plugins", root.get("plugin-roots").asText());
+        Files.deleteIfExists(result.configPath());
+    }
+
     @Test
     public void testMergeWithExistingConfig() throws IOException {
         // Create existing config

tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerIntegrationTest.java

@@ -100,6 +100,18 @@ public void testBasic() throws Exception {
         testBaseline();
     }
 
+    /** TIKA-4834: the docs permit comments in the config; the server must start from one. */
+    @Test
+    public void testCommentedConfig() throws Exception {
+        startProcess(new String[]{"-config", getConfig("tika-config-server-comments.json")});
+        awaitServerStartup();
+        Response response = WebClient
+                .create(endPoint + RMETA_PATH)
+                .accept("application/json")
+                .put(ClassLoader.getSystemResourceAsStream(TEST_HELLO_WORLD));
+        assertEquals(200, response.getStatus());
+    }
+
     @Test
     public void testBasicWithPipes() throws Exception {
         // Test that pipes-based parsing works for normal documents

tika-server/tika-server-core/src/test/resources/configs/tika-config-server-comments.json

@@ -0,0 +1,20 @@
+// TIKA-4834: comments everywhere the docs say they are allowed
+{
+  /* block comment before a key */
+  "server": {
+    "port": 9999, // trailing comment
+    "allowPipes": true,
+    "endpoints": [
+      "rmeta",
+      // comment inside an array
+      "tika"
+    ]
+  },
+  "pipes": {
+    "numClients": 1,
+    "forkedJvmArgs": [
+      "-Xmx256m"
+    ]
+  },
+  "plugin-roots": "target/plugins"
+}