[GitHub] [camel-kamelets] davsclaus commented on a diff in pull request #1162: chore: Introduce Kamelet input/output data types

[GitBox <gi...@apache.org>]

davsclaus commented on code in PR #1162:
URL: https://github.com/apache/camel-kamelets/pull/1162#discussion_r1033333572


##########
library/camel-kamelets-utils/src/main/java/org/apache/camel/kamelets/utils/format/converter/aws2/s3/AWS2S3BinaryOutputType.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * 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.kamelets.utils.format.converter.aws2.s3;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+import org.apache.camel.CamelExecutionException;
+import org.apache.camel.Exchange;
+import org.apache.camel.InvalidPayloadException;
+import org.apache.camel.kamelets.utils.format.spi.DataTypeConverter;
+import org.apache.camel.kamelets.utils.format.spi.annotations.DataType;
+import software.amazon.awssdk.utils.IoUtils;
+
+/**
+ * Binary output type.
+ */
+@DataType(scheme = "aws2-s3", name = "binary", mediaType = "application/octet-stream")
+public class AWS2S3BinaryOutputType implements DataTypeConverter {
+
+    @Override
+    public void convert(Exchange exchange) {
+        if (exchange.getMessage().getBody() instanceof byte[]) {
+            return;
+        }
+
+        try {
+            InputStream is = exchange.getMessage().getBody(InputStream.class);
+            if (is != null) {
+                exchange.getMessage().setBody(IoUtils.toByteArray(is));

Review Comment:
   Why are we use AWS util for this? Does it do something special, or can we just use plain Camel / Java for this



##########
library/camel-kamelets-utils/src/main/java/org/apache/camel/kamelets/utils/format/spi/DataTypeConverterResolver.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.kamelets.utils.format.spi;
+
+import java.util.Optional;
+
+import org.apache.camel.CamelContext;
+
+/**
+ * Represents a resolver of data type converters from a URI to be able to lazy load them using some discovery mechanism.
+ */
+@FunctionalInterface
+public interface DataTypeConverterResolver {
+
+    /**
+     * Attempts to resolve the converter for the given URI.
+     *
+     * @param scheme
+     * @param name
+     * @param camelContext
+     * @return
+     */
+    Optional<DataTypeConverter> resolve(String scheme, String name, CamelContext camelContext);

Review Comment:
   You are welcome to add javadoc for the parameters (yeah I not I am picky - but this is a SPI so end users and others may need to understand these interfaces)



##########
library/camel-kamelets-utils/src/main/resources/META-INF/services/org/apache/camel/DataTypeConverter:
##########
@@ -0,0 +1,20 @@
+#
+# 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.
+#
+
+org.apache.camel.kamelets.utils.format.converter.standard

Review Comment:
   In the core camel project we source generate these META-INF files. We should do this in the future for kamelets too



##########
library/camel-kamelets-utils/src/main/java/org/apache/camel/kamelets/utils/format/converter/standard/JsonModelDataType.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.kamelets.utils.format.converter.standard;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.camel.CamelExecutionException;
+import org.apache.camel.Exchange;
+import org.apache.camel.InvalidPayloadException;
+import org.apache.camel.component.jackson.JacksonDataFormat;
+import org.apache.camel.kamelets.utils.format.spi.DataTypeConverter;
+import org.apache.camel.kamelets.utils.format.spi.annotations.DataType;
+
+/**
+ * Data type converter able to unmarshal to given unmarshalType using jackson data format.
+ * <p/>
+ * Unmarshal type should be given as a fully qualified class name in the exchange properties.
+ */
+@DataType(name = "jsonObject")
+public class JsonModelDataType implements DataTypeConverter {
+
+    public static final String JSON_DATA_TYPE_KEY = "CamelJsonModelDataType";
+
+    @Override
+    public void convert(Exchange exchange) {
+        if (!exchange.hasProperties() || !exchange.getProperties().containsKey(JSON_DATA_TYPE_KEY)) {
+            return;
+        }
+
+        String type = exchange.getProperty(JSON_DATA_TYPE_KEY, String.class);
+        try (JacksonDataFormat dataFormat = new JacksonDataFormat(new ObjectMapper(), Class.forName(type))) {

Review Comment:
   1) Yes at first we can create a shared object mapper on the class level, not per invoked exchange.
   
   2) You should use Camels' ClassResolver API to load classes, not Class.forName



##########
library/camel-kamelets-utils/src/main/java/org/apache/camel/kamelets/utils/format/spi/DataTypeConverter.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.kamelets.utils.format.spi;
+
+import org.apache.camel.Exchange;
+import org.apache.camel.kamelets.utils.format.spi.annotations.DataType;
+
+@FunctionalInterface
+public interface DataTypeConverter {

Review Comment:
   This is a SPI so would be good to have full javadoc



##########
library/camel-kamelets-utils/src/main/java/org/apache/camel/kamelets/utils/format/spi/DataTypeRegistry.java:
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.kamelets.utils.format.spi;
+
+import java.util.Optional;
+
+/**
+ * Registry for data types. Data type loaders should be used to add types to the registry.
+ * <p/>
+ * The registry is able to perform a lookup of a specific data type.
+ */
+public interface DataTypeRegistry {
+
+    /**
+     * Registers a new default data type converter.
+     * @param scheme
+     * @param converter
+     */
+    void addDataTypeConverter(String scheme, DataTypeConverter converter);
+
+    /**
+     * Registers a new default data type converter.
+     * @param converter
+     */
+    default void addDataTypeConverter(DataTypeConverter converter) {
+        addDataTypeConverter("camel", converter);

Review Comment:
   anything special when the scheme is camel ?



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