Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions astrbot/core/platform/sources/aiocqhttp/aiocqhttp_message_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,30 @@ async def send_message(
payload["user_id"] = session_id
await bot.call_action("send_private_forward_msg", **payload)
elif isinstance(seg, File):
# 优先使用 NapCat 的 upload API 上传文件(上传成功后会自动发送)
file_path = seg.file
if file_path and session_id and session_id.isdigit():
sid = int(session_id)
try:
if is_group:
await bot.call_action(
"upload_group_file",
group_id=sid,
file=file_path,
name=seg.name or "file",
)
else:
await bot.call_action(
"upload_private_file",
user_id=sid,
file=file_path,
name=seg.name or "file",
)
continue # 上传成功,upload API 自带发送功能
except Exception as e:
from astrbot.api import logger

logger.error(f"上传文件失败,回退到直接发送: {e}")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

这里有几点可以优化:

  1. 路径存在性与绝对路径转换

    • 建议在调用 upload_group_file / upload_private_file 之前,先使用 os.path.exists(file_path) 检查文件是否在本地存在。这样可以避免对网络 URL 或不存在的文件路径进行无意义的上传 API 调用。
    • 建议使用 os.path.abspath(file_path) 将文件路径转换为绝对路径。因为 OneBot 客户端(如 NapCat)可能运行在不同的工作目录下,使用相对路径可能会导致其无法正确找到并上传文件。
  2. 日志级别调整

    • 当前在上传失败回退到直接发送时,使用了 logger.error。由于对于不支持这些自定义上传 API 的 OneBot 客户端,每次发送文件都会触发此回退逻辑,使用 logger.error 会导致日志中充斥大量的“错误”信息。建议将其降级为 logger.warning
  3. 重构与测试(根据项目规范)

    • 避免代码重复:在实现类似功能(如直接发送与引用发送附件)时,建议将逻辑重构为共享的辅助函数(helper function),以避免代码重复。
    • 单元测试:新增的附件处理功能应当伴随相应的单元测试,以确保其稳定性和正确性。
file_path = seg.file
import os
if file_path and os.path.exists(file_path) and session_id and session_id.isdigit():
    file_path = os.path.abspath(file_path)
    sid = int(session_id)
    try:
        if is_group:
            await bot.call_action(
                "upload_group_file",
                group_id=sid,
                file=file_path,
                name=seg.name or "file",
            )
        else:
            await bot.call_action(
                "upload_private_file",
                user_id=sid,
                file=file_path,
                name=seg.name or "file",
            )
        continue
    except Exception as e:
        from astrbot.api import logger
        logger.warning(f"上传文件失败,回退到直接发送: {e}")
References
  1. When implementing similar functionality for different cases (e.g., direct vs. quoted attachments), refactor the logic into a shared helper function to avoid code duplication.
  2. New functionality, such as handling attachments, should be accompanied by corresponding unit tests.

Comment thread
sourcery-ai[bot] marked this conversation as resolved.
Outdated
d = await cls._from_segment_to_dict(seg)
await cls._dispatch_send(bot, event, is_group, session_id, [d])
else:
Expand Down
219 changes: 219 additions & 0 deletions tests/unit/test_aiocqhttp_file_message.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
"""测试 aiocqhttp 平台发送文件消息时的行为。

验证 File 段通过 upload_group_file / upload_private_file API 上传,
以及各种回退场景。
"""

import pytest
from unittest.mock import AsyncMock, MagicMock

# 先导入 astrbot.api 完成模块初始化,避免 star_tools → aiocqhttp_message_event 的循环导入
import astrbot.api # noqa: F401

from astrbot.core.message import components as Comp
from astrbot.core.message.message_event_result import MessageChain
from astrbot.core.platform.sources.aiocqhttp.aiocqhttp_message_event import (
AiocqhttpMessageEvent,
)


@pytest.mark.asyncio
async def test_file_group_uses_upload_group_file_api(tmp_path):
"""群聊发送本地文件:应调用 upload_group_file API,不走 send_group_msg。"""
testFile = tmp_path / "test.txt"
testFile.write_text("hello world", encoding="utf-8")

fileComp = Comp.File(name="test.txt", file=str(testFile))
bot = MagicMock()
bot.send_group_msg = AsyncMock()
bot.send_private_msg = AsyncMock()
Comment on lines +20 to +29
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add a test for non-digit session_id to cover the session_id.isdigit() guard.

Currently we only cover session_id=None. Please also add a case where session_id is a non-empty, non-numeric string (e.g. "abc123") to confirm the upload path is skipped and _dispatch_send is used instead. For example, a test_non_digit_session_id_falls_back_to_dispatch_send (for both group and private, if applicable) that:

  • passes a File segment with session_id="abc123";
  • asserts bot.call_action is not called;
  • asserts the appropriate fallback send path (send_group_msg / send_private_msg) or the expected exception from _dispatch_send.

Suggested implementation:

    bot.call_action.assert_not_called()
    # 回退到 send_group_msg
    bot.send_group_msg.assert_called_once()


@pytest.mark.asyncio
async def test_non_digit_session_id_group_falls_back_to_dispatch_send(tmp_path):
    """群聊:session_id 为非数字字符串时应回退到 _dispatch_send,不调用 upload API。"""
    testFile = tmp_path / "test.txt"
    testFile.write_text("hello world", encoding="utf-8")

    fileComp = Comp.File(name="test.txt", file=str(testFile))
    bot = MagicMock()
    bot.send_group_msg = AsyncMock()
    bot.send_private_msg = AsyncMock()
    bot.call_action = AsyncMock()

    await AiocqhttpMessageEvent.send_message(
        bot=bot,
        message_chain=MessageChain([fileComp]),
        is_group=True,
        session_id="abc123",
    )

    # 非数字 session_id,上传 API 应被跳过
    bot.call_action.assert_not_called()
    # 回退到 send_group_msg
    bot.send_group_msg.assert_called_once()
    bot.send_private_msg.assert_not_called()


@pytest.mark.asyncio
async def test_non_digit_session_id_private_falls_back_to_dispatch_send(tmp_path):
    """私聊:session_id 为非数字字符串时应回退到 _dispatch_send,不调用 upload API。"""
    testFile = tmp_path / "test.txt"
    testFile.write_text("hello world", encoding="utf-8")

    fileComp = Comp.File(name="test.txt", file=str(testFile))
    bot = MagicMock()
    bot.send_group_msg = AsyncMock()
    bot.send_private_msg = AsyncMock()
    bot.call_action = AsyncMock()

    await AiocqhttpMessageEvent.send_message(
        bot=bot,
        message_chain=MessageChain([fileComp]),
        is_group=False,
        session_id="abc123",
    )

    # 非数字 session_id,上传 API 应被跳过
    bot.call_action.assert_not_called()
    # 回退到 send_private_msg
    bot.send_private_msg.assert_called_once()
    bot.send_group_msg.assert_not_called()

These edits assume that:

  1. The snippet with bot.call_action.assert_not_called() and bot.send_group_msg.assert_called_once() belongs to test_no_local_file_falls_back_to_dispatch_send, and that pytest, MagicMock, AsyncMock, Comp, MessageChain, and AiocqhttpMessageEvent are already imported (as shown in the partial file).
  2. No additional fixtures or helpers are required for these tests; if your existing tests use shared fixtures for bot construction or message setup, you may want to refactor these new tests to reuse those fixtures instead of instantiating MagicMock/AsyncMock directly.

bot.call_action = AsyncMock()

await AiocqhttpMessageEvent.send_message(
bot=bot,
message_chain=MessageChain([fileComp]),
is_group=True,
session_id="123456",
)

bot.call_action.assert_called_once_with(
"upload_group_file",
group_id=123456,
file=str(testFile.resolve()),
name="test.txt",
)
bot.send_group_msg.assert_not_called()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Consider adding a private-chat variant for multiple-file uploads.

You already verify that each file in a group chat triggers a separate upload_group_file call. To fully exercise the new behavior, please add an analogous private-chat test (e.g. test_multiple_files_each_uses_upload_private_api) that checks:

  • bot.call_action is invoked once per file with "upload_private_file" and the expected user_id/file/name.
  • send_private_msg is never called.

This keeps group and private flows symmetric and protects against regressions in one path only.

Suggested implementation:

@pytest.mark.asyncio
async def test_multiple_files_each_uses_upload_api(tmp_path):
    """多个 File 段:每个都独立调用 group upload API。"""
    file1 = tmp_path / "a.txt"
    file2 = tmp_path / "b.txt"
    file1.write_text("aaa", encoding="utf-8")
    file2.write_text("bbb", encoding="utf-8")

    comp1 = Comp.File(name="a.txt", file=str(file1))
    comp2 = Comp.File(name="b.txt", file=str(file2))

    bot = MagicMock()

    await AiocqhttpMessageEvent.send_message(
        bot=bot,
        message_chain=MessageChain([comp1, comp2]),
        is_group=True,
        session_id="123456",
    )

    assert bot.call_action.call_count == 2
    bot.call_action.assert_any_call(
        "upload_group_file",
        group_id="123456",
        file=str(file1),
        name="a.txt",
    )
    bot.call_action.assert_any_call(
        "upload_group_file",
        group_id="123456",
        file=str(file2),
        name="b.txt",
    )

    # 发送文件时不应直接调用发送消息接口
    assert not bot.send_group_msg.called


@pytest.mark.asyncio
async def test_multiple_files_each_uses_upload_private_api(tmp_path):
    """多个 File 段:每个都独立调用 private upload API。"""
    file1 = tmp_path / "a.txt"
    file2 = tmp_path / "b.txt"
    file1.write_text("aaa", encoding="utf-8")
    file2.write_text("bbb", encoding="utf-8")

    comp1 = Comp.File(name="a.txt", file=str(file1))
    comp2 = Comp.File(name="b.txt", file=str(file2))

    bot = MagicMock()

    await AiocqhttpMessageEvent.send_message(
        bot=bot,
        message_chain=MessageChain([comp1, comp2]),
        is_group=False,
        user_id=123456,
        session_id="123456",
    )

    assert bot.call_action.call_count == 2
    bot.call_action.assert_any_call(
        "upload_private_file",
        user_id=123456,
        file=str(file1),
        name="a.txt",
    )
    bot.call_action.assert_any_call(
        "upload_private_file",
        user_id=123456,
        file=str(file2),
        name="b.txt",
    )

    # 发送文件时不应直接调用发送私聊消息接口
    assert not bot.send_private_msg.called
  1. The exact arguments to AiocqhttpMessageEvent.send_message (e.g. group_id, user_id, session_id) and the upload_group_file/upload_private_file call signatures should be aligned with the rest of this test file. You may need to adjust group_id="123456"/user_id=123456/session_id="123456" to match existing conventions.
  2. If ANY from unittest.mock is preferred in your tests (to avoid hard-coding IDs), you can replace 123456 with ANY and ensure from unittest.mock import ANY is present at the top of the file.
  3. If the existing test_multiple_files_each_uses_upload_api already has assertions beyond the snippet you provided, merge the new body carefully to avoid duplicating or conflicting assertions.

bot.send_private_msg.assert_not_called()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add a test for defaulting name to 'file' when seg.name is missing.

Right now all tests pass a File with an explicit name. Since the implementation uses seg.name or "file", please add a test (e.g. test_file_without_name_defaults_to_file) that:

  • creates Comp.File(file=str(testFile)) without a name;
  • calls send_message for group/private;
  • asserts bot.call_action is invoked with name="file".

This ensures the default-name behavior is covered and guarded against regressions.

Suggested implementation:

    bot.send_group_msg.assert_not_called()
    bot.send_private_msg.assert_not_called()


@pytest.mark.asyncio
async def test_file_group_without_name_defaults_to_file(tmp_path):
    """群聊发送本地文件且未提供 name:应使用默认 name='file' 并调用 upload_group_file。"""
    testFile = tmp_path / "test.txt"
    testFile.write_text("hello world", encoding="utf-8")

    fileComp = Comp.File(file=str(testFile))
    bot = MagicMock()
    bot.send_group_msg = AsyncMock()
    bot.send_private_msg = AsyncMock()
    bot.call_action = AsyncMock()

    await AiocqhttpMessageEvent.send_message(
        bot=bot,
        message_chain=MessageChain([fileComp]),
        is_group=True,
        session_id="123456",
    )

    bot.call_action.assert_called_once_with(
        "upload_group_file",
        group_id=123456,
        file=str(testFile.resolve()),
        name="file",
    )
    bot.send_group_msg.assert_not_called()
    bot.send_private_msg.assert_not_called()


@pytest.mark.asyncio
async def test_file_private_without_name_defaults_to_file(tmp_path):
    """私聊发送本地文件且未提供 name:应使用默认 name='file' 并调用 upload_private_file。"""
    testFile = tmp_path / "doc.pdf"
    testFile.write_text("pdf content", encoding="utf-8")

    fileComp = Comp.File(file=str(testFile))
    bot = MagicMock()
    bot.send_group_msg = AsyncMock()
    bot.send_private_msg = AsyncMock()
    bot.call_action = AsyncMock()

    await AiocqhttpMessageEvent.send_message(
        bot=bot,
        message_chain=MessageChain([fileComp]),
        is_group=False,
        session_id="789012",
    )

    bot.call_action.assert_called_once_with(
        "upload_private_file",
        user_id=789012,
        file=str(testFile.resolve()),
        name="file",
    )
    bot.send_private_msg.assert_not_called()


@pytest.mark.asyncio

These edits assume that Comp, MessageChain, and AiocqhttpMessageEvent are already imported earlier in test_aiocqhttp_file_message.py, consistent with the existing tests. If they are not, add the appropriate imports (mirroring how the existing file-upload tests reference them).



@pytest.mark.asyncio
async def test_file_private_uses_upload_private_file_api(tmp_path):
"""私聊发送本地文件:应调用 upload_private_file API,不走 send_private_msg。"""
testFile = tmp_path / "doc.pdf"
testFile.write_text("pdf content", encoding="utf-8")

fileComp = Comp.File(name="doc.pdf", file=str(testFile))
bot = MagicMock()
bot.send_group_msg = AsyncMock()
bot.send_private_msg = AsyncMock()
bot.call_action = AsyncMock()

await AiocqhttpMessageEvent.send_message(
bot=bot,
message_chain=MessageChain([fileComp]),
is_group=False,
session_id="789012",
)

bot.call_action.assert_called_once_with(
"upload_private_file",
user_id=789012,
file=str(testFile.resolve()),
name="doc.pdf",
)
bot.send_private_msg.assert_not_called()


@pytest.mark.asyncio
async def test_file_and_plain_mixed_file_uses_upload_api(tmp_path):
"""File 与 Plain 混合时:Plain 走 send_group_msg,File 走 upload API。"""
testFile = tmp_path / "data.txt"
testFile.write_text("sample", encoding="utf-8")

fileComp = Comp.File(name="data.txt", file=str(testFile))
plainComp = Comp.Plain(text="请查收文件")

bot = MagicMock()
bot.send_group_msg = AsyncMock()
bot.send_private_msg = AsyncMock()
bot.call_action = AsyncMock()

await AiocqhttpMessageEvent.send_message(
bot=bot,
message_chain=MessageChain([plainComp, fileComp]),
is_group=True,
session_id="123456",
)

# Plain 走 send_group_msg
assert bot.send_group_msg.call_count == 1
plainMessage = bot.send_group_msg.call_args.kwargs["message"]
assert plainMessage[0]["type"] == "text"
assert plainMessage[0]["data"]["text"] == "请查收文件"

# File 走 upload API
bot.call_action.assert_called_once_with(
"upload_group_file",
group_id=123456,
file=str(testFile.resolve()),
name="data.txt",
)


@pytest.mark.asyncio
async def test_upload_failure_falls_back_to_dispatch_send(tmp_path):
"""upload API 失败时回退到 _from_segment_to_dict + _dispatch_send。"""
testFile = tmp_path / "test.txt"
testFile.write_text("fallback", encoding="utf-8")

fileComp = Comp.File(name="test.txt", file=str(testFile))
bot = MagicMock()
bot.send_group_msg = AsyncMock()
bot.send_private_msg = AsyncMock()
bot.call_action = AsyncMock(side_effect=RuntimeError("upload failed"))

await AiocqhttpMessageEvent.send_message(
bot=bot,
message_chain=MessageChain([fileComp]),
is_group=True,
session_id="123456",
)

# upload 尝试了但失败了
bot.call_action.assert_called_once()
# 回退到 send_group_msg
bot.send_group_msg.assert_called_once()
fallbackMessage = bot.send_group_msg.call_args.kwargs["message"]
assert fallbackMessage[0]["type"] == "file"
assert "file:///" in str(fallbackMessage[0]["data"]["file"])


@pytest.mark.asyncio
async def test_no_local_file_falls_back_to_dispatch_send():
"""无本地文件(纯 URL)时回退到 _from_segment_to_dict + _dispatch_send。"""
fileComp = Comp.File(name="remote.txt", url="https://example.com/remote.txt")
bot = MagicMock()
bot.send_group_msg = AsyncMock()
bot.send_private_msg = AsyncMock()
bot.call_action = AsyncMock()

await AiocqhttpMessageEvent.send_message(
bot=bot,
message_chain=MessageChain([fileComp]),
is_group=True,
session_id="123456",
)

# 无本地文件路径,不应调用 upload API
bot.call_action.assert_not_called()
# 回退到 send_group_msg
bot.send_group_msg.assert_called_once()


@pytest.mark.asyncio
async def test_no_session_id_falls_back_to_dispatch_send(tmp_path):
"""session_id 为 None 时回退到 _dispatch_send(会抛 ValueError)。"""
testFile = tmp_path / "test.txt"
testFile.write_text("hello", encoding="utf-8")

fileComp = Comp.File(name="test.txt", file=str(testFile))
bot = MagicMock()
bot.send = AsyncMock()
bot.send_group_msg = AsyncMock()
bot.send_private_msg = AsyncMock()

with pytest.raises(ValueError, match="无法发送消息"):
await AiocqhttpMessageEvent.send_message(
bot=bot,
message_chain=MessageChain([fileComp]),
is_group=True,
session_id=None,
)


@pytest.mark.asyncio
async def test_multiple_files_each_uses_upload_api(tmp_path):
"""多个 File 段:每个都独立调用 upload API。"""
file1 = tmp_path / "a.txt"
file2 = tmp_path / "b.txt"
file1.write_text("aaa", encoding="utf-8")
file2.write_text("bbb", encoding="utf-8")

comp1 = Comp.File(name="a.txt", file=str(file1))
comp2 = Comp.File(name="b.txt", file=str(file2))

bot = MagicMock()
bot.send_group_msg = AsyncMock()
bot.call_action = AsyncMock()

await AiocqhttpMessageEvent.send_message(
bot=bot,
message_chain=MessageChain([comp1, comp2]),
is_group=True,
session_id="123456",
)

assert bot.call_action.call_count == 2
bot.call_action.assert_any_call(
"upload_group_file",
group_id=123456,
file=str(file1.resolve()),
name="a.txt",
)
bot.call_action.assert_any_call(
"upload_group_file",
group_id=123456,
file=str(file2.resolve()),
name="b.txt",
)
bot.send_group_msg.assert_not_called()
Loading