From 298078b536b8ab6c672b083b8b4ce64753a487da Mon Sep 17 00:00:00 2001 From: Fiber <48828021+leafliber@users.noreply.github.com> Date: Sat, 27 Jun 2026 16:06:14 +0800 Subject: [PATCH] Fix: KV storage not cleared on plugin uninstall. Improved cleanup logic and updated i18n strings to indicate database KV data removal. (#8291) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: 完善插件卸载时的清理逻辑，新增KV数据清理，更新了多语言文案以说明会清理数据库KV数据 * fix: 修复插件关闭时不清理KV的问题，更新单元测试 * refactor: 统一插件ID生成逻辑 将插件ID生成逻辑抽离到StarMetadata类中，移除重复的代码实现， 同时在__post_init__中自动补全plugin_id字段。 * refactor: 将plugin_id属性从方法转换为属性，确保在属性赋值后正确计算 --- astrbot/core/star/star.py | 6 + astrbot/core/star/star_manager.py | 28 ++- .../locales/en-US/features/extension.json | 2 +- .../locales/ru-RU/features/extension.json | 2 +- .../locales/zh-CN/features/extension.json | 2 +- tests/test_plugin_manager.py | 231 ++++++++++++++++++ tests/unit/test_star_base.py | 54 ++++ 7 files changed, 315 insertions(+), 10 deletions(-) diff --git a/astrbot/core/star/star.py b/astrbot/core/star/star.py index d4628e254..255c9e2b2 100644 --- a/astrbot/core/star/star.py +++ b/astrbot/core/star/star.py @@ -75,6 +75,12 @@ class StarMetadata: pages: list[dict] = field(default_factory=list) """插件注册的 Pages 元数据。""" + @property + def plugin_id(self) -> str: + p_name = (self.name or "unknown").lower().replace("/", "_") + p_author = (self.author or "unknown").lower().replace("/", "_") + return f"{p_author}/{p_name}" + def __str__(self) -> str: return f"Plugin {self.name} ({self.version}) by {self.author}: {self.desc}" diff --git a/astrbot/core/star/star_manager.py b/astrbot/core/star/star_manager.py index 4ebf9a982..0b5fec046 100644 --- a/astrbot/core/star/star_manager.py +++ b/astrbot/core/star/star_manager.py @@ -777,6 +777,7 @@ class PluginManager: "display_name": metadata.display_name, "support_platforms": metadata.support_platforms, "astrbot_version": metadata.astrbot_version, + "plugin_id": metadata.plugin_id, } ) except Exception as metadata_error: @@ -1036,12 +1037,11 @@ class PluginManager: logger.info(metadata) metadata.config = plugin_config - p_name = (metadata.name or "unknown").lower().replace("/", "_") - p_author = (metadata.author or "unknown").lower().replace("/", "_") - plugin_id = f"{p_author}/{p_name}" + plugin_id = metadata.plugin_id - # 在实例化前注入类属性，保证插件 __init__ 可读取这些值 + # inject class attributes before instantiation so __init__ can read them if metadata.star_cls_type: + p_author, p_name = plugin_id.split("/") setattr(metadata.star_cls_type, "name", p_name) setattr(metadata.star_cls_type, "author", p_author) setattr(metadata.star_cls_type, "plugin_id", plugin_id) @@ -1316,11 +1316,12 @@ class PluginManager: f"清理安装失败插件配置失败: {plugin_config_path}，原因: {e!s}", ) - def _cleanup_plugin_optional_artifacts( + async def _cleanup_plugin_optional_artifacts( self, *, root_dir_name: str, plugin_label: str, + plugin_id: str | None = None, delete_config: bool, delete_data: bool, ) -> None: @@ -1355,6 +1356,13 @@ class PluginManager: f"删除插件持久化数据失败 ({data_dir_name}, {plugin_label}): {e!s}", ) + if plugin_id: + try: + await self.context.get_db().clear_preferences("plugin", plugin_id) + logger.info(f"已清除插件 {plugin_label}({plugin_id}) 的 KV 数据") + except Exception as e: + logger.warning(f"清除插件 KV 数据失败 ({plugin_label}): {e!s}") + def _track_failed_install_dir( self, *, @@ -1557,9 +1565,12 @@ class PluginManager: f"移除插件成功，但是删除插件文件夹失败: {e!s}。您可以手动删除该文件夹，位于 addons/plugins/ 下。", ) - self._cleanup_plugin_optional_artifacts( + plugin_id = plugin.plugin_id + + await self._cleanup_plugin_optional_artifacts( root_dir_name=root_dir_name, plugin_label=plugin_name, + plugin_id=plugin_id, delete_config=delete_config, delete_data=delete_data, ) @@ -1603,16 +1614,19 @@ class PluginManager: ) plugin_label = dir_name + plugin_id = None if isinstance(failed_info, dict): plugin_label = ( failed_info.get("display_name") or failed_info.get("name") or dir_name ) + plugin_id = failed_info.get("plugin_id") - self._cleanup_plugin_optional_artifacts( + await self._cleanup_plugin_optional_artifacts( root_dir_name=dir_name, plugin_label=plugin_label, + plugin_id=plugin_id, delete_config=delete_config, delete_data=delete_data, ) diff --git a/dashboard/src/i18n/locales/en-US/features/extension.json b/dashboard/src/i18n/locales/en-US/features/extension.json index a23e7edf1..59bf8005e 100644 --- a/dashboard/src/i18n/locales/en-US/features/extension.json +++ b/dashboard/src/i18n/locales/en-US/features/extension.json @@ -208,7 +208,7 @@ "deleteConfig": "Also delete plugin configuration file", "deleteData": "Also delete plugin persistent data", "configHint": "Configuration file located in data/config directory", - "dataHint": "Deletes data in data/plugin_data and data/plugins_data" + "dataHint": "Deletes data in data/plugin_data and data/plugins_data, as well as KV preference data in the database" }, "install": { "title": "Install Extension", diff --git a/dashboard/src/i18n/locales/ru-RU/features/extension.json b/dashboard/src/i18n/locales/ru-RU/features/extension.json index 76395097f..2163972be 100644 --- a/dashboard/src/i18n/locales/ru-RU/features/extension.json +++ b/dashboard/src/i18n/locales/ru-RU/features/extension.json @@ -207,7 +207,7 @@ "deleteConfig": "Удалить файл конфигурации плагина", "deleteData": "Удалить сохраненные данные плагина", "configHint": "Конфиг находится в data/config", - "dataHint": "Данные находятся в data/plugin_data и data/plugins_data" + "dataHint": "Данные находятся в data/plugin_data и data/plugins_data, а также KV-данные в базе данных" }, "install": { "title": "Установка плагина", diff --git a/dashboard/src/i18n/locales/zh-CN/features/extension.json b/dashboard/src/i18n/locales/zh-CN/features/extension.json index 60c696e2d..46dc454b3 100644 --- a/dashboard/src/i18n/locales/zh-CN/features/extension.json +++ b/dashboard/src/i18n/locales/zh-CN/features/extension.json @@ -208,7 +208,7 @@ "deleteConfig": "同时删除插件配置文件", "deleteData": "同时删除插件持久化数据", "configHint": "配置文件位于 data/config 目录", - "dataHint": "删除 data/plugin_data 和 data/plugins_data 目录下的数据" + "dataHint": "删除 data/plugin_data、plugins_data 目录下的数据，以及数据库中插件的 KV 数据" }, "install": { "title": "安装插件", diff --git a/tests/test_plugin_manager.py b/tests/test_plugin_manager.py index 0f29b961c..723998f28 100644 --- a/tests/test_plugin_manager.py +++ b/tests/test_plugin_manager.py @@ -1439,3 +1439,234 @@ async def test_ensure_plugin_requirements_does_not_mask_install_error_when_clean ) assert any("删除临时插件依赖文件失败" in log for log in warning_logs) + + +# --- Tests for plugin_id KV cleanup logic --- + + +@pytest.mark.asyncio +async def test_cleanup_plugin_optional_artifacts_clears_kv_when_plugin_id_present( + plugin_manager_pm: PluginManager, monkeypatch +): + cleared = [] + + class MockDB: + async def clear_preferences(self, scope, scope_id): + cleared.append((scope, scope_id)) + + monkeypatch.setattr( + plugin_manager_pm.context, "get_db", MockDB, raising=False + ) + + await plugin_manager_pm._cleanup_plugin_optional_artifacts( + root_dir_name="test_plugin", + plugin_label="TestPlugin", + plugin_id="test_author/test_plugin", + delete_config=False, + delete_data=True, + ) + + assert cleared == [("plugin", "test_author/test_plugin")] + + +@pytest.mark.asyncio +async def test_cleanup_plugin_optional_artifacts_skips_kv_when_plugin_id_none( + plugin_manager_pm: PluginManager, monkeypatch +): + cleared = [] + + class MockDB: + async def clear_preferences(self, scope, scope_id): + cleared.append((scope, scope_id)) + + monkeypatch.setattr( + plugin_manager_pm.context, "get_db", MockDB, raising=False + ) + + await plugin_manager_pm._cleanup_plugin_optional_artifacts( + root_dir_name="test_plugin", + plugin_label="TestPlugin", + plugin_id=None, + delete_config=False, + delete_data=True, + ) + + assert cleared == [] + + +@pytest.mark.asyncio +async def test_uninstall_plugin_reads_plugin_id_from_metadata( + plugin_manager_pm: PluginManager, monkeypatch +): + cleanup_calls = [] + + mock_star = MockStar() + mock_star.root_dir_name = TEST_PLUGIN_DIR + mock_star.name = TEST_PLUGIN_NAME + mock_star.module_path = "data.plugins.helloworld.main" + mock_star.reserved = False + mock_star.star_cls = None + mock_star.plugin_id = "mock_author/mock_name" + + cast(Any, plugin_manager_pm.context).stars.append(mock_star) + + monkeypatch.setattr( + plugin_manager_pm, "_terminate_plugin", lambda p: asyncio.sleep(0) + ) + monkeypatch.setattr( + plugin_manager_pm, "_unbind_plugin", lambda n, m: asyncio.sleep(0) + ) + monkeypatch.setattr( + "astrbot.core.star.star_manager.remove_dir", + lambda p: None, + ) + + async def mock_cleanup( + *, root_dir_name, plugin_label, plugin_id, delete_config, delete_data + ): + cleanup_calls.append( + { + "root_dir_name": root_dir_name, + "plugin_label": plugin_label, + "plugin_id": plugin_id, + } + ) + + monkeypatch.setattr( + plugin_manager_pm, "_cleanup_plugin_optional_artifacts", mock_cleanup + ) + + await plugin_manager_pm.uninstall_plugin( + TEST_PLUGIN_NAME, delete_config=False, delete_data=True + ) + + assert len(cleanup_calls) == 1 + assert cleanup_calls[0]["plugin_id"] == "mock_author/mock_name" + + +@pytest.mark.asyncio +async def test_uninstall_plugin_handles_disabled_plugin_with_plugin_id( + plugin_manager_pm: PluginManager, monkeypatch +): + cleanup_calls = [] + + mock_star = MockStar() + mock_star.root_dir_name = TEST_PLUGIN_DIR + mock_star.name = TEST_PLUGIN_NAME + mock_star.module_path = "data.plugins.helloworld.main" + mock_star.star_cls = None + mock_star.plugin_id = "mock_author/mock_name" + + cast(Any, plugin_manager_pm.context).stars.append(mock_star) + + monkeypatch.setattr( + plugin_manager_pm, "_terminate_plugin", lambda p: asyncio.sleep(0) + ) + monkeypatch.setattr( + plugin_manager_pm, "_unbind_plugin", lambda n, m: asyncio.sleep(0) + ) + monkeypatch.setattr( + "astrbot.core.star.star_manager.remove_dir", + lambda p: None, + ) + + async def mock_cleanup( + *, root_dir_name, plugin_label, plugin_id, delete_config, delete_data + ): + cleanup_calls.append( + { + "root_dir_name": root_dir_name, + "plugin_label": plugin_label, + "plugin_id": plugin_id, + } + ) + + monkeypatch.setattr( + plugin_manager_pm, "_cleanup_plugin_optional_artifacts", mock_cleanup + ) + + await plugin_manager_pm.uninstall_plugin( + TEST_PLUGIN_NAME, delete_config=False, delete_data=True + ) + + assert len(cleanup_calls) == 1 + assert cleanup_calls[0]["plugin_id"] == "mock_author/mock_name" + + +@pytest.mark.asyncio +async def test_uninstall_failed_plugin_passes_plugin_id_from_record( + plugin_manager_pm: PluginManager, monkeypatch +): + cleanup_calls = [] + + plugin_manager_pm.failed_plugin_dict[TEST_PLUGIN_DIR] = { + "name": TEST_PLUGIN_NAME, + "display_name": "Hello World", + "plugin_id": "astrbot_team/helloworld", + } + + monkeypatch.setattr( + "astrbot.core.star.star_manager.remove_dir", + lambda p: None, + ) + + async def mock_cleanup( + *, root_dir_name, plugin_label, plugin_id, delete_config, delete_data + ): + cleanup_calls.append( + { + "root_dir_name": root_dir_name, + "plugin_label": plugin_label, + "plugin_id": plugin_id, + } + ) + + monkeypatch.setattr( + plugin_manager_pm, "_cleanup_plugin_optional_artifacts", mock_cleanup + ) + + await plugin_manager_pm.uninstall_failed_plugin( + TEST_PLUGIN_DIR, delete_config=False, delete_data=True + ) + + assert len(cleanup_calls) == 1 + assert cleanup_calls[0]["plugin_id"] == "astrbot_team/helloworld" + + +@pytest.mark.asyncio +async def test_uninstall_failed_plugin_without_plugin_id_in_record( + plugin_manager_pm: PluginManager, monkeypatch +): + cleanup_calls = [] + + plugin_manager_pm.failed_plugin_dict[TEST_PLUGIN_DIR] = { + "name": TEST_PLUGIN_NAME, + "display_name": "Hello World", + } + + monkeypatch.setattr( + "astrbot.core.star.star_manager.remove_dir", + lambda p: None, + ) + + async def mock_cleanup( + *, root_dir_name, plugin_label, plugin_id, delete_config, delete_data + ): + cleanup_calls.append( + { + "root_dir_name": root_dir_name, + "plugin_label": plugin_label, + "plugin_id": plugin_id, + } + ) + + monkeypatch.setattr( + plugin_manager_pm, "_cleanup_plugin_optional_artifacts", mock_cleanup + ) + + await plugin_manager_pm.uninstall_failed_plugin( + TEST_PLUGIN_DIR, delete_config=False, delete_data=True + ) + + assert len(cleanup_calls) == 1 + assert cleanup_calls[0]["plugin_id"] is None diff --git a/tests/unit/test_star_base.py b/tests/unit/test_star_base.py index 77e35ba2c..0a6f924c1 100644 --- a/tests/unit/test_star_base.py +++ b/tests/unit/test_star_base.py @@ -162,6 +162,60 @@ class TestStarBase: assert len(star_registry) >= initial_count +class TestStarMetadataPluginId: + """Tests for StarMetadata.plugin_id derived view. + + Regression: previously `plugin_id` was set in `__post_init__`, which only + fires at dataclass construction. The plugin load flow constructs + StarMetadata empty (no name/author) and fills them via attribute + assignment later, so `plugin_id` stayed None and crashed the downstream + `plugin_id.split("/")`. Now it's a property recomputed on every access. + """ + + def test_plugin_id_defaults_to_unknown_when_empty(self): + from astrbot.core.star.star import StarMetadata + + assert StarMetadata().plugin_id == "unknown/unknown" + + def test_plugin_id_uses_name_and_author(self): + from astrbot.core.star.star import StarMetadata + + metadata = StarMetadata(name="Hello", author="AstrBot") + assert metadata.plugin_id == "astrbot/hello" + + def test_plugin_id_recomputes_after_attribute_assignment(self): + from astrbot.core.star.star import StarMetadata + + metadata = StarMetadata() + metadata.name = "A" + metadata.author = "B" + assert metadata.plugin_id == "b/a" + + def test_plugin_id_lowercases_and_escapes_slash(self): + from astrbot.core.star.star import StarMetadata + + metadata = StarMetadata(name="A/B", author="C") + assert metadata.plugin_id == "c/a_b" + + def test_plugin_id_reflects_latest_name_after_change(self): + from astrbot.core.star.star import StarMetadata + + metadata = StarMetadata(name="old", author="author") + assert metadata.plugin_id == "author/old" + metadata.name = "new" + assert metadata.plugin_id == "author/new" + + def test_plugin_id_only_name_set(self): + from astrbot.core.star.star import StarMetadata + + assert StarMetadata(name="OnlyName").plugin_id == "unknown/onlyname" + + def test_plugin_id_only_author_set(self): + from astrbot.core.star.star import StarMetadata + + assert StarMetadata(author="OnlyAuthor").plugin_id == "onlyauthor/unknown" + + class TestNoCircularImports: """Test that there are no circular import issues."""