Skip to content

CAMEL-23503: Add HiveMQ component - #25905

Open
ramu11 wants to merge 2 commits into
apache:mainfrom
ramu11:CAMEL-23503
Open

CAMEL-23503: Add HiveMQ component#25905
ramu11 wants to merge 2 commits into
apache:mainfrom
ramu11:CAMEL-23503

Conversation

@ramu11

@ramu11 ramu11 commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

CAMEL-23503: Add native HiveMQ component

Description

This PR adds a new native camel-hivemq component to Apache Camel, providing MQTT connectivity using the modern HiveMQ Java Client SDK.

While Camel already provides MQTT support through generic components such as camel-paho, this component provides a dedicated HiveMQ-based implementation with native Camel endpoint/configuration support and integration with Camel's test infrastructure.

Key capabilities

  • Dedicated HiveMQ endpoint

    • Adds the hivemq: endpoint for publishing messages to and consuming messages from MQTT brokers.
    • Supports broker connection configuration such as host, port, credentials, and other MQTT client settings.
  • QoS and retained message support

    • Supports MQTT QoS levels including QoS 1 (AT_LEAST_ONCE) and QoS 2 (EXACTLY_ONCE).
    • Supports MQTT retained messages through the CamelHiveMQRetained header.
  • Camel test infrastructure integration

    • Adds the test-infra-hivemq module integration.
    • Uses Testcontainers and HiveMQService to start a real HiveMQ broker for integration testing.
    • Adds integration coverage for publish/subscribe, topic overrides, QoS, retained messages, dynamic sends, and component configuration.

Testing

Validated locally with:
mvn -pl components/camel-hivemq -am -DskipTests install
mvn -pl components/camel-hivemq -DskipITs test
mvn clean install -Psourcecheck -pl components/camel-hivemq -am

Results:

  • Maven build: SUCCESS
  • Tests: 4 passed, 0 failures, 0 errors
  • git diff --check: clean
  • git diff --cached --check: clean

@ramu11
ramu11 requested review from apupier and davsclaus August 30, 2026 14:28
@github-actions

Copy link
Copy Markdown
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@davsclaus

Copy link
Copy Markdown
Contributor

There are uncommitted changes
HEAD detached at pull/25905/merge
Changes not staged for commit:
(use "git add ..." to update what will be committed)
(use "git restore ..." to discard changes in working directory)
modified: bom/camel-bom/pom.xml
modified: catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/components.properties
modified: components/camel-hivemq/src/generated/java/org/apache/camel/component/hivemq/HiveMQComponentConfigurer.java
modified: components/camel-hivemq/src/generated/java/org/apache/camel/component/hivemq/HiveMQEndpointConfigurer.java
modified: components/camel-hivemq/src/generated/java/org/apache/camel/component/hivemq/HiveMQEndpointUriFactory.java
modified: core/camel-main/src/generated/resources/org/apache/camel/main/components.properties
modified: dsl/camel-componentdsl/src/generated/java/org/apache/camel/builder/component/ComponentsBuilderFactory.java
modified: dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/EndpointBuilderFactory.java
modified: dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/EndpointBuilders.java
modified: dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/EndpointHeaderBuilders.java
modified: dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/StaticEndpointBuilders.java
modified: dsl/camel-kamelet-main/src/generated/resources/camel-component-known-dependencies.properties

Untracked files:
(use "git add ..." to include in what will be committed)
catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/components/hivemq.json
docs/components/modules/ROOT/examples/json/hivemq.json
dsl/camel-componentdsl/src/generated/java/org/apache/camel/builder/component/dsl/HivemqComponentBuilderFactory.java
dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/dsl/HiveMQEndpointBuilderFactory.java

@ramu11

ramu11 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

The CI generated-source check is modifying the generated HiveMQ endpoint DSL file with whitespace-only changes. The working tree was clean before generation, and the generated diff contains no functional changes. I've committed the generated output so the PR reflects the current generator output. Please advise if the generated-source change is expected or if the underlying generator/build configuration should be corrected.

I checked further CI is regenerating the endpoint DSL files with trailing whitespace on blank Javadoc lines. Running generate-sources locally reproduces this across existing generated files (EndpointHeaderBuilders.java, StaticEndpointBuilders.java) as well as the new HiveMQ generated file. It appears to be a generator/template formatting issue rather than something specific to this PR. Should this be addressed in the generator/template?

@davsclaus

Copy link
Copy Markdown
Contributor

There are uncommitted changes
HEAD detached at pull/25905/merge
Changes not staged for commit:
(use "git add ..." to update what will be committed)
(use "git restore ..." to discard changes in working directory)
modified: components/camel-hivemq/src/generated/java/org/apache/camel/component/hivemq/HiveMQComponentConfigurer.java
modified: components/camel-hivemq/src/generated/java/org/apache/camel/component/hivemq/HiveMQEndpointConfigurer.java
modified: components/camel-hivemq/src/generated/java/org/apache/camel/component/hivemq/HiveMQEndpointUriFactory.java
modified: dsl/camel-componentdsl/src/generated/java/org/apache/camel/builder/component/ComponentsBuilderFactory.java
modified: dsl/camel-componentdsl/src/generated/java/org/apache/camel/builder/component/dsl/HivemqComponentBuilderFactory.java
modified: dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/EndpointHeaderBuilders.java
modified: dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/StaticEndpointBuilders.java
modified: dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/dsl/HiveMQEndpointBuilderFactory.java

@ramu11

ramu11 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

thinkpadp16vgen1:~/Development/git-projects/camel$ git status
On branch CAMEL-23503
Your branch is up to date with 'origin/CAMEL-23503'.

nothing to commit, working tree clean
thinkpadp16vgen1:/Development/git-projects/camel$ git diff --check
thinkpadp16vgen1:
/Development/git-projects/camel$ git diff --stat
thinkpadp16vgen1:/Development/git-projects/camel$ git diff -- components/camel-hivemq/src/generated/java/org/apache/camel/component/hivemq/HiveMQComponentConfigurer.java
thinkpadp16vgen1:
/Development/git-projects/camel$

Thanks. Just to clarify, my CAMEL-23503 branch is clean locally (git status, git diff --check, and git diff --stat all show no changes).

I noticed your checkout is at pull/25905/merge, so I assume the generated files were modified after running the build/code-generation step on the PR merge ref.

Could you please let me know which command was run before these files became modified, and whether the resulting diffs are only whitespace changes? I want to make sure we are comparing the same generated output before making any changes to the PR.

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution — this is a solid first cut at a native HiveMQ component. The endpoint/producer/consumer/SendDynamicAware structure follows Camel's standard component shape, and there's decent IT coverage for pub/sub, QoS, retained messages, and dynamic topics.

I found a few issues that should be addressed before merge, called out inline below:

  1. username/password are configured but never used to authenticate the MQTT connection (HiveMQEndpoint.createClient()). Anyone setting credentials on the URI will silently connect unauthenticated.
  2. password isn't marked secret = true, so it won't be masked in traces, management endpoints, or logged URIs.
  3. HiveMQComponentITTest.java is misnamed — because it ends in ITTest.java rather than IT.java, it matches Surefire's unit-test include pattern (**/*Test.java) instead of being excluded like every other IT class here, so mvn test will try to spin up a real broker via Testcontainers. It's also a near-duplicate of HiveMQComponentPubSubIT.
  4. No component documentation page. There's no src/main/docs/hivemq-component.adoc, so the generated docs page and nav entry that every other component has won't exist.

I've also left a couple of minor/non-blocking notes (assertion style, blocking join() calls without a timeout, a leftover speculative comment in a test).

This review is limited to this repo's contribution conventions and code inspection — it isn't a substitute for CodeRabbit/Sourcery or static analysis (SonarCloud), and I haven't run the suite against a live broker myself.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

builder.sslWithDefaultConfig();
}

return builder.useMqttVersion5().buildAsync();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. username/password are now applied when creating the HiveMQ client using the client's simple authentication configuration. Authentication options are therefore no longer ignored. The existing authentication tests also cover the supported username/password combinations.

/**
* Password for authentication with the HiveMQ broker.
*/
@UriParam(label = "security")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

password should be marked secret = true (in addition to label = "security"), per this project's convention for sensitive @UriParams. 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. The password URI parameter is now marked with secret = true so it is treated as sensitive data and masked in generated metadata, endpoint URIs, management, and logging.

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

public class HiveMQComponentITTest extends CamelTestSupport {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. HiveMQComponentITTest.java has been removed because it requires a real broker but matches the unit-test naming pattern (*Test.java). Its basic pub/sub coverage is already provided by HiveMQComponentPubSubIT, so keeping the duplicate test is unnecessary.

protected void doStart() throws Exception {
super.doStart();
client = endpoint.createClient();
client.connect().join();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. The blocking connect().join() calls are now bounded with a connection timeout so that an unreachable broker does not block Camel component startup indefinitely. The same handling is applied to both consumer and producer startup.

properties,
new HashMap<>());

// Use createPreProcessor if SendDynamicAware uses processor-based preparation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. Since createPreProcessor() always returns a non-null processor for this component, the test now directly asserts the processor is non-null and invokes it without the unnecessary null guard.

@Croway Croway left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution — a native HiveMQ component is a welcome addition, especially with camel-paho deprecated since 4.21. I reviewed the component against the actual hivemq-mqtt-client 1.3.3 API surface (all client API claims below were verified against the published jar), with a focus on whether the important client capabilities are exposed. Requesting changes on the following blockers (inline comments have details):

Blocking:

  1. Wire or remove the four dead URI options. username/password (already flagged in the previous review), and additionally version and cleanStart are declared but never passed to the client — users configuring them get silently different behavior than requested.
  2. Automatic reconnect + resubscribe. Without automaticReconnect(...) and a resubscribe-on-reconnect strategy, any broken TCP connection permanently kills the consumer while the route stays Started (zombie route). This is the client's headline feature and table stakes for an MQTT/IoT component.
  3. Consumer callback runs on the client's Netty event-loop thread. A blocking route stalls the MQTT client's own I/O. The subscribe call chain supports .executor(Executor) — wire it to a Camel thread pool.
  4. TLS: SSLContextParameters support. Only sslWithDefaultConfig() is possible today — no custom truststore or mTLS. Camel's JSSE config maps directly onto the client's sslConfig(MqttClientSslConfig).
  5. HiveMQSendDynamicAware is unregistered dead code (missing @SendDynamic), and its resolveStaticUri() would produce an invalid URI if it were registered. Register + fix + test it for real, or remove it.
  6. All 7 test files are missing the ASF license header — RAT/sourcecheck fails on these, so the -Psourcecheck result reported in the PR description is not reproducible.
  7. Missing src/main/docs/hivemq-component.adoc and CI is red on the uncommitted-generated-changes check (the committed src/generated output doesn't match what the build regenerates) — both already flagged; regenerating from a clean tree after adding the doc page should settle it.

Items already raised in the previous review (auth not wired, secret = true on password, HiveMQComponentITTest naming, unbounded join()) still stand — I won't repeat them inline.

Other notable findings (inline): CamelHiveMQQos header type is inconsistent between docs (Integer) and code (MqttQos enum); producer/consumer clientId collision when clientId is configured; null-body publish behavior is undefined/untested.

Client APIs worth exposing as follow-ups (present in 1.3.3, not blocking, but worth JIRAs so the component page can state what's unsupported):

  • Last Will & Testament: willPublish(...) (paho-mqtt5 parity: willTopic/willPayload/willQos/willRetained)
  • Connection tuning: keepAlive, sessionExpiryInterval (needed for cleanStart=false to be meaningful in MQTT 5), transport/connect timeouts
  • MQTT 5 publish properties, both directions (headers on publish, header mapping on consume): userProperties, contentType, responseTopic, correlationData, messageExpiryInterval, payloadFormatIndicator — the request/response pattern is a key reason to choose MQTT 5
  • MQTT 5 subscription options: noLocal, retainAsPublished, retainHandling
  • Manual acknowledgement: manualAcknowledgement(true) + Mqtt5Publish.acknowledge() for ack-after-processing at QoS 1/2 (paho-mqtt5: manualAcksEnabled)
  • WebSocket transport: webSocketConfig(...) (common for cloud brokers)
  • Connected/disconnected listeners exposed for observability

Also: the pinned hivemq-mqtt-client 1.3.3 is from Oct 2023 — latest on Maven Central is 1.3.6 (June 2025); please bump. For reference, camel-paho-mqtt5 (the currently active MQTT component) exposes ~28 options covering most of the list above — since this component is positioned as the native successor, it would be good for the docs to state the current coverage honestly.

This review was generated by Claude Code (AI) on behalf of Croway. It may contain inaccuracies — please verify suggestions before applying.

* MQTT protocol version to use for the connection.
*/
@UriParam(defaultValue = "MQTT_5_0")
private MqttVersion version = MqttVersion.MQTT_5_0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

version is a dead option: HiveMQEndpoint.createClient() unconditionally calls .useMqttVersion5(), so setting version=MQTT_3_1_1 (or MQTT_3_1) silently does nothing — the client's Mqtt3 flavor (useMqttVersion3() / Mqtt3AsyncClient) is never used. Please either wire this option (branching to the Mqtt3 client API) or remove it until MQTT 3 support is actually implemented, so users don't get a false sense of protocol choice.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. The version option was not actually used because the component always creates an MQTT 5 client via useMqttVersion5(). Since MQTT 3 support is not implemented, the unused version option has been removed to avoid exposing a configuration option that has no effect.

* Whether to initiate a clean session upon connecting to the broker.
*/
@UriParam(defaultValue = "true")
private boolean cleanStart = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleanStart is a dead option: both consumer and producer call client.connect() with defaults, so this value is never passed via connectWith().cleanStart(...). Note also that in MQTT 5, cleanStart=false alone does not give a persistent session — the client's default sessionExpiryInterval is 0 (session ends on disconnect), so sessionExpiryInterval should be exposed alongside it for this option to be meaningful.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. cleanStart is now explicitly passed through connectWith().cleanStart(...) for both producer and consumer, so the configured value is no longer ignored.

sessionExpiryInterval is not exposed in this PR; adding persistent-session configuration would be a separate enhancement.

return consumer;
}

public Mqtt5AsyncClient createClient() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two connection-robustness issues in createClient():

  1. No automatic reconnect. Any broken TCP connection permanently kills the consumer: the route stays Started but consumes nothing, and nothing resubscribes. The builder offers automaticReconnect(...)/automaticReconnectWithDefaultConfig() plus addConnectedListener/addDisconnectedListener for exactly this — and a resubscribe-on-reconnect step is needed too, since with cleanStart=true the session (and its subscriptions) is gone after a drop. camel-paho-mqtt5 exposes automaticReconnect and just received a 4.23 fix for the equivalent zombie-route case; a new MQTT component shouldn't reintroduce that failure mode.

  2. clientId collision. Every producer and consumer builds its own client from the same copied configuration. If clientId is set and a route has both a consumer and a producer (or two endpoints share component config), two connections use the same client identifier and the broker must disconnect the older one (MQTT spec), causing a reconnect fight that's hard to debug. Consider a uniquifying suffix per client instance, or document the constraint clearly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. Automatic reconnect is now enabled using automaticReconnectWithDefaultConfig(). The HiveMQ client also handles resubscription when the session is lost, which covers the default cleanStart=true behavior.

For configured clientId, we retain the user-provided value to avoid breaking broker ACLs. Reusing the same clientId across multiple connections can cause broker-side collisions, and this constraint is documented.

}

if (configuration.isSsl()) {
builder.sslWithDefaultConfig();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TLS is limited to sslWithDefaultConfig(). Camel's convention is an sslContextParameters URI option resolving a JSSE SSLContextParameters from the registry — without it, custom truststores and mutual TLS (the norm for MQTT broker deployments) are impossible. The client supports this directly via sslConfig(MqttClientSslConfig) (key/trustmanager factories), so the mapping is straightforward.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged. sslContextParameters is not currently supported. The existing implementation uses HiveMQ's sslWithDefaultConfig() when ssl=true.

Custom Camel SSLContextParameters / truststore / keystore / mTLS integration would require additional TLS configuration and wiring, so this is left as follow-up work. The limitation is already documented.

No changes made for this comment.

client.subscribeWith()
.topicFilter(endpoint.getTopic())
.qos(endpoint.getConfiguration().getQos())
.callback(this::onMessage)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The subscribe callback runs on the client's Netty event-loop thread, so the entire Camel route executes on the client's I/O thread — a slow or blocking route stalls the MQTT client itself (including keep-alive/ack traffic, which can get the client disconnected by the broker). This exact call chain supports .executor(Executor) — please wire it to a thread pool from Camel's ExecutorServiceManager (and ideally process exchanges asynchronously via AsyncProcessor, mirroring the DefaultAsyncProducer used on the producer side).

Related (fine as a follow-up, but worth documenting): messages are auto-acknowledged on receipt, before the route processes them, so an exception in the route loses the message even at QoS 1/2. The client supports .manualAcknowledgement(true) + Mqtt5Publish.acknowledge() for ack-after-processing (paho-mqtt5 exposes this as manualAcksEnabled). Until then the docs should state the at-most-once processing semantics.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed both points.

Message processing is dispatched from the HiveMQ callback to a Camel-managed executor and uses AsyncProcessor, preventing route processing from blocking the HiveMQ I/O thread.
The current auto-acknowledgement behavior is documented, including that acknowledgement is not tied to route processing success/failure. Manual acknowledgement is documented as a limitation/follow-up.

No further changes are required.

public static final String MQTT_TOPIC = "CamelHiveMQTopic";

@Metadata(description = "The QoS level of the message.", javaType = "Integer")
public static final String MQTT_QOS = "CamelHiveMQQos";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Header type inconsistency: this documents javaType = "Integer", but HiveMQConsumer sets the header as an MqttQos enum and HiveMQProducer reads it as MqttQos.class. A user following this doc and setting an Integer header (e.g. 1) will fail type conversion — there is no Integer→MqttQos converter registered. Please pick one representation: either map to/from int at the component boundary, or document the header as com.hivemq.client.mqtt.datatypes.MqttQos.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. CamelHiveMQQos is now documented as MqttQos, matching the type set by HiveMQConsumer and consumed by HiveMQProducer. The generated component metadata has also been updated accordingly.

boolean retained = exchange.getIn().getHeader(HiveMQConstants.MQTT_RETAINED, endpoint.getConfiguration().isRetained(),
Boolean.class);

byte[] payload = exchange.getIn().getBody(byte[].class);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the exchange body is null, payload is null here and the behavior of Mqtt5Publish.builder().payload(null) is undefined/untested. MQTT explicitly allows empty payloads (publishing an empty retained message is how you clear a retained topic), so a null body should map deliberately to a no-payload publish rather than relying on the builder's null handling. Worth an explicit branch + test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. A null Camel body is explicitly converted to an empty MQTT payload before publishing. Added HiveMQEmptyPayloadIT to verify that a null body results in an empty MQTT payload on the consumer side. No further changes are required.

import org.apache.camel.util.StringHelper;

public class HiveMQSendDynamicAware extends SendDynamicAwareSupport {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is currently dead code: it's missing the @SendDynamic("hivemq") annotation, so no META-INF/services/org/apache/camel/send-dynamic/hivemq file is generated and Camel never discovers it for toD (compare JmsSendDynamicAware / PahoSendDynamicAware). HiveMQSendDynamicIT passes trivially because toD simply creates a per-topic endpoint without this optimization ever running.

Additionally, if it were registered, resolveStaticUri() returns "hivemq:" with an empty path — but topic is a required = true @UriPath, so creating the static endpoint would fail. PahoSendDynamicAware handles this by keeping the original URI's topic as the static endpoint and overriding per-message via header.

Please either register it, fix the static URI, and add a real test (assert only one hivemq: endpoint exists after sending to multiple dynamic topics), or remove the class for now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the resolveStaticUri() issue following the existing Paho/JMS SendDynamicAware pattern. The dynamic topic template is retained in the static URI and the resolved topic is applied through the override header.

The @SendDynamic("hivemq") registration was already present.

Added/updated HiveMQSendDynamicIT to send to multiple dynamic topics and verify that only one hivemq: endpoint is created while both messages are delivered successfully.

Unit tests and the integration test pass. will push the changes soon

@Croway Croway left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of the updated changeset — I verified each item from my previous review against the new code and the CI logs, rather than only the replies.

Verified fixed ✅

  • version removed; cleanStart wired via connectWith().cleanStart(...); username/password wired via Mqtt5SimpleAuth with secret = true.
  • Automatic reconnect via automaticReconnectWithDefaultConfig() — and I confirm the resubscribe claim: hivemq-mqtt-client restores subscriptions after reconnect even when the session expired, since 1.2.0 (hivemq/hivemq-mqtt-client#297). My earlier assumption that resubscribe was missing was wrong — thanks for the correction.
  • Consumer callback dispatched off the HiveMQ/Netty I/O thread to a Camel-managed executor, with proper exchange release handling.
  • @SendDynamic("hivemq") registered, resolveStaticUri() now follows the JMS/Paho pattern, and HiveMQSendDynamicIT asserts a single static endpoint services multiple dynamic topics.
  • CamelHiveMQQos documented as MqttQos; null body → empty payload (with HiveMQEmptyPayloadIT); ASF license headers on all test files; HiveMQComponentITTest removed; dependency bumped to 1.3.6; docs page added with a clear limitations / clientId / acknowledgement section.

Still blocking ❌

  1. CI is still red with the same There are uncommitted changes failure. The committed generated files are stale after the option changes. Please run a full local build and commit the regenerated output — the job log lists them: catalog/.../components/hivemq.json, catalog/.../docs.properties, HiveMQComponentConfigurer / HiveMQEndpointConfigurer / HiveMQEndpointUriFactory, ComponentsBuilderFactory / HivemqComponentBuilderFactory, EndpointHeaderBuilders / StaticEndpointBuilders / HiveMQEndpointBuilderFactory, docs/components/modules/ROOT/nav.adoc, plus the two docs-sync copies of hivemq-component.adoc (untracked in the CI workspace).
  2. Context startup can now hang forever when the broker is unreachable — see the inline comment on HiveMQEndpoint.connect(). The earlier reply said the blocking connect is now "bounded with a connection timeout", but no timeout is set anywhere in the diff, and with automatic reconnect enabled the connect future never completes on an unreachable broker.
  3. Stopping a route while the client is reconnecting leaks a live client — see the inline comment on HiveMQConsumer.doStop() (same pattern in HiveMQProducer.doStop()).

Acceptable as follow-up — please file JIRAs so they are not lost

  • sslContextParameters support (the documented limitation is reasonable for a Preview component).
  • The follow-up client API list from my previous review: will messages (LWT), keepAlive / sessionExpiryInterval / timeouts, MQTT 5 publish properties in both directions, subscription options (noLocal, retainAsPublished, retainHandling), manual acknowledgement, WebSocket transport, connected/disconnected listeners.

Non-blocking note: newDefaultThreadPool is a multi-threaded pool, so once dispatch is concurrent, per-topic message ordering is no longer guaranteed. Worth a sentence in the docs, or a future option to process with a single thread for users who rely on MQTT ordering.


This review was generated by Claude Code (AI) on behalf of Croway.

client.connectWith()
.cleanStart(configuration.isCleanStart())
.send()
.join();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This join() has no timeout, and with automaticReconnectWithDefaultConfig() enabled the connect future never completes when the broker is unreachable — the client keeps retrying internally and the caller blocks forever. This is long-standing documented behavior of the client: hivemq/hivemq-mqtt-client#302 (closed as stale, not fixed) and the still-open follow-up hivemq/hivemq-mqtt-client#622. Since this is called from doStart() of both the consumer and the producer, a broker that is down at deployment time now hangs Camel context startup indefinitely — the reply on the earlier thread mentioned a bounded connection timeout, but none is set in the code.

Suggestion: use client.connectWith().cleanStart(...).send().toCompletableFuture().get(timeout, TimeUnit.SECONDS) (or orTimeout) with a sensible default, and surface the timeout as a connectTimeout URI option. On timeout, call disconnect() so the auto-reconnector does not keep running behind a failed start.

Claude Code on behalf of Croway

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. doStop() now uses the centralized stopClient() path instead of relying only on isConnected().

The stop path marks the client for cancellation, and the disconnect/reconnect listeners ensure that an in-flight or scheduled auto-reconnect cannot leave the client connected after Camel has stopped.

The same handling is applied to both HiveMQConsumer and HiveMQProducer.

Addressed. The initial connect() wait is now bounded to 30 seconds using orTimeout(...). On timeout/failure, the client is stopped so the automatic reconnect process does not continue behind a failed Camel startup.

Added HiveMQEndpointConnectTest covering the unreachable-broker case and stopping the client during an in-flight connection.

Focused tests pass: 9/9, BUILD SUCCESS.


@Override
protected void doStop() throws Exception {
if (client != null && client.getState().isConnected()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

client.getState().isConnected() is only true in the CONNECTED state. With automatic reconnect enabled, a client that is currently in DISCONNECTED_RECONNECT / CONNECTING_RECONNECT (broker briefly down, network blip) fails this check, so disconnect() is skipped — and the auto-reconnector stays alive: the client will reconnect and resubscribe after Camel has stopped the route, leaking a connection and continuing to consume messages that no route processes.

Suggestion: call client.disconnect() whenever the state is not DISCONNECTED (or unconditionally and tolerate the already-disconnected error), so the reconnector is cancelled. The same pattern is in HiveMQProducer.doStop().

Claude Code on behalf of Croway

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. doStop() now delegates to the centralized stopClient() logic, which handles the auto-reconnect states and ensures the reconnect loop is cancelled during shutdown. The same handling is applied to both consumer and producer.

@Croway

Croway commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Thanks for working on it @ramu11 , there is a small conflict
dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/StaticEndpointBuilders.java may you solve it?

@ramu11

ramu11 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for working on it @ramu11 , there is a small conflict dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/StaticEndpointBuilders.java may you solve it?

Merged the latest upstream/main into CAMEL-23503 to bring the branch up to date and resolve the merge conflicts. The large number of changed files is primarily due to upstream changes and regenerated/generated artifacts; the HiveMQ implementation itself remains in the original CAMEL-23503 commit. All conflicts have been resolved and the working tree is clean.

@Croway

Croway commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

@ramu11 there are some uncommitted files:

There are uncommitted changes
HEAD detached at pull/25905/merge
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/components/hivemq.json
	modified:   catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs.properties
	modified:   components/camel-hivemq/src/generated/java/org/apache/camel/component/hivemq/HiveMQComponentConfigurer.java
	modified:   components/camel-hivemq/src/generated/java/org/apache/camel/component/hivemq/HiveMQEndpointConfigurer.java
	modified:   components/camel-hivemq/src/generated/java/org/apache/camel/component/hivemq/HiveMQEndpointUriFactory.java
	modified:   docs/components/modules/ROOT/nav.adoc
	modified:   dsl/camel-componentdsl/src/generated/java/org/apache/camel/builder/component/ComponentsBuilderFactory.java
	modified:   dsl/camel-componentdsl/src/generated/java/org/apache/camel/builder/component/dsl/HivemqComponentBuilderFactory.java
	modified:   dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/EndpointHeaderBuilders.java
	modified:   dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/StaticEndpointBuilders.java
	modified:   dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/dsl/HiveMQEndpointBuilderFactory.java

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/hivemq-component.adoc
	docs/components/modules/ROOT/pages/hivemq-component.adoc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants