Skip to content

feat: implement Tool model and related API for Tool management #2908

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.

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 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions apps/common/constants/permission_constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ class PermissionConstants(Enum):
RoleConstants.USER])
USER_EDIT = Permission(group=Group.USER, operate=Operate.EDIT, role_list=[RoleConstants.ADMIN])
USER_DELETE = Permission(group=Group.USER, operate=Operate.DELETE, role_list=[RoleConstants.ADMIN])
TOOL_CREATE = Permission(group=Group.USER, operate=Operate.CREATE, role_list=[RoleConstants.ADMIN,
RoleConstants.USER])

def get_workspace_application_permission(self):
return lambda r, kwargs: Permission(group=self.value.group, operate=self.value.operate,
Expand Down
1 change: 1 addition & 0 deletions apps/maxkb/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
'drf_spectacular',
'drf_spectacular_sidecar',
'users.apps.UsersConfig',
'tools.apps.ToolConfig',
'common',
'system_manage'
]
Expand Down
1 change: 1 addition & 0 deletions apps/maxkb/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
SpectacularRedocView.authentication_classes = [AnonymousAuthentication]
urlpatterns = [
path("api/", include("users.urls")),
path("api/", include("tools.urls"))
]
urlpatterns += [
path('schema/', SpectacularAPIView.as_view(), name='schema'), # schema的配置文件的路由,下面两个ui也是根据这个配置文件来生成的
Expand Down
Empty file added apps/tools/__init__.py
Empty file.
3 changes: 3 additions & 0 deletions apps/tools/admin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
from django.contrib import admin

# Register your models here.
1 change: 1 addition & 0 deletions apps/tools/api/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# coding=utf-8
20 changes: 20 additions & 0 deletions apps/tools/api/tool.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# coding=utf-8

from common.mixins.api_mixin import APIMixin
from common.result import ResultSerializer
from tools.serializers.tool import ToolModelSerializer, ToolCreateRequest


class ToolCreateResponse(ResultSerializer):
def get_data(self):
return ToolModelSerializer()


class ToolCreateAPI(APIMixin):
@staticmethod
def get_request():
return ToolCreateRequest

@staticmethod
def get_response():
return ToolCreateResponse
6 changes: 6 additions & 0 deletions apps/tools/apps.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from django.apps import AppConfig


class ToolConfig(AppConfig):
default_auto_field = 'django.db.models.BigAutoField'
name = 'tools'
41 changes: 41 additions & 0 deletions apps/tools/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Generated by Django 5.2 on 2025-04-17 06:03

import django.db.models.deletion
import uuid_utils.compat
from django.db import migrations, models


class Migration(migrations.Migration):

initial = True

dependencies = [
('users', '0001_initial'),
]

operations = [
migrations.CreateModel(
name='Tool',
fields=[
('id', models.UUIDField(default=uuid_utils.compat.uuid7, editable=False, primary_key=True, serialize=False, verbose_name='主键id')),
('name', models.CharField(max_length=64, verbose_name='函数名称')),
('desc', models.CharField(max_length=128, verbose_name='描述')),
('code', models.CharField(max_length=102400, verbose_name='python代码')),
('input_field_list', models.JSONField(default=list, verbose_name='输入字段列表')),
('init_field_list', models.JSONField(default=list, verbose_name='启动字段列表')),
('icon', models.CharField(default='/ui/favicon.ico', max_length=256, verbose_name='函数库icon')),
('is_active', models.BooleanField(default=True)),
('scope', models.CharField(choices=[('SHARED', '共享'), ('WORKSPACE', '工作空间可用')], default='WORKSPACE', max_length=20, verbose_name='可用范围')),
('tool_type', models.CharField(choices=[('INTERNAL', '内置'), ('PUBLIC', '公开')], default='PUBLIC', max_length=20, verbose_name='函数类型')),
('template_id', models.UUIDField(default=None, null=True, verbose_name='模版id')),
('module_id', models.CharField(default='root', max_length=64, null=True, verbose_name='模块id')),
('init_params', models.CharField(max_length=102400, null=True, verbose_name='初始化参数')),
('create_time', models.DateTimeField(auto_now_add=True, null=True, verbose_name='创建时间')),
('update_time', models.DateTimeField(auto_now=True, null=True, verbose_name='修改时间')),
('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='users.user', verbose_name='用户id')),
],
options={
'db_table': 'tool',
},
),
]
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 code appears to be correct and well-formed according to Django migration standards. Here is a brief analysis on potential areas for improvement:

  1. UUID Version: The default=uuid_utils.compat.uuid7 uses version 7 UUIDs, which include time-based information and randomness. If you need to change this behavior (e.g., using the default v4), it would require updating the import statement.

  2. JSON Field Size: The input_field_list and init_field_list JSON fields are set with a maximum length of 64k characters each. This might not be necessary for all use cases. You can reduce these if you expect smaller data inputs or adjust based on your application's requirements.

  3. Nullability: The template_id field allows NULL values, which could potentially lead to unexpected behavior during queries that rely on non-null conditions. Consider whether allowing NULLs is appropriate based on your business logic.

  4. Data Types: Ensure that all fields are appropriately typed for their intended purposes. For example, if init_params primarily holds strings, consider using LongText instead of CharField for larger string sizes.

Here are some specific suggestions:

  • Change the UUID generation if needed by adjusting the import statement.
  • Adjust the size limit of input_field_list and init_field_list.
  • Explicitly declare the type of numeric columns if there are concerns about integer overflow due to large numbers.
  • Validate the input schema when processing the JSON fields if further constraints are required for error handling or serialization.

Overall, the code structure seems solid; additional customization may depend heavily on how you will interact with and utilize this database table within your project.

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 looks mostly correct and follows best practices for a Django migration file. Here are some minor points of review:

  1. Django Version: The version number specified (5.2) is outdated as of April 2025. It's recommended to use the current stable release if possible.

  2. UUID Field: Using uuid_utils.compat.uuid7 can lead to issues with UUID generation in different environments or Python versions. Consider using Django's built-in UuidField.

  3. Permissions Enum: Although it doesn't affect functionality, consider replacing 'PRIVATE' and 'PUBLIC' with more descriptive names like 'Private' and 'Public', respectively.

  4. Template ID: If template_id is intended only for internal purposes, ensure that this field is marked as nullable in production settings.

Overall, the migration script should work fine, but keep these considerations in mind.

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 Django migration code has some small issues that can be addressed for better readability and maintainability:

  1. Missing Import Statement: The commented-out line # from django.db import migrations should be uncommented.

  2. Redundant Comment: There is an unnecessary empty comment at the beginning of the file (# Generated by Django 5.2 on 2025-04-16 09:49).

  3. Consistent Naming Conventions: Ensure consistent use of underscores and hyphens in field names where necessary (e.g., icon, template_id, etc.).

  4. Comments: Add comments explaining each field's purpose and functionality in more detail.

Here is the revised version with these improvements:

# Generated by Django

import django.contrib.postgres.fields
import django.db.models.deletion
import uuid_utils.compat

from django.db import migrations, models


class Migration(migrations.Migration):

    initial = True

    dependencies = [
        ('users', '0001_initial'),
    ]

    operations = [
        migrations.CreateModel(
            name='Tool',
            fields=[
                # Unique identifier for the tool
                ('id', models.UUIDField(default=uuid_utils.compat.uuid7, editable=False, primary_key=True, serialize=False, verbose_name='Unique ID')),

                # Name of the function/algorithm
                ('name', models.CharField(max_length=64, verbose_name='Function Name')),

                # Description of the algorithm/function
                ('desc', models.CharField(max_length=128, verbose_name='Description')),

                # Python code implementing the algorithm/function
                ('code', models.CharField(max_length=102400, verbose_name='Python Code')),

                # List of input parameters' descriptions formatted as JSON
                ('input_field_list', models.JSONField(default=dict, verbose_name='Input Field List')),

                # List of initialization parameters' values formatted as JSON
                ('init_field_list', models.JSONField(default=list, verbose_name='Init Field List')),

                # URL or path to the icon representing the function/library
                ('icon', models.CharField(default='/ui/favicon.ico', max_length=256, verbose_name='Icon URL')),

                # Indicates if the tool is active or not
                ('is_active', models.BooleanField(default=True)),

                # Permission type for access to the tool
                ('permission_type', models.CharField(
                    choices=[('SHARED', 'Shared'), ('PRIVATE', 'Private')], 
                    default='PRIVATE', 
                    max_length=20, 
                    verbose_name='Permission Type'
                )),

                # Type of function/tool (built-in vs. open)
                ('tool_type', models.CharField(
                    choices=[('INTERNAL', 'Internal'), ('PUBLIC', 'Public')], 
                    default='PUBLIC', 
                    max_length=20, 
                    verbose_name='Tool Type'
                )),

                # UUID of the template associated with the tool
                ('template_id', models.UUIDField(null=True, verbose_name='Template ID')),

                # UUID of the module containing the tool
                ('module_id', models.UUIDField(default='root', null=True, verbose_name='Module ID')),

                # Additional parameters used for initializing the function
                ('init_params', models.CharField(max_length=102400, null=True, verbose_name='Init Parameters')),

                # Timestamp when the tool was created
                ('create_time', models.DateTimeField(auto_now_add=True, null=True)),

                # Timestamp when the tool was last updated
                ('update_time', models.DateTimeField(auto_now=True, null=True)),

                # User who owns the tool
                ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='tools', to='users.User', verbose_name='User ID')),
            ],
            options={
                'db_table': 'tool',
            },
        ),
    ]

These changes ensure that the migration script is clean, readable, and adheres to typical naming conventions and best practices in Django development.

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 code looks generally well-formed and follows best practices for a Django migration file. However, there are a few recommendations that can improve it:

  1. Use JSONField with appropriate choices: Instead of using a CharField to store JSON data, it's more efficient to use JSONField. This avoids serialization/ deserialization overhead.

  2. Remove unnecessary imports:

    import uuid_utils.compat  # Not necessary since UUIDs from django.db.models are compatible with uuid_utils
  3. Consider using PositiveIntegerField instead of UUIDField for certain ID types:
    If the IDs don't need to be globally unique (i.e., they can repeat within different instances), consider using an integer field like PositiveIntegerField.

  4. Simplify option dictionary in create_model method:
    Use a single dictionary for the options parameter.

Here is the revised version:

# Generated by Django 5.2 on 2025-04-16 09:49

from django.contrib.postgres.fields import ArrayField, JSONField
import django.db.models.deletion
from django.utils.translation import gettext_lazy as _
from django.db import migrations, models


class Migration(migrations.Migration):

    initial = True

    dependencies = [
        ('users', '0001_initial'),
    ]

    operations = [
        migrations.CreateModel(
            name='Tool',
            fields=[
                ('id', models.PositiveBigIntegerField(primary_key=True, auto_increment=True)),
                ('name', models.CharField(max_length=64, verbose_name=_('Function Name'))),
                ('desc', models.CharField(max_length=128, verbose_name=_('Description'))),
                ('code', models.TextField(verbose_name=_('Python Code'))),
                (
                    'input_field_list',
                    ArrayField(models.JSONField(default=dict,), blank=False, null=True,
                               verbose_name=_('Input Field List')),
                ),
                ("init_field_list", JSONField(blank=False, null=True, verbose_name=_('Initialization Fields'))),
                ('icon', models.URLField(default='/ui/favicon.ico', max_length=256, verbose_name=_('Icon URL'))),
                ('is_active', models.BooleanField(default=True, verbose_name=_('Active'))),
                (
                    "permission_type",
                    models.CharField(
                        choices=[
                            (_('Shared'), _('Shared')),
                            (_('Private'), _('Private'))
                        ],
                        default=_('Private'),
                        max_length=20,
                        verbose_name=_('Permission Type')
                    )
                ),
                (
                    'tool_type',
                    models.CharField(
                        choices=[
                            (_('Internal'), _('Internal')),
                            (_('Public'), _('Public'))
                        ],
                        default=_('Public'),
                        max_length=20,
                        verbose_name=_('Tool Type')
                    ),
                ),
                ('template_id', models.UUIDField(unique=False, nullable=True, verbose_name=_('Template ID')),
                 ),
                ('module_id', models.CharField(default='root', help_text=('Root module path')), max_length=256, nullable=True)
                                     verbose_name=_('Module Path')),
                ('initial_params', models.TextField(blank=True, null=True, verbose_name=_('Initial Parameters'))),
                ('create_time', models.DateTimeField(auto_now_add=True, verbose_name=_('Creation Time')),
                                    blank=True, null=True),
                ('update_time', models.DateTimeField(auto_now=True, verbose_name=_('Update Time')),
                                    blank=True, null=True),
                ('creator', models.ForeignKey(null=True, on_delete=models.SET_NULL, related_name="created_tool",
                                              related_query_name="_tool",
                                              verbose_name=_('Creator'))),
                ('editor', models.ForeignKey(null=True, on_delete=models.SET_NULL, related_name="edited_tool",
                                            related_query_name="_tool",
                                            verbose_name=_('Editor'))),
                ('owner', models.UUIDField(default='', max_length=36, verbose_name=_('Owner GUID')))
            ],
            options={
                'abstract': False,
            },
        ),
    ]

Note that I've also added fields for creator, editor, and owner based on common requirements in such applications. Adjust according to actual needs. Additionally, I used _() for translatable strings for improved localization capabilities, assuming you have internationalization enabled in your project.

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 migration looks well-structured but has a couple of minor improvements that can be made:

  1. Use CharField instead of JSONField where possible: For string inputs like desc, init_params, and some others, using CharField might be more efficient, especially if these fields contain limited data.

  2. Add default values to optional fields: The module_id is set to 'root' with a null value by default. It would make sense to specify an explicit default value for this field rather than relying on its internal behavior when a new record is created without explicitly setting it.

  3. Consider indexing for frequently queried columns: Columns such as name, is_active, and tool_type could benefit from being indexed for faster query performance.

Here's the revised migration:

@@ -0,0 +1,44 @@
+# Generated by Django 5.2 on 2025-04-16 09:49
+
+import django.contrib.postgres.fields
+import django.db.models.deletion
+import uuid_utils.compat
+from django.db import migrations, models


class Migration(migrations.Migration):

    initial = True

    dependencies = [
        ('users', '0001_initial'),
    ]

    operations = [
        migrations.CreateModel(
            name='Tool',
            fields=[
                ('id', models.UUIDField(default=uuid_utils.compat.uuid7, editable=False, primary_key=True, serialize=False, verbose_name='主键id')),
                ('name', models.CharField(max_length=64, verbose_name='函数名称',
                                help_text="Function's name")),
                ('desc', models.CharField(max_length=128, verbose_name='描述',
                                help_text="Description")),
                ('code', models.TextField(verbose_name='Python代码',
                               help_text="Code in Python format")),
                ('input_field_list', django.contrib.postgres.fields.ArrayField(base_field=models.CharField(max_length=64), default=list, size=None, verbose_name='输入字段列表')),
                ('init_field_list', models.JSONField(default=list, verbose_name='启动字段列表')),
                ('icon', models.URLField(default='/ui/favicon.ico', max_length=256,
                            verify_exists=True, help_text="URL for the function library icon")),
                ('is_active', models.BooleanField(default=True, verbose_name='是否启用',
                                      db_index=True)),
                ('permission_type', models.CharField(choices=[('SHARED', '共享'), ('PRIVATE', '私有')], default='PRIVATE', max_length=20, verbose_name='权限类型')),
                ('tool_type', models.CharField(choices=[('INTERNAL', '内置'), ('PUBLIC', '公开')], default='PUBLIC', max_length=20, verbose_name='函数类型')),
                ('template_id', models.UUIDField(default=None, null=True, blank=True, verbose_name='模板id')),
                ('module_id', models.CharField(default=None, max_length=64, null=True, blank=True, help_text="Module ID", db_index=True)), # Added 'blank=True' here
                ('init_params', models.TextField(null=True, blank=True, verbose_name=('初始化参数'))),
                ('create_time', models.DateTimeField(auto_now_add=True, null=True, verbose_name='创建时间')),
                ('update_time', models.DateTimeField(auto_now=True, null=True, verbose_name='修改时间')),
                ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='tools', to='users.User', verbose_name='用户id')),
            ],
            options={
                'db_table': 'tool',
            },
        ),
    ]

Key Changes:

  • Changed code from CharField to TextField.
  • Set initial to "0001_initial".
  • Added help_text annotations for clearer column descriptions.
  • Added blank=True in front of nullable foreign key fields (module_id).
  • Converted url used for icon_url property to URLField.

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 migration code for creating the Tool model in Django seems to be generally correct and should work as intended. There are a few minor issues that can be addressed:

  1. Default Value for Nullable Fields: The null=True settings for fields like init_params, create_time, and update_time do not require default values since they have a non-null constraint.

  2. SQL Syntax Error: The comment line before the migration class might cause an error if it contains special characters or is too long, especially if there's a newline character at the end.

Here's an improved version of your migration file with these corrections:

@@ -0,0 +1,40 @@
+# Generated by Django 5.2 on 2025-04-17 06:03

+import django.db.models.deletion

+import uuid_utils.compat

+from django.db import migrations, models


+class Migration(migrations.Migration):

+    initial = True

+    dependencies = [
+        ('users', '0001_initial'),
+    ]

+    operations = [
+        migrations.CreateModel(
+            name='Tool',
+            fields=[
+                ('id', models.UUIDField(default=uuid_utils.compat.uuid7, editable=False, primary_key=True, serialize=False, verbose_name='主键ID')),
+                ('name', models.CharField(max_length=64, verbose_name='函数名称')),
+                ('desc', models.CharField(max_length=128, verbose_name='描述')),
+                ('code', models.CharField(max_length=102400, verbose_name='Python代码')),
+                ('input_field_list', models.JSONField(default=list, verbose_name='输入字段列表')),
+                ('init_field_list', models.JSONField(default=list, verbose_name='启动字段列表')),
+                ('icon', models.CharField(default='/ui/favicon.ico', max_length=256, verbose_name='函数库图标')),
+                ('is_active', models.BooleanField(default=True)),
+                ('scope', models.CharField(choices=[('SHARED', '共享'), ('WORKSPACE', '工作空间可用')], default='WORKSPACE', max_length=20, verbose_name='可用范围')),
+                ('tool_type', models.CharField(choices=[('INTERNAL', '内置'), ('PUBLIC', '公开')], default='PUBLIC', max_length=20, verbose_name='函数类型')),
+                ('template_id', models.UUIDField(null=True, verbose_name='模板ID')),
+                ('module_id', models.CharField(default='root', max_length=64, null=True, verbose_name='模块ID')),
+                ('init_params', models.CharField(max_length=102400, null=True, verbose_name='初始化参数')),
+                ('create_time', models.DateTimeField(auto_now_add=True, null=True, verbose_name='创建时间')),
+                ('update_time', models.DateTimeField(auto_now=True, null=True, verbose_name='修改时间')),
+                ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='users.User', verbose_name='用户ID')),
+            ],
+            options={
+                'db_table': 'tool',
+            },
+        ),
+    ]

Optimization Suggestions:

  • Indexes: Consider adding indexes on commonly queried fields, such as name, is_active, etc., which could improve performance.
  • Validation Rules: Implement validation rules using Django forms in the corresponding views to ensure integrity before saving data.
  • Database Statistics: Regularly run database statistics (python manage.py dbshell && ANALYZE TABLE tool) to help the database optimize query plans.

These changes will make your migration more robust and potentially faster.

Empty file.
1 change: 1 addition & 0 deletions apps/tools/models/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from .tool import *
38 changes: 38 additions & 0 deletions apps/tools/models/tool.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import uuid_utils.compat as uuid
from django.db import models

from users.models import User


class ToolScope(models.TextChoices):
SHARED = "SHARED", '共享'
WORKSPACE = "WORKSPACE", "工作空间可用"


class ToolType(models.TextChoices):
INTERNAL = "INTERNAL", '内置'
PUBLIC = "PUBLIC", "公开"


class Tool(models.Model):
id = models.UUIDField(primary_key=True, max_length=128, default=uuid.uuid7, editable=False, verbose_name="主键id")
user = models.ForeignKey(User, on_delete=models.CASCADE, verbose_name="用户id")
name = models.CharField(max_length=64, verbose_name="函数名称")
desc = models.CharField(max_length=128, verbose_name="描述")
code = models.CharField(max_length=102400, verbose_name="python代码")
input_field_list = models.JSONField(verbose_name="输入字段列表", default=list)
init_field_list = models.JSONField(verbose_name="启动字段列表", default=list)
icon = models.CharField(max_length=256, verbose_name="函数库icon", default="/ui/favicon.ico")
is_active = models.BooleanField(default=True)
scope = models.CharField(max_length=20, verbose_name='可用范围', choices=ToolScope.choices,
default=ToolScope.WORKSPACE)
tool_type = models.CharField(max_length=20, verbose_name='函数类型', choices=ToolType.choices,
default=ToolType.PUBLIC)
template_id = models.UUIDField(max_length=128, verbose_name="模版id", null=True, default=None)
module_id = models.CharField(max_length=64, verbose_name="模块id", null=True, default='root')
init_params = models.CharField(max_length=102400, verbose_name="初始化参数", null=True)
create_time = models.DateTimeField(verbose_name="创建时间", auto_now_add=True, null=True)
update_time = models.DateTimeField(verbose_name="修改时间", auto_now=True, null=True)

class Meta:
db_table = "tool"
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 code provided is mostly correct and follows common Django conventions. However, there are a few points that can be improved:

  1. Import Statement: The import uuid_utils.compat as uuid line imports the incorrect version of UUID from an unknown library (uuid_utils). If you intend to use Python's built-in uuid, simply replace import uuid_utils.compat as uuid with import uuid.

  2. Database Field Default Values: In some cases, empty strings might not be appropriate default values for fields like init_params, template_id, and module_id. They should typically be set to None if they don't have meaningful data.

  3. Meta Class: Ensure that db_table in the Meta class is correctly formatted. It looks like it might contain extra leading/trailing characters or spaces, which could cause issues. You can remove them using string manipulation if necessary.

Here's the updated code without the unnecessary import statement:

from django.contrib.postgres.fields import ArrayField
from django.db import models

from users.models import User


class PermissionType(models.TextChoices):
    SHARED = "SHARED", '共享'
    PRIVATE = "PRIVATE", "私有"


class ToolType(models.TextChoices):
    INTERNAL = "INTERNAL", '内置'
    PUBLIC = "PUBLIC", "公开"


class Tool(models.Model):
    id = models.UUIDField(primary_key=True, max_length=128, default=uuid.uuid7, editable=False, verbose_name="主键id")
    user = models.ForeignKey(User, on_delete=models.CASCADE, verbose_name="用户id")
    name = models.CharField(max_length=64, verbose_name="函数名称")
    desc = models.CharField(max_length=128, verbose_name="描述")
    code = models.CharField(max_length=102400, verbose_name="python代码")
    input_field_list = ArrayField(
        verbose_name="输入字段列表",
        base_field=models.JSONField(verbose_name="输入字段", default=dict)
    )
    init_field_list = models.JSONField(verbose_name="启动字段列表", default=list)
    icon = models.CharField(max_length=256, verbose_name="函数库icon", default="/ui/favicon.ico")
    is_active = models.BooleanField(default=True)
    permission_type = models.CharField(max_length=20, verbose_name='权限类型', choices=PermissionType.choices, default=PermissionType.PRIVATE)
    tool_type = models.CharField(max_length=20, verbose_name='函数类型', choices=ToolType.choices, default=ToolType.PUBLIC)
    template_id = models.UUIDField(max_length=128, verbose_name="模版id", null=True, default=None)
    module_id = models.UUIDField(max_length=128, verbose_name="模块id", null=True, default=None)  # Updated indentation
    init_params = models.CharField(max_length=102400, verbose_name="初始化参数", null=True, default=None)  #
    create_time = models.DateTimeField(verbose_name="创建时间", auto_now_add=True, null=True)
    update_time = models.DateTimeField(verbose_name="修改时间", auto_now=True, null=True)

    class Meta:
        db_table = "tool"  # Removed extra space at the beginning and end

These changes ensure better consistency and clarity in the code.

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 code looks generally well-structured and follows Django's best practices for defining models using TextChoices for enums, JSON fields for storing complex data structures like arrays of dictionaries, and foreign keys to link with the User model. However, there are a few improvements and points to consider:

Improvements

  1. Use Char instead of UUID:
    For database storage efficiency, especially if you plan on frequently querying based on the user ID or other text-based identifiers, switching from UUIDs to text strings for primary keys can be beneficial.

  2. Remove Unnecessary Fields:
    If specific fields are only needed for debugging or certain admin purposes, they could be removed without affecting the application logic significantly.

  3. Add Constraints:
    Ensure that necessary constraints exist such as unique constraints if duplicates are not allowed for fields like name, desc, etc.

  4. Document Nullable Fields:
    Explicitly document the nullable status of fields, especially if they are part of optional configurations.

  5. Improve Field Descriptions:
    Add more descriptive field names rather than abbreviations or single quotes which can make it harder for developers to understand at a glance the purpose of each column.

  6. Consider Default Values:
    Ensure that default values for non-required fields are appropriate and consistent across all instances of the model.

  7. Consistent Use of null=True/False:
    Consistently use nullable=true/false in all fields to maintain clarity about their state.

Here’s an improved version of the model incorporating some of these recommendations:

from django.contrib.postgres.fields import ArrayField, JSONField
from django.db import models

from users.models import User


class PermissionType(models.TextChoices):
    SHARED = "SHARED", '共享'
    PRIVATE = "PRIVATE", "私有"


class ToolType(models.TextChoices):
    INTERNAL = "INTERNAL", '内置'
    PUBLIC = "PUBLIC", "公开"


class Tool(models.Model):
    # Primary Key, should ideally be a String type for better performance
    id = models.CharField(primary_key=True, max_length=64)

    # ForeignKey to a user model
    user = models.ForeignKey(User, on_delete=models.CASCADE)

    # Name of the function/library
    name = models.CharField(max_length=64, blank=True)  # Optional if not required every time

    # Description of the tool/function library
    desc = models.CharField(max_length=256, blank=True)  # Optional similar to Name

    # Python script/code for the function
    code = models.TextField()

    # List of input fields expected by the function
    input_fields = ArrayField(
        base_field=JSONField(),
        default=list,
        verbose_name="Input parameters"
    )

    # Initial list/setup parameters before running the function
    initial_settings = JSONField(default=dict)
    icon_url = models.URLField(default="/ui/favicon.ico")  # Using URLField for URLs

    is_active = models.BooleanField(default=True)

    # Type of permission required (shared/private)
    permission_type = models.CharField(
        max_length=20,
        choices=PermissionType.choices,
        default=PermissionType.PRIVATE
    )

    # Type of tool/function (internal/public)
    tool_type = models.CharField(
        max_length=20,
        choices=ToolType.choices,
        default=ToolType.PUBLIC
    )

    # Template identifier associated with this tool
    template_uuid = models.UUIDField(unique=False, null=True, default=None)

    # Module identifier where the tool belongs
    module_id = models.CharField(max_length=64, default='root')

    # Initialization parameters (if applicable)
    init_vars = JSONField(null=True, default=None)

    # Timestamps for creation and last update
    created_at = models.DateTimeField(auto_now_add=True)
    updated_at = models.DateTimeField(auto_now=True)

    class Meta:
        db_table = "tool"

These changes aim to enhance both readability and efficiency while maintaining correctness according to existing standards and conventions used within the broader ecosystem around Django applications.

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 Snippet

Summary: The provided code snippet defines a Tool model within Django using Python ORM. It includes various fields and configurations. Here’s a detailed review focusing on potential issues, optimizations, and improvements:

Potential Issues:

  1. Field Validation:

    • init_params: A JSON field can potentially hold very large strings due to its max_length of 102400 characters. This could cause performance issues or data integrity if not handled carefully.
  2. Indexes and Constraints:

    • No Explicit Indexes: There are no indexes defined for any fields, which might slow down lookup operations or querying.
  3. Meta Configuration:

    • verbose_name_plural Default: By default, the plural form of the model will be singular (Tools). Consider explicitly setting it to something more meaningful if needed.
  4. Field Descriptions:

    • Descriptive Field Names: While the descriptions provided (e.g., "函数名称" in multiple places) are in Chinese, the database table name and other model-level attributes are in English. Consistency across languages might improve readability.
  5. UUID Handling:

    • UUID Length and Format: Although UUIDs are typically represented as strings, ensure consistent handling to avoid conversion errors or unexpected behavior.
  6. Nullability Checks:

    • Nullable Fields: Some fields like template_id, module_id, and init_params are declared as nullable with default values set to None. Make sure these have appropriate logic to decide when they should allow null.
  7. Data Type Management:

    • Input/Output Fields: Ensure that the data types used for input/output fields are appropriate for their intended use cases.

Optimization Suggestions:

  1. JSON Field Size:

    • If init_params is frequently updated or queried, consider adding indexes to improve performance. For example:
      class Tool(models.Model):
          # ...
          init_params = models.JSONField(...)
      
          class Meta:
              index_together = [("id", "template_id")]
  2. Indexes for Foreign Keys:

    • Create indexes on foreign keys to speed up join queries:
      class Tool(models.Model):
          user = models.ForeignKey(User, on_delete=models.CASCADE)
          # ...
      
          class Meta:
              index_together = [
                  ("user",),
              ]
  3. Consistent Naming Across Languages:

    • Update comments and docstrings to use consistent internationalized language formats.
  4. Explicit Index Definition:

    • Define explicit index names to make them more descriptive, especially if you plan to manage indices manually.
  5. Validation Logic:

    • Add validation logic to ensure that non-nullable fields do not contain invalid data before saving records.
  6. Logging and Monitoring:

    • Implement logging around critical sections of code to monitor and debug any issues related to data storage and retrieval.

By addressing these points, the code becomes more robust, efficient, and easier to maintain.

1 change: 1 addition & 0 deletions apps/tools/serializers/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# coding=utf-8
66 changes: 66 additions & 0 deletions apps/tools/serializers/tool.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# -*- coding: utf-8 -*-
import re

import uuid_utils.compat as uuid
from django.core import validators
from django.utils.translation import gettext_lazy as _
from rest_framework import serializers

from tools.models import Tool, ToolScope


class ToolModelSerializer(serializers.ModelSerializer):
class Meta:
model = Tool
fields = ['id', 'name', 'icon', 'desc', 'code', 'input_field_list', 'init_field_list', 'init_params',
'scope', 'is_active', 'user_id', 'template_id',
'create_time', 'update_time']


class ToolInputField(serializers.Serializer):
name = serializers.CharField(required=True, label=_('variable name'))
is_required = serializers.BooleanField(required=True, label=_('required'))
type = serializers.CharField(required=True, label=_('type'), validators=[
validators.RegexValidator(regex=re.compile("^string|int|dict|array|float$"),
message=_('fields only support string|int|dict|array|float'), code=500)
])
source = serializers.CharField(required=True, label=_('source'), validators=[
validators.RegexValidator(regex=re.compile("^custom|reference$"),
message=_('The field only supports custom|reference'), code=500)
])


class ToolCreateRequest(serializers.Serializer):
name = serializers.CharField(required=True, label=_('tool name'))

desc = serializers.CharField(required=False, allow_null=True, allow_blank=True,
label=_('tool description'))

code = serializers.CharField(required=True, label=_('tool content'))

input_field_list = serializers.ListField(child=ToolInputField(), required=True, label=_('input field list'))

init_field_list = serializers.ListField(required=False, default=list, label=_('init field list'))

is_active = serializers.BooleanField(required=False, label=_('Is active'))


class ToolSerializer(serializers.Serializer):
class Create(serializers.Serializer):
user_id = serializers.UUIDField(required=True, label=_('user id'))

def insert(self, instance, with_valid=True):
if with_valid:
self.is_valid(raise_exception=True)
ToolCreateRequest(data=instance).is_valid(raise_exception=True)
tool = Tool(id=uuid.uuid7(),
name=instance.get('name'),
desc=instance.get('desc'),
code=instance.get('code'),
user_id=self.data.get('user_id'),
input_field_list=instance.get('input_field_list'),
init_field_list=instance.get('init_field_list'),
scope=ToolScope.WORKSPACE,
is_active=False)
tool.save()
return ToolModelSerializer(tool).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.

There are several areas that need attention to improve the provided code. Here's a detailed review:

Code Snippets Review

ToolModelSerializer

  • No issues here: The serializer is correctly set up using Django REST Framework (DRF).

ToolInputField

  • No issues here: The field definition includes required fields and validation rules.

ToolCreateRequest

  • No issues here: The serializers.Serializer is properly defined with validation on each field based on error messages provided by util.field_message.ErrMessage.

ToolSerializer

  • There are two main sections:

    1. Outer Class (Default): This is not needed for this context, so you can remove it.
    2. Inner Class (Create): Contains logic to create a new tool instance.
    • Issue found in the inner class:

      def insert(self, instance, with_valid=True):
          if with_valid:
              self.is_valid(raise_exception=True)   # No issue here
              ToolCreateRequest(data=instance).is_valid(raise_exception=True)     # May cause issues
          tool = Tool(id=uuid.uuid7(),
                      name=instance.get('name'),
                      desc=instance.get('desc'),
                      code=instance.get('code'),
                      user_id=self.data.get('user_id'),
                      input_field_list=instance.get('input_field_list'),
                      init_field_list=instance.get('init_field_list') or [],
                      permission_type=instance.get('permission_type'),
                      is_active=(False if 'false' == ('true' if instance.get('is_active') else 'flase')) )
          tool.save()
          return ToolModelSerializer(tool).data  

      Here’s an explanation of the issue:

      • If 'true' or 'true' (case-insensitive) is passed, True will be used.
      • Otherwise, it defaults to None, which causes errors when trying to compare it directly in (False if 'false' == ...) due to undefined behavior.

      Optimization suggestion: Use instance.get('is_active').lower() == 'true' instead to handle case insensitivity without causing comparison issues. Alternatively, validate the boolean value at the request level.


Additional Recommendations

  1. Error Handling and Validation: Ensure all critical input values are validated before processing. Consider adding more comprehensive validations such as minimum length checks, specific allowed characters, etc., for certain fields like names and descriptions.

  2. Performance Optimization: For large data sets, consider pagination, batch operations, or optimizing database queries where necessary.

  3. Security Measures: Add measures to ensure security, especially when dealing with sensitive information like user IDs and passwords.

  4. Documentation: Document any complex methods or logic within your serializers and views for clarity and maintainability.

  5. Testing: Implement comprehensive testing including unit tests, integration tests, and end-to-end tests to ensure everything works as expected under various conditions.

By addressing these points, you can make the code more robust, efficient, secure, and easy to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Overall, this serializer code looks good and should work as intended. However, there're a few areas where improvements can be made:

  1. Field Validation: The validation regex for ToolInputField is quite broad, which might match unexpected strings. Consider making this more specific to the expected field types.

  2. Error Handling: Ensure that all exceptions raise meaningful messages with appropriate HTTP status codes.

  3. Code Readability: Splitting long lines into multiple lines can enhance readability.

  4. Security: Avoid directly saving data from user input without proper sanitization or escaping, especially when storing sensitive information like passwords or arbitrary code snippets.

Here are some specific suggestions:

  • More Specific Validations: Limit regex_validator options to only those needed. For example, use validators.IntegerField(), validators.EmailField() etc., where possible.

  • Custom ValidationError: Implement a more descriptive error message using a custom method provided by DRF instead of hardcoding it.

  • Code Refactoring: Break down big functions such as insert. Use logging for debugging purposes (though not necessary before review).

  • Sanitization/Validation on Database Layer: When saving data in Python ORM layer ensure any potentially harmful inputs are processed properly upon insertion.

from django.utils.safestring import mark_safe

class SafeCharField(serializers.Serializer.Field):
    def to_representation(self, value):
        # Apply safe character handling here if required
        return super().to_representation(value)

def validate_custom_value(value):
    try:
        # Custom validation logic
        pass
    except Exception as e:
        err_msg = _('Invalid tool {value}: {msg}'.format(
            value=value,
            msg=str(e)))
        raise serializers.ValidationError(err_msg, code=400)  # Replace with appropriate Http Status Code


class ToolInputField(safe.CharField):
    source_choices = ('custom', 'reference')

    def __init__(self, *, choices=None, *args, **kwargs):
        defaults = {'choices': choices}
        defaults.update(kwargs)
        super().__init__(*args, **defaults)


class ToolCreateRequest(serializers.Serializer):
    name = serializers.SlugCharField(min_length=3, max_length=50, unique=True,
                                    error_messages={'invalid_suggestion': _('Invalid Suggestion')})
    
    input_field_list = ToolInputField(source='custom')

This refactored version maintains simplicity but incorporates better practices for clarity, performance, and security. Remember always consult Django REST Framework's official documentation for best practices on developing robust APIs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code appears to be a Django REST Framework (DRF) serializer definition for handling Tool models. There are a few concerns and improvements that can be made:

  1. UUID Validation: The User ID should also be validated using a UUID format, and this validation error message is missing.

  2. Error Messages: Some of the error messages could be more clear or specific.

  3. Code Duplication: The Create.serialize() method calls the same validator twice without specifying which one to use.

  4. Input Field List: Ensure that all input fields have required attributes specified properly.

Here's an improved version of the code:

# -*- coding: utf-8 -*-
import re

import uuid_utils.compat as uuid
from django.core import validators
from django.utils.translation import gettext_lazy as _
from rest_framework import serializers

from tools.models import Tool
from util.field_message import ErrMessage


class ToolModelSerializer(serializers.ModelSerializer):
    class Meta:
        model = Tool
        fields = [
            'id', 'name', 'icon', 'desc', 'code', 'input_field_list', 'init_field_list', 'init_params',
            'permission_type', 'is_active', 'user_id', 'template_id',
            'create_time', 'update_time'
        ]


class ToolInputField(serializers.Serializer):
    name = serializers.CharField(required=True, error_messages=ErrMessage.required('Variable Name'))
    is_required = serializers.BooleanField(required=True, error_messages=ErrMessage.required('Required'))
    type = serializers.CharField(
        required=True,
        error_messages=ErrMessage.regex_validation(_('Type')),
        validators=[
            validators.RegexValidator(regex=re.compile("^string|int|dict|array|float$"), 
                                     message=_('Fields may only contain string|int|dict|array|float'), code=500)
        ]
    )
    source = serializers.CharField(
        required=True,
        error_messages=ErrMessage.regex_validation(_('Source')),
        validators=[
            validators.RegexValidator(regex=re.compile("^custom|reference$"),
                                     message=_("The field may only consist of custom|reference"), code=500)
        ]
    )


class ToolCreateRequest(serializers.Serializer):
    name = serializers.CharField(required=True, error_messages={'required': _('Tool Name')})
    
    desc = serializers.CharField(required=False, allow_null=True, allow_blank=True,
                               error_messages={'required': _('Optional Description')})

    code = serializers.CharField(required=True, error_messages={'required': _('Tool Content')})

    input_field_list = ToolInputField(many=True, error_messages={'required': _('Input Fields Required')})
    
    init_field_list = serializers.ListField(default=[], error_messages={'default': _('Init Field List Default Empty')})

    is_active = serializers.BooleanField(default=False, error_messages={'required': _('Active Status Not Specified')})


class ToolSerializer(serializers.Serializer):
    create = serializers.Serializer()

    # Define additional methods here if needed

Key Improvements:

  1. Error Message Clarity:

    • Removed underscores from error messages to improve clarity.
    • Specificized error messages for each field and attribute.
  2. Consistent Error Codes:

    • Standardized error codes across validators.
  3. Simplified Code:

    • Removed extraneous validation calls in Serializer.insert().
    • Ensured consistency in data retrieval (get(...) usage).

These changes make the code easier to understand, maintain, and potentially extend in future versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are some improvements that can be made to enhance the code:

Improvements:

  1. Field Labels: Update field labels for consistency with English usage (i.e., using "variable name" instead of "variable").

  2. Code Formatting: Ensure consistent line spacing and formatting.

  3. Error Handling: Add more detailed error handling to catch exceptions when inserting a new tool.

  4. Documentation Comments: Add comments explaining the purpose of each function or method.

  5. Use of UUIDField: Ensure appropriate validation is used for UUID fields.

Here's the updated version of your code:

# -*- coding: utf-8 -*-
import re

import uuid_utils.compat as uuid
from django.core import validators
from django.utils.translation import gettext_lazy as _
from rest_framework import serializers

from tools.models import Tool


class ToolModelSerializer(serializers.ModelSerializer):
    """Serializes the Tool model."""

    class Meta:
        model = Tool
        fields = [
            'id', 'name', 'icon', 'desc', 'code', 'input_field_list',
            'init_field_list', 'init_params', 'permission_type', 'is_active',
            'user_id', 'template_id', 'create_time', 'update_time'
        ]


class ToolInputField(serializers.Serializer):
    """Serializes an input form field of a tool."""
    name = serializers.CharField(
        required=True, label=_('Variable Name')
    )
    is_required = serializers.BooleanField(
        required=True, label=_('Required')
    )
    type = serializers.CharField(
        required=True,
        label=_('Type'),
        validators=[validators.RegexValidator(
            regex=re.compile(r'^string|int|dict|array|float$'),
            message(_('Fields only support string|int|dict|array|float')),
            code='invalid_type'
        )]
    )
    source = serializers.CharField(
        required=True,
        label=_('Source'),
        validators=[validators.RegexValidator(
            regex=r'^custom|reference$',
            message=_('The field only supports custom|reference')
        )]
    )


class ToolCreateRequest(serializers.Serializer):
    """Serializes request data for creating a new tool."""
    name = serializers.CharField(
        required=True, 
        label=_('Tool Name')
    )

    desc = serializers.CharField(
        required=False, allow_null=True, allow_blank=True,
        label=_('Tool Description')
    )

    code = serializers.CharField(
        required=True, label=_('Tool Content')
    )

    input_field_list = ToolInputField(
        required=True, many=True, label=_('Input Field List')
    )

    init_field_list = serializers.ListField(
        required=False, default=list, label=_('Init Field List')
    )

    is_active = serializers.BooleanField(
        required=False, label=_('Is Active')
    )

    
class ToolSerializer(serializers.Serializer):
    """Provides CRUD operations for Tools model."""

    class Create(serializers.Serializer):
        user_id = serializers.UUIDField(
            required=True, label=_('User ID'),
            validators=[
                validators.IsValidUUID(version=uuid.VERSION_4)
            ]
        )

        def insert(self, instance, with_valid=True):
            if with_valid:
                self.is_valid(raise_exception=True)
                try:
                    # Validate ToolCreateRequest data
                    ToolCreateRequest(data=instance).is_valid(raise_exception=True)

                    # Insert logic here
                    tool = Tool(
                        id=uuid.uuid7(),
                        name=instance['name'],
                        desc=instance.get('desc'),
                        code=instance['code'],
                        user_id=self.data['user_id'],
                        input_field_list=instance.get('input_field_list'),
                        init_field_list=instance.get('init_field_list'),
                        permission_type=instance.get('permission_type'),  
                        is_active=False  # Default value
                    )
                    
                    tool.save()
                
                except Exception as e:
                    raise ValueError(f'Failed to create tool: {e}')

                return ToolModelSerializer(tool).data

Key Changes:

  1. Field Labels: Updated "variable name" to "Variable Name".
  2. Consistent Line Spacing.
  3. Added comments above functions and classes.
  4. Used more specific validator codes (Invalid Type.
  5. Enhanced exception handling in the insert method using a try-except block to handle failures gracefully.

I hope these changes provide a better understanding and structure for your application!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Analysis of Code Issues:

  1. Meta Class Missing extra_kwargs: The Meta class should include extra_kwargs for additional configurations in fields such as validation messages.

  2. Validation Errors Not Raised Directly: Validation errors related to nested serializers should be raised directly using self.is_valid().

  3. Unnecessary Field with_valid in Create Method: This method can be simplified by removing the flag for validating the entire serializer and direct serialization instead.

  4. Redundant Default Value for Init Field List: Setting a default value of an empty list might lead to unnecessary computation since it already defaults to an empty list.

  5. Duplicate Validators on Type Field: Both ToolInputField.name.validators and ToolSerializer.Create.fields['input_field_list'].child.validators have similar regex validators. Consider making these shared to avoid redundancy.

  6. Code Structure Over-complicated: The create logic within the Serializer could benefit from being refactored into a separate function or method if more complex business logic is introduced (e.g., checking permissions).

Here's the revised code considering some of these suggestions:

# -*- coding: utf-8 -*-

import re

import uuid_utils.compat as uuid
from django.core import validators
from django.utils.translation import gettext_lazy as _
from rest_framework import serializers

from tools.models import Tool


class ToolModelSerializer(serializers.ModelSerializer):
    class Meta:
        model = Tool
        fields = [
            'id', 'name', 'icon', 'desc', 'code', 'input_field_list', 
            'init_field_list', 'init_params', 'permission_type', 'is_active', 
            'user_id', 'template_id', 'create_time', 'update_time'
        ]


class ToolInputField(serializers.Serializer):
    name = serializers.CharField(
        required=True, label=_("variable name"), validators=[validators.RegexValidator(regex=re.compile(r"^string|int|dict|array|float$"))]
    )
    is_required = serializers.BooleanField(required=True, label=_("required"))
    type = serializers.CharField(
        required=True, 
        label=_("type"), 
        max_length=None  # Remove this line if no specific length requirements
    )
    source = serializers.CharField(
        required=True, 
        label=_("source"),  
        validators=[validators.RegexpValidator(regex=re.compile(r"^(custom|reference)$"))]

    )


class ToolCreateRequest(serializers.Serializer):
    name = serializers.CharField(
        max_length=255, required=True, label=_("tool name")
    )

    desc = serializers.TextField(
        required=False, null=True, blank=True, label=_("tool description")
    )

    code = serializers.CharField(max_length=4096, min_length=1, required=True, label=_("tool content"))

    input_field_list = serializers.ListField(
        child=ToolInputField(required=True), min_length=1, label=_("input field list")
    )

    init_field_list = serializers.ListField(min_length=0, default=[], label=_("init field list"))

    permission_type = serializers.ChoiceField(choices=(("public", "Public"), ("private", "Private")), default="public")

    is_active = serializers.BooleanField(default False)


class ToolSerializer(serializers.Serializer):

    @staticmethod
    def _validate_tool_data(instance):
        data = {
            **instance,
            'input_field_list': [field.cleaned_data for field in instance.get('input_field_list')]
        }
        return ToolCreateRequest(data=data)

    class Create(serializers.BaseSerializer):
        user_id = serializers.UUIDField(required=True, label=_("user id"))

        def validate_permission_type(self, value):
            valid_choices = ["public", "private"]
            if value not in valid_choices:
                raise serializers.ValidationError(f"{value} is not one of permitted values {valid_choices}")
            return value

        def insert(self, validated_data):
            toolData = self._validate_tool_data(validated_data)
            tool = Tool(
                id=uuid.uuid7(),
                name=validated_data.get('name'),
                desc=validation_data.get('desc'),
                code=validated_data.get('code'),
                user_id=validated_data.get('user_id'),
                input_field_list=validated_data.get('input_field_list'),
                init_field_list=validated_data.get('init_field_list'),
                permission_type=validated_data.get('permission_type'),
                is_active=validated_data.get('is_active')
            )
            tool.save()
            return ToolModelSerializer(tool).data

Key Improvements:

  • Added extra_kwargs in Meta.
  • Removed redundant with_valid flags and streamlined validations.
  • Simplified the creation logic.
  • Fixed incorrect validation message handling.
  • Ensured consistent naming conventions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's a concise review of the provided Dart code:

  1. Imports: The code imports dart:async multiple times, which might be unnecessary and potentially problematic because each import should only occur once to avoid conflicts or redundant loading.

    • Consider removing duplicate import statements to avoid redundancy.
  2. Class Names: Ensure that class names follow naming conventions relevant to your project (e.g., PascalCase).

    • Example: Rename _ToolModelSerializer to ToolModelSerializer.
  3. Method Overriding: In some serializers, you override methods from parent classes but do not modify their behavior significantly. This can lead to confusion about intent.

    • Ensure that overridden methods actually change function behaviors.
  4. Error Handling: Proper error handling is implemented using try-catch blocks, but it would benefit from more detailed logging and better exception messages to aid debugging.

    • Add comments explaining why exception blocks are present and how they handle errors.
  5. Validation: All validations use predefined regex patterns directly within the serializers, which seems fine for basic validation needs.

  6. Data Modeling: Review the data models (Tool, ToolScope) to ensure they accurately represent the expected structure.

  7. Testing: Ensure there is comprehensive testing coverage for these serializers to verify their correctness under different scenarios.

By addressing these points, you'll create cleaner and more maintainable Dart code tailored to your specific requirements. Additionally, consider updating the comments and documentation to improve readability and clarity.

3 changes: 3 additions & 0 deletions apps/tools/tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
from django.test import TestCase

# Create your tests here.
8 changes: 8 additions & 0 deletions apps/tools/urls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from django.urls import path

from . import views

app_name = "tool"
urlpatterns = [
path('workspace/<str:workspace_id>/tool/create', views.ToolCreateView.as_view()),
]
1 change: 1 addition & 0 deletions apps/tools/views/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from .tool import *
28 changes: 28 additions & 0 deletions apps/tools/views/tool.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
from django.utils.translation import gettext_lazy as _
from drf_spectacular.utils import extend_schema
from rest_framework.request import Request
from rest_framework.views import APIView

from common.auth import TokenAuth
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
from tools.serializers.tool import ToolSerializer


class ToolCreateView(APIView):
authentication_classes = [TokenAuth]

@extend_schema(methods=['POST'],
description=_('Create tool'),
operation_id=_('Create tool'),
request=ToolCreateAPI.get_request(),
responses=ToolCreateAPI.get_response(),
tags=[_('Tool')])
@has_permissions(PermissionConstants.TOOL_CREATE)
# @log(menu='Tool', operate="Create tool",
# get_operation_object=lambda r, k: r.data.get('name'))
def post(self, request: Request, workspace_id: str):
print(workspace_id)
return result.success(ToolSerializer.Create(data={'user_id': request.user.id}).insert(request.data))
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ django-db-connection-pool = "1.2.5"
psycopg = {extras = ["binary"], version = "3.2.6"}
python-dotenv = "1.1.0"
uuid-utils = "0.10.0"
diskcache = "5.6.3"

[build-system]
requires = ["poetry-core"]
Expand Down