gnodet-bot commented on code in PR #26937:
URL: https://github.com/apache/camel/pull/26937#discussion_r4130885264


##########
components/camel-jackson3/src/main/java/org/apache/camel/component/jackson3/AbstractJacksonDataFormat.java:
##########
@@ -737,64 +737,179 @@ private boolean resolveObjectMapper() {
         return objectMapperFoundRegistry;
     }
 
-    private void doEnableFeaures() {
+    private boolean trySetSerializationFeature(String featureValueName, 
boolean state) {
+        SerializationFeature feature
+                = 
getCamelContext().getTypeConverter().tryConvertTo(SerializationFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetDeserializationFeature(String featureValueName, 
boolean state) {
+        DeserializationFeature feature
+                = 
getCamelContext().getTypeConverter().tryConvertTo(DeserializationFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetMapperFeature(String featureValueName, boolean 
state) {
+        MapperFeature feature = 
getCamelContext().getTypeConverter().tryConvertTo(MapperFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetDateTimeFeature(String featureValueName, boolean 
state) {
+        DateTimeFeature feature = 
getCamelContext().getTypeConverter().tryConvertTo(DateTimeFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetEnumFeature(String featureValueName, boolean state) {
+        EnumFeature feature = 
getCamelContext().getTypeConverter().tryConvertTo(EnumFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetJsonNodeFeature(String featureValueName, boolean 
state) {
+        JsonNodeFeature feature = 
getCamelContext().getTypeConverter().tryConvertTo(JsonNodeFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetStreamReadFeature(String featureValueName, boolean 
state) {
+        StreamReadFeature feature
+                = 
getCamelContext().getTypeConverter().tryConvertTo(StreamReadFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetStreamWriteFeature(String featureValueName, boolean 
state) {
+        StreamWriteFeature feature
+                = 
getCamelContext().getTypeConverter().tryConvertTo(StreamWriteFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private void doEnableFeatures() {
         Iterator<?> it = ObjectHelper.createIterator(enableFeatures);
         while (it.hasNext()) {
             String enable = it.next().toString();
-            // it can be different kind
-            SerializationFeature sf
-                    = 
getCamelContext().getTypeConverter().tryConvertTo(SerializationFeature.class, 
enable);
-            if (sf != null) {
-                ObjectMapper om = objectMapper.rebuild().enable(sf).build();
-                setObjectMapper(om);
-                continue;
-            }
-            DeserializationFeature df
-                    = 
getCamelContext().getTypeConverter().tryConvertTo(DeserializationFeature.class, 
enable);
-            if (df != null) {
-                ObjectMapper om = objectMapper.rebuild().enable(df).build();
-                setObjectMapper(om);
-                continue;
-            }
-            MapperFeature mf = 
getCamelContext().getTypeConverter().tryConvertTo(MapperFeature.class, enable);
-            if (mf != null) {
-                ObjectMapper om = objectMapper.rebuild().enable(mf).build();
-                setObjectMapper(om);
-                continue;
-            }
-            DateTimeFeature dtf = 
getCamelContext().getTypeConverter().tryConvertTo(DateTimeFeature.class, 
enable);
-            if (dtf != null) {
-                ObjectMapper om = objectMapper.rebuild().enable(dtf).build();
-                setObjectMapper(om);
-                continue;
-            }
-            EnumFeature ef = 
getCamelContext().getTypeConverter().tryConvertTo(EnumFeature.class, enable);
-            if (ef != null) {
-                ObjectMapper om = objectMapper.rebuild().enable(ef).build();
-                setObjectMapper(om);
-                continue;
+            long dotCount = enable.chars().filter(ch -> ch == '.').count();
+            if (dotCount > 1)

Review Comment:
   ⚠️ **Still missing braces** — re-raised from davsclaus's previous review. 
Please add block braces to match the project style (same fix as the `if 
(feature == null)` lines).
   
   ```suggestion
               if (dotCount > 1) {
                   throw new IllegalArgumentException("Enable feature: " + 
enable + " cannot contain more than one '.'");
               }
   ```



##########
components/camel-jackson3/src/main/java/org/apache/camel/component/jackson3/AbstractJacksonDataFormat.java:
##########
@@ -737,64 +737,179 @@ private boolean resolveObjectMapper() {
         return objectMapperFoundRegistry;
     }
 
-    private void doEnableFeaures() {
+    private boolean trySetSerializationFeature(String featureValueName, 
boolean state) {
+        SerializationFeature feature
+                = 
getCamelContext().getTypeConverter().tryConvertTo(SerializationFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetDeserializationFeature(String featureValueName, 
boolean state) {
+        DeserializationFeature feature
+                = 
getCamelContext().getTypeConverter().tryConvertTo(DeserializationFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetMapperFeature(String featureValueName, boolean 
state) {
+        MapperFeature feature = 
getCamelContext().getTypeConverter().tryConvertTo(MapperFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetDateTimeFeature(String featureValueName, boolean 
state) {
+        DateTimeFeature feature = 
getCamelContext().getTypeConverter().tryConvertTo(DateTimeFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetEnumFeature(String featureValueName, boolean state) {
+        EnumFeature feature = 
getCamelContext().getTypeConverter().tryConvertTo(EnumFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetJsonNodeFeature(String featureValueName, boolean 
state) {
+        JsonNodeFeature feature = 
getCamelContext().getTypeConverter().tryConvertTo(JsonNodeFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetStreamReadFeature(String featureValueName, boolean 
state) {
+        StreamReadFeature feature
+                = 
getCamelContext().getTypeConverter().tryConvertTo(StreamReadFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private boolean trySetStreamWriteFeature(String featureValueName, boolean 
state) {
+        StreamWriteFeature feature
+                = 
getCamelContext().getTypeConverter().tryConvertTo(StreamWriteFeature.class, 
featureValueName);
+        if (feature == null) {
+            return false;
+        }
+        setObjectMapper(objectMapper.rebuild().configure(feature, 
state).build());
+        return true;
+    }
+
+    private void doEnableFeatures() {
         Iterator<?> it = ObjectHelper.createIterator(enableFeatures);
         while (it.hasNext()) {
             String enable = it.next().toString();
-            // it can be different kind
-            SerializationFeature sf
-                    = 
getCamelContext().getTypeConverter().tryConvertTo(SerializationFeature.class, 
enable);
-            if (sf != null) {
-                ObjectMapper om = objectMapper.rebuild().enable(sf).build();
-                setObjectMapper(om);
-                continue;
-            }
-            DeserializationFeature df
-                    = 
getCamelContext().getTypeConverter().tryConvertTo(DeserializationFeature.class, 
enable);
-            if (df != null) {
-                ObjectMapper om = objectMapper.rebuild().enable(df).build();
-                setObjectMapper(om);
-                continue;
-            }
-            MapperFeature mf = 
getCamelContext().getTypeConverter().tryConvertTo(MapperFeature.class, enable);
-            if (mf != null) {
-                ObjectMapper om = objectMapper.rebuild().enable(mf).build();
-                setObjectMapper(om);
-                continue;
-            }
-            DateTimeFeature dtf = 
getCamelContext().getTypeConverter().tryConvertTo(DateTimeFeature.class, 
enable);
-            if (dtf != null) {
-                ObjectMapper om = objectMapper.rebuild().enable(dtf).build();
-                setObjectMapper(om);
-                continue;
-            }
-            EnumFeature ef = 
getCamelContext().getTypeConverter().tryConvertTo(EnumFeature.class, enable);
-            if (ef != null) {
-                ObjectMapper om = objectMapper.rebuild().enable(ef).build();
-                setObjectMapper(om);
-                continue;
+            long dotCount = enable.chars().filter(ch -> ch == '.').count();
+            if (dotCount > 1)
+                throw new IllegalArgumentException("Enable feature: " + enable 
+ " cannot contain more than one '.'");
+
+            String featureClassName = null;
+            String featureValueName = enable;
+
+            if (dotCount == 1) {
+                String[] parts = enable.split("\\.", 2);
+                featureClassName = parts[0];
+                featureValueName = parts[1];
             }
-            JsonNodeFeature jnf = 
getCamelContext().getTypeConverter().tryConvertTo(JsonNodeFeature.class, 
enable);
-            if (jnf != null) {
-                ObjectMapper om = objectMapper.rebuild().enable(jnf).build();
-                setObjectMapper(om);
-                continue;
+
+            if (featureClassName != null) {
+                if (!switch (featureClassName) {
+                    case "SerializationFeature" -> 
trySetSerializationFeature(featureValueName, true);
+                    case "DeserializationFeature" -> 
trySetDeserializationFeature(featureValueName, true);
+                    case "MapperFeature" -> 
trySetMapperFeature(featureValueName, true);
+                    case "DateTimeFeature" -> 
trySetDateTimeFeature(featureValueName, true);
+                    case "EnumFeature" -> trySetEnumFeature(featureValueName, 
true);
+                    case "JsonNodeFeature" -> 
trySetJsonNodeFeature(featureValueName, true);
+                    case "StreamReadFeature" -> 
trySetStreamReadFeature(featureValueName, true);
+                    case "StreamWriteFeature" -> 
trySetStreamWriteFeature(featureValueName, true);
+                    default -> false;
+                }) {
+                    throw new IllegalArgumentException(
+                            "Enable feature: " + enable
+                                                       + " cannot be converted 
to an accepted enum of types 
[SerializationFeature,DeserializationFeature,MapperFeature,DateTimeFeature,EnumFeature,JsonNodeFeature,StreamReadFeature,StreamWriteFeature]");
+                }
+            } else {
+                if (!trySetSerializationFeature(featureValueName, true)
+                        && !trySetDeserializationFeature(featureValueName, 
true)
+                        && !trySetMapperFeature(featureValueName, true)
+                        && !trySetDateTimeFeature(featureValueName, true)
+                        && !trySetEnumFeature(featureValueName, true)
+                        && !trySetJsonNodeFeature(featureValueName, true)
+                        && !trySetStreamReadFeature(featureValueName, true)
+                        && !trySetStreamWriteFeature(featureValueName, true)) {
+                    throw new IllegalArgumentException(
+                            "Enable feature: " + featureValueName
+                                                       + " cannot be converted 
to an accepted enum of types 
[SerializationFeature,DeserializationFeature,MapperFeature,DateTimeFeature,EnumFeature,JsonNodeFeature,StreamReadFeature,StreamWriteFeature]");
+                }
             }
-            StreamReadFeature srf = 
getCamelContext().getTypeConverter().tryConvertTo(StreamReadFeature.class, 
enable);
-            if (srf != null) {
-                ObjectMapper om = objectMapper.rebuild().enable(srf).build();
-                setObjectMapper(om);
-                continue;
+        }
+    }
+
+    private void doDisableFeatures() {
+        Iterator<?> it = ObjectHelper.createIterator(disableFeatures);
+        while (it.hasNext()) {
+            String disable = it.next().toString();
+            long dotCount = disable.chars().filter(ch -> ch == '.').count();
+            if (dotCount > 1)
+                throw new IllegalArgumentException("Disable feature: " + 
disable + " cannot contain more than one '.'");

Review Comment:
   ⚠️ **Still missing braces** — same issue in `doDisableFeatures()`. Re-raised 
from davsclaus's previous review.
   
   ```suggestion
               if (dotCount > 1) {
                   throw new IllegalArgumentException("Disable feature: " + 
disable + " cannot contain more than one '.'");
               }
   ```



##########
core/camel-core-model/src/main/java/org/apache/camel/model/dataformat/JsonDataFormat.java:
##########
@@ -100,11 +100,13 @@ public class JsonDataFormat extends DataFormatDefinition 
implements ContentTypeH
     private String moduleRefs;
     @XmlAttribute
     @Metadata(label = "advanced",
-              description = "Set of features to enable on the Jackson 
com.fasterxml.jackson.databind.ObjectMapper. Multiple features can be separated 
by comma.")
+              description = "Set of features to enable on the Jackson 
com.fasterxml.jackson.databind.ObjectMapper. Multiple features can be separated 
by comma."
+                            + " Jackson 3 features can be declared in 
qualified manner (ClassName.FEATURE) for multiple identical feature names.")

Review Comment:
   💡 **Wording** — re-raised from davsclaus's previous review. Suggested 
rewording:
   
   ```suggestion
                 description = "Set of features to enable on the Jackson 
com.fasterxml.jackson.databind.ObjectMapper. Multiple features can be separated 
by comma."
                               + " When using Jackson 3, a feature can be 
qualified with its enum class name (e.g. SerializationFeature.WRAP_ROOT_VALUE) 
to tell apart features with the same name.")
   ```



##########
core/camel-core-model/src/main/java/org/apache/camel/model/dataformat/JsonDataFormat.java:
##########
@@ -100,11 +100,13 @@ public class JsonDataFormat extends DataFormatDefinition 
implements ContentTypeH
     private String moduleRefs;
     @XmlAttribute
     @Metadata(label = "advanced",
-              description = "Set of features to enable on the Jackson 
com.fasterxml.jackson.databind.ObjectMapper. Multiple features can be separated 
by comma.")
+              description = "Set of features to enable on the Jackson 
com.fasterxml.jackson.databind.ObjectMapper. Multiple features can be separated 
by comma."
+                            + " Jackson 3 features can be declared in 
qualified manner (ClassName.FEATURE) for multiple identical feature names.")
     private String enableFeatures;
     @XmlAttribute
     @Metadata(label = "advanced",
-              description = "Set of features to disable on the Jackson 
com.fasterxml.jackson.databind.ObjectMapper. Multiple features can be separated 
by comma.")
+              description = "Set of features to disable on the Jackson 
com.fasterxml.jackson.databind.ObjectMapper. Multiple features can be separated 
by comma."
+                            + " Jackson 3 features can be declared in 
qualified manner (ClassName.FEATURE) for multiple identical feature names.")

Review Comment:
   💡 **Wording** — same wording fix for `disableFeatures`.
   
   ```suggestion
                 description = "Set of features to disable on the Jackson 
com.fasterxml.jackson.databind.ObjectMapper. Multiple features can be separated 
by comma."
                               + " When using Jackson 3, a feature can be 
qualified with its enum class name (e.g. SerializationFeature.WRAP_ROOT_VALUE) 
to tell apart features with the same name.")
   ```



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