Skip to content

Refactor CacheManager to use generic type T #87

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JohnRichard4096 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Refactor CacheManager to use generic type T #87

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 5 additions & 5 deletions nonebot_plugin_value/_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from collections.abc import Hashable
from dataclasses import dataclass, field
from functools import lru_cache
from typing import Any, Generic, TypeVar
from typing import Generic, TypeVar

from typing_extensions import Self

Expand All @@ -33,7 +33,7 @@
max_size: int = 1000

# LRU实现
_cache: OrderedDict[str, BaseData] = field(default_factory=lambda: OrderedDict())
_cache: OrderedDict[str, T] = field(default_factory=lambda: OrderedDict())

def __post_init__(self):
if self.max_size <= 0:
Expand All @@ -48,7 +48,7 @@
return True

# 添加新数据
self._cache[data_id] = data

Check failure on line 51 in nonebot_plugin_value/_cache.py

View workflow job for this annotation

GitHub Actions / build 

Argument of type "UserAccountData | BaseData" cannot be assigned to parameter "value" of type "T@Cache" in function "__setitem__"
  Type "UserAccountData | BaseData" is not assignable to type "T@Cache" (reportArgumentType)
self._cache.move_to_end(data_id)

# 如果超出最大大小，删除最久未使用的项（第一个）
Expand Down Expand Up @@ -84,9 +84,9 @@
return Lock()


class CacheManager:
class CacheManager(Generic[T]):
_instance = None
_cached: dict[CacheCategoryEnum, Cache[Any]]
_cached: dict[CacheCategoryEnum, Cache[T]]
Comment on lines +87 to +89
Copy link
Contributor

@sourcery-ai sourcery-ai bot Dec 31, 2025

Choose a reason for hiding this comment

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

issue (bug_risk)： 在单例管理器上使用 Generic[T]，在用不同类型参数实例化时会导致令人困惑或不健全的类型行为。

由于 _instance 是共享的，CacheManager[int]() 和 CacheManager[str]() 实际上引用的是同一个对象，但在类型检查器看来却是不同的泛型类型。这会使 _cached 和 get_cache 变得类型不安全。如果你只需要一个全局管理器，请从 CacheManager 中移除 Generic[T]，只让 Cache 保持泛型。如果你确实需要按 T 区分的管理器，请让 _instance 与类型相关（例如针对每个 T 拥有一个单独的单例实例）。

Original comment in English

issue (bug_risk): Using Generic[T] on a singleton manager can lead to confusing or unsound typing when instantiated with different type parameters.

Since _instance is shared, CacheManager[int]() and CacheManager[str]() refer to the same object but appear as different generic types to the type checker. This can make _cached and get_cache type-unsafe. If you only need one global manager, drop Generic[T] from CacheManager and keep only Cache generic. If you truly need per-T managers, make _instance type-specific (e.g., one singleton instance per T).


def __new__(cls) -> Self:
if cls._instance is None:
Expand All @@ -96,7 +96,7 @@

async def get_cache(
self, category: CacheCategoryEnum, max_size: int = 1000
) -> Cache[Any]:
) -> Cache[T]:
Comment on lines 97 to +99
Copy link
Contributor

@sourcery-ai sourcery-ai bot Dec 31, 2025

Choose a reason for hiding this comment

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

suggestion： get_cache 的泛型返回类型可能无法体现不同分类之间缓存内容的异质性。

在这里使用 Cache[T] 意味着所有 CacheCategoryEnum 的值都使用相同的 T。如果不同分类存放的值类型不同，在重用 CacheManager 时，这就会变得具有误导性并且类型不安全。根据实际使用情况，可以考虑对 CacheManager 使用非泛型的 Cache[Any]，或者引入一个显式的“分类→类型”映射（例如通过 Protocol 或辅助工具），从而让不同缓存类型在类型检查器中是可区分的。

建议实现：

class CacheManager:
    _instance = None
    _cached: dict[CacheCategoryEnum, Cache[Any]]
    async def get_cache(
        self, category: CacheCategoryEnum, max_size: int = 1000
    ) -> Cache[Any]:
  1. 从该模块中移除未使用的泛型类型参数 T：
    • 如果 T 只用于 CacheManager，则删除相应的 TypeVar 声明。
    • 如果没有其他类使用 Generic，则从导入中删除 Generic。
  2. 如果目前有调用方使用了 CacheManager[T] 的注解或期望一个泛型 CacheManager，请更新它们以不带类型参数地引用 CacheManager。
  3. 确保已经从 typing 中导入了 Any（from typing import Any），如果尚未导入的话。
Original comment in English

suggestion: The generic return type of get_cache might not reflect per-category heterogeneity of cache contents.

Using Cache[T] here implies all CacheCategoryEnum values use the same T. If categories hold different value types, this becomes misleading and type-unsafe as CacheManager is reused. Depending on actual usage, consider either using a non-generic Cache[Any] for CacheManager, or introducing an explicit category→type mapping (e.g. via a Protocol or helper) so different cache types are distinguishable to the type checker.

Suggested implementation:

class CacheManager:
    _instance = None
    _cached: dict[CacheCategoryEnum, Cache[Any]]
    async def get_cache(
        self, category: CacheCategoryEnum, max_size: int = 1000
    ) -> Cache[Any]:
  1. Remove the unused generic type parameter T from this module:
    • Drop T from any TypeVar declaration if it is only used for CacheManager.
    • Remove Generic from the imports if no other class uses it.
  2. If any callers currently annotate CacheManager[T] or expect a generic CacheManager, update them to reference CacheManager without type parameters.
  3. Ensure Any is imported from typing if it is not already (from typing import Any).

# 为不同类别创建具有不同大小的缓存
if category not in self._cached:
self._cached[category] = Cache(max_size=max_size)
Expand Down
Loading