Fix validation in `AttributeSerializer` (#6447)

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

@@ -32,6 +32,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed SAM plugin (403 code for workers in organizations) (<https://github.com/opencv/cvat/pull/6514>)
 - Using initial frame from query parameter to open specific frame in a job
  (<https://github.com/opencv/cvat/pull/6506>)
+- Server-side validation for attribute specifications
+  (<https://github.com/opencv/cvat/pull/6447>)
 
 ### Security
 - TDB

cvat/apps/dataset_manager/tests/test_formats.py

@@ -259,7 +259,8 @@ class TaskExportTest(_DbTestBase):
                             "name": "parked",
                             "mutable": True,
                             "input_type": "checkbox",
-                            "default_value": False
+                            "default_value": "false",
+                            "values": [],
                         },
                     ]
                 },
@@ -561,7 +562,8 @@ class FrameMatchingTest(_DbTestBase):
                             "name": "parked",
                             "mutable": True,
                             "input_type": "checkbox",
-                            "default_value": False
+                            "default_value": "false",
+                            "values": [],
                         },
                     ]
                 },
@@ -723,7 +725,8 @@ class TaskAnnotationsImportTest(_DbTestBase):
                             "name": "parked",
                             "mutable": True,
                             "input_type": "checkbox",
-                            "default_value": False
+                            "default_value": "false",
+                            "values": [],
                         }
                     ]
                 },

cvat/apps/engine/serializers.py

@@ -212,8 +212,15 @@ class UserSerializer(serializers.ModelSerializer):
             '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 AttributeSerializer(serializers.ModelSerializer):
         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=[],

cvat/apps/engine/tests/test_rest_api.py

@@ -1811,7 +1811,8 @@ class ProjectImportExportAPITestCase(ApiTestBase):
                             "name": "bool_attribute",
                             "mutable": True,
                             "input_type": AttributeType.CHECKBOX,
-                            "default_value": "true"
+                            "default_value": "true",
+                            "values": [],
                         }],
                     }, {
                         "name": "person",
@@ -2572,7 +2573,8 @@ class TaskCreateAPITestCase(ApiTestBase):
                     "name": "my_attribute",
                     "mutable": True,
                     "input_type": AttributeType.CHECKBOX,
-                    "default_value": "true"
+                    "default_value": "true",
+                    "values": [],
                 }]
             }]
         }
@@ -2895,7 +2897,8 @@ class TaskImportExportAPITestCase(ApiTestBase):
                         "name": "bool_attribute",
                         "mutable": True,
                         "input_type": AttributeType.CHECKBOX,
-                        "default_value": "true"
+                        "default_value": "true",
+                        "values": [],
                     }],
                     }, {
                         "name": "person",
@@ -2915,7 +2918,8 @@ class TaskImportExportAPITestCase(ApiTestBase):
                         "name": "bool_attribute",
                         "mutable": True,
                         "input_type": AttributeType.CHECKBOX,
-                        "default_value": "true"
+                        "default_value": "true",
+                        "values": [],
                     }],
                     }, {
                         "name": "person",
@@ -4649,7 +4653,8 @@ class JobAnnotationAPITestCase(ApiTestBase):
                             "name": "parked",
                             "mutable": True,
                             "input_type": "checkbox",
-                            "default_value": "false"
+                            "default_value": "false",
+                            "values": [],
                         },
                     ]
                 },

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": []
           }
         ]
       },