Skip to content

MQTT Client V5 Implementation (4.x branch) - #269

Open
dometec wants to merge 9 commits into
vert-x3:4.xfrom
dometec:client_mqtt5_vertx4
Open

MQTT Client V5 Implementation (4.x branch)#269
dometec wants to merge 9 commits into
vert-x3:4.xfrom
dometec:client_mqtt5_vertx4

Conversation

@dometec

@dometec dometec commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

This is a backport of #265 to the 4.x branch.
The tests I’ve run so far are OK, but I’m leaving this as a draft (like #265) because I want to run additional.

@dometec
dometec force-pushed the client_mqtt5_vertx4 branch 2 times, most recently from 805eeec to 4eb4fe1 Compare May 7, 2026 21:22
@dometec dometec changed the title MQTT Client V5 Implementation Vertx4 MQTT Client V5 Implementation (4.x branch) May 11, 2026
@dometec
dometec marked this pull request as ready for review May 12, 2026 07:11

@ozangunalp ozangunalp 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.

@dometec I've pointed out some flagrant things I saw about correctness.

Comment thread .github/workflows/ci-client-mqtt5.yml Outdated
Comment thread src/main/java/io/vertx/mqtt/MqttClientOptions.java Outdated
Comment thread src/main/java/io/vertx/mqtt/MqttClientOptions.java Outdated
Comment thread src/main/java/io/vertx/mqtt/MqttClientOptions.java
Comment thread src/main/java/io/vertx/mqtt/MqttClient.java Outdated

@Override
public synchronized void pause() {
connOption.setAutoRead(false);

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.

what if the ChannelConfig connOption is not yet set ? Shall we accept pause/resume after we receive ConnAck ?

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.

connOption is set at connection time (as connection), moreover that pause is called for sure after the client get some messages

private String[] pickServer(String serverReference) {
if (serverReference == null || serverReference.trim().isEmpty()) return null;
String[] entries = serverReference.split(",");
String entry = entries[ThreadLocalRandom.current().nextInt(entries.length)].trim();

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.

@vietj would you have any guidance to avoid the usage of ThreadLocalRandom ?

Comment thread src/main/java/io/vertx/mqtt/impl/MqttClientImpl.java Outdated
Comment thread src/main/java/io/vertx/mqtt/impl/MqttClientImpl.java Outdated
Comment thread src/main/java/io/vertx/mqtt/impl/MqttClientImpl.java Outdated
@ozangunalp

Copy link
Copy Markdown
Contributor

@dometec would you have time to also open the PR targeting Vert.x 5 (master) ?

dometec and others added 9 commits July 27, 2026 23:58
Port of upstream commit b7c3bd8 from client_mqtt5_master onto the vertx4
branch. Functional scope unchanged from the original commit:
- Manage Publish v5
- Updated MqttEndpointImpl to handle PUBCOMP messages with reason codes.
- Enhance authentication and subscription features
- Modified MqttServerConnection to allow AUTH messages before CONNACK.
- Added support for Wildcard and Shared Subscription properties in
  MqttConnAckMessage.
- Implemented corresponding methods in MqttConnAckMessageImpl to retrieve
  new properties.
- Added tests in Mqtt5ClientConnectTest for user properties in CONNECT
  packets.
- Enhanced Mqtt5ClientDisconnectTest to verify server-initiated DISCONNECT
  handling.
- Updated Mqtt5ClientPublishTest to check for maximum packet size
  enforcement.
- Added tests in Mqtt5ClientSubscribeTest for handling of wildcard and
  shared subscriptions.
- Created Mqtt5ClientWillTest to ensure correct encoding of will message
  properties.
- Manage Server Redirect
- Manage Subscription Identifier
- Manage TopicAlias from Broker to Client

Breaking Changes:
- Removed willFlag client side (info derived from the presence of
  willTopic and willPayload).
- Will options serialized as object in json.

Adaptations for vertx4 / older Netty:
- Coexistence of legacy Handler<AsyncResult<>> overloads with the new
  Future-based v5 API (connect, disconnect, publish, unsubscribe).
- MqttProperties.<NAME> rewritten to
  MqttProperties.MqttPropertyType.<NAME>.value() because Netty 4.1.133
  exposes property ids only via the enum.
- String.isBlank() replaced with trim().isEmpty() (Java 8 compatibility).
- List.of(...) / Map.of(...) replaced with Arrays.asList /
  Collections.singletonMap in tests.
- connect(...) calls disambiguated with explicit
  (Map<String,String>) null casts.
- Imported io.vertx.core.net.* to keep JksOptions / PfxOptions /
  PemKeyCertOptions / PemTrustOptions visible.
- Pulled MqttAuthenticationExchangeMessage / MqttAuthenticateReasonCode /
  MqttAuthenticationExchangeMessageImpl from master as transitive
  dependencies.

Server-side AUTH feature is incomplete: only a stub handleAuth was added
to MqttEndpointImpl, and Mqtt5ClientAuthTest was dropped. The full
authentication-exchange API requires upstream commit 892e923, which is
not part of this port.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Mqtt5ServerRedirectTest: replace Future.compose() chain in @after with
  nested callback-style closes. Under Vert.x 4 the compose chain hits a
  RejectedExecutionException ("event executor terminated") because the
  intermediate future is dispatched onto a context that vertx.close()
  has already torn down, leaving the test framework hanging until the
  120s outer timeout. Also replace Future.all (Vert.x 5) with
  CompositeFuture.all.
- Mqtt5ClientFlowControlTest: same Future.all -> CompositeFuture.all
  substitution; List<Future<?>> tightened to List<Future> to match the
  Vert.x 4 CompositeFuture.all(List<Future>) overload.

The redirect logic in MqttClientImpl was already correct; the failures
were purely a test-side incompatibility with the Vert.x 4 future API.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n current instance and add validation for QoS
…t-side authentication flow and provide example for SCRAM-SHA-256
…efactor; not on vertx4)

Ports the portable subset of the last 4 commits on client_mqtt5_master:
- f3820d7: README now states the client is compliant with both 3.1.1 and 5.0
- f6dd00f: drop unused imports (MqttConnAckMessage, java.util.Map,
  MqttServerOptions) in the touched MQTT 5 test classes

269067a is already satisfied/N-A on vertx4 (no publish(int id,..) family,
no example10, @GenIgnore import and 2-arg authenticationExchange doc already
present). d96cad9 is skipped: it relies on the server-side
MqttEndpoint.authenticationExchange, which vertx4 does not have.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dometec
dometec force-pushed the client_mqtt5_vertx4 branch from 9fad186 to 57e2b5a Compare July 27, 2026 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants