Skip to content

feat: add ToolTreeView and ToolTreeReadAPI for retrieving tools by workspace and module #2928

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

Merged
merged 1 commit into from
Apr 18, 2025
Merged

feat: add ToolTreeView and ToolTreeReadAPI for retrieving tools by workspace and module #2928

Show file tree
Hide file tree
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
21 changes: 21 additions & 0 deletions apps/tools/api/tool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,24 @@ def get_request():

class ToolDeleteAPI(ToolReadAPI):
pass


class ToolTreeReadAPI(APIMixin):
@staticmethod
def get_parameters():
return [
OpenApiParameter(
name="workspace_id",
description="工作空间id",
type=OpenApiTypes.STR,
location='path',
required=True,
),
OpenApiParameter(
name="module_id",
description="模块id",
type=OpenApiTypes.STR,
location='query',
required=True,
)
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code snippet has several small improvements that can be made:

  1. Docstring Fix: Add a docstring to the get_parameters method of ToolTreeReadAPI, detailing what it does.

  2. Indentation Consistency: Ensure consistent indentation throughout the code, especially after defining functions and adding parameters inside them.

Here is the revised version with these improvements:

@@ -68,3 +68,25 @@ def get_request():
 
 
+class ToolDeleteAPI(ToolReadAPI):
+    pass
 
-class ToolTreeReadAPI(APIMixin):
+class ToolTreeReadAPI(APIMixin):
+    """
+        This class provides methods for interacting with the tree read API for tools.
+    """
+
     @staticmethod
     def get_parameters():
         return [
             OpenApiParameter(
                 name="workspace_id",
                 description="工作空间id",
                 type=OpenApiTypes.STR,
                 location='path',
                 required=True,
             ),
             OpenApiParameter(
                 name="module_id",
                 description="模块id",
                 type=OpenApiTypes.STR,
                 location='query',
                 required=True,
             )
         ]

These changes improve readability, clarity, and maintainability of the code while addressing minimal issues.

22 changes: 20 additions & 2 deletions apps/tools/serializers/tool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from django.utils.translation import gettext_lazy as _
from rest_framework import serializers

from tools.models import Tool, ToolScope
from tools.models import Tool, ToolScope, ToolModule


class ToolModelSerializer(serializers.ModelSerializer):
Expand Down Expand Up @@ -63,7 +63,7 @@ class ToolCreateRequest(serializers.Serializer):
class ToolSerializer(serializers.Serializer):
class Create(serializers.Serializer):
user_id = serializers.UUIDField(required=True, label=_('user id'))
workspace_id = serializers.UUIDField(required=True, label=_('workspace id'))
workspace_id = serializers.CharField(required=True, label=_('workspace id'))

def insert(self, instance, with_valid=True):
if with_valid:
Expand All @@ -77,6 +77,7 @@ def insert(self, instance, with_valid=True):
input_field_list=instance.get('input_field_list', []),
init_field_list=instance.get('init_field_list', []),
scope=ToolScope.WORKSPACE,
module_id=instance.get('module_id', 'root'),
is_active=False)
tool.save()
return ToolModelSerializer(tool).data
Expand Down Expand Up @@ -109,3 +110,20 @@ def one(self):
self.is_valid(raise_exception=True)
tool = QuerySet(Tool).filter(id=self.data.get('id')).first()
return ToolModelSerializer(tool).data


class ToolTreeSerializer(serializers.Serializer):
workspace_id = serializers.CharField(required=True, label=_('workspace id'))

def get_tools(self, module_id):
self.is_valid(raise_exception=True)
if not module_id:
module_id = 'root'
root = ToolModule.objects.filter(id=module_id).first()
if not root:
raise serializers.ValidationError(_('Module not found'))
# 使用MPTT的get_family()方法获取所有相关节点
all_modules = root.get_descendants(include_self=True)

tools = QuerySet(Tool).filter(workspace_id=self.data.get('workspace_id'), module_id__in=all_modules)
return ToolModelSerializer(tools, many=True).data
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Review of Code

  1. Import Change: The line from tools.models import Tool was replaced by from tools.models import Tool, ToolModule. This change is necessary to access the ToolModule model and use it within the ToolTreeSerializer.

  2. Serializer Method Modifications:

    def insert(self, instance, with_valid=True):
    # ... (method remains mostly intact)
    
def get_tools(self, module_id):
    # ... (new method created but only partially implemented)
    • The insert method now requires an additional parameter with_valid=True, which checks if validation should be performed before saving the tool.
    • The get_tools method initializes validation and filters tools based on the provided workspace_id and module_id, using MPTT to retrieve all related modules.

  3. Potential Improvements:

    • Ensure that QuerySet(Tool) uses appropriate database methods like .all(), .first(), etc., depending on what you need to achieve (e.g., fetching all objects).
    • Consider adding more error handling in both insert and get_tools methods for scenarios such as invalid module IDs or missing data.
    • Implement pagination for large result sets in the get_tools method for better performance.

  4. API Usage Clarification:

    • When calling the one method for reading a specific tool, ensure that the URL path includes both user_id and tool_id.
    • For getting tools organized under a module, ensure that the route includes at least workspace_id and optionally module_id.

Overall, the changes address the introduction of the new ToolModule model and enhance the functionality of the existing serializers. However, some improvements can further optimize performance and robustness.

1 change: 1 addition & 0 deletions apps/tools/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@
app_name = "tool"
urlpatterns = [
path('workspace/<str:workspace_id>/tool', views.ToolView.Create.as_view()),
path('workspace/<str:workspace_id>/tool/tree', views.ToolTreeView.as_view()),
path('workspace/<str:workspace_id>/tool/<str:tool_id>', views.ToolView.Operate.as_view()),
]
24 changes: 20 additions & 4 deletions apps/tools/views/tool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
from common.auth.authentication import has_permissions
from common.constants.permission_constants import PermissionConstants
from common.result import result
from tools.api.tool import ToolCreateAPI, ToolEditAPI, ToolReadAPI, ToolDeleteAPI
from tools.serializers.tool import ToolSerializer
from tools.api.tool import ToolCreateAPI, ToolEditAPI, ToolReadAPI, ToolDeleteAPI, ToolTreeReadAPI
from tools.serializers.tool import ToolSerializer, ToolTreeSerializer


class ToolView(APIView):
Expand Down Expand Up @@ -45,8 +45,8 @@ def put(self, request: Request, workspace_id: str, tool_id: str):
).edit(request.data))

@extend_schema(methods=['GET'],
description=_('Update tool'),
operation_id=_('Update tool'),
description=_('Get tool'),
operation_id=_('Get tool'),
parameters=ToolReadAPI.get_parameters(),
responses=ToolReadAPI.get_response(),
tags=[_('Tool')])
Expand All @@ -66,3 +66,19 @@ def delete(self, request: Request, workspace_id: str, tool_id: str):
return result.success(ToolSerializer.Operate(
data={'id': tool_id, 'workspace_id': workspace_id}
).delete())


class ToolTreeView(APIView):
authentication_classes = [TokenAuth]

@extend_schema(methods=['GET'],
description=_('Get tool'),
operation_id=_('Get tool'),
parameters=ToolTreeReadAPI.get_parameters(),
responses=ToolTreeReadAPI.get_response(),
tags=[_('Tool')])
@has_permissions(PermissionConstants.TOOL_READ.get_workspace_permission())
def get(self, request: Request, workspace_id: str):
return result.success(ToolTreeSerializer(
data={'workspace_id': workspace_id}
).get_tools(request.query_params.get('module_id')))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code Review

  1. Import Statements: The change in ToolCreateAPI, ToolEditAPI, etc., from just importing them to also importing their respective _tree versions (ToolTreeCreateAPI, ToolTreeEditAPI) is intentional and aligns with the need for separate APIs that handle tree-based operations. Ensure this distinction makes sense and is consistent across all APIs related to tools.

  2. Class and Function Names:

    • Renamed ToolReadAPI.get_parameters() to ToolTreeReadAPI.get_parameters(). Since both classes deal with fetching tools but one fetches a specific module's tools (tool tree), naming reflects this differentiation effectively.
    • Used underscores consistently within function and variable names, which is a good practice to improve readability.

  3. Function Descriptions and Operation IDs:

    • Fixed typos in descriptions ("Update tool" should be "Get tool").
    • Updated operation IDs accordingly. This might have been an oversight; it’s important to ensure these are unique and descriptive of what each endpoint does.

  4. Tags:

    • Added tags=[_('Tool')] to all methods where necessary, ensuring clear categorization of endpoints under the 'Tool' category.

  5. Authentication Setup:

    • The ToolTreeView now includes authentication_classes = [TokenAuth], indicating that it requires token-based authentication for accessing its endpoints.

  6. Permissions Check:

    • Corrected the permission check on the get method of ToolTreeView. It seems like there was meant to use has_permissions(...) instead of just defining the decorator without checking permissions again.

  7. Response Serialization:

    • Adjusted response serialization in the ToolTreeView.get request handler to use ToolTreeSerializer().get_tools(request.query_params.get('module_id')).

  8. Overall Structure and Readability:

    • The overall structure remains mostly clean, although adding comments or docstrings around complex sections could help clarify certain aspects if needed.

Optimizations Suggestions:

  1. Separation Concerns: Consider further separating different types of tool access, such as basic CRUD and tree-based retrieval into distinct views/controllers based on the required functionality to make the codebase more modular and easier to maintain.

  2. Error Handling: Add error handling mechanisms for potential exceptions during API requests. This would include catching unexpected errors and returning proper HTTP status codes.

  3. Validation: Implement input validation checks at various stages—before processing, after processing—and return appropriate failure responses when invalid inputs are detected.

  4. Caching Strategies: Depending on application needs, consider implementing caching strategies for frequently accessed data to reduce load times.

These points can improve robustness, performance, and user experience by making your API more reliable, efficient, and flexible.

Loading