gnodet-bot commented on code in PR #27168: URL: https://github.com/apache/camel/pull/27168#discussion_r4152098913
########## components/camel-odata/src/main/java/org/apache/camel/component/odata/ODataConfiguration.java: ########## @@ -0,0 +1,168 @@ +/* + * 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.odata; + +import org.apache.camel.spi.UriParam; +import org.apache.camel.spi.UriParams; +import org.apache.camel.support.jsse.SSLContextParameters; + +@UriParams +public class ODataConfiguration { + + @UriParam(label = "producer", description = "The OData operation to perform") + private ODataOperation operation = ODataOperation.READ_SET; + + @UriParam(label = "producer", description = "The OData $filter query parameter") + private String filter; + + @UriParam(label = "producer", description = "The OData $select query parameter") + private String select; + + @UriParam(label = "producer", description = "The OData $expand query parameter") + private String expand; + + @UriParam(label = "producer", description = "The OData $orderby query parameter") + private String orderBy; + + @UriParam(label = "producer", description = "The OData $top query parameter") + private Integer top; + + @UriParam(label = "producer", description = "The OData $skip query parameter") + private Integer skip; + + @UriParam(label = "producer", description = "The OData $count query parameter") + private Boolean count; + + @UriParam(label = "security", description = "Authentication method to use (e.g., Basic, Bearer)") + private String authMethod; + + @UriParam(label = "security", description = "Username for Basic authentication") + private String authUsername; + + @UriParam(label = "security", description = "Password for Basic authentication") + private String authPassword; Review Comment: 🔴 **Security:** `authPassword` is not annotated with `secret = true` on its `@UriParam`. This means the password will appear in plain text in endpoint URIs, logs, JMX, and management APIs. Same issue on `authBearerToken` at line 62. The generated `odata.json` confirms this — both properties have `"secret": false` and `SECRET_PROPERTY_NAMES` is empty. ```suggestion @UriParam(label = "security", secret = true, description = "Password for Basic authentication") private String authPassword; ``` ########## components/camel-odata/src/main/java/org/apache/camel/component/odata/ODataConfiguration.java: ########## @@ -0,0 +1,168 @@ +/* + * 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.odata; + +import org.apache.camel.spi.UriParam; +import org.apache.camel.spi.UriParams; +import org.apache.camel.support.jsse.SSLContextParameters; + +@UriParams +public class ODataConfiguration { + + @UriParam(label = "producer", description = "The OData operation to perform") + private ODataOperation operation = ODataOperation.READ_SET; + + @UriParam(label = "producer", description = "The OData $filter query parameter") + private String filter; + + @UriParam(label = "producer", description = "The OData $select query parameter") + private String select; + + @UriParam(label = "producer", description = "The OData $expand query parameter") + private String expand; + + @UriParam(label = "producer", description = "The OData $orderby query parameter") + private String orderBy; + + @UriParam(label = "producer", description = "The OData $top query parameter") + private Integer top; + + @UriParam(label = "producer", description = "The OData $skip query parameter") + private Integer skip; + + @UriParam(label = "producer", description = "The OData $count query parameter") + private Boolean count; + + @UriParam(label = "security", description = "Authentication method to use (e.g., Basic, Bearer)") + private String authMethod; + + @UriParam(label = "security", description = "Username for Basic authentication") + private String authUsername; + + @UriParam(label = "security", description = "Password for Basic authentication") + private String authPassword; + + @UriParam(label = "security", description = "Bearer token for Bearer authentication") + private String authBearerToken; + + @UriParam(label = "security", description = "To use a custom SSLContextParameters") Review Comment: 🔴 **Security:** Same issue — bearer token must be marked as secret. ```suggestion @UriParam(label = "security", secret = true, description = "Bearer token for Bearer authentication") private String authBearerToken; ``` ########## components/camel-odata/src/main/java/org/apache/camel/component/odata/ODataComponent.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.component.odata; + +import java.net.URI; +import java.util.LinkedHashMap; +import java.util.Map; + +import org.apache.camel.Endpoint; +import org.apache.camel.spi.annotations.Component; +import org.apache.camel.support.DefaultComponent; + +@Component("odata") +public class ODataComponent extends DefaultComponent { + + @Override + protected Endpoint createEndpoint(String uri, String remaining, Map<String, Object> parameters) + throws Exception { + + ODataEndpoint endpoint = new ODataEndpoint(uri, this); + + endpoint.setHttpUri(new URI(remaining)); + + // Create a copy of parameters before setProperties consumes known properties + Map<String, Object> customParams = new LinkedHashMap<>(parameters); Review Comment: ⚠️ **Dead code:** `customParams` is assigned but never read. Remove it. ```suggestion ``` ########## components/camel-odata/src/main/java/org/apache/camel/component/odata/ODataHelper.java: ########## @@ -0,0 +1,67 @@ +/* + * 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.odata; + +import java.util.List; +import java.util.Map; + +import org.apache.camel.util.json.DeserializationException; +import org.apache.camel.util.json.Jsoner; + +public final class ODataHelper { + + private ODataHelper() { + } + + public static String toJson(Object value) { + if (value == null) { + return null; + } + + if (value instanceof String string) { + return string; + } + + return Jsoner.serialize(value); + } + + @SuppressWarnings("unchecked") + public static Map<String, Object> parseJsonObject(String value) throws DeserializationException { + if (value == null || value.isBlank()) { + return Map.of(); + } + + Object parsed = Jsoner.deserialize(value); + if (!(parsed instanceof Map)) { + throw new IllegalArgumentException("Expected an OData JSON object response"); + } + + return (Map<String, Object>) parsed; + } + + @SuppressWarnings("unchecked") + public static List<Map<String, Object>> extractValueList(Map<String, Object> response) { + Object value = response.get("value"); + + if (!(value instanceof List)) { + throw new IllegalArgumentException("OData response does not contain a value array"); + } + + return (List<Map<String, Object>>) value; + } Review Comment: ⚠️ **Dead code:** `extractValueList()` is not called anywhere in the codebase. Either use it in `ODataProducer.process()` to parse `READ_SET` responses, or remove it. ########## components/camel-odata/pom.xml: ########## @@ -0,0 +1,77 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<!-- + + 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. + +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd";> + + <modelVersion>4.0.0</modelVersion> + + <parent> + <groupId>org.apache.camel</groupId> + <artifactId>components</artifactId> + <version>4.23.0-SNAPSHOT</version> + </parent> + + <artifactId>camel-odata</artifactId> + <packaging>jar</packaging> + + <name>Camel :: OData</name> + <description>Camel OData Component</description> + + <properties> + <firstVersion>4.23.0-SNAPSHOT</firstVersion> Review Comment: ⚠️ **Convention:** `firstVersion` should be `4.23.0`, not `4.23.0-SNAPSHOT`. The generated `odata.json` under `src/generated/resources/odata.json` also has `4.23.0-SNAPSHOT` as `firstVersion` while the component descriptor under `META-INF` has the correct `4.23.0`. This inconsistency will persist in release artifacts. ```suggestion <firstVersion>4.23.0</firstVersion> ``` ########## components/camel-odata/src/main/docs/odata-component.adoc: ########## @@ -0,0 +1,359 @@ += OData Component +:doctitle: OData +:shortname: odata +:artifactid: camel-odata +:description: Camel OData Component +:since: 4.23 +:supportlevel: Preview +:tabs-sync-option: +:component-header: Only producer is supported + +*Since Camel {since}* + +*{component-header}* + +The OData component allows you to interact with OData (Open Data Protocol) services using standard OData CRUD operations. + +The component delegates HTTP transport to Camel HTTP while providing OData-specific operation, key, query option, authentication, and response handling. + +Maven users will need to add the following dependency to their `pom.xml` for this component: + +## [source,xml] Review Comment: 🔴 **Documentation:** Every code block in this file uses `## [source,xml]` / `## [source,java]` instead of the correct AsciiDoc syntax `[source,xml]`. The `##` prefix is a Markdown heading marker and will render as literal text or garbled headings in the Antora-generated docs. This affects all ~17 code blocks in the file. Additionally, lines 278 and 308 have unescaped quotes in Java examples: ``` .setHeader("CamelODataETag", constant("W/"etag-value"")) ``` This is invalid Java. It should be: ``` .setHeader("CamelODataETag", constant("W/\"etag-value\"")) ``` The entire AsciiDoc file needs a formatting pass — every code listing block delimiter and closing is broken. -- 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]
