RongtongJin commented on code in PR #10444:
URL: https://github.com/apache/rocketmq/pull/10444#discussion_r3388114635
##########
common/src/main/java/org/apache/rocketmq/common/message/Message.java:
##########
@@ -103,10 +103,12 @@ public String getUserProperty(final String name) {
}
public String getProperty(final String name) {
+ // Read-only access shouldn't allocate. Original side-effect of
creating an
+ // empty map on every get is wasted when the message has no properties
yet
+ // (e.g. freshly constructed produce request before tags/keys are set).
if (null == this.properties) {
- this.properties = new HashMap<>();
+ return null;
Review Comment:
Changing `getProperty()` from lazy-initializing `properties` to returning
`null` changes a public API side effect. Some callers may currently rely on a
read accessor to make `getProperties()` non-null before mutating it; this
codebase already has patterns like `getProperty(...)` followed by
`getProperties().put(...)`. If we keep the no-allocation read path, please make
those write paths explicit, for example by using `putProperty` /
`MessageAccessor.putProperty` or another ensure-properties helper, and add a
compatibility test so messages created with the no-arg constructor or decoded
without properties do not surface a new NPE.
--
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]
