davsclaus commented on code in PR #25433:
URL: https://github.com/apache/camel/pull/25433#discussion_r3764854376
##########
bom/camel-bom/pom.xml:
##########
@@ -66,6 +66,21 @@
<artifactId>camel-ai-tool</artifactId>
<version>4.23.0-SNAPSHOT</version>
</dependency>
+ <dependency>
+ <groupId>org.apache.camel</groupId>
+ <artifactId>camel-alibaba-common</artifactId>
+ <version>4.22.0-SNAPSHOT</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.camel</groupId>
+ <artifactId>camel-alibaba-mns</artifactId>
+ <version>4.22.0-SNAPSHOT</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.camel</groupId>
+ <artifactId>camel-alibaba-oss</artifactId>
+ <version>4.22.0-SNAPSHOT</version>
+ </dependency>
Review Comment:
**Blocking:** The three new BOM entries hardcode `4.22.0-SNAPSHOT` instead
of `4.23.0-SNAPSHOT`. The `parent/pom.xml` and all module `pom.xml` files
correctly use `4.23.0-SNAPSHOT`. This version mismatch will break dependency
management for any project importing the Camel BOM.
Note: the Camel BOM uses hardcoded versions (not `${project.version}`), so
these must match the current project version.
```suggestion
<dependency>
<groupId>org.apache.camel</groupId>
<artifactId>camel-alibaba-common</artifactId>
<version>4.23.0-SNAPSHOT</version>
</dependency>
<dependency>
<groupId>org.apache.camel</groupId>
<artifactId>camel-alibaba-mns</artifactId>
<version>4.23.0-SNAPSHOT</version>
</dependency>
<dependency>
<groupId>org.apache.camel</groupId>
<artifactId>camel-alibaba-oss</artifactId>
<version>4.23.0-SNAPSHOT</version>
</dependency>
```
##########
components/camel-alibaba/camel-alibaba-oss/src/main/java/org/apache/camel/component/alibaba/oss/OSSProducer.java:
##########
@@ -0,0 +1,377 @@
+/*
+ * 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.component.alibaba.oss;
+
+import java.io.File;
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import com.aliyun.sdk.service.oss2.OSSClient;
+import com.aliyun.sdk.service.oss2.models.BucketSummary;
+import com.aliyun.sdk.service.oss2.models.CopyObjectRequest;
+import com.aliyun.sdk.service.oss2.models.CopyObjectResult;
+import com.aliyun.sdk.service.oss2.models.DeleteObjectRequest;
+import com.aliyun.sdk.service.oss2.models.DeleteObjectResult;
+import com.aliyun.sdk.service.oss2.models.GetObjectRequest;
+import com.aliyun.sdk.service.oss2.models.GetObjectResult;
+import com.aliyun.sdk.service.oss2.models.HeadObjectRequest;
+import com.aliyun.sdk.service.oss2.models.HeadObjectResult;
+import com.aliyun.sdk.service.oss2.models.ListBucketsRequest;
+import com.aliyun.sdk.service.oss2.models.ListBucketsResult;
+import com.aliyun.sdk.service.oss2.models.ListObjectsRequest;
+import com.aliyun.sdk.service.oss2.models.ListObjectsResult;
+import com.aliyun.sdk.service.oss2.models.ObjectSummary;
+import com.aliyun.sdk.service.oss2.models.PutObjectRequest;
+import com.aliyun.sdk.service.oss2.models.PutObjectResult;
+import com.aliyun.sdk.service.oss2.transport.BinaryData;
+import com.google.gson.Gson;
+import org.apache.camel.Exchange;
+import org.apache.camel.WrappedFile;
+import org.apache.camel.component.alibaba.oss.constants.OSSOperations;
+import org.apache.camel.component.alibaba.oss.constants.OSSProperties;
+import org.apache.camel.component.alibaba.oss.models.ClientConfigurations;
+import org.apache.camel.support.DefaultProducer;
+import org.apache.camel.util.ObjectHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class OSSProducer extends DefaultProducer {
+ private static final Logger LOG =
LoggerFactory.getLogger(OSSProducer.class);
+
+ private final OSSEndpoint endpoint;
+ private OSSClient ossClient;
+ private Gson gson;
+
+ public OSSProducer(OSSEndpoint endpoint) {
+ super(endpoint);
+ this.endpoint = endpoint;
+ }
+
+ @Override
+ protected void doInit() throws Exception {
+ super.doInit();
+ this.gson = new Gson();
+ }
+
+ @Override
+ public void process(Exchange exchange) throws Exception {
+ ClientConfigurations clientConfigurations = new ClientConfigurations();
+
+ if (ossClient == null) {
+ this.ossClient = endpoint.initClient();
+ }
+
Review Comment:
**Important — exchange properties vs headers:** The `updateClientConfigs()`
method reads runtime overrides from `exchange.getProperty(OSSProperties.*)`,
but the Camel convention is to use **message headers**
(`exchange.getIn().getHeader(...)`) for dynamic parameter overrides. Exchange
properties are for internal exchange metadata.
The docs table in `alibaba-oss-component.adoc` has the column header
"Header" but the code reads from properties — users following the documentation
will call `setHeader("CamelAlibabaOssOperation", ...)` and the producer will
silently ignore it.
Recommendation: change `OSSProperties` to be used as header constants (like
`OSSHeaders` already is for the consumer), and read from
`exchange.getIn().getHeader(...)` instead of `exchange.getProperty(...)`.
##########
components/camel-alibaba/camel-alibaba-oss/src/main/java/org/apache/camel/component/alibaba/oss/OSSProducer.java:
##########
@@ -0,0 +1,377 @@
+/*
+ * 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.component.alibaba.oss;
+
+import java.io.File;
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import com.aliyun.sdk.service.oss2.OSSClient;
+import com.aliyun.sdk.service.oss2.models.BucketSummary;
+import com.aliyun.sdk.service.oss2.models.CopyObjectRequest;
+import com.aliyun.sdk.service.oss2.models.CopyObjectResult;
+import com.aliyun.sdk.service.oss2.models.DeleteObjectRequest;
+import com.aliyun.sdk.service.oss2.models.DeleteObjectResult;
+import com.aliyun.sdk.service.oss2.models.GetObjectRequest;
+import com.aliyun.sdk.service.oss2.models.GetObjectResult;
+import com.aliyun.sdk.service.oss2.models.HeadObjectRequest;
+import com.aliyun.sdk.service.oss2.models.HeadObjectResult;
+import com.aliyun.sdk.service.oss2.models.ListBucketsRequest;
+import com.aliyun.sdk.service.oss2.models.ListBucketsResult;
+import com.aliyun.sdk.service.oss2.models.ListObjectsRequest;
+import com.aliyun.sdk.service.oss2.models.ListObjectsResult;
+import com.aliyun.sdk.service.oss2.models.ObjectSummary;
+import com.aliyun.sdk.service.oss2.models.PutObjectRequest;
+import com.aliyun.sdk.service.oss2.models.PutObjectResult;
+import com.aliyun.sdk.service.oss2.transport.BinaryData;
+import com.google.gson.Gson;
+import org.apache.camel.Exchange;
+import org.apache.camel.WrappedFile;
+import org.apache.camel.component.alibaba.oss.constants.OSSOperations;
+import org.apache.camel.component.alibaba.oss.constants.OSSProperties;
+import org.apache.camel.component.alibaba.oss.models.ClientConfigurations;
+import org.apache.camel.support.DefaultProducer;
+import org.apache.camel.util.ObjectHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class OSSProducer extends DefaultProducer {
+ private static final Logger LOG =
LoggerFactory.getLogger(OSSProducer.class);
+
+ private final OSSEndpoint endpoint;
+ private OSSClient ossClient;
+ private Gson gson;
+
+ public OSSProducer(OSSEndpoint endpoint) {
+ super(endpoint);
+ this.endpoint = endpoint;
+ }
+
+ @Override
+ protected void doInit() throws Exception {
+ super.doInit();
+ this.gson = new Gson();
+ }
+
Review Comment:
**Important — JSON serialization via Gson:** All producer operations
serialize results to JSON strings using `gson.toJson(...)`. This is
non-standard in Camel:
1. Most components return the SDK result object or typed `Map<String,
Object>` as the body, letting Camel's type converter system handle
serialization.
2. Users who want to work with the structured data must parse JSON back,
defeating the purpose of an integration framework.
3. This introduces a hard runtime dependency on Gson when Jackson is already
ubiquitous in the Camel ecosystem.
Consider returning the `Map<String, Object>` directly (or the SDK result
objects). Users can convert to JSON themselves if needed via Camel's data
format system.
##########
components/camel-alibaba/camel-alibaba-oss/src/main/java/org/apache/camel/component/alibaba/oss/OSSEndpoint.java:
##########
@@ -0,0 +1,240 @@
+/*
+ * 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.component.alibaba.oss;
+
+import com.aliyun.sdk.service.oss2.OSSClient;
+import org.apache.camel.Category;
+import org.apache.camel.Consumer;
+import org.apache.camel.Processor;
+import org.apache.camel.Producer;
+import org.apache.camel.component.alibaba.common.models.ServiceKeys;
+import org.apache.camel.component.alibaba.oss.constants.OSSHeaders;
+import org.apache.camel.spi.Metadata;
+import org.apache.camel.spi.UriEndpoint;
+import org.apache.camel.spi.UriParam;
+import org.apache.camel.spi.UriPath;
+import org.apache.camel.support.ScheduledPollEndpoint;
+
+/**
+ * Alibaba Cloud Object Storage Service (OSS) component
+ */
+@UriEndpoint(firstVersion = "4.23.0", scheme = "alibaba-oss", title = "Alibaba
Object Storage Service (OSS)",
+ syntax = "alibaba-oss:operation",
+ category = { Category.CLOUD }, headersClass = OSSHeaders.class)
+public class OSSEndpoint extends ScheduledPollEndpoint {
+
+ @UriPath(description = "Operation to be performed", displayName =
"Operation", label = "producer")
+ @Metadata(required = true)
+ private String operation;
+
+ @UriParam(description = "OSS service region", displayName = "Service
region")
+ @Metadata(required = true)
+ private String region;
+
+ @UriParam(description = "OSS endpoint URL. Carries higher precedence than
region based client initialization",
+ displayName = "Endpoint url")
+ private String endpoint;
+
+ @UriParam(description = "Configuration object for cloud service
authentication", displayName = "Service Configuration",
+ security = "secret", label = "security")
+ private ServiceKeys serviceKeys;
+
+ @UriParam(description = "Access key for the cloud user", displayName =
"API access key (AK)",
+ security = "secret", label = "security")
+ @Metadata(required = true)
+ private String accessKey;
Review Comment:
**Important:** Missing `secret = true` on `@UriParam`. The `security =
"secret"` attribute controls the security *policy* framework (e.g.,
`camel.main.profile = prod`), but it does **not** mask the value in logs, JMX,
and management APIs. For that, `secret = true` is required.
Per CLAUDE.md: *"Mark sensitive parameters with `secret = true` on
`@UriParam` or `@Metadata` (passwords, tokens, API keys)"*.
The MNS module correctly uses `secret = true` — this module should do the
same. Apply the same fix to the `secretKey` and `serviceKeys` fields below.
```suggestion
@UriParam(description = "Access key for the cloud user", displayName =
"API access key (AK)",
secret = true, security = "secret", label = "security")
@Metadata(required = true)
private String accessKey;
```
##########
components/camel-alibaba/camel-alibaba-mns/src/main/java/org/apache/camel/component/alibaba/mns/MNSEndpoint.java:
##########
@@ -0,0 +1,261 @@
+/*
+ * 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.component.alibaba.mns;
+
+import com.aliyun.mns.client.MNSClient;
+import org.apache.camel.Category;
+import org.apache.camel.Consumer;
+import org.apache.camel.Processor;
+import org.apache.camel.Producer;
+import org.apache.camel.component.alibaba.common.models.ServiceKeys;
+import org.apache.camel.component.alibaba.mns.constants.MNSHeaders;
+import org.apache.camel.component.alibaba.mns.constants.MNSOperations;
+import org.apache.camel.spi.Metadata;
+import org.apache.camel.spi.UriEndpoint;
+import org.apache.camel.spi.UriParam;
+import org.apache.camel.spi.UriPath;
+import org.apache.camel.support.ScheduledPollEndpoint;
+
+/**
+ * Send and receive messages to/from Alibaba Cloud Message Service (MNS).
+ */
+@UriEndpoint(firstVersion = "4.23.0", scheme = "alibaba-mns", title = "Alibaba
Message Service (MNS)",
+ syntax = "alibaba-mns:queueName", category = { Category.CLOUD,
Category.MESSAGING },
+ headersClass = MNSHeaders.class)
+public class MNSEndpoint extends ScheduledPollEndpoint {
+
+ @UriPath(description = "Queue name, or topic name when using the topic URI
syntax", displayName = "Queue Name")
+ @Metadata(required = true)
+ private String queueName;
+
+ @UriParam(description = "Operation to perform", displayName = "Operation",
+ enums =
"sendMessage,receiveMessage,deleteMessage,publishMessage")
+ private String operation;
+
+ @UriParam(description = "Alibaba Cloud region", displayName = "Region")
+ @Metadata(required = true)
+ private String region;
+
+ @UriParam(description = "MNS account endpoint, for example
https://123456.mns.cn-hangzhou.aliyuncs.com";,
+ displayName = "Account Endpoint")
+ @Metadata(required = true)
+ private String accountEndpoint;
+
+ @UriParam(description = "Access key for the cloud user", displayName =
"Access Key", secret = true)
+ private String accessKey;
+
+ @UriParam(description = "Secret key for the cloud user", displayName =
"Secret Key", secret = true)
+ private String secretKey;
+
+ @UriParam(description = "Configuration object for cloud service
authentication", displayName = "Service Keys",
+ security = "secret")
+ private ServiceKeys serviceKeys;
Review Comment:
**Moderate — inconsistent annotation style:** MNS correctly uses `secret =
true` here, but is missing `label = "security"` for catalog grouping (which OSS
has). For consistency across the two modules, both endpoints should use `secret
= true` (for masking) AND `label = "security"` (for catalog grouping).
```suggestion
@UriParam(description = "Access key for the cloud user", displayName =
"Access Key",
secret = true, label = "security")
private String accessKey;
```
Apply the same to the `secretKey` and `serviceKeys` fields.
##########
components/camel-alibaba/camel-alibaba-mns/src/main/java/org/apache/camel/component/alibaba/mns/constants/MNSProperties.java:
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.component.alibaba.mns.constants;
+
+public final class MNSProperties {
+
+ public static final String OPERATION = "CamelAlibabaMnsOperation";
+ public static final String QUEUE_NAME = "CamelAlibabaMnsQueueName";
+ public static final String TOPIC_NAME = "CamelAlibabaMnsTopicName";
+ public static final String RECEIPT_HANDLE = "CamelAlibabaMnsReceiptHandle";
+
+ public static final String MESSAGE_ID = "CamelAlibabaMnsMessageId";
+ public static final String REQUEST_ID = "CamelAlibabaMnsRequestId";
Review Comment:
**Minor:** `MNSProperties.RECEIPT_HANDLE` and `MNSHeaders.RECEIPT_HANDLE`
both resolve to `"CamelAlibabaMnsReceiptHandle"`. In
`MNSUtils.resolveReceiptHandle()`, the method checks 3 different sources
(property by `MNSProperties.RECEIPT_HANDLE`, header by
`MNSProperties.RECEIPT_HANDLE`, header by `MNSHeaders.RECEIPT_HANDLE`) — but
the last two are identical. Consider removing the duplicate constant or
consolidating the lookup.
##########
components/camel-alibaba/camel-alibaba-oss/src/main/java/org/apache/camel/component/alibaba/oss/OSSEndpoint.java:
##########
@@ -0,0 +1,240 @@
+/*
+ * 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.component.alibaba.oss;
+
+import com.aliyun.sdk.service.oss2.OSSClient;
+import org.apache.camel.Category;
+import org.apache.camel.Consumer;
+import org.apache.camel.Processor;
+import org.apache.camel.Producer;
+import org.apache.camel.component.alibaba.common.models.ServiceKeys;
+import org.apache.camel.component.alibaba.oss.constants.OSSHeaders;
+import org.apache.camel.spi.Metadata;
+import org.apache.camel.spi.UriEndpoint;
+import org.apache.camel.spi.UriParam;
+import org.apache.camel.spi.UriPath;
+import org.apache.camel.support.ScheduledPollEndpoint;
+
+/**
+ * Alibaba Cloud Object Storage Service (OSS) component
+ */
+@UriEndpoint(firstVersion = "4.23.0", scheme = "alibaba-oss", title = "Alibaba
Object Storage Service (OSS)",
+ syntax = "alibaba-oss:operation",
+ category = { Category.CLOUD }, headersClass = OSSHeaders.class)
Review Comment:
**Moderate — URI design for consumer use:** The syntax
`alibaba-oss:operation` requires an operation as the URI path, but this
component supports both producer and consumer. When used as a consumer
(`from("alibaba-oss:listObjects?bucketName=...")`), the user must provide an
operation name that is meaningless for consumers.
Consider using `bucketName` as the path parameter (similar to
`camel-aws2-s3`) and making `operation` a query parameter, e.g.:
```
syntax = "alibaba-oss:bucketName"
```
This would make the consumer URI more natural:
`from("alibaba-oss:my-bucket?deleteAfterRead=true")`.
--
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]
