From 06f9229858d59d9ebeb731684464abee0543b3d0 Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Thu, 13 Jul 2023 16:40:44 +0300 Subject: [PATCH 1/2] Fix test data to conform to the API schema for attributes --- cvat/apps/dataset_manager/tests/test_formats.py | 9 ++++++--- cvat/apps/engine/tests/test_rest_api.py | 15 ++++++++++----- cvat/apps/lambda_manager/tests/assets/tasks.json | 6 ++++-- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/cvat/apps/dataset_manager/tests/test_formats.py b/cvat/apps/dataset_manager/tests/test_formats.py index e1fa84d874d..c710013d547 100644 --- a/cvat/apps/dataset_manager/tests/test_formats.py +++ b/cvat/apps/dataset_manager/tests/test_formats.py @@ -259,7 +259,8 @@ def _generate_task(self, images, **overrides): "name": "parked", "mutable": True, "input_type": "checkbox", - "default_value": False + "default_value": "false", + "values": [], }, ] }, @@ -561,7 +562,8 @@ def _generate_task(self, images): "name": "parked", "mutable": True, "input_type": "checkbox", - "default_value": False + "default_value": "false", + "values": [], }, ] }, @@ -723,7 +725,8 @@ def _generate_task(self, images, annotation_format, **overrides): "name": "parked", "mutable": True, "input_type": "checkbox", - "default_value": False + "default_value": "false", + "values": [], } ] }, diff --git a/cvat/apps/engine/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py index 5663e47e4a7..4cb37551c55 100644 --- a/cvat/apps/engine/tests/test_rest_api.py +++ b/cvat/apps/engine/tests/test_rest_api.py @@ -1811,7 +1811,8 @@ def _create_project(project_data): "name": "bool_attribute", "mutable": True, "input_type": AttributeType.CHECKBOX, - "default_value": "true" + "default_value": "true", + "values": [], }], }, { "name": "person", @@ -2572,7 +2573,8 @@ def test_api_v2_tasks_admin(self): "name": "my_attribute", "mutable": True, "input_type": AttributeType.CHECKBOX, - "default_value": "true" + "default_value": "true", + "values": [], }] }] } @@ -2895,7 +2897,8 @@ def _create_task(task_data, media_data): "name": "bool_attribute", "mutable": True, "input_type": AttributeType.CHECKBOX, - "default_value": "true" + "default_value": "true", + "values": [], }], }, { "name": "person", @@ -2915,7 +2918,8 @@ def _create_task(task_data, media_data): "name": "bool_attribute", "mutable": True, "input_type": AttributeType.CHECKBOX, - "default_value": "true" + "default_value": "true", + "values": [], }], }, { "name": "person", @@ -4649,7 +4653,8 @@ def _create_task(self, owner, assignee, annotation_format=""): "name": "parked", "mutable": True, "input_type": "checkbox", - "default_value": "false" + "default_value": "false", + "values": [], }, ] }, diff --git a/cvat/apps/lambda_manager/tests/assets/tasks.json b/cvat/apps/lambda_manager/tests/assets/tasks.json index 019716f3896..7ae07a1c0d0 100644 --- a/cvat/apps/lambda_manager/tests/assets/tasks.json +++ b/cvat/apps/lambda_manager/tests/assets/tasks.json @@ -18,7 +18,8 @@ "name": "parked", "mutable": true, "input_type": "checkbox", - "default_value": false + "default_value": "false", + "values": [] } ] }, @@ -47,7 +48,8 @@ "name": "parked", "mutable": true, "input_type": "checkbox", - "default_value": false + "default_value": "false", + "values": [] } ] }, From d5c66e5b539f5cd4156d69f88a1f49fbba04e27c Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Mon, 10 Jul 2023 16:12:06 +0300 Subject: [PATCH 2/2] Fix validation in `AttributeSerializer` The current implementation of `to_internal_value` doesn't call the base class method, which is supposed to carry out generic validation of serializer fields. This means that we essentially don't validate attribute fields beyond what is done incidentally by database contraints. Fix it by removing the overridden method entirely, and adding a special field class for the `values` field. --- CHANGELOG.md | 2 ++ cvat/apps/engine/serializers.py | 24 ++++++++---------------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 604f34deb96..d3d4b708240 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - \[SDK\] SDK should not change input data in models () - 3D job can not be opened in validation mode () - Memory leak related to unclosed av container () +- Server-side validation for attribute specifications + () ### Security - TDB diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index f3303ea0ab0..52987755f25 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -212,8 +212,15 @@ class Meta: 'last_login': { 'allow_null': True } } +class DelimitedStringListField(serializers.ListField): + def to_representation(self, value): + return super().to_representation(value.split('\n')) + + def to_internal_value(self, data): + return '\n'.join(super().to_internal_value(data)) + class AttributeSerializer(serializers.ModelSerializer): - values = serializers.ListField(allow_empty=True, + values = DelimitedStringListField(allow_empty=True, child=serializers.CharField(allow_blank=True, max_length=200), ) @@ -221,21 +228,6 @@ class Meta: model = models.AttributeSpec fields = ('id', 'name', 'mutable', 'input_type', 'default_value', 'values') - # pylint: disable=no-self-use - def to_internal_value(self, data): - attribute = data.copy() - attribute['values'] = '\n'.join(data.get('values', [])) - return attribute - - def to_representation(self, instance): - if instance: - rep = super().to_representation(instance) - rep['values'] = instance.values.split('\n') - else: - rep = instance - - return rep - class SublabelSerializer(serializers.ModelSerializer): id = serializers.IntegerField(required=False) attributes = AttributeSerializer(many=True, source='attributespec_set', default=[],