diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index b6be62d9b..c07b00f78 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -2,6 +2,7 @@ import datetime import math import re import zoneinfo +from decimal import Decimal import magic from celery import states @@ -9,7 +10,9 @@ from django.conf import settings from django.contrib.auth.models import Group from django.contrib.auth.models import User from django.contrib.contenttypes.models import ContentType -from django.core.validators import URLValidator +from django.core.validators import DecimalValidator +from django.core.validators import MaxLengthValidator +from django.core.validators import integer_validator from django.utils.crypto import get_random_string from django.utils.text import slugify from django.utils.translation import gettext as _ @@ -41,6 +44,7 @@ from documents.models import UiSettings from documents.parsers import is_mime_type_supported from documents.permissions import get_groups_with_only_permission from documents.permissions import set_permissions_for_object +from documents.validators import uri_validator # https://www.django-rest-framework.org/api-guide/serializers/#example @@ -489,17 +493,29 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): def validate(self, data): """ - For some reason, URLField validation is not run against the value - automatically. Force it to run against the value + Probably because we're kind of doing it odd, validation from the model + doesn't run against the field "value", so we have to re-create it here. + + Don't like it, but it is better than returning an HTTP 500 when the database + hates the value """ data = super().validate(data) field: CustomField = data["field"] - if ( - field.data_type == CustomField.FieldDataType.URL - and data["value"] is not None - and len(data["value"]) > 0 - ): - URLValidator()(data["value"]) + if "value" in data and data["value"] is not None: + if ( + field.data_type == CustomField.FieldDataType.URL + and len(data["value"]) > 0 + ): + uri_validator(data["value"]) + elif field.data_type == CustomField.FieldDataType.INT: + integer_validator(data["value"]) + elif field.data_type == CustomField.FieldDataType.MONETARY: + DecimalValidator(max_digits=12, decimal_places=2)( + Decimal(str(data["value"])), + ) + elif field.data_type == CustomField.FieldDataType.STRING: + MaxLengthValidator(limit_value=128)(data["value"]) + return data def reflect_doclinks( diff --git a/src/documents/tests/test_api_custom_fields.py b/src/documents/tests/test_api_custom_fields.py index 15abcd053..690e92690 100644 --- a/src/documents/tests/test_api_custom_fields.py +++ b/src/documents/tests/test_api_custom_fields.py @@ -333,19 +333,17 @@ class TestCustomField(DirectoriesMixin, APITestCase): }, format="json", ) - from pprint import pprint - pprint(resp.json()) self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(CustomFieldInstance.objects.count(), 0) self.assertEqual(len(doc.custom_fields.all()), 0) - def test_custom_field_value_validation(self): + def test_custom_field_value_url_validation(self): """ GIVEN: - Document & custom field exist WHEN: - - API request to set a field value + - API request to set a field value to something which is or is not a link THEN: - HTTP 400 is returned - No field instance is created or attached to the document @@ -360,31 +358,62 @@ class TestCustomField(DirectoriesMixin, APITestCase): name="Test Custom Field URL", data_type=CustomField.FieldDataType.URL, ) - custom_field_int = CustomField.objects.create( - name="Test Custom Field INT", - data_type=CustomField.FieldDataType.INT, - ) + for value in ["not a url", "file:"]: + with self.subTest(f"Test value {value}"): + resp = self.client.patch( + f"/api/documents/{doc.id}/", + data={ + "custom_fields": [ + { + "field": custom_field_url.id, + "value": value, + }, + ], + }, + format="json", + ) + + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(CustomFieldInstance.objects.count(), 0) + self.assertEqual(len(doc.custom_fields.all()), 0) resp = self.client.patch( f"/api/documents/{doc.id}/", data={ "custom_fields": [ { "field": custom_field_url.id, - "value": "not a url", + "value": "tel:+1-816-555-1212", }, ], }, format="json", ) - self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(CustomFieldInstance.objects.count(), 0) - self.assertEqual(len(doc.custom_fields.all()), 0) + self.assertEqual(resp.status_code, status.HTTP_200_OK) - self.assertRaises( - Exception, - self.client.patch, + def test_custom_field_value_integer_validation(self): + """ + GIVEN: + - Document & custom field exist + WHEN: + - API request to set a field value to something not an integer + THEN: + - HTTP 400 is returned + - No field instance is created or attached to the document + """ + doc = Document.objects.create( + title="WOW", + content="the content", + checksum="123", + mime_type="application/pdf", + ) + custom_field_int = CustomField.objects.create( + name="Test Custom Field INT", + data_type=CustomField.FieldDataType.INT, + ) + + resp = self.client.patch( f"/api/documents/{doc.id}/", data={ "custom_fields": [ @@ -397,6 +426,81 @@ class TestCustomField(DirectoriesMixin, APITestCase): format="json", ) + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(CustomFieldInstance.objects.count(), 0) + self.assertEqual(len(doc.custom_fields.all()), 0) + + def test_custom_field_value_monetary_validation(self): + """ + GIVEN: + - Document & custom field exist + WHEN: + - API request to set a field value to something not a valid monetary decimal + THEN: + - HTTP 400 is returned + - No field instance is created or attached to the document + """ + doc = Document.objects.create( + title="WOW", + content="the content", + checksum="123", + mime_type="application/pdf", + ) + custom_field_money = CustomField.objects.create( + name="Test Custom Field MONETARY", + data_type=CustomField.FieldDataType.MONETARY, + ) + + resp = self.client.patch( + f"/api/documents/{doc.id}/", + data={ + "custom_fields": [ + { + "field": custom_field_money.id, + # Too many places past decimal + "value": 12.123, + }, + ], + }, + format="json", + ) + + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(CustomFieldInstance.objects.count(), 0) + self.assertEqual(len(doc.custom_fields.all()), 0) + + def test_custom_field_value_short_text_validation(self): + """ + GIVEN: + - Document & custom field exist + WHEN: + - API request to set a field value to a too long string + THEN: + - HTTP 400 is returned + - No field instance is created or attached to the document + """ + doc = Document.objects.create( + title="WOW", + content="the content", + checksum="123", + mime_type="application/pdf", + ) + custom_field_string = CustomField.objects.create( + name="Test Custom Field STRING", + data_type=CustomField.FieldDataType.STRING, + ) + + resp = self.client.patch( + f"/api/documents/{doc.id}/", + data={ + "custom_fields": [ + {"field": custom_field_string.id, "value": "a" * 129}, + ], + }, + format="json", + ) + + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(CustomFieldInstance.objects.count(), 0) self.assertEqual(len(doc.custom_fields.all()), 0) diff --git a/src/documents/validators.py b/src/documents/validators.py new file mode 100644 index 000000000..0ebf15697 --- /dev/null +++ b/src/documents/validators.py @@ -0,0 +1,29 @@ +from urllib.parse import urlparse + +from django.core.exceptions import ValidationError +from django.utils.translation import gettext_lazy as _ + + +def uri_validator(value) -> None: + """ + Raises a ValidationError if the given value does not parse as an + URI looking thing, which we're defining as a scheme and either network + location or path value + """ + try: + parts = urlparse(value) + if not parts.scheme: + raise ValidationError( + _(f"Unable to parse URI {value}, missing scheme"), + params={"value": value}, + ) + elif not parts.netloc and not parts.path: + raise ValidationError( + _(f"Unable to parse URI {value}, missing net location or path"), + params={"value": value}, + ) + except Exception as e: + raise ValidationError( + _(f"Unable to parse URI {value}"), + params={"value": value}, + ) from e