From c28c80fd4cd249ff54ae895563122e8e7b02f9c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Sun, 3 Jan 2021 13:40:06 +0100 Subject: [PATCH] Add option for disabled bookmark URL validation (#36) --- .../migrations/0005_auto_20210103_1212.py | 19 ++++ bookmarks/models.py | 5 +- bookmarks/tests/test_bookmark_validation.py | 90 +++++++++++++++++++ bookmarks/validators.py | 14 +++ siteroot/settings/base.py | 3 + 5 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 bookmarks/migrations/0005_auto_20210103_1212.py create mode 100644 bookmarks/tests/test_bookmark_validation.py create mode 100644 bookmarks/validators.py diff --git a/bookmarks/migrations/0005_auto_20210103_1212.py b/bookmarks/migrations/0005_auto_20210103_1212.py new file mode 100644 index 0000000..11e5a63 --- /dev/null +++ b/bookmarks/migrations/0005_auto_20210103_1212.py @@ -0,0 +1,19 @@ +# Generated by Django 2.2.13 on 2021-01-03 12:12 + +import bookmarks.validators +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('bookmarks', '0004_auto_20200926_1028'), + ] + + operations = [ + migrations.AlterField( + model_name='bookmark', + name='url', + field=models.CharField(max_length=2048, validators=[bookmarks.validators.BookmarkURLValidator()]), + ), + ] diff --git a/bookmarks/models.py b/bookmarks/models.py index 613f139..b5e08b1 100644 --- a/bookmarks/models.py +++ b/bookmarks/models.py @@ -5,6 +5,7 @@ from django.contrib.auth import get_user_model from django.db import models from bookmarks.utils import unique +from bookmarks.validators import BookmarkURLValidator class Tag(models.Model): @@ -32,7 +33,7 @@ def build_tag_string(tag_names: List[str], delimiter: str = ','): class Bookmark(models.Model): - url = models.URLField(max_length=2048) + url = models.CharField(max_length=2048, validators=[BookmarkURLValidator()]) title = models.CharField(max_length=512, blank=True) description = models.TextField(blank=True) website_title = models.CharField(max_length=512, blank=True, null=True) @@ -71,7 +72,7 @@ class Bookmark(models.Model): class BookmarkForm(forms.ModelForm): # Use URLField for URL - url = forms.URLField() + url = forms.CharField(validators=[BookmarkURLValidator()]) tag_string = forms.CharField(required=False) # Do not require title and description in form as we fill these automatically if they are empty title = forms.CharField(max_length=512, diff --git a/bookmarks/tests/test_bookmark_validation.py b/bookmarks/tests/test_bookmark_validation.py new file mode 100644 index 0000000..bae6056 --- /dev/null +++ b/bookmarks/tests/test_bookmark_validation.py @@ -0,0 +1,90 @@ +import datetime + +from django.contrib.auth import get_user_model +from django.core.exceptions import ValidationError +from django.test import TestCase, override_settings + +from bookmarks.models import BookmarkForm, Bookmark + +User = get_user_model() + +ENABLED_URL_VALIDATION_TEST_CASES = [ + ('thisisnotavalidurl', False), + ('http://domain', False), + ('unknownscheme://domain.com', False), + ('http://domain.com', True), + ('http://www.domain.com', True), + ('https://domain.com', True), + ('https://www.domain.com', True), +] + +DISABLED_URL_VALIDATION_TEST_CASES = [ + ('thisisnotavalidurl', True), + ('http://domain', True), + ('unknownscheme://domain.com', True), + ('http://domain.com', True), + ('http://www.domain.com', True), + ('https://domain.com', True), + ('https://www.domain.com', True), +] + + +class BookmarkValidationTestCase(TestCase): + + def setUp(self) -> None: + self.user = User.objects.create_user('testuser', 'test@example.com', 'password123') + + @override_settings(LD_DISABLE_URL_VALIDATION=False) + def test_bookmark_model_should_validate_url_if_not_disabled_in_settings(self): + self._run_bookmark_model_url_validity_checks(ENABLED_URL_VALIDATION_TEST_CASES) + + @override_settings(LD_DISABLE_URL_VALIDATION=True) + def test_bookmark_model_should_not_validate_url_if_disabled_in_settings(self): + self._run_bookmark_model_url_validity_checks(DISABLED_URL_VALIDATION_TEST_CASES) + + def test_bookmark_form_should_validate_required_fields(self): + form = BookmarkForm(data={'url': ''}) + + self.assertEqual(len(form.errors), 1) + self.assertIn('required', str(form.errors)) + + form = BookmarkForm(data={'url': None}) + + self.assertEqual(len(form.errors), 1) + self.assertIn('required', str(form.errors)) + + @override_settings(LD_DISABLE_URL_VALIDATION=False) + def test_bookmark_form_should_validate_url_if_not_disabled_in_settings(self): + self._run_bookmark_form_url_validity_checks(ENABLED_URL_VALIDATION_TEST_CASES) + + @override_settings(LD_DISABLE_URL_VALIDATION=True) + def test_bookmark_form_should_not_validate_url_if_disabled_in_settings(self): + self._run_bookmark_form_url_validity_checks(DISABLED_URL_VALIDATION_TEST_CASES) + + def _run_bookmark_model_url_validity_checks(self, cases): + for case in cases: + url, expectation = case + bookmark = Bookmark( + url=url, + date_added=datetime.datetime.now(), + date_modified=datetime.datetime.now(), + owner=self.user + ) + + try: + bookmark.full_clean() + self.assertTrue(expectation, 'Did not expect validation error') + except ValidationError as e: + self.assertFalse(expectation, 'Expected validation error') + self.assertTrue('url' in e.message_dict, 'Expected URL validation to fail') + + def _run_bookmark_form_url_validity_checks(self, cases): + for case in cases: + url, expectation = case + form = BookmarkForm(data={'url': url}) + + if expectation: + self.assertEqual(len(form.errors), 0) + else: + self.assertEqual(len(form.errors), 1) + self.assertIn('Enter a valid URL', str(form.errors)) diff --git a/bookmarks/validators.py b/bookmarks/validators.py new file mode 100644 index 0000000..ec88743 --- /dev/null +++ b/bookmarks/validators.py @@ -0,0 +1,14 @@ +from django.conf import settings +from django.core import validators + + +class BookmarkURLValidator(validators.URLValidator): + """ + Extends default Django URLValidator and cancels validation if it is disabled in settings. + This allows to switch URL validation on/off dynamically which helps with testing + """ + def __call__(self, value): + if settings.LD_DISABLE_URL_VALIDATION: + return + + super().__call__(value) \ No newline at end of file diff --git a/siteroot/settings/base.py b/siteroot/settings/base.py index 535c667..f225b27 100644 --- a/siteroot/settings/base.py +++ b/siteroot/settings/base.py @@ -157,3 +157,6 @@ REST_FRAMEWORK = { # Registration switch ALLOW_REGISTRATION = False + +# URL validation flag +LD_DISABLE_URL_VALIDATION = os.getenv('LD_DISABLE_URL_VALIDATION', False) in (True, 'True', '1')