davsclaus commented on code in PR #25498: URL: https://github.com/apache/camel/pull/25498#discussion_r3841562312
########## components/camel-alibaba/camel-alibaba-sls/src/main/java/org/apache/camel/component/alibaba/sls/AlibabaSlsUtils.java: ########## @@ -0,0 +1,215 @@ +/* + * 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 java.util.HashMap; +import java.util.Map; + +import com.aliyun.sls20201230.Client; +import com.aliyun.sls20201230.models.GetLogsRequest; +import com.aliyun.sls20201230.models.GetLogsResponse; +import com.aliyun.sls20201230.models.ListLogStoresRequest; +import com.aliyun.sls20201230.models.ListLogStoresResponse; +import com.aliyun.sls20201230.models.ListLogStoresResponseBody; +import com.aliyun.sls20201230.models.LogGroup; +import com.aliyun.sls20201230.models.PutLogsRequest; +import com.aliyun.sls20201230.models.PutLogsResponse; +import com.aliyun.teaopenapi.models.Config; +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.sls.constants.AlibabaSlsProperties; +import org.apache.camel.component.alibaba.sls.models.ClientConfigurations; +import org.apache.camel.util.ObjectHelper; + +public final class AlibabaSlsUtils { + + private AlibabaSlsUtils() { + } + + public static Client createClient(AlibabaSlsEndpoint endpoint) throws Exception { + if (ObjectHelper.isEmpty(endpoint.getRegion()) && ObjectHelper.isEmpty(endpoint.getEndpoint())) { + throw new IllegalArgumentException("Region or endpoint is required"); + } + + return new Client( + createTeaConfig( + endpoint.getAccessKey(), + endpoint.getSecretKey(), + endpoint.getServiceKeys(), + endpoint.getRegion(), + endpoint.getEndpoint())); + } + + private static Config createTeaConfig( + String accessKey, String secretKey, ServiceKeys serviceKeys, String region, String endpoint) { + Config config = new Config(); + config.setAccessKeyId(OpenApiClientSupport.resolveAccessKey(accessKey, serviceKeys)); + config.setAccessKeySecret(OpenApiClientSupport.resolveSecretKey(secretKey, serviceKeys)); + if (ObjectHelper.isNotEmpty(region)) { + config.setRegionId(region); + } + if (ObjectHelper.isNotEmpty(endpoint)) { + config.setEndpoint(endpoint); + } + return config; + } + + public static ClientConfigurations createClientConfigurations(AlibabaSlsEndpoint endpoint, Exchange exchange) { + ClientConfigurations configuration = new ClientConfigurations(); + configuration + .setOperation( + OpenApiClientSupport.resolveString(exchange, AlibabaSlsProperties.OPERATION, endpoint.getOperation())); + configuration.setProject( + OpenApiClientSupport.resolveString(exchange, AlibabaSlsProperties.PROJECT, endpoint.getProject())); + configuration.setLogStoreName( + OpenApiClientSupport.resolveString(exchange, AlibabaSlsProperties.LOG_STORE_NAME, endpoint.getLogStoreName())); + configuration.setQuery(OpenApiClientSupport.resolveString(exchange, AlibabaSlsProperties.QUERY, endpoint.getQuery())); + configuration.setFrom(OpenApiClientSupport.resolveInteger(exchange, AlibabaSlsProperties.FROM, endpoint.getFrom())); + configuration.setTo(OpenApiClientSupport.resolveInteger(exchange, AlibabaSlsProperties.TO, endpoint.getTo())); + configuration.setLine(resolveLong(exchange, AlibabaSlsProperties.LINE, endpoint.getLine())); + configuration.setOffset(resolveLong(exchange, AlibabaSlsProperties.OFFSET, endpoint.getOffset())); + configuration.setTopic(OpenApiClientSupport.resolveString(exchange, AlibabaSlsProperties.TOPIC, endpoint.getTopic())); + configuration.setReverse(resolveBoolean(exchange, AlibabaSlsProperties.REVERSE, endpoint.getReverse())); + configuration.setLogstoreName( + OpenApiClientSupport.resolveString(exchange, AlibabaSlsProperties.LOGSTORE_NAME, null)); + configuration.setMode(OpenApiClientSupport.resolveString(exchange, AlibabaSlsProperties.MODE, null)); + configuration.setListOffset(OpenApiClientSupport.resolveInteger(exchange, AlibabaSlsProperties.OFFSET, null)); Review Comment: This reuses `AlibabaSlsProperties.OFFSET` (`CamelAlibabaSlsOffset`) — the same header already used for `getLogs`'s offset on line 85 — to resolve the `listLogStores` offset. `AlibabaSlsProperties` only defines one `OFFSET` constant, so a route can't set independent offsets for `getLogs` and `listLogStores` in the same exchange/route, and setting the `getLogs` offset header would unintentionally also affect a subsequent `listLogStores` call (or vice versa). Every other `listLogStores`-specific field (`mode`, `logstoreName` filter, `size`, `telemetryType`) got its own dedicated property/header — this looks like a copy-paste slip rather than intentional sharing. Suggest adding a dedicated `LIST_OFFSET = "CamelAlibabaSlsListOffset"` constant and using that here instead. ########## tooling/maven/camel-package-maven-plugin/src/main/java/org/apache/camel/maven/packaging/MojoHelper.java: ########## @@ -149,7 +149,9 @@ public static List<Path> getComponentPath(Path dir) { dir.resolve("camel-alibaba-fc"), dir.resolve("camel-alibaba-sms"), dir.resolve("camel-alibaba-kms"), - dir.resolve("camel-alibaba-eventbridge")); + dir.resolve("camel-alibaba-sls"), Review Comment: Nit: `camel-alibaba-sls` is inserted before `camel-alibaba-eventbridge`, breaking alphabetical order within this list (the list wasn't strictly alphabetical before this PR either, so purely cosmetic). Non-blocking. ########## components/camel-alibaba/camel-alibaba-sls/src/main/java/org/apache/camel/component/alibaba/sls/models/ClientConfigurations.java: ########## @@ -0,0 +1,156 @@ +/* + * 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.models; + +public class ClientConfigurations { + + private String operation; + private String project; + private String logStoreName; + private String query; + private Integer from; + private Integer to; + private Long line; + private Long offset; + private String topic; + private Boolean reverse; + private String logstoreName; Review Comment: Nit: `logstoreName` (this field, a prefix filter for `listLogStores`) differs from `logStoreName` above (line 23, the target log store for `getLogs`/`putLogs`) only by capitalization. Easy to misread or typo when extending this class — consider a clearer name like `logstoreNamePrefix`. Non-blocking. -- 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]
