[GitHub] [camel] davsclaus commented on a diff in pull request #10493: CAMEL-18698: Add support for multiple input/output data types on components

["davsclaus (via GitHub)" <gi...@apache.org>]

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


##########
core/camel-core-model/src/main/java/org/apache/camel/model/transformer/LoadTransformerDefinition.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.camel.model.transformer;
+
+import jakarta.xml.bind.annotation.XmlAccessType;
+import jakarta.xml.bind.annotation.XmlAccessorType;
+import jakarta.xml.bind.annotation.XmlAttribute;
+import jakarta.xml.bind.annotation.XmlType;
+
+import org.apache.camel.spi.Metadata;
+
+/**
+ * Loads one to many {@link org.apache.camel.spi.Transformer} via {@link org.apache.camel.spi.TransformerLoader}.
+ * Supports classpath scan to load transformer implementations configured for instance via annotation configuration.
+ */
+@Metadata(label = "transformation")
+@XmlType(name = "loadTransformer")
+@XmlAccessorType(XmlAccessType.FIELD)
+public class LoadTransformerDefinition extends TransformerDefinition {
+
+    @XmlAttribute
+    private String packageScan;
+
+    @XmlAttribute
+    private Boolean defaults = false;

Review Comment:
   Use String type, as all options in EIP should be configurable via placeholder. See other EIPs how they do it for "boolean" types.



##########
core/camel-core-processor/src/main/java/org/apache/camel/processor/transformer/bytes/ByteArrayDataTypeTransformer.java:
##########
@@ -0,0 +1,41 @@
+/*
+ * 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.camel.processor.transformer.bytes;

Review Comment:
   Do you really need this to be in its own package for just 1 class? IMHO move it to parent package



##########
core/camel-core-processor/src/main/java/org/apache/camel/processor/transformer/text/StringDataTypeTransformer.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.camel.processor.transformer.text;

Review Comment:
   Do you really need this to be in its own package for just 1 class? IMHO move it to parent package



##########
docs/user-manual/modules/ROOT/pages/transformer.adoc:
##########
@@ -134,6 +175,66 @@ XML DSL:
 <customTransformer className="com.example.MyCustomTransformer" fromType="xml" toType="json"/>
 ----
 
+== Load Transformer Options
+
+Users are able to preload known default transformers. Also users may load transformers via classpath scan.
+
+[width="100%",cols="25%,75%",options="header",]
+|===
+| Name | Description
+| defaults | Loads known default transformer implementations (e.g. plain-text, application-octet-stream)
+| location | Classpath location to scan for transformer implementations. Transformer implementations must use the `org.apache.camel.spi.DataTypeTransformer` annotation to get recognized by the scanner.
+|===
+
+Here is an example to load default Transformer classes:
+
+Java DSL:

Review Comment:
   Same here with TABs



##########
core/camel-core-reifier/src/main/java/org/apache/camel/reifier/TransformReifier.java:
##########
@@ -31,6 +32,10 @@ public TransformReifier(Route route, ProcessorDefinition<?> definition) {
 
     @Override
     public Processor createProcessor() throws Exception {
+        if (definition.getToType() != null) {
+            return new DataTypeProcessor(definition.getFromType(), definition.getToType());

Review Comment:
   Hmm I think we may need to throw an illegal argument exception if you have configured toType and also expression. It seems that this is not possible.



##########
docs/user-manual/modules/ROOT/pages/transformer.adoc:
##########
@@ -30,20 +31,60 @@ a wildcard.
 | Data Format Transformer | Transform with using Data Format
 | Endpoint Transformer | Transform with using Endpoint
 | Custom Transformer | Transform with using custom transformer class. Transformer must be a subclass of `org.apache.camel.spi.Transformer`
+| Loading Transformer | Loads multiple transformer implementations (e.g. via annotation classpath scan). Also preloads known default Camel transformer implementations.
 |===
 
 === Common Options
 
-All transformers have following common options to specify which data type is supported by the transformer. `scheme` or both of `fromType` and `toType` must be specified.
+All transformers have following common options to specify which data type is supported by the transformer. `name` or both of `fromType` and `toType` must be specified.
 
 [width="100%",cols="25%,75%",options="header",]
 |===
 | Name | Description
-| scheme | Type of data model like `xml` or `json`. For example if `xml` is specified, the transformer is applied for all java -&gt; xml and xml -&gt; java transformation.
+| scheme | The supported data type scheme. It is possible to just reference a scheme like `xml` or `json`. For example if `xml` is specified, the transformer is applied for all java -&gt; xml and xml -&gt; java transformation.
+| name | The name of the transformer. If name is specified users may use a combination of a scheme and name (e.g. `xml:Order`) to reference the transformer in a route.
 | fromType | xref:transformer.adoc[Data type] to transform from.
 | toType | xref:transformer.adoc[Data type] to transform to.
 |===
 
+Transformer implementations may use `scheme:name` or the combination of `fromType/toType` as an identifier.
+
+When using the `scheme:name` identifier users may reference the transformer by its full name in a route.
+
+Java DSL:

Review Comment:
   Use the new TABS for having examples in different DSLs



##########
dsl/camel-yaml-dsl/camel-yaml-dsl/src/test/groovy/org/apache/camel/dsl/yaml/KameletBindingLoaderTest.groovy:
##########
@@ -621,4 +622,100 @@ class KameletBindingLoaderTest extends YamlTestSupport {
         context.resolvePropertyPlaceholders("{{MY_ENV}}") == "cheese"
     }
 
+    def "kamelet binding with input/output data types"() {

Review Comment:
   Can you also add tests that are not kamelet bindings, eg a plain route



-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org