davsclaus commented on code in PR #25905: URL: https://github.com/apache/camel/pull/25905#discussion_r3892321018
########## components/camel-hivemq/src/main/java/org/apache/camel/component/hivemq/HiveMQEndpoint.java: ########## @@ -0,0 +1,110 @@ +/* + * 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.hivemq; + +import com.hivemq.client.mqtt.MqttClient; +import com.hivemq.client.mqtt.MqttClientBuilder; +import com.hivemq.client.mqtt.mqtt5.Mqtt5AsyncClient; +import org.apache.camel.Category; +import org.apache.camel.Consumer; +import org.apache.camel.Processor; +import org.apache.camel.Producer; +import org.apache.camel.spi.EndpointServiceLocation; +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; + +@UriEndpoint(firstVersion = "4.23.0", scheme = "hivemq", title = "HiveMQ", syntax = "hivemq:topic", + category = { Category.MESSAGING, Category.IOT }, headersClass = HiveMQConstants.class) +public class HiveMQEndpoint extends DefaultEndpoint implements EndpointServiceLocation { + + /** + * The MQTT topic name or pattern to subscribe to or publish on. + */ + @UriPath + @Metadata(required = true) + private String topic; + + /** + * The HiveMQ component configuration options. + */ + @UriParam + @Metadata(description = "To use a custom HiveMQConfiguration") + private HiveMQConfiguration configuration; + + public HiveMQEndpoint(String uri, HiveMQComponent component, HiveMQConfiguration configuration, String topic) { + super(uri, component); + this.configuration = configuration; + this.topic = topic; + } + + @Override + public Producer createProducer() throws Exception { + return new HiveMQProducer(this); + } + + @Override + public Consumer createConsumer(Processor processor) throws Exception { + HiveMQConsumer consumer = new HiveMQConsumer(this, processor); + configureConsumer(consumer); + return consumer; + } + + public Mqtt5AsyncClient createClient() { + MqttClientBuilder builder = MqttClient.builder() + .serverHost(configuration.getHost()) + .serverPort(configuration.getPort()); + + if (configuration.getClientId() != null) { + builder.identifier(configuration.getClientId()); + } + + if (configuration.isSsl()) { + builder.sslWithDefaultConfig(); + } + + return builder.useMqttVersion5().buildAsync(); Review Comment: `createClient()` never applies `configuration.getUsername()`/`getPassword()` to the builder (e.g. via `.simpleAuth(Mqtt5SimpleAuth.builder().username(...).password(...).build())`). As written, the `username`/`password` URI options are dead — a user who sets them will connect without authentication and get no error or warning. This should call the HiveMQ client's simple-auth builder when a username is configured. ########## components/camel-hivemq/src/main/java/org/apache/camel/component/hivemq/HiveMQConfiguration.java: ########## @@ -0,0 +1,177 @@ +/* + * 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.hivemq; + +import com.hivemq.client.mqtt.MqttVersion; +import com.hivemq.client.mqtt.datatypes.MqttQos; +import org.apache.camel.spi.Metadata; +import org.apache.camel.spi.UriParam; +import org.apache.camel.spi.UriParams; + +@UriParams +public class HiveMQConfiguration implements Cloneable { + + /** + * Hostname or IP address of the HiveMQ MQTT broker. + */ + @UriParam(defaultValue = HiveMQConstants.DEFAULT_HOST) + private String host = HiveMQConstants.DEFAULT_HOST; + + /** + * Port number of the HiveMQ MQTT broker. + */ + @UriParam(defaultValue = "1883") + private int port = HiveMQConstants.DEFAULT_PORT; + + /** + * Client identifier used when connecting to the HiveMQ broker. + */ + @UriParam + private String clientId; + + /** + * MQTT protocol version to use for the connection. + */ + @UriParam(defaultValue = "MQTT_5_0") + private MqttVersion version = MqttVersion.MQTT_5_0; + + /** + * Default Quality of Service (QoS) level to use for messages. + */ + @UriParam(defaultValue = "AT_LEAST_ONCE") + private MqttQos qos = MqttQos.AT_LEAST_ONCE; + + /** + * Whether published messages should be retained by the MQTT broker. + */ + @UriParam(defaultValue = "false") + private boolean retained; + + /** + * Whether to initiate a clean session upon connecting to the broker. + */ + @UriParam(defaultValue = "true") + private boolean cleanStart = true; + + /** + * Username for authentication with the HiveMQ broker. + */ + @UriParam(label = "security") + @Metadata(label = "security") + private String username; + + /** + * Password for authentication with the HiveMQ broker. + */ + @UriParam(label = "security") Review Comment: `password` should be marked `secret = true` (in addition to `label = "security"`), per this project's convention for sensitive `@UriParam`s. The generated catalog currently shows `"secret": false` for this field, meaning the password won't be masked in traces, the management console, or logged endpoint URIs. ########## components/camel-hivemq/src/test/java/org/apache/camel/component/hivemq/HiveMQSendDynamicAwareTest.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.hivemq; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.camel.Exchange; +import org.apache.camel.impl.DefaultCamelContext; +import org.apache.camel.spi.SendDynamicAware.DynamicAwareEntry; +import org.apache.camel.support.DefaultExchange; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class HiveMQSendDynamicAwareTest { + + private DefaultCamelContext camelContext; + private HiveMQSendDynamicAware sendDynamicAware; + + @BeforeEach + void setUp() { + camelContext = new DefaultCamelContext(); + sendDynamicAware = new HiveMQSendDynamicAware(); + sendDynamicAware.setCamelContext(camelContext); + sendDynamicAware.setScheme("hivemq"); + } + + @Test + @DisplayName("Prepare dynamic awareness injects override header") + void testPrepareInjectsHeader() throws Exception { + Exchange exchange = new DefaultExchange(camelContext); + + Map<String, Object> properties = new HashMap<>(); + properties.put("topic", "dynamic/sensors/temperature"); + + DynamicAwareEntry entry = new DynamicAwareEntry( + "hivemq:dynamic/sensors/temperature", + "hivemq:dynamic/sensors/temperature", + properties, + new HashMap<>()); + + // Use createPreProcessor if SendDynamicAware uses processor-based preparation Review Comment: Minor: this comment and the `if (processor != null)` guard read like uncertainty about whether `createPreProcessor` can return null here, rather than an intentional case for this component. Since `HiveMQSendDynamicAware.createPreProcessor` always returns a non-null `Processor`, this can likely be simplified to a direct call + assert. ########## components/camel-hivemq/src/main/java/org/apache/camel/component/hivemq/HiveMQConsumer.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.hivemq; + +import com.hivemq.client.mqtt.mqtt5.Mqtt5AsyncClient; +import com.hivemq.client.mqtt.mqtt5.message.publish.Mqtt5Publish; +import org.apache.camel.Exchange; +import org.apache.camel.Processor; +import org.apache.camel.support.DefaultConsumer; + +public class HiveMQConsumer extends DefaultConsumer { + + private final HiveMQEndpoint endpoint; + private Mqtt5AsyncClient client; + + public HiveMQConsumer(HiveMQEndpoint endpoint, Processor processor) { + super(endpoint, processor); + this.endpoint = endpoint; + } + + @Override + protected void doStart() throws Exception { + super.doStart(); + client = endpoint.createClient(); + client.connect().join(); Review Comment: Minor/question: `client.connect().join()` blocks indefinitely with no timeout. If the broker is unreachable, consumer startup (and the same pattern in `HiveMQProducer.doStart()`) could hang forever instead of failing fast. Worth considering a bounded wait. ########## components/camel-hivemq/src/test/java/org/apache/camel/component/hivemq/HiveMQComponentITTest.java: ########## @@ -0,0 +1,58 @@ +/* + * 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.hivemq; + +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.component.mock.MockEndpoint; +import org.apache.camel.test.infra.hivemq.services.HiveMQService; +import org.apache.camel.test.infra.hivemq.services.HiveMQServiceFactory; +import org.apache.camel.test.junit6.CamelTestSupport; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +public class HiveMQComponentITTest extends CamelTestSupport { Review Comment: This class needs a real broker (via `HiveMQServiceFactory`) but is named `*ITTest.java`, not `*IT.java`. The parent POM's Surefire config includes `**/*Test.java` and only excludes `**/*IT.java`, so this file matches the unit-test include pattern and will run under plain `mvn test` (unlike `HiveMQComponentPubSubIT` and the other IT classes in this PR, which correctly end in `IT.java`). It also duplicates `HiveMQComponentPubSubIT`'s basic pub/sub scenario almost exactly — suggest deleting this file rather than renaming it, since the coverage is already provided there. -- 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]
