[ISSUE #10990] Skip pop retry arrival notification when properties are null

[unbridled-41]

Which Issue(s) This PR Fixes

• Fixes [Bug] Pop long-polling: retry topic message with null properties throws NPE in the reput thread and stalls broker-wide dispatch #10990

Brief Description

PopLongPollingService#notifyMessageArrivingFromRetry dereferences the dispatch request's properties map without a null check:

String originGroup = properties.get(MessageConst.PROPERTY_ORIGIN_GROUP);   // NPE when properties == null

A DispatchRequest legitimately carries a null properties map (MessageDecoder.string2messageProperties returns null for a message stored without properties — e.g. written by a non-Java client on a %RETRY%-prefixed topic). The store's own code acknowledges nullability: DefaultMessageStore#notifyMessageArrive4MultiQueue guards prop == null and PullRequestHoldService#notifyMessageArriving guards properties != null — only the pop retry branch misses it.

The blast radius is broker-wide: the listener is invoked from DefaultMessageStore.ReputMessageService#doReput before reputFromOffset is advanced, and doReput only catches RocksDBException. The NPE propagates to ServiceThread.run, which logs and loops — then re-reads the same commitlog record and throws again forever, so all dispatch stops (consume queues stop advancing, consumers see a full outage) until manual intervention, and the poison message survives restarts.

This PR returns early when properties == null: a retry-topic message without properties cannot be mapped back to an origin group, so there is no long-polling request to wake up. This matches the guards used by the sibling listeners.

How Did You Test This Change?

Added PopLongPollingServiceTest#testNotifyMessageArrivingFromRetryWithoutProperties, which invokes notifyMessageArrivingWithRetryTopic on a %RETRY% topic with a null properties map. It throws NPE on the unfixed code and passes (no wake-up) with this change.

mvn -pl broker test -Dtest=PopLongPollingServiceTest passes (13/13).

[RockteMQ-AI]

Summary

This PR adds a defensive null check for the properties parameter in notifyMessageArrivingFromRetry, preventing potential NPE when retry topic messages arrive without properties (e.g., from non-Java clients). The fix is minimal, well-commented, and includes a test case.

LGTM — code changes look good.

---

Automated review by github-manager-bot

[unbridled-41]

Test evidence (before → after)

PopLongPollingServiceTest#testNotifyMessageArrivingFromRetryWithoutProperties was verified in both directions on JDK 8 (mvn -pl broker test -Dtest=PopLongPollingServiceTest):

On the unfixed code (fix reverted, test kept):

java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)" because "properties" is null
	at ...PopLongPollingServiceTest.testNotifyMessageArrivingFromRetryWithoutProperties
  → invoked via notifyMessageArrivingWithRetryTopic → notifyMessageArrivingFromRetry("%RETRY%group", ...)
Tests run: 1, Failures: 0, Errors: 1, Skipped: 0

That is exactly the NPE the reput thread hits: DefaultMessageStore.ReputMessageService#doReput only catches RocksDBException, and the exception is thrown before reputFromOffset is advanced, so ServiceThread.run logs and re-reads the same commitlog record forever — broker-wide dispatch stall from a single properties-less message on a %RETRY% topic.

With this PR: Tests run: 12, Failures: 0, Errors: 0, Skipped: 0 (11 pre-existing + 1 new; the test also verifies via never() that no wake-up is attempted for a null-properties retry message).

For consistency reference: DefaultMessageStore#notifyMessageArrive4MultiQueue guards prop == null and PullRequestHoldService#notifyMessageArriving guards properties != null — this PR adds the same guard to the pop retry branch, it does not introduce a new pattern.

Side note on CI: the workflow runs for this PR are in action_required state (first-time contributor) and will start once a maintainer approves them.