Refactor Permissions class for type hints and methods #8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
评审指南此 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
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
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.
Comment on lines
+51
to
+58
| 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 已在之前的循环中填充。
第二个循环重复了第一个循环的工作，可以移除以简化代码并提高效率。
Suggested change
| 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.
Suggested change
| 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)
Suggested change
| 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)
Suggested change
| if node_dict.get(current_node + "." + "*"): | |
| if node_dict.get(f"{current_node}.*"): |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: