feat: implement MQTT Last Will and Testament (LWT) by wy471x · Pull Request #6902 · apache/shenyu

wy471x · 2026-08-10T15:11:57Z

Parse and store will fields (topic, message, QoS, retain) in WillRepository on CONNECT. Publish will message to subscribers on ungraceful disconnect via channelInactive hook. Clear will on graceful DISCONNECT. Fix DISCONNECT message dispatch in MqttFactory that was previously dropped.

Make sure that:

You have read the contribution guidelines.
You submit test cases (unit or integration tests) that back your changes.
Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

Summary

Core changes:

WillRepository.java (new) — A ConcurrentHashMap-backed repository that stores WillEntry (topic, message bytes, QoS, retain flag) keyed by Netty Channel. Implements the existing
BaseRepository interface.
Connect.java — On CONNECT, if isWillFlag() is true, extracts the will fields from the MQTT CONNECT payload and stores them via WillRepository.
Disconnect.java — On graceful DISCONNECT, removes the will from WillRepository (so it won't fire). Replaces the old // todo Last words placeholder.
MqttTransportHandler.java — Overrides channelInactive() to detect ungraceful disconnects. If a will is still present for that channel when it goes inactive, it publishes the
will via Publish.publishWill() and then clears it.
Publish.java — New publishWill() static method that looks up subscribers for the will topic and writes the will message as an MQTT PUBLISH packet to each active subscriber
channel.
MqttFactory.java — Fixes a bug where DISCONNECT was falling through to PUBACK/default instead of calling messageType.disconnect(ctx).
pom.xml — Adds JUnit Jupiter, Mockito, and maven-surefire-plugin with --add-opens JVM args for testing.

Tests (5 new test files):

ConnectTest, DisconnectTest, MqttTransportHandlerTest, PublishWillTest, and WillRepositoryTest — covering will storage on connect, will clearance on disconnect, will firing on channel inactivity, inactive channel skipping, empty subscriber handling, and QoS/retain flag propagation.

close #6852

Parse and store will fields (topic, message, QoS, retain) in WillRepository on CONNECT. Publish will message to subscribers on ungraceful disconnect via channelInactive hook. Clear will on graceful DISCONNECT. Fix DISCONNECT message dispatch in MqttFactory that was previously dropped. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Copilot

Pull request overview

Implements MQTT Last Will and Testament (LWT) support in the shenyu-protocol-mqtt module by persisting will data on CONNECT, clearing it on graceful DISCONNECT, and publishing it on ungraceful disconnect via Netty channelInactive.

Changes:

Add WillRepository to store per-connection LWT entries.
Publish stored wills on channelInactive and clear wills on DISCONNECT.
Add unit tests and module test dependencies/config to validate LWT behavior.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
shenyu-protocol/shenyu-protocol-mqtt/src/main/java/org/apache/shenyu/protocol/mqtt/Connect.java	Store will fields from CONNECT into `WillRepository`.
shenyu-protocol/shenyu-protocol-mqtt/src/main/java/org/apache/shenyu/protocol/mqtt/Disconnect.java	Clear will on graceful DISCONNECT before closing channel.
shenyu-protocol/shenyu-protocol-mqtt/src/main/java/org/apache/shenyu/protocol/mqtt/MqttTransportHandler.java	Publish + clear will on ungraceful disconnect (`channelInactive`).
shenyu-protocol/shenyu-protocol-mqtt/src/main/java/org/apache/shenyu/protocol/mqtt/Publish.java	Add `publishWill(...)` helper to emit will PUBLISH packets to subscribers.
shenyu-protocol/shenyu-protocol-mqtt/src/main/java/org/apache/shenyu/protocol/mqtt/MqttFactory.java	Ensure DISCONNECT is dispatched to `messageType.disconnect(ctx)`.
shenyu-protocol/shenyu-protocol-mqtt/src/main/java/org/apache/shenyu/protocol/mqtt/repositories/WillRepository.java	New repository for storing will topic/message/QoS/retain by Channel.
shenyu-protocol/shenyu-protocol-mqtt/src/test/java/org/apache/shenyu/protocol/mqtt/ConnectTest.java	Tests will persistence on CONNECT.
shenyu-protocol/shenyu-protocol-mqtt/src/test/java/org/apache/shenyu/protocol/mqtt/DisconnectTest.java	Tests will clearance on DISCONNECT.
shenyu-protocol/shenyu-protocol-mqtt/src/test/java/org/apache/shenyu/protocol/mqtt/MqttTransportHandlerTest.java	Tests will firing/removal on `channelInactive`.
shenyu-protocol/shenyu-protocol-mqtt/src/test/java/org/apache/shenyu/protocol/mqtt/PublishWillTest.java	Tests will publishing behavior to subscribers.
shenyu-protocol/shenyu-protocol-mqtt/src/test/java/org/apache/shenyu/protocol/mqtt/repositories/WillRepositoryTest.java	Tests repository CRUD semantics for will entries.
shenyu-protocol/shenyu-protocol-mqtt/pom.xml	Add JUnit/Mockito deps and configure Surefire argLine for tests.

Suppressed comments (1)

shenyu-protocol/shenyu-protocol-mqtt/src/main/java/org/apache/shenyu/protocol/mqtt/repositories/WillRepository.java:75

WillEntry#getMessage currently returns the internal byte[] directly, allowing external callers to mutate repository state. Return a defensive copy instead.

        public byte[] getMessage() {
            return message;
        }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+        // store will if present
+        if (msg.variableHeader().isWillFlag()) {
+            WillRepository.WillEntry will = new WillRepository.WillEntry(
+                    msg.payload().willTopic(),
+                    msg.payload().willMessageInBytes(),
+                    msg.variableHeader().willQos(),
+                    msg.variableHeader().isWillRetain());
+            Singleton.INST.get(WillRepository.class).add(ctx.channel(), will);
+        }


+    @Override
+    public void channelInactive(final ChannelHandlerContext ctx) throws Exception {
+        WillRepository.WillEntry will = Singleton.INST.get(WillRepository.class).get(ctx.channel());
+        if (Objects.nonNull(will)) {
+            Publish.publishWill(will);
+            Singleton.INST.get(WillRepository.class).remove(ctx.channel());
+        }
+        super.channelInactive(ctx);
+    }


+        public WillEntry(final String topic, final byte[] message, final int qos, final boolean retain) {
+            this.topic = topic;
+            this.message = message;
+            this.qos = qos;
+            this.retain = retain;
+        }


+    @AfterEach
+    public void tearDown() {
+        channel.close();
+    }


Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…emented

Aias00

Review: #6902 — feat: implement MQTT Last Will and Testament (LWT)

Verdict: ✅ Approve (with one consistency follow-up)

Solid, well-tested feature implementation. The lifecycle handling is correct and the test coverage is excellent.

What's correct

The MqttFactory DISCONNECT fix is the linchpin. Original code had case PUBACK: case DISCONNECT: default: break; — so DISCONNECT messages were silently dropped and Disconnect.disconnect() was never invoked. Without this fix, graceful-disconnect will removal couldn't work at all. Good catch, and it's required for the rest of the feature to function.
Correct will lifecycle:
- CONNECT with isWillFlag() → WillRepository.add(channel, will) ✅
- Graceful DISCONNECT → WillRepository.remove(channel) → will never fires ✅
- Ungraceful disconnect → MqttTransportHandler.channelInactive() sees the will present → Publish.publishWill(will) → removes it ✅
- The inactive channel is excluded from receiving its own will (channel.isActive() guard) ✅
- No double-publish: channelInactive removes the will immediately and Netty fires it once per close ✅
WillRepository is a clean ConcurrentHashMap<Channel, WillEntry> keyed by Channel (not clientId), so reconnects with a new channel don't collide, and testReplaceWillEntryOnReconnect covers the replace path.
publishWill null-guards topic/message, derives a valid packetId (0 for AT_MOST_ONCE, random otherwise), and respects the will QoS/retain flags.
Test coverage is genuinely thorough: ConnectTest (store / no-will / qos0 / retain), DisconnectTest (clears will / no will / removes channel), MqttTransportHandlerTest (fires+removes / no will / post-disconnect), PublishWillTest (active / inactive-skip / empty / qos+retain), and WillRepositoryTest. The pom changes (junit-jupiter, mockito, --add-opens for JDK 17) are the right scaffolding to support them.

Suggestion (non-blocking)

Wildcard-aware will delivery. Publish.publishWill uses SubscribeRepository.get(will.getTopic()) — an exact lookup. A client subscribed to e.g. status/# will not receive a will published to status/client-001, even though normal publish routing should match it. Since #6906 (wildcard matching) adds getChannelsByTopic, it would be consistent to route the will through that here too. Not blocking (LWT-to-wildcard-subscriber is an edge case), but worth aligning.
Retained will semantics. A will with retain=true is sent with the RETAIN flag, but there's no evidence the broker persists retained messages for later subscribers. That's a broader retained-message gap, outside this PR's scope — just flagging so it's tracked.

Verdict

Approving. The implementation is correct, the essential DISCONNECT routing bug is fixed, and the tests back the behavior end-to-end. Address the wildcard-delivery point as a small follow-up.

…emented

Publish.publishWill used an exact SubscribeRepository.get() lookup, so clients subscribed to wildcard filters (e.g. status/#) never received wills published to concrete topics like status/client-001. Port the TopicMatcher and SubscribeRepository.getChannelsByTopic from apache#6906 and route will delivery through it, consistent with normal publish routing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

wy471x · 2026-08-15T16:15:39Z

Review: #6902 — feat: implement MQTT Last Will and Testament (LWT)

Verdict: ✅ Approve (with one consistency follow-up)

Solid, well-tested feature implementation. The lifecycle handling is correct and the test coverage is excellent.

What's correct

The MqttFactory DISCONNECT fix is the linchpin. Original code had case PUBACK: case DISCONNECT: default: break; — so DISCONNECT messages were silently dropped and Disconnect.disconnect() was never invoked. Without this fix, graceful-disconnect will removal couldn't work at all. Good catch, and it's required for the rest of the feature to function.

Correct will lifecycle:

CONNECT with isWillFlag() → WillRepository.add(channel, will) ✅

Graceful DISCONNECT → WillRepository.remove(channel) → will never fires ✅

Ungraceful disconnect → MqttTransportHandler.channelInactive() sees the will present → Publish.publishWill(will) → removes it ✅

The inactive channel is excluded from receiving its own will (channel.isActive() guard) ✅

No double-publish: channelInactive removes the will immediately and Netty fires it once per close ✅

WillRepository is a clean ConcurrentHashMap<Channel, WillEntry> keyed by Channel (not clientId), so reconnects with a new channel don't collide, and testReplaceWillEntryOnReconnect covers the replace path.

publishWill null-guards topic/message, derives a valid packetId (0 for AT_MOST_ONCE, random otherwise), and respects the will QoS/retain flags.

Test coverage is genuinely thorough: ConnectTest (store / no-will / qos0 / retain), DisconnectTest (clears will / no will / removes channel), MqttTransportHandlerTest (fires+removes / no will / post-disconnect), PublishWillTest (active / inactive-skip / empty / qos+retain), and WillRepositoryTest. The pom changes (junit-jupiter, mockito, --add-opens for JDK 17) are the right scaffolding to support them.

Suggestion (non-blocking)

Wildcard-aware will delivery. Publish.publishWill uses SubscribeRepository.get(will.getTopic()) — an exact lookup. A client subscribed to e.g. status/# will not receive a will published to status/client-001, even though normal publish routing should match it. Since feat: implement MQTT wildcard subscription matching #6906 (wildcard matching) adds getChannelsByTopic, it would be consistent to route the will through that here too. Not blocking (LWT-to-wildcard-subscriber is an edge case), but worth aligning.

Retained will semantics. A will with retain=true is sent with the RETAIN flag, but there's no evidence the broker persists retained messages for later subscribers. That's a broader retained-message gap, outside this PR's scope — just flagging so it's tracked.

Verdict

Approving. The implementation is correct, the essential DISCONNECT routing bug is fixed, and the tests back the behavior end-to-end. Address the wildcard-delivery point as a small follow-up.

Thank you for the code review on this PR.

Fix: Wildcard-aware will delivery — previously Publish.publishWill used SubscribeRepository.get(topic), an exact-match lookup, so a client subscribed to status/# never received a will published to status/client-001.

Ported TopicMatcher and SubscribeRepository.getChannelsByTopic (identical to PR feat: implement MQTT wildcard subscription matching #6906 code) into the LWT branch
publishWill now routes through getChannelsByTopic, consistent with normal publish routing
Updated PublishWillTest/MqttTransportHandlerTest stubs; added wildcard-subscriber test and ported TopicMatcherTest/SubscribeRepositoryTest
Fixed checkstyle violation (== null → Objects.isNull)

dengliming requested a lite review from Copilot August 11, 2026 04:10

Copilot started reviewing on behalf of dengliming August 11, 2026 04:10 View session

Copilot AI reviewed Aug 11, 2026

View reviewed changes

yu199195 and others added 2 commits August 11, 2026 18:22

Potential fix for pull request finding

ab95921

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Merge branch 'master' into feat_Last-Will-&-Testament-entirely-unimpl…

22a7e8f

…emented

Aias00 previously approved these changes Aug 14, 2026

View reviewed changes

Aias00 and others added 5 commits August 14, 2026 14:54

Merge branch 'master' into feat_Last-Will-&-Testament-entirely-unimpl…

74d51d4

…emented

Merge branch 'master' into feat_Last-Will-&-Testament-entirely-unimpl…

b37ff8b

…emented

Merge branch 'master' into feat_Last-Will-&-Testament-entirely-unimpl…

20e61b2

…emented

Merge branch 'master' into feat_Last-Will-&-Testament-entirely-unimpl…

cfc9cb8

…emented

wy471x dismissed Aias00’s stale review via 61384ab August 15, 2026 15:58

Navigation Menu

Uh oh!

feat: implement MQTT Last Will and Testament (LWT) - #6902

feat: implement MQTT Last Will and Testament (LWT)#6902
wy471x wants to merge 8 commits into
apache:masterfrom
wy471x:feat_Last-Will-&-Testament-entirely-unimplemented

wy471x commented Aug 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Aias00 left a comment

Uh oh!

wy471x commented Aug 15, 2026

Review: #6902 — feat: implement MQTT Last Will and Testament (LWT)

What's correct

Suggestion (non-blocking)

Verdict

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

wy471x commented Aug 10, 2026

Summary

Core changes:

Tests (5 new test files):

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Aias00 left a comment

Choose a reason for hiding this comment

Review: #6902 — feat: implement MQTT Last Will and Testament (LWT)

What's correct

Suggestion (non-blocking)

Verdict

Uh oh!

wy471x commented Aug 15, 2026

Review: #6902 — feat: implement MQTT Last Will and Testament (LWT)

What's correct

Suggestion (non-blocking)

Verdict

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants