.4062073974432528:5337c62effc52e81e0c6db154c8cf31c_69f81cbed510364f7a8b65f8.69f81d07d510364f7a8b6606.69f81d07b7ead4df0c307bb6:Trae CN.T(2026/5/4 12:13:59)#191
Open
CGW516 wants to merge 1 commit into
Conversation
新增离线消息分表存储及消息已读状态管理 添加P2P和群组消息的已读状态记录接口 实现会话级别的消息已读标记功能 支持查询未读消息数量及已读人数统计 优化用户信息缓存服务以支持注册用户ID查询
Contributor
📝 WalkthroughWalkthroughThis PR introduces a comprehensive message read-receipt system for a chat application, enabling clients to track message-read status at P2P, conversation, and group levels. It adds protocol messages, database persistence with sharding, service implementations, client SDK APIs, and server handlers for processing and querying read-receipt state. ChangesMessage Read-Receipt System
Sequence Diagram(s)sequenceDiagram
participant Client
participant CIMServer as CIM Server
participant RouteService as Route Service
participant PersistenceLayer as Persistence<br/>(MySQL)
participant SenderServer as Sender's Server
rect rgba(100, 150, 255, 0.5)
Note over Client,PersistenceLayer: P2P Message Read Flow
Client->>CIMServer: P2PMsgRead Request<br/>(messageId, userId, sendUserId)
CIMServer->>CIMServer: ReadReceiptHandler<br/>extracts & validates
CIMServer->>RouteService: p2pMsgRead(reqVO)
RouteService->>PersistenceLayer: saveP2PMsgReadStatus()
PersistenceLayer-->>RouteService: Status saved
RouteService->>SenderServer: pushReadReceiptToSender()<br/>SendMsg via ServerApi
SenderServer-->>RouteService: ACK
RouteService-->>CIMServer: Success
CIMServer-->>Client: Success
end
rect rgba(150, 255, 100, 0.5)
Note over Client,PersistenceLayer: Unread Count Query Flow
Client->>Client: getP2PUnreadCount<br/>(conversationId)
Client->>RouteService: Query via RouteApi
RouteService->>PersistenceLayer: countUnreadP2PMsgs()
PersistenceLayer-->>RouteService: Count result
RouteService-->>Client: UnreadCountResVO
end
rect rgba(255, 200, 100, 0.5)
Note over Client,PersistenceLayer: Group Message Read Flow
Client->>CIMServer: GroupMsgRead Request<br/>(messageId, groupId, totalMembers)
CIMServer->>RouteService: groupMsgRead(reqVO)
RouteService->>PersistenceLayer: markGroupMsgAsRead()<br/>incrementGroupReadCount()
PersistenceLayer-->>RouteService: Status updated
RouteService->>SenderServer: pushGroupReadCount<br/>to all registered users
RouteService-->>Client: Success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (11)
cim-persistence/cim-persistence-api/src/main/java/com/crossoverjie/cim/persistence/api/service/EnhancedOfflineMsgStore.java-18-18 (1)
18-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
findByMessageIdreturns a singleOfflineMsgbut may match multiple rows in the sharded schema.The sharding key is
receive_user_id(8 shards). For group messages, the samemessageIdis stored once per recipient and can land in different shards.findByMessageId(Long messageId)provides no shard-key hint, so:
- The implementation must fan-out across all 8 shards.
- Multiple rows with the same
messageIdcan exist; returning only one silently discards the rest.Consider either:
- Adding
Long receiveUserIdas a second parameter so the shard can be resolved directly, or- Changing the return type to
List<OfflineMsg>if cross-recipient lookup is genuinely needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-persistence/cim-persistence-api/src/main/java/com/crossoverjie/cim/persistence/api/service/EnhancedOfflineMsgStore.java` at line 18, The method EnhancedOfflineMsgStore.findByMessageId(Long messageId) returns a single OfflineMsg but the sharded schema uses receive_user_id (8 shards) so the same messageId can exist in multiple shards/rows; you must either (A) change the signature to findByMessageId(Long messageId, Long receiveUserId) so the shard is resolved deterministically (update all implementations and callers to pass receiveUserId and route to the correct shard), or (B) change the return type to List<OfflineMsg> and implement a fan-out across all 8 shards to collect all matching rows for messageId (update implementations and callers to handle multiple results); ensure any DAO/SQL layer and tests are updated to reflect the chosen approach.cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/vo/req/P2PMsgReadReqVO.java-27-29 (1)
27-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
sendUserIdis@NotNullhere but is not null-checked inReadReceiptHandler.handleP2PMsgRead.In
ReadReceiptHandler.handleP2PMsgRead(lines 31–32 of that file), onlymessageIdanduserIdare guarded:if (messageId == null || userId == null) { ... return; }
sendUserIdis extracted withparseLong(...), which returnsnullon a missing or malformed value, and is never validated. The resulting VO with anullsendUserIdis then passed torouteApi.p2pMsgRead(reqVO), where@Validwill throw a constraint-violation exception. Because the handler swallows all exceptions, every P2P read receipt that omitsSEND_USER_IDsilently drops with only an error log — no retry, no feedback to the caller.Add
sendUserIdto the null-check guard in the handler:-if (messageId == null || userId == null) { - log.error("Invalid P2P msg read request: messageId={}, userId={}", messageId, userId); +if (messageId == null || userId == null || sendUserId == null) { + log.error("Invalid P2P msg read request: messageId={}, userId={}, sendUserId={}", messageId, userId, sendUserId); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/vo/req/P2PMsgReadReqVO.java` around lines 27 - 29, ReadReceiptHandler.handleP2PMsgRead currently only guards messageId and userId but parses sendUserId via parseLong which may return null; update handleP2PMsgRead to include sendUserId in the null-check guard (e.g., if (messageId == null || userId == null || sendUserId == null) { ... return; }) so you validate sendUserId before constructing P2PMsgReadReqVO and calling routeApi.p2pMsgRead(reqVO); locate the check in ReadReceiptHandler.handleP2PMsgRead and add sendUserId to the condition and corresponding log/early-return behavior.cim-persistence/cim-persistence-mysql/src/main/resources/mapper/offlinemsg/OfflineMsgShardingMapper.xml-26-28 (1)
26-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
fetchOfflineMsgIdsWithCursorlacks alimitparameter — potential unbounded query.Its sibling
fetchOfflineMsgsWithCursorat Line 17 accepts a@Param("limit") Integer limit, but this cursor method omits it. For a user with many accumulated offline messages, this will fetch all IDs in a single query. Add alimitparameter consistent with the other cursor method.🐛 Proposed fix
List<Long> fetchOfflineMsgIdsWithCursor(`@Param`("tableName") String tableName, `@Param`("receiveUserId") Long receiveUserId, - `@Param`("lastMessageId") Long lastMessageId); + `@Param`("lastMessageId") Long lastMessageId, + `@Param`("limit") Integer limit);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-persistence/cim-persistence-mysql/src/main/resources/mapper/offlinemsg/OfflineMsgShardingMapper.xml` around lines 26 - 28, The method fetchOfflineMsgIdsWithCursor is missing a limit parameter which allows an unbounded result set; update its signature to accept a `@Param`("limit") Integer limit (matching the sibling fetchOfflineMsgsWithCursor) and ensure callers and mapper XML use that limit to constrain the query so only up to `limit` IDs are returned; refer to fetchOfflineMsgIdsWithCursor and fetchOfflineMsgsWithCursor when making the change.cim-persistence/cim-persistence-mysql/src/main/resources/mapper/readreceipt/GroupMsgReadCountMapper.xml-6-6 (1)
6-6:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
${tableName}is vulnerable to SQL injection.
${tableName}uses MyBatis string substitution (not parameterized), so it is interpolated directly into the SQL. If any caller path accepts a user-influenced value without strict server-side allowlist validation, this is exploitable for SQL injection. Ensure thattableNameis always computed exclusively from the server-sideShardingStrategyand never derived from user input, and add an explicit allowlist check before any mapper call.This applies equally to
GroupMsgReadStatusMapper.xmlandConversationReadRecordMapper.xml.cim-persistence/cim-persistence-mysql/src/main/java/com/crossoverjie/cim/persistence/mysql/offlinemsg/mapper/OfflineMsgShardingMapper.java-26-28 (1)
26-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
fetchOfflineMsgIdsWithCursoris missing alimitparameter, risking unbounded result sets.Unlike
fetchOfflineMsgsWithCursor(lines 17–20) which has@Param("limit") Integer limit, this method fetches IDs with no upper bound. For a user with a large message backlog, the underlying SQL returns all qualifying rows into aList<Long>in memory, creating a potential OOM and slow-query risk.🔧 Proposed fix
List<Long> fetchOfflineMsgIdsWithCursor(`@Param`("tableName") String tableName, `@Param`("receiveUserId") Long receiveUserId, - `@Param`("lastMessageId") Long lastMessageId); + `@Param`("lastMessageId") Long lastMessageId, + `@Param`("limit") Integer limit);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-persistence/cim-persistence-mysql/src/main/java/com/crossoverjie/cim/persistence/mysql/offlinemsg/mapper/OfflineMsgShardingMapper.java` around lines 26 - 28, The method fetchOfflineMsgIdsWithCursor currently lacks a limit parameter, which can return unbounded ID lists and cause OOM/slow queries; update the OfflineMsgShardingMapper.fetchOfflineMsgIdsWithCursor signature to accept `@Param`("limit") Integer limit, update the corresponding SQL mapping (XML or annotation) to apply LIMIT #{limit}, and adjust any callers to pass a sane limit (matching fetchOfflineMsgsWithCursor usage) so the returned List<Long> is bounded.cim-forward-route/src/main/java/com/crossoverjie/cim/route/service/impl/UserInfoCacheServiceImpl.java-59-74 (1)
59-74:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
redisTemplate.keys()is a blocking O(N) RedisKEYScall — replace withSCAN.The Spring Data Redis API itself documents
keys()as "non-interruptible" and warns it "may cause performance issues", recommendingRedisOperations.scan(ScanOptions)for large datasets. Because Redis is single-threaded, this command blocks all other operations, and its O(N) complexity can lead to severe production server blocking as data grows.Replace with a cursor-based
SCANusing try-with-resources:🔧 Proposed fix
+import org.springframework.data.redis.core.ScanOptions; +import java.io.IOException; + `@Override` public Set<Long> getRegisteredUserIds() { Set<Long> userIds = new HashSet<>(); - Set<String> keys = redisTemplate.keys(ACCOUNT_PREFIX + "*"); - if (keys != null) { - for (String key : keys) { - try { - String userIdStr = key.replace(ACCOUNT_PREFIX, ""); - userIds.add(Long.parseLong(userIdStr)); - } catch (NumberFormatException e) { - log.warn("Skipping invalid account key: {}", key); - } + ScanOptions options = ScanOptions.scanOptions().match(ACCOUNT_PREFIX + "*").count(200).build(); + try (var cursor = redisTemplate.scan(options)) { + cursor.forEachRemaining(key -> { + try { + userIds.add(Long.parseLong(key.replace(ACCOUNT_PREFIX, ""))); + } catch (NumberFormatException e) { + log.warn("Skipping invalid account key: {}", key); + } + }); + } catch (IOException e) { + log.error("Error scanning Redis keys for prefix {}", ACCOUNT_PREFIX, e); } return userIds; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-forward-route/src/main/java/com/crossoverjie/cim/route/service/impl/UserInfoCacheServiceImpl.java` around lines 59 - 74, getRegisteredUserIds currently calls redisTemplate.keys(ACCOUNT_PREFIX + "*"), which performs a blocking O(N) KEYS call; replace it with a cursor-based SCAN using RedisOperations.scan(ScanOptions) to avoid blocking. Modify getRegisteredUserIds to build ScanOptions.match(ACCOUNT_PREFIX + "*"), obtain a Cursor<byte[]> (or Cursor<String>) via redisTemplate.execute or redisTemplate.opsForValue().getOperations().scan, iterate the cursor in a try-with-resources block to parse and collect IDs (strip ACCOUNT_PREFIX then Long.parseLong), log and skip NumberFormatException as before, and ensure the cursor is closed automatically. Use the existing ACCOUNT_PREFIX and preserve the method signature and returned Set<Long>.cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/RouteApi.java-61-63 (1)
61-63:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign the batch read-count contract with the controller.
RouteController.getGroupMsgsReadCount(...)is bound asPOSTand readsmessageIdsfrom the request body (RouteController.java, Lines 400-405), but this client contract does not declare the method asPOSTand leavesmessageIdsunannotated. That mismatch is likely to make the SDK call atRouteManager.java, Lines 158-160 send a shape the controller cannot bind.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/RouteApi.java` around lines 61 - 63, The client interface RouteApi.getGroupMsgsReadCount must match the controller: change the mapping to a POST (e.g., add `@PostMapping` or `@RequestMapping`(method = RequestMethod.POST)) and annotate the messageIds parameter with `@RequestBody` so the list is read from the request body; also fix the groupId parameter to use the correct `@RequestParam` annotation if needed (ensure the method signature in RouteApi matches RouteController.getGroupMsgsReadCount).cim-client-sdk/src/main/java/com/crossoverjie/cim/client/sdk/RouteManager.java-92-140 (1)
92-140:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPropagate read-receipt failures back to the caller.
These three async methods swallow exceptions and ignore non-success
BaseResponsecodes, so the returned future completes successfully even when the route layer rejects the request. That makes the new synchronousClientwrappers look successful because.get()never sees the failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-client-sdk/src/main/java/com/crossoverjie/cim/client/sdk/RouteManager.java` around lines 92 - 140, The three async methods sendP2PMsgRead, sendConversationRead, and sendGroupMsgRead currently swallow exceptions and ignore non-success BaseResponse results from routeApi, so change each to propagate failures to the returned CompletableFuture: capture the BaseResponse returned by routeApi.* (p2pMsgRead, conversationRead, groupMsgRead), check its success code/flag and if not successful throw an appropriate RuntimeException (or build a specific exception) so the CompletableFuture completes exceptionally; also remove the try/catch that swallows exceptions (or re-throw inside the catch) so any thrown exceptions propagate to callers (e.g., via CompletableFuture.supplyAsync/runAsync semantics) and callers calling .get() will see the failure.cim-persistence/cim-persistence-mysql/src/main/java/com/crossoverjie/cim/persistence/mysql/readreceipt/ReadReceiptServiceImpl.java-134-148 (1)
134-148:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMake the group read-count update atomic.
This is a classic select-then-insert/increment race: concurrent reads for the same
(groupId, messageId)can both observeexisting == nulland attempt the insert path, or interleave between the read and increment paths. Move this to a single SQL upsert/increment statement so the count stays correct under parallel read receipts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-persistence/cim-persistence-mysql/src/main/java/com/crossoverjie/cim/persistence/mysql/readreceipt/ReadReceiptServiceImpl.java` around lines 134 - 148, The current incrementGroupReadCount method does a select-then-insert/increment leading to races; change it to a single atomic upsert/update at the mapper level: replace the select+conditional insert/increment logic in incrementGroupReadCount with one call to a new mapper method (e.g., groupMsgReadCountMapper.upsertIncrementReadCount) that executes a single SQL statement such as INSERT ... ON DUPLICATE KEY UPDATE read_count = read_count + 1 (and sets total_members when inserting) using the same table name from ShardingStrategy.getGroupMsgReadCountTableName; keep GroupMsgReadCount as the insert payload but move concurrency handling into the mapper SQL to ensure atomicity.cim-forward-route/src/main/java/com/crossoverjie/cim/route/controller/RouteController.java-114-131 (1)
114-131:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPersist the group metadata you build here.
propertiesis dead code right now: nothing passes it to persistence, andOfflineMsgServiceImpl.saveOfflineMsg(...)only storessendUserId,receiveUserId, andmsg(OfflineMsgServiceImpl.java, Lines 15-24). Offline group messages will therefore be reloaded without the group/conversation context needed to distinguish them from P2P messages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-forward-route/src/main/java/com/crossoverjie/cim/route/controller/RouteController.java` around lines 114 - 131, saveGroupOfflineMsg builds a properties map but never persists it, so group messages lose conversation metadata; fix by passing those properties into the persistence call: either add a properties field/setter to P2PReqVO and set it before calling offlineMsgService.saveOfflineMsg(p2pReqVO), or add an overloaded saveOfflineMsg method on OfflineMsgService that accepts (P2PReqVO, Map<String,String>) and update OfflineMsgServiceImpl to persist the map alongside sendUserId/receiveUserId/msg; update the call in saveGroupOfflineMsg to use the new/extended API so properties are stored with the offline message.cim-forward-route/src/main/java/com/crossoverjie/cim/route/service/impl/ReadReceiptRouteServiceImpl.java-107-126 (1)
107-126:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMake group read updates idempotent.
incrementGroupReadCount(...)runs even whengetGroupMsgReadStatus(...)finds an existing read-status row, so a duplicate read event from the same user will still bumpreadCount. The increment should only happen on an unread→read transition, ideally based on the persistence layer's affected-row result.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-forward-route/src/main/java/com/crossoverjie/cim/route/service/impl/ReadReceiptRouteServiceImpl.java` around lines 107 - 126, handleGroupMsgRead currently always calls readReceiptService.incrementGroupReadCount which causes duplicate increments; change the flow so incrementGroupReadCount is only invoked when the user's status transitions unread→read. Concretely, have markGroupMsgAsRead or the DAO method return whether it actually changed state (e.g., affected-row count) and use that boolean/result to decide to call incrementGroupReadCount; alternatively, call getGroupMsgReadStatus first and only call incrementGroupReadCount when status is null or status.isRead indicates unread before you save via saveGroupMsgReadStatus. Ensure you reference handleGroupMsgRead, readReceiptService.markGroupMsgAsRead, readReceiptService.getGroupMsgReadStatus, readReceiptService.saveGroupMsgReadStatus and readReceiptService.incrementGroupReadCount when implementing this guard.
🟡 Minor comments (3)
cim-common/src/main/java/com/crossoverjie/cim/common/constant/Constants.java-69-71 (1)
69-71:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
READ_TYPE_UNKNOWN = 0constant to mirror the proto enum default.The
ReadTypeproto enum definesREAD_TYPE_UNKNOWN = 0as the zero/default value, butConstantsonly declaresREAD_TYPE_SINGLE = 1andREAD_TYPE_CONVERSATION = 2. Any code path that receives a raw0read-type value has no named constant to compare against, leaving an unhandled state.🐛 Proposed fix
+ public static final Integer READ_TYPE_UNKNOWN = 0; + public static final Integer READ_TYPE_SINGLE = 1; public static final Integer READ_TYPE_CONVERSATION = 2;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-common/src/main/java/com/crossoverjie/cim/common/constant/Constants.java` around lines 69 - 71, Add the missing zero/default constant to the Constants class: declare public static final Integer READ_TYPE_UNKNOWN = 0 alongside the existing READ_TYPE_SINGLE and READ_TYPE_CONVERSATION in class Constants so code that receives a proto ReadType value of 0 can compare against a named constant (use the same Integer type as READ_TYPE_SINGLE/READ_TYPE_CONVERSATION and place it near those declarations).cim-common/src/main/proto/cim.proto-29-39 (1)
29-39:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winProto request messages are missing fields required by their handlers, and are not used as data carriers.
ConversationReadRequestis missingconversation_type(which the handler inReadReceiptHandler.javavalidates as a mandatory field) andgroup_id/target_user_id(both extracted by the handler).GroupMsgReadRequestis missingtotal_members.More broadly, the handlers in
ReadReceiptHandler.java(context snippet 1) bypass these proto messages entirely and read all fields from the genericRequest.propertiesmap. This meansP2PMsgReadRequest,ConversationReadRequest, andGroupMsgReadRequestare defined but never serialized or deserialized, making them dead schema. Either populate these messages on the client side and decode them in the handler, or remove them to avoid misleading documentation.♻️ Proposed additions to align the schema with handler requirements
message ConversationReadRequest{ int64 user_id = 1; string conversation_id = 2; int64 before_message_id = 3; + ConversationType conversation_type = 4; + int64 target_user_id = 5; + int64 group_id = 6; } message GroupMsgReadRequest{ int64 message_id = 1; int64 group_id = 2; int64 user_id = 3; + int32 total_members = 4; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-common/src/main/proto/cim.proto` around lines 29 - 39, ConversationReadRequest and GroupMsgReadRequest are missing fields required by ReadReceiptHandler.java (and the handler currently bypasses these protos by reading Request.properties), so either add the missing fields to the proto and switch the handler to deserialize and use them, or remove the dead proto messages. Specifically, update ConversationReadRequest to include conversation_type, group_id and target_user_id (with appropriate scalar types consistent with existing protos) and update GroupMsgReadRequest to include total_members; then modify ReadReceiptHandler.java to parse the incoming Request payload into P2PMsgReadRequest/ConversationReadRequest/GroupMsgReadRequest and use those typed fields instead of Request.properties (or if you choose removal, delete the proto messages and adjust any client code to stop sending them).cim-server/src/main/java/com/crossoverjie/cim/server/kit/ReadReceiptHandler.java-27-33 (1)
27-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
userId == nullis dead code — auto-boxed primitives can never be null; proto3 default0Lslips through.
request.getRequestId()returns a primitivelongwhich is auto-boxed toLongand can never benull. In proto3, an unsetint64field defaults to0. A malformed request with norequestIdtherefore bypasses this guard (as0L != null) and propagates torouteApi.p2pMsgRead(...)with an invalid user ID. The same dead-code null check exists inhandleConversationRead(line 54) andhandleGroupMsgRead(line 89).🔧 Proposed fix (apply to all three handlers)
- if (messageId == null || userId == null) { - log.error("Invalid P2P msg read request: messageId={}, userId={}", messageId, userId); + if (messageId == null || userId == null || userId <= 0L) { + log.error("Invalid P2P msg read request: messageId={}, userId={}", messageId, userId);- if (userId == null || conversationId == null || conversationType == null) { + if (userId == null || userId <= 0L || conversationId == null || conversationType == null) {- if (messageId == null || groupId == null || userId == null) { + if (messageId == null || groupId == null || userId == null || userId <= 0L) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-server/src/main/java/com/crossoverjie/cim/server/kit/ReadReceiptHandler.java` around lines 27 - 33, The null-check on userId in ReadReceiptHandler is dead because request.getRequestId() returns a primitive long (auto-boxed) so unset proto3 fields are 0, not null; update the validation in the P2P, conversation and group handlers (methods handling request.getRequestId() — e.g., the handlers where you call routeApi.p2pMsgRead(...), routeApi.conversationRead(...), routeApi.groupMsgRead(...)) to treat 0 (or <=0 if you prefer stricter validation) as invalid instead of null, i.e., replace/augment the userId null check with a check like userId == 0 (or userId <= 0) and reject/return on that condition; also ensure other parsed IDs (e.g., sendUserId from parseLong(...)) are validated for null/invalid values as appropriate before calling routeApi.
🧹 Nitpick comments (7)
sql/02-offline-msg-sharding.sql (2)
9-9: ⚡ Quick win
created_at DATETIMEhas no default value — rows inserted without setting this column will haveNULL.Add
DEFAULT CURRENT_TIMESTAMPto ensure all rows are timestamped automatically and to preventNULLpollution in theidx_created_atindex.💡 Suggested fix
- created_at DATETIME, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sql/02-offline-msg-sharding.sql` at line 9, The created_at column is defined as "created_at DATETIME" which can be NULL and pollute idx_created_at; modify the column definition for created_at to include a default timestamp (e.g., DEFAULT CURRENT_TIMESTAMP) and consider adding NOT NULL so rows inserted without an explicit timestamp are automatically populated and the idx_created_at index doesn't contain NULLs.
1-15: ⚡ Quick winUse
CREATE TABLE IF NOT EXISTSto make the scripts re-runnable.Plain
CREATE TABLEstatements cause hard failures on re-execution; addingIF NOT EXISTSto all eight table definitions (in both SQL files) makes the scripts safe to run in local setups and CI.💡 Suggested pattern
-CREATE TABLE offline_msg_0 +CREATE TABLE IF NOT EXISTS offline_msg_0Apply the same change to
offline_msg_1throughoffline_msg_7.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sql/02-offline-msg-sharding.sql` around lines 1 - 15, Change each CREATE TABLE statement to CREATE TABLE IF NOT EXISTS for all eight partition tables (offline_msg_0 through offline_msg_7) in both SQL files so the scripts are re-runnable; locate the table definitions (e.g., the offline_msg_0 block shown) and update the header to "CREATE TABLE IF NOT EXISTS offline_msg_X" for each partition, preserving the existing columns, indexes and table options exactly.cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/vo/req/ConversationReadReqVO.java (1)
27-29: ⚡ Quick win
conversationTypeasIntegerprovides no type safety — an out-of-range value passes@NotNullsilently.The
@Schemadescription documents only values0(P2P) and1(GROUP), but anyInteger(e.g.,99) satisfies@NotNulland passes validation. Consider using an enum:♻️ Suggested refactor
- `@NotNull`(message = "conversationType can't be null") - `@Schema`(requiredMode = Schema.RequiredMode.REQUIRED, description = "conversation type: 0-P2P, 1-GROUP", example = "0") - private Integer conversationType; + `@NotNull`(message = "conversationType can't be null") + `@Schema`(requiredMode = Schema.RequiredMode.REQUIRED, description = "conversation type: P2P or GROUP", example = "P2P") + private ConversationType conversationType;Where
ConversationTypeis:public enum ConversationType { P2P, GROUP }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/vo/req/ConversationReadReqVO.java` around lines 27 - 29, Replace the raw Integer conversationType in ConversationReadReqVO with a dedicated enum (e.g., ConversationType {P2P, GROUP}) to enforce allowed values; update the field type in ConversationReadReqVO to ConversationType, keep `@NotNull`, and adjust the `@Schema` annotation to reflect the enum values and a suitable example (or map enum to desired integer representation if external API expects ints), and update any code that constructs/parses ConversationReadReqVO (deserializers, controllers, tests) to use or convert to the new ConversationType enum.cim-common/src/main/proto/cim.proto (1)
41-48: ⚡ Quick win
read_typeshould use theReadTypeenum instead ofint32.The
ReadTypeenum is defined in the same file. Usingint32here drops proto-level type safety and breaks the symmetry with the enum definition.♻️ Proposed fix
message ReadReceiptPush{ int64 message_id = 1; int64 reader_user_id = 2; string reader_user_name = 3; - int32 read_type = 4; + ReadType read_type = 4; int64 read_at = 5; map<string, string> properties = 6; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-common/src/main/proto/cim.proto` around lines 41 - 48, The ReadReceiptPush message currently declares read_type as int32; change its type to the existing ReadType enum to restore proto-level type safety and symmetry with the enum definition: update the field declaration in ReadReceiptPush (read_type = 4) to use ReadType instead of int32 and keep the field number/tag unchanged; ensure any references or generated code consumers are rebuilt to reflect the enum type change.cim-common/src/main/java/com/crossoverjie/cim/common/constant/Constants.java (1)
51-75: 💤 Low valueJava integer constants duplicate proto enum ordinals without a compile-time link.
CONVERSATION_TYPE_P2P = 0,CONVERSATION_TYPE_GROUP = 1,READ_TYPE_SINGLE = 1,READ_TYPE_CONVERSATION = 2,MSG_TYPE_*, etc., all mirror values from the newly added proto enums in the same PR. If a proto enum value is ever renumbered,Constantssilently diverges and causes hard-to-diagnose mismatches at runtime. Consider referencing the generated proto enum values directly (e.g.,ConversationType.CONVERSATION_TYPE_P2P_VALUE) in the service/route layer rather than maintaining a parallel integer copy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-common/src/main/java/com/crossoverjie/cim/common/constant/Constants.java` around lines 51 - 75, Replace the hard-coded integer constants in Constants.java (e.g., MSG_TYPE_TEXT, MSG_TYPE_IMAGE, MSG_TYPE_VOICE, MSG_TYPE_VIDEO, MSG_TYPE_FILE, CONVERSATION_TYPE_P2P, CONVERSATION_TYPE_GROUP, READ_TYPE_SINGLE, READ_TYPE_CONVERSATION) with references to the generated proto enum values (e.g., use ConversationType.CONVERSATION_TYPE_P2P_VALUE and the corresponding MessageType/ReadType enum *_VALUE fields) in callers (service/route layer) so there is a single source of truth; remove or deprecate the duplicate Integer fields in Constants.java and update usages to import and use the proto enum VALUE constants (or the enum instances) instead to avoid divergence when proto ordinals change.cim-persistence/cim-persistence-api/src/main/java/com/crossoverjie/cim/persistence/api/service/ReadReceiptService.java (1)
35-35: 💤 Low valueAmbiguous
totalMembersparameter inincrementGroupReadCount.It is unclear whether
totalMembersrepresents the current group size (used to compute a read percentage), a delta of new readers, or something else. This ambiguity can lead to wrong implementations downstream. Consider adding a Javadoc comment clarifying the semantics, or renaming the parameter (e.g.groupSize) to make the intent self-evident.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-persistence/cim-persistence-api/src/main/java/com/crossoverjie/cim/persistence/api/service/ReadReceiptService.java` at line 35, The parameter name totalMembers in the interface method incrementGroupReadCount(Long groupId, Long messageId, int totalMembers) is ambiguous; rename it to groupSize (or add a Javadoc) to clarify it represents the current total number of members in the group used for read-percentage calculations, then update the method signature in ReadReceiptService and all implementing classes and callers to use groupSize (or add the Javadoc above incrementGroupReadCount explaining the exact semantics: that it is the current group size, not a delta or number-of-new-readers).cim-persistence/cim-persistence-mysql/src/main/resources/mapper/readreceipt/GroupMsgReadCountMapper.xml (1)
62-76: ⚡ Quick win
selectUnreadByConversationhas noLIMIT— unbounded result set.For active conversations this query could return thousands of rows, causing memory pressure and slow response times. Consider adding a
LIMIT(with cursor-based pagination if required) unless the caller always bounds the result externally.This applies to the same statement in
GroupMsgReadStatusMapper.xmlandConversationReadRecordMapper.xml.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cim-persistence/cim-persistence-mysql/src/main/resources/mapper/readreceipt/GroupMsgReadCountMapper.xml` around lines 62 - 76, The select id="selectUnreadByConversation" query currently returns an unbounded result set; update this mapper to enforce a LIMIT (e.g., add "LIMIT #{limit} OFFSET #{offset}" or "LIMIT #{limit}" with a caller-supplied param) and expose parameters (limit/offset or a cursor token) so callers can page results; apply the same change to the corresponding statement in GroupMsgReadStatusMapper.xml and ConversationReadRecordMapper.xml, and ensure the DAO/service methods that call selectUnreadByConversation accept and pass the new pagination parameters (receiveUserId, conversationId, tableName remain unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@cim-forward-route/src/main/java/com/crossoverjie/cim/route/service/impl/ReadReceiptRouteServiceImpl.java`:
- Around line 97-98: ReadReceiptRouteServiceImpl calls reqVO.getTotalMembers()
but ConversationReadReqVO has no totalMembers field, causing a compile error;
either add a totalMembers property (with getter/setter) to ConversationReadReqVO
in the shared API/SDK contract and bump any dependent SDKs so
ReadReceiptRouteServiceImpl can call reqVO.getTotalMembers(), or remove that
usage and resolve the count server-side by fetching the group member count
before calling readReceiptService.incrementGroupReadCount(reqVO.getGroupId(),
msg.getMessageId(), totalMembers); update the code paths that construct
ConversationReadReqVO accordingly if you choose the contract change.
In
`@cim-forward-route/src/main/java/com/crossoverjie/cim/route/service/ReadReceiptRouteService.java`:
- Around line 1-10: This file declares the persistence API interface
ReadReceiptService in the route module with the wrong package; either delete
this duplicate interface and have the route module depend on and import
com.crossoverjie.cim.persistence.api.service.ReadReceiptService, or if you truly
need a route-layer facade create a new interface/class in package
com.crossoverjie.cim.route.service (e.g. RouteReadReceiptService) that
adapts/forwards to the persistence ReadReceiptService; do not keep an identical
ReadReceiptService declaration here—remove the file or rename and relocate the
interface to the correct package and wire it to the persistence API.
In
`@cim-persistence/cim-persistence-mysql/src/main/java/com/crossoverjie/cim/persistence/mysql/readreceipt/mapper/ConversationReadRecordMapper.java`:
- Line 10: The interface in ConversationReadRecordMapper.java is incorrectly
declared as public interface P2PMsgReadStatusMapper causing a filename/type
mismatch and duplicate symbol errors; rename the interface to
ConversationReadRecordMapper and update its generic/entity type to use
ConversationReadRecord (instead of the P2PMsgReadStatus* type), and verify the
other mapper files do not declare the same public interface name so you avoid
duplicate public interface definitions.
In
`@cim-persistence/cim-persistence-mysql/src/main/java/com/crossoverjie/cim/persistence/mysql/readreceipt/mapper/GroupMsgReadCountMapper.java`:
- Line 10: The public interface name in each mapper file is wrong: files like
GroupMsgReadCountMapper.java, GroupMsgReadStatusMapper.java, and
ConversationReadRecordMapper.java currently declare public interface
P2PMsgReadStatusMapper which causes a compiler error; rename the public
interface in each file to match the file name (e.g., change
P2PMsgReadStatusMapper to GroupMsgReadCountMapper in
GroupMsgReadCountMapper.java, to GroupMsgReadStatusMapper in
GroupMsgReadStatusMapper.java, and to ConversationReadRecordMapper in
ConversationReadRecordMapper.java) and update any generic/entity type
declarations or imports so each interface references the correct entity type
(e.g., GroupMsgReadStatus for group mappers and ConversationReadRecord for the
conversation mapper).
In
`@cim-persistence/cim-persistence-mysql/src/main/java/com/crossoverjie/cim/persistence/mysql/readreceipt/mapper/GroupMsgReadStatusMapper.java`:
- Line 10: The interface declaration is using the wrong name and entity type:
change the interface from P2PMsgReadStatusMapper to GroupMsgReadStatusMapper and
update any generic/entity references from P2PMsgReadStatus to GroupMsgReadStatus
(and adjust the import) so the mapper matches the persistence service API;
verify the interface name and the entity type in GroupMsgReadStatusMapper, and
fix any references in that file to the correct GroupMsgReadStatus symbol.
In
`@cim-persistence/cim-persistence-mysql/src/main/java/com/crossoverjie/cim/persistence/mysql/readreceipt/ReadReceiptServiceImpl.java`:
- Around line 113-116: The code uses extractGroupIdFromMessageId(messageId)
which returns the messageId instead of the message's group id, causing shard
lookups to use the wrong shard; fix getGroupMsgReadStatus by first loading the
message record to obtain its real groupId (e.g., via the existing
message/messageMapper or messageService lookup by messageId), then call
ShardingStrategy.getGroupMsgReadTableName(groupId) and pass that tableName to
groupMsgReadStatusMapper.selectByMessageIdAndUserId; apply the same change to
the other occurrence (the block around lines 163-165/handleGroupMsgRead) so all
group read-status lookups use the actual groupId rather than the messageId.
In
`@cim-persistence/cim-persistence-mysql/src/main/resources/mapper/readreceipt/ConversationReadRecordMapper.xml`:
- Line 3: The mapper XML's namespace is incorrect: change the namespace
attribute value from
"com.crossoverjie.cim.persistence.mysql.readreceipt.mapper.P2PMsgReadStatusMapper"
to
"com.crossoverjie.cim.persistence.mysql.readreceipt.mapper.ConversationReadRecordMapper"
in the ConversationReadRecordMapper XML (the <mapper> element) so this file no
longer duplicates the P2PMsgReadStatusMapper namespace and avoids colliding
statement IDs.
In
`@cim-persistence/cim-persistence-mysql/src/main/resources/mapper/readreceipt/GroupMsgReadCountMapper.xml`:
- Line 3: The three mapper XMLs currently share the same MyBatis namespace which
causes duplicate statement registration; open GroupMsgReadCountMapper.xml,
GroupMsgReadStatusMapper.xml and ConversationReadRecordMapper.xml and set each
<mapper namespace="..."> to the correct, distinct Java mapper interface name
(e.g.
com.crossoverjie.cim.persistence.mysql.readreceipt.mapper.GroupMsgReadCountMapper,
com.crossoverjie.cim.persistence.mysql.readreceipt.mapper.GroupMsgReadStatusMapper,
com.crossoverjie.cim.persistence.mysql.readreceipt.mapper.ConversationReadRecordMapper)
so the duplicated statement IDs (insert, insertBatch,
selectByMessageIdAndUserId, etc.) are uniquely qualified by their proper
namespace.
- Around line 25-46: The insertBatch mapper (insert id="insertBatch") can emit
invalid SQL when the "list" is empty; wrap the VALUES clause with an XML guard
so no INSERT is produced for empty input (e.g., add <if test="list != null and
list.size() > 0"> before the VALUES and closing </if> after the </foreach>),
or alternatively handle the short-circuit in the service layer before calling
insertBatch; apply the same fix to GroupMsgReadStatusMapper.xml and
ConversationReadRecordMapper.xml to prevent empty-list SQL generation.
In
`@cim-persistence/cim-persistence-mysql/src/main/resources/mapper/readreceipt/GroupMsgReadStatusMapper.xml`:
- Line 3: The mapper's namespace is incorrectly set to P2PMsgReadStatusMapper
and conflicts with other mappers; change the mapper namespace attribute value
from
"com.crossoverjie.cim.persistence.mysql.readreceipt.mapper.P2PMsgReadStatusMapper"
to
"com.crossoverjie.cim.persistence.mysql.readreceipt.mapper.GroupMsgReadStatusMapper",
update any statement ids if they collide with P2P counterparts (or keep names
but rely on the new namespace), and adjust the SQL inside this mapper (table
names, column names and resultType/resultMap) to operate on GroupMsgReadStatus
entities/columns instead of P2P/GroupMsgReadCount variants so the statements
target GroupMsgReadStatus data.
In
`@cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/vo/res/GroupReadCountResVO.java`:
- Around line 1-33: The file currently declares GroupMsgReadReqVO in package
com.crossoverjie.cim.route.api.vo.req while the filename and directory indicate
it should be GroupReadCountResVO in package
com.crossoverjie.cim.route.api.vo.res; fix by replacing the incorrect class with
a proper response VO named GroupReadCountResVO in package
com.crossoverjie.cim.route.api.vo.res (remove or move GroupMsgReadReqVO to its
correct file if needed), define appropriate response fields such as Long
messageId, Integer readCount, Integer totalMembers (and any getters/setters or
Lombok annotations consistent with project style), and ensure the public class
name matches the filename and declared package matches the directory.
In
`@cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/vo/res/UnreadCountResVO.java`:
- Around line 1-33: The file UnreadCountResVO.java currently defines the wrong
public class and package: it contains GroupMsgReadReqVO in package
com.crossoverjie.cim.route.api.vo.req which causes a compilation mismatch;
change this file to declare the correct class UnreadCountResVO in package
com.crossoverjie.cim.route.api.vo.res (remove or move GroupMsgReadReqVO to its
proper file under vo.req), and implement the expected response VO fields (e.g.,
unreadCount, maybe groupId/userId as needed) so the filename, package, and
public class name match and the missing UnreadCountResVO type is provided for
callers.
In `@sql/02-offline-msg-sharding.sql`:
- Around line 1-127: The two migration files both create the same tables
offline_msg_0 … offline_msg_7, causing duplicate CREATE TABLE errors; fix by
keeping the table DDL in only one migration (either
sql/02-offline-msg-sharding.sql or sql/03-read-receipt.sql) and removing the
duplicate CREATE TABLE blocks from the other file, or consolidate the DDL into a
shared migration and have the other file reference those tables (alternatively
if you must keep both files, change the duplicate CREATEs to CREATE TABLE IF NOT
EXISTS and ensure migration ordering), targeting the CREATE statements for
offline_msg_0..offline_msg_7 to resolve the conflict.
In `@sql/03-read-receipt.sql`:
- Around line 1-127: The file accidentally duplicates the offline_msg_* sharded
tables; remove all CREATE TABLE offline_msg_0 … offline_msg_7 blocks and replace
them with the read-receipt schema (e.g., CREATE TABLE message_read_status or
message_read_receipt) that tracks message_id (BIGINT), reader_user_id (BIGINT),
read_at (DATETIME), status/TINYINT, optional read_count/INT, a composite primary
key or unique index on (message_id, reader_user_id), and useful indexes
(message_id, reader_user_id, read_at); ensure ENGINE=InnoDB and DEFAULT
CHARSET=utf8mb4 and include column names above to match other schema
conventions.
---
Major comments:
In
`@cim-client-sdk/src/main/java/com/crossoverjie/cim/client/sdk/RouteManager.java`:
- Around line 92-140: The three async methods sendP2PMsgRead,
sendConversationRead, and sendGroupMsgRead currently swallow exceptions and
ignore non-success BaseResponse results from routeApi, so change each to
propagate failures to the returned CompletableFuture: capture the BaseResponse
returned by routeApi.* (p2pMsgRead, conversationRead, groupMsgRead), check its
success code/flag and if not successful throw an appropriate RuntimeException
(or build a specific exception) so the CompletableFuture completes
exceptionally; also remove the try/catch that swallows exceptions (or re-throw
inside the catch) so any thrown exceptions propagate to callers (e.g., via
CompletableFuture.supplyAsync/runAsync semantics) and callers calling .get()
will see the failure.
In
`@cim-forward-route/src/main/java/com/crossoverjie/cim/route/controller/RouteController.java`:
- Around line 114-131: saveGroupOfflineMsg builds a properties map but never
persists it, so group messages lose conversation metadata; fix by passing those
properties into the persistence call: either add a properties field/setter to
P2PReqVO and set it before calling offlineMsgService.saveOfflineMsg(p2pReqVO),
or add an overloaded saveOfflineMsg method on OfflineMsgService that accepts
(P2PReqVO, Map<String,String>) and update OfflineMsgServiceImpl to persist the
map alongside sendUserId/receiveUserId/msg; update the call in
saveGroupOfflineMsg to use the new/extended API so properties are stored with
the offline message.
In
`@cim-forward-route/src/main/java/com/crossoverjie/cim/route/service/impl/ReadReceiptRouteServiceImpl.java`:
- Around line 107-126: handleGroupMsgRead currently always calls
readReceiptService.incrementGroupReadCount which causes duplicate increments;
change the flow so incrementGroupReadCount is only invoked when the user's
status transitions unread→read. Concretely, have markGroupMsgAsRead or the DAO
method return whether it actually changed state (e.g., affected-row count) and
use that boolean/result to decide to call incrementGroupReadCount;
alternatively, call getGroupMsgReadStatus first and only call
incrementGroupReadCount when status is null or status.isRead indicates unread
before you save via saveGroupMsgReadStatus. Ensure you reference
handleGroupMsgRead, readReceiptService.markGroupMsgAsRead,
readReceiptService.getGroupMsgReadStatus,
readReceiptService.saveGroupMsgReadStatus and
readReceiptService.incrementGroupReadCount when implementing this guard.
In
`@cim-forward-route/src/main/java/com/crossoverjie/cim/route/service/impl/UserInfoCacheServiceImpl.java`:
- Around line 59-74: getRegisteredUserIds currently calls
redisTemplate.keys(ACCOUNT_PREFIX + "*"), which performs a blocking O(N) KEYS
call; replace it with a cursor-based SCAN using
RedisOperations.scan(ScanOptions) to avoid blocking. Modify getRegisteredUserIds
to build ScanOptions.match(ACCOUNT_PREFIX + "*"), obtain a Cursor<byte[]> (or
Cursor<String>) via redisTemplate.execute or
redisTemplate.opsForValue().getOperations().scan, iterate the cursor in a
try-with-resources block to parse and collect IDs (strip ACCOUNT_PREFIX then
Long.parseLong), log and skip NumberFormatException as before, and ensure the
cursor is closed automatically. Use the existing ACCOUNT_PREFIX and preserve the
method signature and returned Set<Long>.
In
`@cim-persistence/cim-persistence-api/src/main/java/com/crossoverjie/cim/persistence/api/service/EnhancedOfflineMsgStore.java`:
- Line 18: The method EnhancedOfflineMsgStore.findByMessageId(Long messageId)
returns a single OfflineMsg but the sharded schema uses receive_user_id (8
shards) so the same messageId can exist in multiple shards/rows; you must either
(A) change the signature to findByMessageId(Long messageId, Long receiveUserId)
so the shard is resolved deterministically (update all implementations and
callers to pass receiveUserId and route to the correct shard), or (B) change the
return type to List<OfflineMsg> and implement a fan-out across all 8 shards to
collect all matching rows for messageId (update implementations and callers to
handle multiple results); ensure any DAO/SQL layer and tests are updated to
reflect the chosen approach.
In
`@cim-persistence/cim-persistence-mysql/src/main/java/com/crossoverjie/cim/persistence/mysql/offlinemsg/mapper/OfflineMsgShardingMapper.java`:
- Around line 26-28: The method fetchOfflineMsgIdsWithCursor currently lacks a
limit parameter, which can return unbounded ID lists and cause OOM/slow queries;
update the OfflineMsgShardingMapper.fetchOfflineMsgIdsWithCursor signature to
accept `@Param`("limit") Integer limit, update the corresponding SQL mapping (XML
or annotation) to apply LIMIT #{limit}, and adjust any callers to pass a sane
limit (matching fetchOfflineMsgsWithCursor usage) so the returned List<Long> is
bounded.
In
`@cim-persistence/cim-persistence-mysql/src/main/java/com/crossoverjie/cim/persistence/mysql/readreceipt/ReadReceiptServiceImpl.java`:
- Around line 134-148: The current incrementGroupReadCount method does a
select-then-insert/increment leading to races; change it to a single atomic
upsert/update at the mapper level: replace the select+conditional
insert/increment logic in incrementGroupReadCount with one call to a new mapper
method (e.g., groupMsgReadCountMapper.upsertIncrementReadCount) that executes a
single SQL statement such as INSERT ... ON DUPLICATE KEY UPDATE read_count =
read_count + 1 (and sets total_members when inserting) using the same table name
from ShardingStrategy.getGroupMsgReadCountTableName; keep GroupMsgReadCount as
the insert payload but move concurrency handling into the mapper SQL to ensure
atomicity.
In
`@cim-persistence/cim-persistence-mysql/src/main/resources/mapper/offlinemsg/OfflineMsgShardingMapper.xml`:
- Around line 26-28: The method fetchOfflineMsgIdsWithCursor is missing a limit
parameter which allows an unbounded result set; update its signature to accept a
`@Param`("limit") Integer limit (matching the sibling fetchOfflineMsgsWithCursor)
and ensure callers and mapper XML use that limit to constrain the query so only
up to `limit` IDs are returned; refer to fetchOfflineMsgIdsWithCursor and
fetchOfflineMsgsWithCursor when making the change.
In `@cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/RouteApi.java`:
- Around line 61-63: The client interface RouteApi.getGroupMsgsReadCount must
match the controller: change the mapping to a POST (e.g., add `@PostMapping` or
`@RequestMapping`(method = RequestMethod.POST)) and annotate the messageIds
parameter with `@RequestBody` so the list is read from the request body; also fix
the groupId parameter to use the correct `@RequestParam` annotation if needed
(ensure the method signature in RouteApi matches
RouteController.getGroupMsgsReadCount).
In
`@cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/vo/req/P2PMsgReadReqVO.java`:
- Around line 27-29: ReadReceiptHandler.handleP2PMsgRead currently only guards
messageId and userId but parses sendUserId via parseLong which may return null;
update handleP2PMsgRead to include sendUserId in the null-check guard (e.g., if
(messageId == null || userId == null || sendUserId == null) { ... return; }) so
you validate sendUserId before constructing P2PMsgReadReqVO and calling
routeApi.p2pMsgRead(reqVO); locate the check in
ReadReceiptHandler.handleP2PMsgRead and add sendUserId to the condition and
corresponding log/early-return behavior.
---
Minor comments:
In
`@cim-common/src/main/java/com/crossoverjie/cim/common/constant/Constants.java`:
- Around line 69-71: Add the missing zero/default constant to the Constants
class: declare public static final Integer READ_TYPE_UNKNOWN = 0 alongside the
existing READ_TYPE_SINGLE and READ_TYPE_CONVERSATION in class Constants so code
that receives a proto ReadType value of 0 can compare against a named constant
(use the same Integer type as READ_TYPE_SINGLE/READ_TYPE_CONVERSATION and place
it near those declarations).
In `@cim-common/src/main/proto/cim.proto`:
- Around line 29-39: ConversationReadRequest and GroupMsgReadRequest are missing
fields required by ReadReceiptHandler.java (and the handler currently bypasses
these protos by reading Request.properties), so either add the missing fields to
the proto and switch the handler to deserialize and use them, or remove the dead
proto messages. Specifically, update ConversationReadRequest to include
conversation_type, group_id and target_user_id (with appropriate scalar types
consistent with existing protos) and update GroupMsgReadRequest to include
total_members; then modify ReadReceiptHandler.java to parse the incoming Request
payload into P2PMsgReadRequest/ConversationReadRequest/GroupMsgReadRequest and
use those typed fields instead of Request.properties (or if you choose removal,
delete the proto messages and adjust any client code to stop sending them).
In
`@cim-server/src/main/java/com/crossoverjie/cim/server/kit/ReadReceiptHandler.java`:
- Around line 27-33: The null-check on userId in ReadReceiptHandler is dead
because request.getRequestId() returns a primitive long (auto-boxed) so unset
proto3 fields are 0, not null; update the validation in the P2P, conversation
and group handlers (methods handling request.getRequestId() — e.g., the handlers
where you call routeApi.p2pMsgRead(...), routeApi.conversationRead(...),
routeApi.groupMsgRead(...)) to treat 0 (or <=0 if you prefer stricter
validation) as invalid instead of null, i.e., replace/augment the userId null
check with a check like userId == 0 (or userId <= 0) and reject/return on that
condition; also ensure other parsed IDs (e.g., sendUserId from parseLong(...))
are validated for null/invalid values as appropriate before calling routeApi.
---
Nitpick comments:
In
`@cim-common/src/main/java/com/crossoverjie/cim/common/constant/Constants.java`:
- Around line 51-75: Replace the hard-coded integer constants in Constants.java
(e.g., MSG_TYPE_TEXT, MSG_TYPE_IMAGE, MSG_TYPE_VOICE, MSG_TYPE_VIDEO,
MSG_TYPE_FILE, CONVERSATION_TYPE_P2P, CONVERSATION_TYPE_GROUP, READ_TYPE_SINGLE,
READ_TYPE_CONVERSATION) with references to the generated proto enum values
(e.g., use ConversationType.CONVERSATION_TYPE_P2P_VALUE and the corresponding
MessageType/ReadType enum *_VALUE fields) in callers (service/route layer) so
there is a single source of truth; remove or deprecate the duplicate Integer
fields in Constants.java and update usages to import and use the proto enum
VALUE constants (or the enum instances) instead to avoid divergence when proto
ordinals change.
In `@cim-common/src/main/proto/cim.proto`:
- Around line 41-48: The ReadReceiptPush message currently declares read_type as
int32; change its type to the existing ReadType enum to restore proto-level type
safety and symmetry with the enum definition: update the field declaration in
ReadReceiptPush (read_type = 4) to use ReadType instead of int32 and keep the
field number/tag unchanged; ensure any references or generated code consumers
are rebuilt to reflect the enum type change.
In
`@cim-persistence/cim-persistence-api/src/main/java/com/crossoverjie/cim/persistence/api/service/ReadReceiptService.java`:
- Line 35: The parameter name totalMembers in the interface method
incrementGroupReadCount(Long groupId, Long messageId, int totalMembers) is
ambiguous; rename it to groupSize (or add a Javadoc) to clarify it represents
the current total number of members in the group used for read-percentage
calculations, then update the method signature in ReadReceiptService and all
implementing classes and callers to use groupSize (or add the Javadoc above
incrementGroupReadCount explaining the exact semantics: that it is the current
group size, not a delta or number-of-new-readers).
In
`@cim-persistence/cim-persistence-mysql/src/main/resources/mapper/readreceipt/GroupMsgReadCountMapper.xml`:
- Around line 62-76: The select id="selectUnreadByConversation" query currently
returns an unbounded result set; update this mapper to enforce a LIMIT (e.g.,
add "LIMIT #{limit} OFFSET #{offset}" or "LIMIT #{limit}" with a caller-supplied
param) and expose parameters (limit/offset or a cursor token) so callers can
page results; apply the same change to the corresponding statement in
GroupMsgReadStatusMapper.xml and ConversationReadRecordMapper.xml, and ensure
the DAO/service methods that call selectUnreadByConversation accept and pass the
new pagination parameters (receiveUserId, conversationId, tableName remain
unchanged).
In
`@cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/vo/req/ConversationReadReqVO.java`:
- Around line 27-29: Replace the raw Integer conversationType in
ConversationReadReqVO with a dedicated enum (e.g., ConversationType {P2P,
GROUP}) to enforce allowed values; update the field type in
ConversationReadReqVO to ConversationType, keep `@NotNull`, and adjust the `@Schema`
annotation to reflect the enum values and a suitable example (or map enum to
desired integer representation if external API expects ints), and update any
code that constructs/parses ConversationReadReqVO (deserializers, controllers,
tests) to use or convert to the new ConversationType enum.
In `@sql/02-offline-msg-sharding.sql`:
- Line 9: The created_at column is defined as "created_at DATETIME" which can be
NULL and pollute idx_created_at; modify the column definition for created_at to
include a default timestamp (e.g., DEFAULT CURRENT_TIMESTAMP) and consider
adding NOT NULL so rows inserted without an explicit timestamp are automatically
populated and the idx_created_at index doesn't contain NULLs.
- Around line 1-15: Change each CREATE TABLE statement to CREATE TABLE IF NOT
EXISTS for all eight partition tables (offline_msg_0 through offline_msg_7) in
both SQL files so the scripts are re-runnable; locate the table definitions
(e.g., the offline_msg_0 block shown) and update the header to "CREATE TABLE IF
NOT EXISTS offline_msg_X" for each partition, preserving the existing columns,
indexes and table options exactly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+97
to
+98
| int totalMembers = reqVO.getTotalMembers() != null ? reqVO.getTotalMembers() : 100; | ||
| readReceiptService.incrementGroupReadCount(reqVO.getGroupId(), msg.getMessageId(), totalMembers); |
Contributor
There was a problem hiding this comment.
ConversationReadReqVO does not expose totalMembers.
ConversationReadReqVO in cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/vo/req/ConversationReadReqVO.java, Lines 1-39 has no totalMembers field, so this call will not compile as written. If this branch needs member counts, either add them to the shared contract and SDK path or resolve them server-side.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@cim-forward-route/src/main/java/com/crossoverjie/cim/route/service/impl/ReadReceiptRouteServiceImpl.java`
around lines 97 - 98, ReadReceiptRouteServiceImpl calls reqVO.getTotalMembers()
but ConversationReadReqVO has no totalMembers field, causing a compile error;
either add a totalMembers property (with getter/setter) to ConversationReadReqVO
in the shared API/SDK contract and bump any dependent SDKs so
ReadReceiptRouteServiceImpl can call reqVO.getTotalMembers(), or remove that
usage and resolve the count server-side by fetching the group member count
before calling readReceiptService.incrementGroupReadCount(reqVO.getGroupId(),
msg.getMessageId(), totalMembers); update the code paths that construct
ConversationReadReqVO accordingly if you choose the contract change.
Comment on lines
+1
to
+10
| package com.crossoverjie.cim.persistence.api.service; | ||
|
|
||
| import com.crossoverjie.cim.persistence.api.pojo.GroupMsgReadCount; | ||
| import com.crossoverjie.cim.persistence.api.pojo.GroupMsgReadStatus; | ||
| import com.crossoverjie.cim.persistence.api.pojo.P2PMsgReadStatus; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public interface ReadReceiptService { | ||
|
|
Contributor
There was a problem hiding this comment.
Wrong package declaration and interface name — compilation failure + incorrect module placement.
Two separate issues:
-
Package/path mismatch: The file lives at
cim-forward-route/src/main/java/com/crossoverjie/cim/route/service/but declarespackage com.crossoverjie.cim.persistence.api.service;. The compiler will fail because the package declaration must match the directory path. -
Duplicate of persistence API: The content is identical to
cim-persistence-api/.../ReadReceiptService.java, including the same interface nameReadReceiptService. Placing a copy of a persistence API interface inside the route module is the wrong abstraction — the route module should depend on the persistence API module, not re-declare it.
If a distinct route-layer facade is needed, the file should declare:
-package com.crossoverjie.cim.persistence.api.service;
+package com.crossoverjie.cim.route.service;
-public interface ReadReceiptService {
+public interface ReadReceiptRouteService {
// route-specific methods, not a copy of the persistence API
}Otherwise, delete this file and have the route module import ReadReceiptService from the persistence API dependency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@cim-forward-route/src/main/java/com/crossoverjie/cim/route/service/ReadReceiptRouteService.java`
around lines 1 - 10, This file declares the persistence API interface
ReadReceiptService in the route module with the wrong package; either delete
this duplicate interface and have the route module depend on and import
com.crossoverjie.cim.persistence.api.service.ReadReceiptService, or if you truly
need a route-layer facade create a new interface/class in package
com.crossoverjie.cim.route.service (e.g. RouteReadReceiptService) that
adapts/forwards to the persistence ReadReceiptService; do not keep an identical
ReadReceiptService declaration here—remove the file or rename and relocate the
interface to the correct package and wire it to the persistence API.
| import java.util.List; | ||
|
|
||
| @Mapper | ||
| public interface P2PMsgReadStatusMapper { |
Contributor
There was a problem hiding this comment.
Same filename/interface name mismatch — compilation failure.
ConversationReadRecordMapper.java also declares public interface P2PMsgReadStatusMapper. The interface should be named ConversationReadRecordMapper and use ConversationReadRecord as its entity type. Having three files in the same package all declaring the same public interface P2PMsgReadStatusMapper would also trigger a duplicate symbol error in addition to the filename mismatch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@cim-persistence/cim-persistence-mysql/src/main/java/com/crossoverjie/cim/persistence/mysql/readreceipt/mapper/ConversationReadRecordMapper.java`
at line 10, The interface in ConversationReadRecordMapper.java is incorrectly
declared as public interface P2PMsgReadStatusMapper causing a filename/type
mismatch and duplicate symbol errors; rename the interface to
ConversationReadRecordMapper and update its generic/entity type to use
ConversationReadRecord (instead of the P2PMsgReadStatus* type), and verify the
other mapper files do not declare the same public interface name so you avoid
duplicate public interface definitions.
| import java.util.List; | ||
|
|
||
| @Mapper | ||
| public interface P2PMsgReadStatusMapper { |
Contributor
There was a problem hiding this comment.
Public interface name must match the file name — compilation failure.
Java requires a public type to be declared in a file whose name matches the type name exactly. GroupMsgReadCountMapper.java declares public interface P2PMsgReadStatusMapper, which will be rejected by the compiler with:
error: class P2PMsgReadStatusMapper is public, should be declared in a file named P2PMsgReadStatusMapper.java
The same problem exists in GroupMsgReadStatusMapper.java and ConversationReadRecordMapper.java (all three declare P2PMsgReadStatusMapper). Each file's interface needs to be renamed to match the file: GroupMsgReadCountMapper, GroupMsgReadStatusMapper, and ConversationReadRecordMapper respectively, with the appropriate entity type for each (e.g. GroupMsgReadStatus for group mappers, ConversationReadRecord for the conversation mapper).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@cim-persistence/cim-persistence-mysql/src/main/java/com/crossoverjie/cim/persistence/mysql/readreceipt/mapper/GroupMsgReadCountMapper.java`
at line 10, The public interface name in each mapper file is wrong: files like
GroupMsgReadCountMapper.java, GroupMsgReadStatusMapper.java, and
ConversationReadRecordMapper.java currently declare public interface
P2PMsgReadStatusMapper which causes a compiler error; rename the public
interface in each file to match the file name (e.g., change
P2PMsgReadStatusMapper to GroupMsgReadCountMapper in
GroupMsgReadCountMapper.java, to GroupMsgReadStatusMapper in
GroupMsgReadStatusMapper.java, and to ConversationReadRecordMapper in
ConversationReadRecordMapper.java) and update any generic/entity type
declarations or imports so each interface references the correct entity type
(e.g., GroupMsgReadStatus for group mappers and ConversationReadRecord for the
conversation mapper).
| import java.util.List; | ||
|
|
||
| @Mapper | ||
| public interface P2PMsgReadStatusMapper { |
Contributor
There was a problem hiding this comment.
Same filename/interface name mismatch as GroupMsgReadCountMapper.java — compilation failure.
GroupMsgReadStatusMapper.java also declares public interface P2PMsgReadStatusMapper. See the corresponding comment on GroupMsgReadCountMapper.java for the full analysis. The interface here should be GroupMsgReadStatusMapper and must use GroupMsgReadStatus (not P2PMsgReadStatus) as its entity type, matching the persistence service API which defines GroupMsgReadStatus operations on this mapper.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@cim-persistence/cim-persistence-mysql/src/main/java/com/crossoverjie/cim/persistence/mysql/readreceipt/mapper/GroupMsgReadStatusMapper.java`
at line 10, The interface declaration is using the wrong name and entity type:
change the interface from P2PMsgReadStatusMapper to GroupMsgReadStatusMapper and
update any generic/entity references from P2PMsgReadStatus to GroupMsgReadStatus
(and adjust the import) so the mapper matches the persistence service API;
verify the interface name and the entity type in GroupMsgReadStatusMapper, and
fix any references in that file to the correct GroupMsgReadStatus symbol.
| @@ -0,0 +1,104 @@ | |||
| <?xml version="1.0" encoding="UTF-8" ?> | |||
| <!DOCTYPE mapper PUBLIC "-//mybatis.org//DTD Mapper 3.0//EN" "http://mybatis.org/dtd/mybatis-3-mapper.dtd" > | |||
| <mapper namespace="com.crossoverjie.cim.persistence.mysql.readreceipt.mapper.P2PMsgReadStatusMapper"> | |||
Contributor
There was a problem hiding this comment.
Same duplicate namespace issue — correct to GroupMsgReadStatusMapper.
This file also uses namespace="com.crossoverjie.cim.persistence.mysql.readreceipt.mapper.P2PMsgReadStatusMapper" and contains the same statement IDs, compounding the duplicate-namespace startup failure already flagged for GroupMsgReadCountMapper.xml. The namespace must be updated to ...mapper.GroupMsgReadStatusMapper and its SQL should operate on GroupMsgReadStatus columns/types.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@cim-persistence/cim-persistence-mysql/src/main/resources/mapper/readreceipt/GroupMsgReadStatusMapper.xml`
at line 3, The mapper's namespace is incorrectly set to P2PMsgReadStatusMapper
and conflicts with other mappers; change the mapper namespace attribute value
from
"com.crossoverjie.cim.persistence.mysql.readreceipt.mapper.P2PMsgReadStatusMapper"
to
"com.crossoverjie.cim.persistence.mysql.readreceipt.mapper.GroupMsgReadStatusMapper",
update any statement ids if they collide with P2P counterparts (or keep names
but rely on the new namespace), and adjust the SQL inside this mapper (table
names, column names and resultType/resultMap) to operate on GroupMsgReadStatus
entities/columns instead of P2P/GroupMsgReadCount variants so the statements
target GroupMsgReadStatus data.
Comment on lines
+1
to
+33
| package com.crossoverjie.cim.route.api.vo.req; | ||
|
|
||
| import com.crossoverjie.cim.common.req.BaseRequest; | ||
| import io.swagger.v3.oas.annotations.media.Schema; | ||
| import jakarta.validation.constraints.NotNull; | ||
| import lombok.AllArgsConstructor; | ||
| import lombok.Builder; | ||
| import lombok.Data; | ||
| import lombok.EqualsAndHashCode; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| @EqualsAndHashCode(callSuper = true) | ||
| @Data | ||
| @Builder | ||
| @AllArgsConstructor | ||
| @NoArgsConstructor | ||
| public class GroupMsgReadReqVO extends BaseRequest { | ||
|
|
||
| @NotNull(message = "messageId can't be null") | ||
| @Schema(requiredMode = Schema.RequiredMode.REQUIRED, description = "message id", example = "1234567890") | ||
| private Long messageId; | ||
|
|
||
| @NotNull(message = "groupId can't be null") | ||
| @Schema(requiredMode = Schema.RequiredMode.REQUIRED, description = "group id", example = "10001") | ||
| private Long groupId; | ||
|
|
||
| @NotNull(message = "userId can't be null") | ||
| @Schema(requiredMode = Schema.RequiredMode.REQUIRED, description = "current user id", example = "1545574049323") | ||
| private Long userId; | ||
|
|
||
| @Schema(description = "group member count", example = "10") | ||
| private Integer totalMembers; | ||
| } |
Contributor
There was a problem hiding this comment.
Wrong class in wrong file — compilation will fail.
The file GroupReadCountResVO.java (in vo/res/) contains:
package com.crossoverjie.cim.route.api.vo.req;
public class GroupMsgReadReqVO extends BaseRequest { ... }
This is a copy of GroupMsgReadReqVO with two separate violations that prevent compilation:
- Java requires the public class name to match the filename (
GroupMsgReadReqVO≠GroupReadCountResVO). - The declared package (
vo.req) does not match the file's directory (vo/res/).
Additionally, the actual GroupReadCountResVO response VO class is entirely absent from the PR. Please replace this file's content with a proper GroupReadCountResVO response VO (e.g., fields for readCount, totalMembers, messageId, etc.) in the correct package com.crossoverjie.cim.route.api.vo.res.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/vo/res/GroupReadCountResVO.java`
around lines 1 - 33, The file currently declares GroupMsgReadReqVO in package
com.crossoverjie.cim.route.api.vo.req while the filename and directory indicate
it should be GroupReadCountResVO in package
com.crossoverjie.cim.route.api.vo.res; fix by replacing the incorrect class with
a proper response VO named GroupReadCountResVO in package
com.crossoverjie.cim.route.api.vo.res (remove or move GroupMsgReadReqVO to its
correct file if needed), define appropriate response fields such as Long
messageId, Integer readCount, Integer totalMembers (and any getters/setters or
Lombok annotations consistent with project style), and ensure the public class
name matches the filename and declared package matches the directory.
Comment on lines
+1
to
+33
| package com.crossoverjie.cim.route.api.vo.req; | ||
|
|
||
| import com.crossoverjie.cim.common.req.BaseRequest; | ||
| import io.swagger.v3.oas.annotations.media.Schema; | ||
| import jakarta.validation.constraints.NotNull; | ||
| import lombok.AllArgsConstructor; | ||
| import lombok.Builder; | ||
| import lombok.Data; | ||
| import lombok.EqualsAndHashCode; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| @EqualsAndHashCode(callSuper = true) | ||
| @Data | ||
| @Builder | ||
| @AllArgsConstructor | ||
| @NoArgsConstructor | ||
| public class GroupMsgReadReqVO extends BaseRequest { | ||
|
|
||
| @NotNull(message = "messageId can't be null") | ||
| @Schema(requiredMode = Schema.RequiredMode.REQUIRED, description = "message id", example = "1234567890") | ||
| private Long messageId; | ||
|
|
||
| @NotNull(message = "groupId can't be null") | ||
| @Schema(requiredMode = Schema.RequiredMode.REQUIRED, description = "group id", example = "10001") | ||
| private Long groupId; | ||
|
|
||
| @NotNull(message = "userId can't be null") | ||
| @Schema(requiredMode = Schema.RequiredMode.REQUIRED, description = "current user id", example = "1545574049323") | ||
| private Long userId; | ||
|
|
||
| @Schema(description = "group member count", example = "10") | ||
| private Integer totalMembers; | ||
| } |
Contributor
There was a problem hiding this comment.
Wrong class in wrong file — same compilation failure as GroupReadCountResVO.java.
UnreadCountResVO.java contains public class GroupMsgReadReqVO in package vo.req — identical mismatch: public class name does not match filename, and the package declaration is wrong for the vo/res/ directory. The actual UnreadCountResVO response VO (expected to carry unread count data) is missing from the PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@cim-rout-api/src/main/java/com/crossoverjie/cim/route/api/vo/res/UnreadCountResVO.java`
around lines 1 - 33, The file UnreadCountResVO.java currently defines the wrong
public class and package: it contains GroupMsgReadReqVO in package
com.crossoverjie.cim.route.api.vo.req which causes a compilation mismatch;
change this file to declare the correct class UnreadCountResVO in package
com.crossoverjie.cim.route.api.vo.res (remove or move GroupMsgReadReqVO to its
proper file under vo.req), and implement the expected response VO fields (e.g.,
unreadCount, maybe groupId/userId as needed) so the filename, package, and
public class name match and the missing UnreadCountResVO type is provided for
callers.
Comment on lines
+1
to
+127
| CREATE TABLE offline_msg_0 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_1 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_2 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_3 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_4 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_5 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_6 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_7 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; |
Contributor
There was a problem hiding this comment.
Identical schema duplicated in sql/03-read-receipt.sql — executing both files will fail.
sql/02-offline-msg-sharding.sql and sql/03-read-receipt.sql both create the same eight tables (offline_msg_0 … offline_msg_7) with identical DDL. Running both scripts sequentially (e.g., during a fresh database setup) will abort with Table 'offline_msg_X' already exists on the first duplicate CREATE TABLE. One of the files must be removed, or the duplicate definitions must be consolidated into a single script.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sql/02-offline-msg-sharding.sql` around lines 1 - 127, The two migration
files both create the same tables offline_msg_0 … offline_msg_7, causing
duplicate CREATE TABLE errors; fix by keeping the table DDL in only one
migration (either sql/02-offline-msg-sharding.sql or sql/03-read-receipt.sql)
and removing the duplicate CREATE TABLE blocks from the other file, or
consolidate the DDL into a shared migration and have the other file reference
those tables (alternatively if you must keep both files, change the duplicate
CREATEs to CREATE TABLE IF NOT EXISTS and ensure migration ordering), targeting
the CREATE statements for offline_msg_0..offline_msg_7 to resolve the conflict.
Comment on lines
+1
to
+127
| CREATE TABLE offline_msg_0 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_1 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_2 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_3 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_4 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_5 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_6 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; | ||
|
|
||
| CREATE TABLE offline_msg_7 | ||
| ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| message_id BIGINT NOT NULL, | ||
| receive_user_id BIGINT NOT NULL, | ||
| content VARCHAR(2000), | ||
| message_type INT, | ||
| status TINYINT, | ||
| created_at DATETIME, | ||
| properties VARCHAR(2000), | ||
| INDEX idx_receive_user_id (receive_user_id), | ||
| INDEX idx_message_id (message_id), | ||
| INDEX idx_created_at (created_at) | ||
| ) ENGINE = InnoDB | ||
| DEFAULT CHARSET = utf8mb4; |
Contributor
There was a problem hiding this comment.
Duplicate of sql/02-offline-msg-sharding.sql — and the actual read-receipt DDL is missing.
This file is an exact copy of sql/02-offline-msg-sharding.sql (offline message sharding tables). Given the filename 03-read-receipt.sql and the PR's stated goal of implementing read-receipt state management, this appears to be a copy-paste error; the actual read-receipt tables (e.g., a message_read_status or similar table tracking reader IDs, timestamps, and read counts) are absent entirely. Please remove the duplicate offline-msg DDL and add the missing read-receipt schema here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sql/03-read-receipt.sql` around lines 1 - 127, The file accidentally
duplicates the offline_msg_* sharded tables; remove all CREATE TABLE
offline_msg_0 … offline_msg_7 blocks and replace them with the read-receipt
schema (e.g., CREATE TABLE message_read_status or message_read_receipt) that
tracks message_id (BIGINT), reader_user_id (BIGINT), read_at (DATETIME),
status/TINYINT, optional read_count/INT, a composite primary key or unique index
on (message_id, reader_user_id), and useful indexes (message_id, reader_user_id,
read_at); ensure ENGINE=InnoDB and DEFAULT CHARSET=utf8mb4 and include column
names above to match other schema conventions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
新增离线消息分表存储及消息已读状态管理
添加P2P和群组消息的已读状态记录接口
实现会话级别的消息已读标记功能
支持查询未读消息数量及已读人数统计
优化用户信息缓存服务以支持注册用户ID查询
Summary by CodeRabbit
Release Notes