Refactor Permissions class for type hints and methods#8
Conversation
评审指南此 PR 重构了 Permissions 类,以添加带有 Self 返回的全面类型提示,将字符串转储整合到单个 __dump 方法中(该方法还会构建一个 node_dict 缓存),改进内部权限搜索以生成类型化元组,并通过利用支持通配符的 node_dict 来简化权限检查。 重构后的 Permissions 类的类图classDiagram
class Permissions {
dict[str, str | dict | bool] permissions_data
dict[str, bool] node_dict
str __permissions_str
__init__(permissions_data: dict[str, str | dict | bool] | None)
__str__()
__search_perm(data: dict[str, Any], parent_key: str = "", result: list[tuple[str, bool]] | None = None) list[tuple[str, bool]]
__dump(overwrite: bool = False)
del_permission(node: str) Self
set_permission(node: str, has_permission: bool, has_parent: bool = False) Self
check_permission(node: str) bool
dump_to_file(filename: str)
load_from_json(filename: str)
from_perm_str(perm_str: str)
dump_data() dict[str, Any]
data: dict[str, Any]
perm_str: str
permissions_str: str
}
使用 node_dict 和通配符进行权限检查逻辑的流程图flowchart TD
A["check_permission(node: str)"] --> B["node_dict.get(node)"]
B -- True --> G["return True"]
B -- False --> C["current_node = ''"]
C --> D["for part in node.split('.')"]
D --> E["node_dict.get(current_node + '.' + '*')"]
E -- True --> G
E -- False --> F["current_node += ('.' if current_node else '') + part"]
F --> H["node_dict.get(current_node)"]
H -- True --> G
H -- False --> D
D --> I["return False"]
文件级变更
提示和命令与 Sourcery 互动
自定义您的体验访问您的 控制面板 以:
获取帮助Original review guide in EnglishReviewer's GuideThis PR refactors the Permissions class to add comprehensive type hints with Self returns, consolidate string dumping into a single __dump method that also builds a node_dict cache, improve the internal permission search to yield typed tuples, and simplify permission checks by leveraging the node_dict with wildcard support. Class diagram for refactored Permissions classclassDiagram
class Permissions {
dict[str, str | dict | bool] permissions_data
dict[str, bool] node_dict
str __permissions_str
__init__(permissions_data: dict[str, str | dict | bool] | None)
__str__()
__search_perm(data: dict[str, Any], parent_key: str = "", result: list[tuple[str, bool]] | None = None) list[tuple[str, bool]]
__dump(overwrite: bool = False)
del_permission(node: str) Self
set_permission(node: str, has_permission: bool, has_parent: bool = False) Self
check_permission(node: str) bool
dump_to_file(filename: str)
load_from_json(filename: str)
from_perm_str(perm_str: str)
dump_data() dict[str, Any]
data: dict[str, Any]
perm_str: str
permissions_str: str
}
Flow diagram for permission check logic with node_dict and wildcardsflowchart TD
A["check_permission(node: str)"] --> B["node_dict.get(node)"]
B -- True --> G["return True"]
B -- False --> C["current_node = ''"]
C --> D["for part in node.split('.')"]
D --> E["node_dict.get(current_node + '.' + '*')"]
E -- True --> G
E -- False --> F["current_node += ('.' if current_node else '') + part"]
F --> H["node_dict.get(current_node)"]
H -- True --> G
H -- False --> D
D --> I["return False"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
你好 - 我已审阅了你的更改,它们看起来很棒!
AI 代理提示
请解决此代码审查中的评论:
## 单独评论
### 评论 1
<location> `src/nonebot_plugin_liteperm/nodelib.py:10` </location>
<code_context>
class Permissions:
permissions_data: dict[str, str | dict | bool]
+ node_dict: dict[str, bool]
+ __permissions_str: str = ""
</code_context>
<issue_to_address>
**问题 (bug_risk):** node_dict 未在 __init__ 中初始化,这可能导致在调用 __dump 之前使用时出现问题。
在调用 __dump 之前访问 node_dict 将引发 AttributeError。在 __init__ 中将 node_dict 初始化为空字典将防止这种情况发生。
</issue_to_address>
### 评论 2
<location> `src/nonebot_plugin_liteperm/nodelib.py:51-58` </location>
<code_context>
+ for n, v in self.__search_perm(data):
+ self.__permissions_str += f"{n} {'true' if v else 'false'}\n"
+ node_dict[n] = v
+ for line in self.__permissions_str.splitlines():
+ if line:
+ perm, arg = line.split(" ")
+ node_dict[perm] = arg == "true"
+ self.node_dict = node_dict
+
</code_context>
<issue_to_address>
**建议:** 冗余的 node_dict 填充:node_dict 已在之前的循环中填充。
第二个循环重复了第一个循环的工作,可以移除以简化代码并提高效率。
```suggestion
for n, v in self.__search_perm(data):
self.__permissions_str += f"{n} {'true' if v else 'false'}\n"
node_dict[n] = v
self.node_dict = node_dict
```
</issue_to_address>
### 评论 3
<location> `src/nonebot_plugin_liteperm/nodelib.py:106` </location>
<code_context>
def check_permission(self, node: str) -> bool:
node = node.strip()
node_dict = self.node_dict
if node_dict.get(node):
return True
current_node = ""
for part in node.split("."):
if node_dict.get(current_node + "." + "*"):
return True
current_node += ("." if current_node else "") + part
if node_dict.get(current_node):
return True
return False
</code_context>
<issue_to_address>
**建议 (代码质量):** 使用 f-string 代替字符串拼接 [×2] ([`use-fstring-for-concatenation`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-fstring-for-concatenation/))
```suggestion
if node_dict.get(f"{current_node}.*"):
```
</issue_to_address>帮助我更有用!请点击每个评论上的 👍 或 👎,我将利用您的反馈来改进您的评论。
Original comment in English
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/nonebot_plugin_liteperm/nodelib.py:10` </location>
<code_context>
class Permissions:
permissions_data: dict[str, str | dict | bool]
+ node_dict: dict[str, bool]
+ __permissions_str: str = ""
</code_context>
<issue_to_address>
**issue (bug_risk):** node_dict is not initialized in __init__, which may cause issues if __dump is not called before usage.
Accessing node_dict before __dump is called will raise an AttributeError. Initializing node_dict in __init__ as an empty dict will prevent this.
</issue_to_address>
### Comment 2
<location> `src/nonebot_plugin_liteperm/nodelib.py:51-58` </location>
<code_context>
+ for n, v in self.__search_perm(data):
+ self.__permissions_str += f"{n} {'true' if v else 'false'}\n"
+ node_dict[n] = v
+ for line in self.__permissions_str.splitlines():
+ if line:
+ perm, arg = line.split(" ")
+ node_dict[perm] = arg == "true"
+ self.node_dict = node_dict
+
</code_context>
<issue_to_address>
**suggestion:** Redundant node_dict population: node_dict is already filled in the previous loop.
The second loop duplicates the work of the first and can be removed to simplify the code and improve efficiency.
```suggestion
for n, v in self.__search_perm(data):
self.__permissions_str += f"{n} {'true' if v else 'false'}\n"
node_dict[n] = v
self.node_dict = node_dict
```
</issue_to_address>
### Comment 3
<location> `src/nonebot_plugin_liteperm/nodelib.py:106` </location>
<code_context>
def check_permission(self, node: str) -> bool:
node = node.strip()
node_dict = self.node_dict
if node_dict.get(node):
return True
current_node = ""
for part in node.split("."):
if node_dict.get(current_node + "." + "*"):
return True
current_node += ("." if current_node else "") + part
if node_dict.get(current_node):
return True
return False
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use f-string instead of string concatenation [×2] ([`use-fstring-for-concatenation`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-fstring-for-concatenation/))
```suggestion
if node_dict.get(f"{current_node}.*"):
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| class Permissions: | ||
| permissions_data: dict[str, str | dict | bool] | ||
| node_dict: dict[str, bool] |
There was a problem hiding this comment.
问题 (bug_risk): node_dict 未在 init 中初始化,这可能导致在调用 __dump 之前使用时出现问题。
在调用 __dump 之前访问 node_dict 将引发 AttributeError。在 init 中将 node_dict 初始化为空字典将防止这种情况发生。
Original comment in English
issue (bug_risk): node_dict is not initialized in init, which may cause issues if __dump is not called before usage.
Accessing node_dict before __dump is called will raise an AttributeError. Initializing node_dict in init as an empty dict will prevent this.
| for n, v in self.__search_perm(data): | ||
| self.__permissions_str += f"{n} {'true' if v else 'false'}\n" | ||
| node_dict[n] = v | ||
| for line in self.__permissions_str.splitlines(): | ||
| if line: | ||
| perm, arg = line.split(" ") | ||
| node_dict[perm] = arg == "true" | ||
| self.node_dict = node_dict |
There was a problem hiding this comment.
建议: 冗余的 node_dict 填充:node_dict 已在之前的循环中填充。
第二个循环重复了第一个循环的工作,可以移除以简化代码并提高效率。
| for n, v in self.__search_perm(data): | |
| self.__permissions_str += f"{n} {'true' if v else 'false'}\n" | |
| node_dict[n] = v | |
| for line in self.__permissions_str.splitlines(): | |
| if line: | |
| perm, arg = line.split(" ") | |
| node_dict[perm] = arg == "true" | |
| self.node_dict = node_dict | |
| for n, v in self.__search_perm(data): | |
| self.__permissions_str += f"{n} {'true' if v else 'false'}\n" | |
| node_dict[n] = v | |
| self.node_dict = node_dict |
Original comment in English
suggestion: Redundant node_dict population: node_dict is already filled in the previous loop.
The second loop duplicates the work of the first and can be removed to simplify the code and improve efficiency.
| for n, v in self.__search_perm(data): | |
| self.__permissions_str += f"{n} {'true' if v else 'false'}\n" | |
| node_dict[n] = v | |
| for line in self.__permissions_str.splitlines(): | |
| if line: | |
| perm, arg = line.split(" ") | |
| node_dict[perm] = arg == "true" | |
| self.node_dict = node_dict | |
| for n, v in self.__search_perm(data): | |
| self.__permissions_str += f"{n} {'true' if v else 'false'}\n" | |
| node_dict[n] = v | |
| self.node_dict = node_dict |
| return True | ||
| current_node = "" | ||
| for part in node.split("."): | ||
| if node_dict.get(current_node + "." + "*"): |
There was a problem hiding this comment.
建议 (代码质量): 使用 f-string 代替字符串拼接 [×2] (use-fstring-for-concatenation)
| if node_dict.get(current_node + "." + "*"): | |
| if node_dict.get(f"{current_node}.*"): |
Original comment in English
suggestion (code-quality): Use f-string instead of string concatenation [×2] (use-fstring-for-concatenation)
| if node_dict.get(current_node + "." + "*"): | |
| if node_dict.get(f"{current_node}.*"): |
Sourcery 摘要
通过添加类型注解、简化权限序列化、缓存节点权限以及启用权限修改的方法链式调用,对 Permissions 类进行了重构。
增强:
__dump_to_str重命名为__dump,并在node_dict中缓存数据__search_perm以返回结构化元组而非字符串行Self使set_permission和del_permission可链式调用check_permission以利用预计算的node_dict实现更快的查找__dump并移除冗余代码路径Original summary in English
Summary by Sourcery
Refactor the Permissions class by adding type annotations, streamlining permission serialization, caching node permissions, and enabling method chaining for permission modifications.
Enhancements: