davsclaus commented on code in PR #27409:
URL: https://github.com/apache/camel/pull/27409#discussion_r4192803200


##########
core/camel-main/src/main/java/org/apache/camel/main/MainHelper.java:
##########
@@ -168,8 +169,7 @@ public void addComponentEnvVariables(Map<String, String> 
env, Properties propert
                 ck = ck.replaceFirst("-", ".");
                 properties.put(ck, v);

Review Comment:
   In custom mode this takes every key left in `env`, not just the 
`CAMEL_COMPONENT_` ones, so leftover `CAMEL_DATAFORMAT_*` / `CAMEL_LANGUAGE_*` 
variables become bogus component keys (`CAMEL_LANGUAGE_FOO_BAR` becomes 
`camel.component.oo.bar`, which fails the startup). Only take keys that start 
with `CAMEL_COMPONENT_` here, and likewise in the data format and language 
branches.



##########
core/camel-main/src/main/java/org/apache/camel/main/MainHelper.java:
##########
@@ -212,8 +211,7 @@ public void addLanguageEnvVariables(Map<String, String> 
env, Properties properti
                 ck = ck.replaceFirst("-", ".");
                 properties.put(ck, v);
             } else {
-                Optional<String> e
-                        = 
languageEnvNames.stream().filter(k::startsWith).findFirst();
+                Optional<String> e = longestMatch(languageEnvNames, k);
                 if (e.isPresent()) {
                     toRemove.add(k);
                     String cname = "camel.language." + 
e.get().substring(15).toLowerCase(Locale.US).replace('_', '-');

Review Comment:
   `toLowerCase` loses the catalog spelling of camelCase names: 
`CAMEL_LANGUAGE_EXCHANGEPROPERTY_TRIM=false` fails the startup with *cannot 
find language with name exchangeproperty*, as the lookup on the classpath is 
case-sensitive. Same for the camelCase data formats above (`avroJackson`, 
`jacksonXml`, ...). Keeping a map from the env name to the catalog name would 
let this use the real name.



##########
core/camel-main/src/test/java/org/apache/camel/main/MainHelperTest.java:
##########
@@ -87,4 +87,68 @@ public void testAddCustomComponentEnvVariables() {
         Assertions.assertEquals("false", 
prop.getProperty("camel.component.foo.pretty-print"));
     }
 
+    /**
+     * Verifies that overlapping component names (e.g. NETTY vs NETTY_HTTP, 
SJMS vs SJMS2, FILE vs FILE_WATCH) are
+     * resolved by longest-match so that each variable is mapped to the 
correct component.
+     */
+    @Test
+    void testLongestMatchComponentEnvVariables() {
+        Map<String, String> env = MainHelper.filterEnvVariables(new String[] { 
"CAMEL_COMPONENT_" });
+        // netty-http must not be mapped to netty
+        env.put("CAMEL_COMPONENT_NETTY_HTTP_MUTE_EXCEPTION", "true");
+        // sjms2 must not be mapped to sjms
+        env.put("CAMEL_COMPONENT_SJMS2_RECOVERY_INTERVAL", "2000");
+        // file-watch must not be mapped to file
+        env.put("CAMEL_COMPONENT_FILE_WATCH_POLL_TIMEOUT", "5000");
+        // netty itself (no suffix clash)
+        env.put("CAMEL_COMPONENT_NETTY_RECEIVE_BUFFER_SIZE", "65536");
+
+        Properties prop = new OrderedProperties();
+        helper.addComponentEnvVariables(env, prop, false);
+
+        Assertions.assertEquals(0, env.size());
+        Assertions.assertEquals(4, prop.size());
+        Assertions.assertEquals("true", 
prop.getProperty("camel.component.netty-http.mute-exception"));
+        Assertions.assertEquals("2000", 
prop.getProperty("camel.component.sjms2.recovery-interval"));
+        Assertions.assertEquals("5000", 
prop.getProperty("camel.component.file-watch.poll-timeout"));
+        Assertions.assertEquals("65536", 
prop.getProperty("camel.component.netty.receive-buffer-size"));
+    }
+
+    /**
+     * Verifies that overlapping language names (e.g. JS vs JSONPATH) are 
resolved by longest-match.
+     */
+    @Test
+    void testLongestMatchLanguageEnvVariables() {
+        Map<String, String> env = MainHelper.filterEnvVariables(new String[] { 
"CAMEL_LANGUAGE_" });
+        // jsonpath must not be mapped to js
+        env.put("CAMEL_LANGUAGE_JSONPATH_SUPPRESS_EXCEPTIONS", "true");
+
+        Properties prop = new OrderedProperties();
+        helper.addLanguageEnvVariables(env, prop, false);
+
+        Assertions.assertEquals(0, env.size());
+        Assertions.assertEquals(1, prop.size());
+        Assertions.assertEquals("true", 
prop.getProperty("camel.language.jsonpath.suppress-exceptions"));
+    }
+
+    /**
+     * Verifies that overlapping dataformat names (e.g. AVRO vs AVROJACKSON) 
are resolved by longest-match.
+     */
+    @Test
+    void testLongestMatchDataFormatEnvVariables() {
+        Map<String, String> env = MainHelper.filterEnvVariables(new String[] { 
"CAMEL_DATAFORMAT_" });
+        // avrojackson must not be mapped to avro
+        env.put("CAMEL_DATAFORMAT_AVROJACKSON_PRETTY_PRINTER", "true");
+        // avro itself
+        env.put("CAMEL_DATAFORMAT_AVRO_SCHEMA_CLASS_NAME", 
"com.example.MySchema");
+
+        Properties prop = new OrderedProperties();
+        helper.addDataFormatEnvVariables(env, prop, false);
+
+        Assertions.assertEquals(0, env.size());
+        Assertions.assertEquals(2, prop.size());
+        Assertions.assertEquals("true", 
prop.getProperty("camel.dataformat.avrojackson.pretty-printer"));

Review Comment:
   This asserts the lowercased key, but `camel.dataformat.avrojackson` cannot 
be resolved (the data format is `avroJackson`), so the test pins down the wrong 
key.



-- 
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]

Reply via email to