Copilot commented on code in PR #10469:
URL: https://github.com/apache/rocketmq/pull/10469#discussion_r3388999784
##########
common/src/main/java/org/apache/rocketmq/common/message/MessageClientIDSetter.java:
##########
@@ -111,8 +111,10 @@ public static int getPidFromID(String msgID) {
return value & 0x0000FFFF;
}
+ private static final ThreadLocal<char[]> UNIQ_ID_BUF =
ThreadLocal.withInitial(() -> new char[LEN * 2]);
+
public static String createUniqID() {
- char[] sb = new char[LEN * 2];
+ char[] sb = UNIQ_ID_BUF.get();
Review Comment:
The variable name `sb` is misleading now that it’s a `char[]` buffer rather
than a `StringBuilder`. Renaming it to something like `buf`, `chars`, or
`uniqIdChars` would make the intent clearer.
##########
common/src/main/java/org/apache/rocketmq/common/message/MessageVersion.java:
##########
@@ -71,12 +71,12 @@ public void putTopicLength(ByteBuffer buffer, int
topicLength) {
}
public static MessageVersion valueOfMagicCode(int magicCode) {
- for (MessageVersion version : MessageVersion.values()) {
- if (version.getMagicCode() == magicCode) {
- return version;
- }
+ if (magicCode == MessageDecoder.MESSAGE_MAGIC_CODE) {
+ return MESSAGE_VERSION_V1;
+ }
+ if (magicCode == MessageDecoder.MESSAGE_MAGIC_CODE_V2) {
+ return MESSAGE_VERSION_V2;
}
-
throw new IllegalArgumentException("Invalid magicCode " + magicCode);
Review Comment:
This hard-codes the mapping to V1/V2 and couples it to `MessageDecoder`
constants, which can drift from `MessageVersion#getMagicCode()` over time
(e.g., adding a new version or changing magic codes requires updating multiple
places). Consider a single source of truth such as a static lookup map keyed by
`getMagicCode()` (built from `values()` once), or a `switch` over known codes
with an explicit comment explaining why the mapping is intentionally limited to
v1/v2.
--
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]
