server/sino-mci
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(send-routing): address final code review findings

Browse Source 
- WeComBotSender.resolveWebhookUrl: use custom URL as-is when no %s placeholder
- WeComBotSender: treat unparseable/blank response body as failure
- ConversationService.isAccountCompatibleWithConversation: ignore scene for single chats
- SendService.selectSendAccountCandidates: apply accountTypes filter for non-conversation records
- SendService: replace account type magic numbers with AccountType constants
- WeComChannelAdapter: use AccountType.WECOM_BOT.getCode() instead of literal 6
- admin-web Conversation.vue: fallback to channelType filter when scene is unset
- add tests for single-chat null scene, webhook_url propagation, and custom URL without %s
This commit is contained in:
金亮
2026-07-15 11:53:51 +08:00
parent c24891a82e
commit 0a326e7ff9
8 changed files with 165 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
backend/mci-channel-wecom/src/main/java/com/sino/mci/channel/wecom/WeComBotSender.java
View File

@@ -62,6 +62,11 @@ public class WeComBotSender {
String url = resolveWebhookUrl(request.getExtra(), key);
ResponseEntity<String> response = restTemplate.postForEntity(url, entity, String.class);
Map<String, Object> responseBody = parseResponseBody(response.getBody());
if (responseBody == null) {
result.setSuccess(false);
result.setMessage("企微群机器人返回无法解析的响应");
return result;
}
int errcode = parseInt(responseBody.get("errcode"));
if (errcode != 0) {
result.setSuccess(false);
@@ -88,21 +93,20 @@ public class WeComBotSender {
if (customUrl.contains("%s")) {
return customUrl.replace("%s", key);
}
log.warn("[WeComBot] custom webhook_url missing %s placeholder; appending key param");
return customUrl.contains("?") ? customUrl + "&key=" + key : customUrl + "?key=" + key;
return customUrl;
}
return String.format(DEFAULT_WEBHOOK_URL, key);
}
private Map<String, Object> parseResponseBody(String body) {
if (body == null || body.isBlank()) {
return new HashMap<>();
return null;
}
try {
return objectMapper.readValue(body, Map.class);
} catch (Exception e) {
log.warn("[WeComBot] 解析响应失败: {}", body, e);
return new HashMap<>();
return null;
}
}

3
backend/mci-channel-wecom/src/main/java/com/sino/mci/channel/wecom/WeComChannelAdapter.java
View File

@@ -5,6 +5,7 @@ import com.sino.mci.channel.common.dto.ChannelInitRequest;
import com.sino.mci.channel.common.dto.ChannelInitResult;
import com.sino.mci.channel.common.dto.SendMessageRequest;
import com.sino.mci.channel.common.dto.SendMessageResult;
import com.sino.mci.channel.common.model.AccountType;
import com.sino.mci.channel.common.model.ChannelType;
import com.sino.mci.channel.common.spi.ChannelAdapter;
import com.sino.mci.channel.wecom.support.WeComServiceFactory;
@@ -86,7 +87,7 @@ public class WeComChannelAdapter implements ChannelAdapter {
}
Integer accountType = getAccountType(request);
if (accountType != null && accountType == 6) {
if (accountType != null && accountType == AccountType.WECOM_BOT.getCode()) {
return weComBotSender.send(request);
}

29
backend/mci-channel-wecom/src/test/java/com/sino/mci/channel/wecom/WeComBotSenderTest.java
View File

@@ -109,4 +109,33 @@ class WeComBotSenderTest {
assertTrue(result.isSuccess(), result.getMessage());
server.verify();
}
@Test
void sendUsesCustomWebhookUrlWithoutPlaceholder() throws Exception {
server.expect(requestTo("https://qyapi.weixin.qq.com/cgi-bin/webhook/send?key=REALKEY"))
.andExpect(content().json("{\"msgtype\":\"text\",\"text\":{\"content\":\"hello\"}}"))
.andRespond(withSuccess("{\"errcode\":0,\"errmsg\":\"ok\"}", MediaType.APPLICATION_JSON));
SendMessageRequest request = new SendMessageRequest();
request.setExtra(Map.of("account_id", "dummy", "webhook_url", "https://qyapi.weixin.qq.com/cgi-bin/webhook/send?key=REALKEY"));
request.setContentType("text");
request.setContent(Map.of("text", "hello"));
SendMessageResult result = sender.send(request);
assertTrue(result.isSuccess(), result.getMessage());
server.verify();
}
@Test
void sendFailsOnUnparseableResponseBody() {
server.expect(requestTo("https://qyapi.weixin.qq.com/cgi-bin/webhook/send?key=dummy"))
.andRespond(withSuccess("not-json", MediaType.TEXT_PLAIN));
SendMessageRequest request = new SendMessageRequest();
request.setExtra(Map.of("account_id", "dummy"));
request.setContentType("text");
request.setContent(Map.of("text", "hello"));
SendMessageResult result = sender.send(request);
assertFalse(result.isSuccess());
assertTrue(result.getMessage().contains("无法解析"));
}
}

4
backend/mci-server/src/main/java/com/sino/mci/channel/service/ConversationService.java
View File

@@ -158,6 +158,10 @@ public class ConversationService {
}
private boolean isAccountCompatibleWithConversation(Conversation conversation, ChannelAccount account) {
// 单聊不强制按场景绑定，保持与 SendService 路由逻辑一致
if (conversation.getConversationType() != null && conversation.getConversationType() == 1) {
return conversation.getChannelType().equalsIgnoreCase(account.getChannelType());
}
Integer scene = conversation.getConversationScene();
if (scene != null) {
return switch (scene) {

21
backend/mci-server/src/main/java/com/sino/mci/send/service/SendService.java
View File

@@ -84,9 +84,9 @@ public class SendService {
private static final int CONVERSATION_TYPE_GROUP = 2;
private static final int CONVERSATION_SCENE_INTERNAL = 1;
private static final int CONVERSATION_SCENE_EXTERNAL = 2;
private static final Set<Integer> ACCOUNT_TYPES_BOT = Set.of(6);
private static final Set<Integer> ACCOUNT_TYPES_PYWECHAT = Set.of(3);
private static final Set<Integer> ACCOUNT_TYPES_WECOM_AGENT = Set.of(2);
private static final Set<Integer> ACCOUNT_TYPES_BOT = Set.of(AccountType.WECOM_BOT.getCode());
private static final Set<Integer> ACCOUNT_TYPES_PYWECHAT = Set.of(AccountType.WECHAT_PERSONAL.getCode());
private static final Set<Integer> ACCOUNT_TYPES_WECOM_AGENT = Set.of(AccountType.WECOM_AGENT.getCode());
private final SendRecordMapper sendRecordMapper;
private final ConversationMapper conversationMapper;
@@ -947,7 +947,17 @@ public class SendService {
private List<Long> selectSendAccountCandidates(SendRecord record, ChannelType channelType, Set<Integer> accountTypes) {
if (record.getConversationId() == null) {
return record.getAccountId() != null ? Collections.singletonList(record.getAccountId()) : Collections.emptyList();
if (record.getAccountId() == null) {
return Collections.emptyList();
}
ChannelAccount account = accountMapper.selectById(record.getAccountId());
if (account == null || !AccountSelector.isHealthy(account, record.getTenantCode(), channelType)) {
return Collections.emptyList();
}
if (accountTypes != null && !accountTypes.contains(account.getAccountType())) {
return Collections.emptyList();
}
return Collections.singletonList(record.getAccountId());
}
String preferredStrategy = extractStrategy(parseChannelStrategy(record.getChannelStrategy()));
return accountSelector.selectSendAccounts(record.getTenantCode(), record.getConversationId(), channelType, accountTypes, preferredStrategy);
@@ -1003,7 +1013,8 @@ public class SendService {
}
private void markAccountUnhealthy(ChannelAccount account) {
if (account == null || account.getAccountType() == null || account.getAccountType() != 3) {
if (account == null || account.getAccountType() == null
|| account.getAccountType() != AccountType.WECHAT_PERSONAL.getCode()) {
return;
}
account.setRiskLevel(2);

76
backend/mci-server/src/test/java/com/sino/mci/send/service/SendServiceFailoverTest.java
View File

@@ -3,6 +3,7 @@ package com.sino.mci.send.service;
import com.sino.mci.channel.adapter.registry.ChannelAdapterRegistry;
import com.sino.mci.channel.common.dto.SendMessageRequest;
import com.sino.mci.channel.common.dto.SendMessageResult;
import com.sino.mci.channel.common.model.AccountType;
import com.sino.mci.channel.common.model.ChannelType;
import com.sino.mci.channel.common.spi.ChannelAdapter;
import com.sino.mci.channel.entity.ChannelAccount;
@@ -39,6 +40,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -79,7 +81,7 @@ class SendServiceFailoverTest {
@BeforeEach
void setUp() {
when(adapterRegistry.getAdapter(ChannelType.WECHAT_PERSONAL)).thenReturn(adapter);
lenient().when(adapterRegistry.getAdapter(ChannelType.WECHAT_PERSONAL)).thenReturn(adapter);
}
@Test
@@ -89,7 +91,7 @@ class SendServiceFailoverTest {
Long backupAccountId = 200L;
when(conversationMapper.selectById(1L)).thenReturn(createExternalConversation());
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECHAT_PERSONAL, Set.of(3), "primary"))
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECHAT_PERSONAL, Set.of(AccountType.WECHAT_PERSONAL.getCode()), "primary"))
.thenReturn(Arrays.asList(primaryAccountId, backupAccountId));
when(accountMapper.selectById(primaryAccountId)).thenReturn(createPywechatAccount(primaryAccountId, "wxid-primary"));
when(accountMapper.selectById(backupAccountId)).thenReturn(createPywechatAccount(backupAccountId, "wxid-backup"));
@@ -131,7 +133,7 @@ class SendServiceFailoverTest {
Long accountId = 100L;
when(conversationMapper.selectById(1L)).thenReturn(createExternalConversation());
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECHAT_PERSONAL, Set.of(3), "primary"))
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECHAT_PERSONAL, Set.of(AccountType.WECHAT_PERSONAL.getCode()), "primary"))
.thenReturn(Collections.singletonList(accountId));
when(accountMapper.selectById(accountId)).thenReturn(createPywechatAccount(accountId, "wxid-primary"));
@@ -166,7 +168,7 @@ class SendServiceFailoverTest {
SendRecord record = createRecord();
when(conversationMapper.selectById(1L)).thenReturn(createExternalConversation());
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECHAT_PERSONAL, Set.of(3), "primary"))
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECHAT_PERSONAL, Set.of(AccountType.WECHAT_PERSONAL.getCode()), "primary"))
.thenReturn(Collections.emptyList());
sendService.dispatchRecord(record);
@@ -179,6 +181,33 @@ class SendServiceFailoverTest {
assertEquals("no_healthy_account", result.get("error"));
}
@Test
void dispatchRecordPropagatesWebhookUrlForWeComBot() {
SendRecord record = createInternalGroupRecord();
Long botAccountId = 100L;
when(adapterRegistry.getAdapter(ChannelType.WECOM)).thenReturn(adapter);
when(conversationMapper.selectById(1L)).thenReturn(createInternalConversation());
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECOM, Set.of(AccountType.WECOM_BOT.getCode()), "primary"))
.thenReturn(Collections.singletonList(botAccountId));
when(accountMapper.selectById(botAccountId)).thenReturn(createWeComBotAccount(botAccountId, "bot-1", "https://custom.example.com/webhook?key=secret"));
SendMessageResult successResult = new SendMessageResult();
successResult.setSuccess(true);
when(adapter.sendMessage(any(SendMessageRequest.class))).thenReturn(successResult);
sendService.dispatchRecord(record);
assertEquals(2, record.getSendStatus().intValue());
assertEquals(botAccountId, record.getAccountId());
ArgumentCaptor<SendMessageRequest> requestCaptor = ArgumentCaptor.forClass(SendMessageRequest.class);
verify(adapter).sendMessage(requestCaptor.capture());
SendMessageRequest request = requestCaptor.getValue();
assertEquals("https://custom.example.com/webhook?key=secret", request.getExtra().get("webhook_url"));
assertEquals(Integer.valueOf(AccountType.WECOM_BOT.getCode()), request.getExtra().get("account_type"));
}
private SendRecord createRecord() {
SendRecord record = new SendRecord();
record.setId(1L);
@@ -194,6 +223,21 @@ class SendServiceFailoverTest {
return record;
}
private SendRecord createInternalGroupRecord() {
SendRecord record = new SendRecord();
record.setId(1L);
record.setTaskId("SND001");
record.setTenantCode("tenant");
record.setConversationId(1L);
record.setAccountId(100L);
record.setChannelType("wecom");
record.setChannelStrategy("{\"channel_type\":\"wecom\",\"strategy\":\"primary\"}");
record.setContentType("text");
record.setContentBody("{\"text\":\"hello\"}");
record.setSendStatus(0);
return record;
}
private Conversation createExternalConversation() {
Conversation conversation = new Conversation();
conversation.setId(1L);
@@ -203,11 +247,21 @@ class SendServiceFailoverTest {
return conversation;
}
private Conversation createInternalConversation() {
Conversation conversation = new Conversation();
conversation.setId(1L);
conversation.setTenantCode("tenant");
conversation.setConversationType(2);
conversation.setConversationScene(1);
conversation.setChannelConversationId("conv-1");
return conversation;
}
private ChannelAccount createPywechatAccount(Long id, String accountId) {
ChannelAccount account = new ChannelAccount();
account.setId(id);
account.setTenantCode("tenant");
account.setAccountType(3);
account.setAccountType(AccountType.WECHAT_PERSONAL.getCode());
account.setChannelType("wechat_personal");
account.setAccountId(accountId);
account.setStatus(1);
@@ -216,4 +270,16 @@ class SendServiceFailoverTest {
account.setLastHeartbeat(java.time.LocalDateTime.now());
return account;
}
private ChannelAccount createWeComBotAccount(Long id, String accountId, String webhookUrl) {
ChannelAccount account = new ChannelAccount();
account.setId(id);
account.setTenantCode("tenant");
account.setAccountType(AccountType.WECOM_BOT.getCode());
account.setChannelType("wecom");
account.setAccountId(accountId);
account.setStatus(1);
account.setExtInfo(webhookUrl);
return account;
}
}

39
backend/mci-server/src/test/java/com/sino/mci/send/service/SendServiceResolveTargetTest.java
View File

@@ -1,6 +1,7 @@
package com.sino.mci.send.service;
import com.sino.mci.channel.adapter.registry.ChannelAdapterRegistry;
import com.sino.mci.channel.common.model.AccountType;
import com.sino.mci.channel.common.model.ChannelType;
import com.sino.mci.channel.entity.ChannelAccount;
import com.sino.mci.channel.entity.Conversation;
@@ -82,7 +83,7 @@ class SendServiceResolveTargetTest {
Long primaryAccountId = 10L;
when(conversationMapper.selectById(1L)).thenReturn(conversation);
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECOM, Set.of(6), "primary"))
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECOM, Set.of(AccountType.WECOM_BOT.getCode()), "primary"))
.thenReturn(Collections.singletonList(primaryAccountId));
when(accountMapper.selectById(primaryAccountId)).thenReturn(createWecomBotAccount(primaryAccountId, 1));
@@ -99,7 +100,7 @@ class SendServiceResolveTargetTest {
conversation.setOwnerAccountId(20L);
when(conversationMapper.selectById(1L)).thenReturn(conversation);
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECOM, Set.of(6), "primary"))
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECOM, Set.of(AccountType.WECOM_BOT.getCode()), "primary"))
.thenReturn(Collections.emptyList());
ChannelAccount disabledAccount = createWecomBotAccount(20L, 0);
when(accountMapper.selectById(20L)).thenReturn(disabledAccount);
@@ -115,7 +116,7 @@ class SendServiceResolveTargetTest {
Long backupAccountId = 20L;
when(conversationMapper.selectById(1L)).thenReturn(conversation);
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECOM, Set.of(6), "primary"))
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECOM, Set.of(AccountType.WECOM_BOT.getCode()), "primary"))
.thenReturn(Collections.singletonList(backupAccountId));
when(accountMapper.selectById(backupAccountId)).thenReturn(createWecomBotAccount(backupAccountId, 1));
@@ -131,7 +132,7 @@ class SendServiceResolveTargetTest {
Long backupAccountId = 20L;
when(conversationMapper.selectById(1L)).thenReturn(conversation);
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECOM, Set.of(6), "backup"))
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECOM, Set.of(AccountType.WECOM_BOT.getCode()), "backup"))
.thenReturn(Collections.singletonList(backupAccountId));
when(accountMapper.selectById(backupAccountId)).thenReturn(createWecomBotAccount(backupAccountId, 1));
@@ -141,7 +142,7 @@ class SendServiceResolveTargetTest {
assertEquals(backupAccountId, targets.get(0).getAccountId());
ArgumentCaptor<String> strategyCaptor = ArgumentCaptor.forClass(String.class);
verify(accountSelector).selectSendAccounts(eq("tenant"), eq(1L), eq(ChannelType.WECOM), eq(Set.of(6)), strategyCaptor.capture());
verify(accountSelector).selectSendAccounts(eq("tenant"), eq(1L), eq(ChannelType.WECOM), eq(Set.of(AccountType.WECOM_BOT.getCode())), strategyCaptor.capture());
assertEquals("backup", strategyCaptor.getValue());
}
@@ -151,7 +152,7 @@ class SendServiceResolveTargetTest {
Long pywechatAccountId = 30L;
when(conversationMapper.selectById(1L)).thenReturn(conversation);
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECHAT_PERSONAL, Set.of(3), "primary"))
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECHAT_PERSONAL, Set.of(AccountType.WECHAT_PERSONAL.getCode()), "primary"))
.thenReturn(Collections.singletonList(pywechatAccountId));
when(accountMapper.selectById(pywechatAccountId)).thenReturn(createPywechatAccount(pywechatAccountId, 1));
@@ -173,6 +174,24 @@ class SendServiceResolveTargetTest {
assertEquals("会话未设置场景，无法自动路由", ex.getMessage());
}
@Test
void resolveConversationTarget_singleChatWithNullSceneUsesChannelStrategy() {
Conversation conversation = createConversation(null);
conversation.setConversationType(1);
Long agentAccountId = 40L;
when(conversationMapper.selectById(1L)).thenReturn(conversation);
when(accountSelector.selectSendAccounts("tenant", 1L, ChannelType.WECOM, null, "primary"))
.thenReturn(Collections.singletonList(agentAccountId));
when(accountMapper.selectById(agentAccountId)).thenReturn(createWecomAgentAccount(agentAccountId, 1));
List<SendTarget> targets = sendService.resolveTestTargets("tenant", createConversationRequest("primary"));
assertEquals(1, targets.size());
assertEquals(agentAccountId, targets.get(0).getAccountId());
assertEquals(ChannelType.WECOM, targets.get(0).getChannelType());
}
@Test
void resolvePersonTarget_rejectsDisabledAccount() {
Person person = createPerson();
@@ -211,7 +230,7 @@ class SendServiceResolveTargetTest {
Person person = createPerson();
ChannelIdentity identity = createIdentity();
ChannelAccount account = createAccount(30L, "wecom", 1);
account.setAccountType(3);
account.setAccountType(AccountType.WECHAT_PERSONAL.getCode());
when(personMapper.selectById(1L)).thenReturn(person);
when(channelIdentityMapper.selectOne(any())).thenReturn(identity);
@@ -300,19 +319,19 @@ class SendServiceResolveTargetTest {
private ChannelAccount createWecomBotAccount(Long id, int status) {
ChannelAccount account = createAccount(id, "wecom", status);
account.setAccountType(6);
account.setAccountType(AccountType.WECOM_BOT.getCode());
return account;
}
private ChannelAccount createWecomAgentAccount(Long id, int status) {
ChannelAccount account = createAccount(id, "wecom", status);
account.setAccountType(2);
account.setAccountType(AccountType.WECOM_AGENT.getCode());
return account;
}
private ChannelAccount createPywechatAccount(Long id, int status) {
ChannelAccount account = createAccount(id, "wechat_personal", status);
account.setAccountType(3);
account.setAccountType(AccountType.WECHAT_PERSONAL.getCode());
account.setOnlineStatus(1);
account.setRiskLevel(1);
account.setLastHeartbeat(java.time.LocalDateTime.now());