atiaomar1978-hub commented on code in PR #25498:
URL: https://github.com/apache/camel/pull/25498#discussion_r3789741999


##########
bom/camel-bom/pom.xml:
##########
@@ -106,6 +106,16 @@
         <artifactId>camel-alibaba-oss</artifactId>
         <version>4.23.0-SNAPSHOT</version>
       </dependency>
+      <dependency>
+        <groupId>org.apache.camel</groupId>

Review Comment:
   Fixed — swapped ordering in `coverage/pom.xml` and 
`docs/components/modules/ROOT/nav.adoc` so `ots` comes before `sls`. The BOM on 
this branch was already in the correct order.



##########
components/camel-alibaba/camel-alibaba-sls/src/main/java/org/apache/camel/component/alibaba/sls/AlibabaSlsEndpoint.java:
##########
@@ -0,0 +1,265 @@
+/*
+ * 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.sls;
+
+import com.aliyun.sls20201230.Client;
+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.sls.constants.AlibabaSlsHeaders;
+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.DefaultEndpoint;
+
+/**
+ * Manage logs on Alibaba Cloud Simple Log Service (SLS).
+ */
+@UriEndpoint(firstVersion = "4.23.0", scheme = "alibaba-sls", title = "Alibaba 
Simple Log Service (SLS)",
+             syntax = "alibaba-sls:operation", category = { Category.CLOUD, 
Category.MONITORING },
+             headersClass = AlibabaSlsHeaders.class, producerOnly = true)
+public class AlibabaSlsEndpoint extends DefaultEndpoint {
+
+    @UriPath(description = "Operation to perform", displayName = "Operation", 
label = "producer",
+             enums = "putLogs,getLogs,listLogStores")
+    @Metadata(required = true)
+    private String operation;
+
+    @UriParam(description = "Alibaba Cloud region", displayName = "Region")
+    @Metadata(required = true)
+    private String region;

Review Comment:
   Fixed — removed `@Metadata(required = true)` from `region` so URIs with 
`endpoint` but no `region` are accepted, matching 
`AlibabaSlsUtils.createClient()` validation.



##########
components/camel-alibaba/camel-alibaba-sls/src/main/java/org/apache/camel/component/alibaba/sls/AlibabaSlsEndpoint.java:
##########
@@ -0,0 +1,265 @@
+/*
+ * 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.sls;
+
+import com.aliyun.sls20201230.Client;
+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.sls.constants.AlibabaSlsHeaders;
+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.DefaultEndpoint;
+
+/**
+ * Manage logs on Alibaba Cloud Simple Log Service (SLS).
+ */
+@UriEndpoint(firstVersion = "4.23.0", scheme = "alibaba-sls", title = "Alibaba 
Simple Log Service (SLS)",
+             syntax = "alibaba-sls:operation", category = { Category.CLOUD, 
Category.MONITORING },
+             headersClass = AlibabaSlsHeaders.class, producerOnly = true)
+public class AlibabaSlsEndpoint extends DefaultEndpoint {
+
+    @UriPath(description = "Operation to perform", displayName = "Operation", 
label = "producer",
+             enums = "putLogs,getLogs,listLogStores")
+    @Metadata(required = true)
+    private String operation;
+
+    @UriParam(description = "Alibaba Cloud region", displayName = "Region")
+    @Metadata(required = true)
+    private String region;
+
+    @UriParam(description = "SLS endpoint URL (e.g. 
cn-hangzhou.log.aliyuncs.com). "
+                            + "Carries higher precedence than region based 
client initialization",
+              displayName = "Endpoint")
+    private String endpoint;
+
+    @UriParam(description = "Access key for the cloud user", displayName = 
"Access Key",
+              secret = true, security = "secret", label = "security")
+    private String accessKey;
+
+    @UriParam(description = "Secret key for the cloud user", displayName = 
"Secret Key",
+              secret = true, security = "secret", label = "security")
+    private String secretKey;
+
+    @UriParam(description = "Configuration object for cloud service 
authentication", displayName = "Service Keys",
+              secret = true, security = "secret", label = "security")
+    private ServiceKeys serviceKeys;
+
+    @UriParam(description = "SLS project name", displayName = "Project")
+    private String project;
+
+    @UriParam(description = "SLS log store name", displayName = "Log Store 
Name")
+    private String logStoreName;
+
+    @UriParam(description = "Log query string for getLogs", displayName = 
"Query", label = "getLogs")
+    private String query;
+
+    @UriParam(description = "Query start time for getLogs (Unix timestamp in 
seconds)", displayName = "From",
+              label = "getLogs")
+    private Integer from;
+
+    @UriParam(description = "Query end time for getLogs (Unix timestamp in 
seconds)", displayName = "To",
+              label = "getLogs")
+    private Integer to;
+
+    @UriParam(description = "Maximum number of log lines to return for 
getLogs", displayName = "Line",
+              label = "getLogs")
+    private Long line;
+
+    @UriParam(description = "Log query offset for getLogs", displayName = 
"Offset", label = "getLogs")
+    private Long offset;
+
+    @UriParam(description = "Log topic filter for getLogs", displayName = 
"Topic", label = "getLogs")
+    private String topic;
+
+    @UriParam(description = "Whether to return logs in reverse order for 
getLogs", displayName = "Reverse",
+              label = "getLogs")
+    private Boolean reverse;
+
+    @UriParam(description = "Autowire an existing SLS client instance", 
displayName = "SLS Client", label = "advanced")
+    @Metadata(autowired = true)
+    private Client slsClient;
+
+    private boolean autowiredSlsClient;
+
+    public AlibabaSlsEndpoint() {
+    }
+
+    public AlibabaSlsEndpoint(String uri, String operation, 
AlibabaSlsComponent component) {
+        super(uri, component);
+        this.operation = operation;
+    }
+
+    @Override
+    public Producer createProducer() throws Exception {
+        return new AlibabaSlsProducer(this);
+    }
+
+    @Override
+    public Consumer createConsumer(Processor processor) throws Exception {
+        throw new UnsupportedOperationException("You cannot consume from this 
endpoint");
+    }
+
+    public Client initClient() throws Exception {
+        if (slsClient != null) {
+            return slsClient;
+        }
+        slsClient = AlibabaSlsUtils.createClient(this);
+        return slsClient;
+    }
+
+    @Override
+    protected void doStop() throws Exception {
+        if (slsClient != null && !autowiredSlsClient) {
+            slsClient = null;
+        }
+        super.doStop();

Review Comment:
   Checked the SDK: `com.aliyun.sls20201230.Client` extends 
`com.aliyun.teaopenapi.Client`, which has no public `close()` or `shutdown()` 
method. Alibaba OpenAPI SDK documentation states the client is GC-managed and 
does not require manual destruction. The other Tea OpenAPI-based Alibaba 
components in this codebase (EventBridge, KMS, FC, SMS) follow the same pattern 
— clear the owned reference on `doStop()` without calling close. OTS differs 
because `SyncClient` exposes an explicit `shutdown()`. Happy to add a close 
call if a future SDK version introduces one.



##########
components/camel-alibaba/camel-alibaba-sls/src/main/java/org/apache/camel/component/alibaba/sls/constants/AlibabaSlsHeaders.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.sls.constants;
+
+import org.apache.camel.spi.Metadata;
+
+public final class AlibabaSlsHeaders {
+
+    @Metadata(label = "producer", description = "SLS project name override", 
javaType = "String")
+    public static final String PROJECT = AlibabaSlsProperties.PROJECT;
+
+    @Metadata(label = "producer", description = "SLS log store name override", 
javaType = "String")
+    public static final String LOG_STORE_NAME = 
AlibabaSlsProperties.LOG_STORE_NAME;
+
+    @Metadata(label = "producer", description = "Log query string override", 
javaType = "String")
+    public static final String QUERY = AlibabaSlsProperties.QUERY;
+
+    @Metadata(label = "producer", description = "Query start time override 
(Unix timestamp in seconds)", javaType = "Integer")
+    public static final String FROM = AlibabaSlsProperties.FROM;
+
+    @Metadata(label = "producer", description = "Query end time override (Unix 
timestamp in seconds)", javaType = "Integer")
+    public static final String TO = AlibabaSlsProperties.TO;
+
+    @Metadata(label = "producer", description = "Maximum number of log lines 
to return", javaType = "Long")
+    public static final String LINE = AlibabaSlsProperties.LINE;
+
+    @Metadata(label = "producer", description = "Log query offset", javaType = 
"Long")
+    public static final String OFFSET = AlibabaSlsProperties.OFFSET;
+
+    @Metadata(label = "producer", description = "Log topic filter override", 
javaType = "String")
+    public static final String TOPIC = AlibabaSlsProperties.TOPIC;
+
+    @Metadata(label = "producer", description = "Whether to return logs in 
reverse order", javaType = "Boolean")
+    public static final String REVERSE = AlibabaSlsProperties.REVERSE;
+
+    @Metadata(label = "producer", description = "Log store name prefix filter 
for listLogStores", javaType = "String")
+    public static final String LOGSTORE_NAME = 
AlibabaSlsProperties.LOGSTORE_NAME;
+
+    @Metadata(label = "producer", description = "List mode for listLogStores", 
javaType = "String")
+    public static final String MODE = AlibabaSlsProperties.MODE;
+
+    @Metadata(label = "producer", description = "Page size for listLogStores", 
javaType = "Integer")
+    public static final String SIZE = AlibabaSlsProperties.SIZE;
+
+    @Metadata(label = "producer", description = "Telemetry type filter for 
listLogStores", javaType = "String")
+    public static final String TELEMETRY_TYPE = 
AlibabaSlsProperties.TELEMETRY_TYPE;
+
+    @Metadata(label = "producer", description = "Alibaba Cloud request id", 
javaType = "String")
+    public static final String REQUEST_ID = AlibabaSlsProperties.REQUEST_ID;
+
+    @Metadata(label = "producer", description = "HTTP status code from SLS 
response", javaType = "Integer")
+    public static final String STATUS_CODE = "CamelAlibabaSlsStatusCode";

Review Comment:
   Fixed — added `STATUS_CODE` to `AlibabaSlsProperties` and referenced it from 
`AlibabaSlsHeaders`, consistent with the other header constants.



##########
components/camel-alibaba/camel-alibaba-ots/src/test/java/org/apache/camel/component/alibaba/ots/AlibabaOtsEndpointTest.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.ots;
+
+import com.alicloud.openservices.tablestore.SyncClient;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+class AlibabaOtsEndpointTest {
+
+    @Test
+    void initClientReturnsCachedInstance() throws Exception {
+        AlibabaOtsEndpoint endpoint = new AlibabaOtsEndpoint();
+        endpoint.setAccessKey("ak");
+        endpoint.setSecretKey("sk");
+        
endpoint.setEndpoint("https://test-instance.cn-hangzhou.ots.aliyuncs.com";);
+        endpoint.setInstanceName("test-instance");
+
+        SyncClient first = endpoint.initClient();
+        SyncClient second = endpoint.initClient();
+
+        assertThat(first).isSameAs(second);

Review Comment:
   Fixed — the test now injects a mock `SyncClient` via `setOtsClient()` and 
verifies caching returns the same instance, without creating a real SDK client 
or triggering network I/O.



##########
components/camel-alibaba/camel-alibaba-ots/src/main/java/org/apache/camel/component/alibaba/ots/AlibabaOtsUtils.java:
##########
@@ -0,0 +1,189 @@
+/*
+ * 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.ots;
+
+import java.util.ArrayList;
+import java.util.Base64;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import com.alicloud.openservices.tablestore.SyncClient;
+import com.alicloud.openservices.tablestore.model.CapacityUnit;
+import com.alicloud.openservices.tablestore.model.Column;
+import com.alicloud.openservices.tablestore.model.ConsumedCapacity;
+import com.alicloud.openservices.tablestore.model.DeleteRowResponse;
+import com.alicloud.openservices.tablestore.model.GetRowResponse;
+import com.alicloud.openservices.tablestore.model.ListTableResponse;
+import com.alicloud.openservices.tablestore.model.PrimaryKey;
+import com.alicloud.openservices.tablestore.model.PrimaryKeyColumn;
+import com.alicloud.openservices.tablestore.model.PutRowResponse;
+import com.alicloud.openservices.tablestore.model.Response;
+import com.alicloud.openservices.tablestore.model.Row;
+import com.alicloud.openservices.tablestore.model.UpdateRowResponse;
+import org.apache.camel.Exchange;
+import org.apache.camel.component.alibaba.common.OpenApiClientSupport;
+import org.apache.camel.component.alibaba.common.models.ServiceKeys;
+import org.apache.camel.component.alibaba.ots.constants.AlibabaOtsProperties;
+import org.apache.camel.component.alibaba.ots.models.ClientConfigurations;
+import org.apache.camel.util.ObjectHelper;
+
+public final class AlibabaOtsUtils {
+
+    private AlibabaOtsUtils() {
+    }
+
+    public static SyncClient createClient(AlibabaOtsEndpoint endpoint) {
+        if (ObjectHelper.isEmpty(endpoint.getEndpoint())) {
+            throw new IllegalArgumentException("Endpoint is required");
+        }
+        if (ObjectHelper.isEmpty(endpoint.getInstanceName())) {
+            throw new IllegalArgumentException("Instance name is required");
+        }
+
+        return new SyncClient(
+                endpoint.getEndpoint(),
+                resolveAccessKey(endpoint.getAccessKey(), 
endpoint.getServiceKeys()),
+                resolveSecretKey(endpoint.getSecretKey(), 
endpoint.getServiceKeys()),
+                endpoint.getInstanceName());
+    }
+
+    public static String resolveAccessKey(String accessKey, ServiceKeys 
serviceKeys) {
+        if (ObjectHelper.isNotEmpty(accessKey)) {
+            return accessKey;
+        }
+        if (serviceKeys != null && 
ObjectHelper.isNotEmpty(serviceKeys.getAccessKey())) {
+            return serviceKeys.getAccessKey();
+        }
+        throw new IllegalArgumentException("Authentication parameter 'access 
key (AK)' not found");
+    }
+
+    public static String resolveSecretKey(String secretKey, ServiceKeys 
serviceKeys) {
+        if (ObjectHelper.isNotEmpty(secretKey)) {
+            return secretKey;
+        }
+        if (serviceKeys != null && 
ObjectHelper.isNotEmpty(serviceKeys.getSecretKey())) {
+            return serviceKeys.getSecretKey();
+        }
+        throw new IllegalArgumentException("Authentication parameter 'secret 
key (SK)' not found");
+    }
+

Review Comment:
   Fixed — removed the duplicate methods and now delegate to 
`OpenApiClientSupport.resolveAccessKey()` / 
`OpenApiClientSupport.resolveSecretKey()`, same as `AlibabaSlsUtils`.



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