wang-jiahua commented on code in PR #10468:
URL: https://github.com/apache/rocketmq/pull/10468#discussion_r3400416279
##########
common/src/main/java/org/apache/rocketmq/common/message/Message.java:
##########
@@ -104,9 +104,8 @@ public String getUserProperty(final String name) {
public String getProperty(final String name) {
if (null == this.properties) {
- this.properties = new HashMap<>();
+ return null;
Review Comment:
Thanks for raising this. This PR (#10468) was specifically split out from
#10444 to give this public API side-effect change its own independent review —
exactly because it warrants careful scrutiny.
The change is intentional and safe because:
1. `hasProperty()` already returns `false` when `properties == null` — same
"null means empty" semantics.
2. All write paths go through `putProperty()` which lazily creates the
HashMap when needed.
3. The pattern `getProperty(x); getProperties().put(y, z)` is fragile and
undocumented; any such caller should use `putProperty()` instead.
Would you prefer we add a brief Javadoc note on `getProperty()` documenting
the null-return behavior?
--
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]
