From 8047ba6c63830224c8cac2bad8a78fa82bd36e70 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?=
 <sascha.issbruecker@googlemail.com>
Date: Wed, 25 Aug 2021 09:20:01 +0200
Subject: [PATCH] Fix importer not validating bookmark models (#149)

---
 bookmarks/services/importer.py               |  1 +
 bookmarks/tests/helpers.py                   | 12 +++++++
 bookmarks/tests/test_bookmark_validation.py  | 21 +++++++++++++
 bookmarks/tests/test_importer.py             | 33 ++++++++++++++++++++
 bookmarks/tests/test_settings_import_view.py |  4 ++-
 5 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 bookmarks/tests/test_importer.py

diff --git a/bookmarks/services/importer.py b/bookmarks/services/importer.py
index a315019..dd3067b 100644
--- a/bookmarks/services/importer.py
+++ b/bookmarks/services/importer.py
@@ -57,6 +57,7 @@ def _import_bookmark_tag(netscape_bookmark: NetscapeBookmark, user: User):
         bookmark.description = netscape_bookmark.description
     bookmark.owner = user
 
+    bookmark.full_clean()
     bookmark.save()
 
     # Set tags
diff --git a/bookmarks/tests/helpers.py b/bookmarks/tests/helpers.py
index a95f50d..b285e45 100644
--- a/bookmarks/tests/helpers.py
+++ b/bookmarks/tests/helpers.py
@@ -1,4 +1,5 @@
 import random
+import logging
 
 from django.contrib.auth.models import User
 from django.utils import timezone
@@ -117,3 +118,14 @@ def random_sentence(num_words: int = None, including_word: str = ''):
     random.shuffle(selected_words)
 
     return ' '.join(selected_words)
+
+
+def disable_logging(f):
+    def wrapper(*args):
+        logging.disable(logging.CRITICAL)
+        result = f(*args)
+        logging.disable(logging.NOTSET)
+
+        return result
+
+    return wrapper
diff --git a/bookmarks/tests/test_bookmark_validation.py b/bookmarks/tests/test_bookmark_validation.py
index bae6056..b6d2529 100644
--- a/bookmarks/tests/test_bookmark_validation.py
+++ b/bookmarks/tests/test_bookmark_validation.py
@@ -34,6 +34,27 @@ class BookmarkValidationTestCase(TestCase):
     def setUp(self) -> None:
         self.user = User.objects.create_user('testuser', 'test@example.com', 'password123')
 
+    def test_bookmark_model_should_not_allow_missing_url(self):
+        bookmark = Bookmark(
+            date_added=datetime.datetime.now(),
+            date_modified=datetime.datetime.now(),
+            owner=self.user
+        )
+
+        with self.assertRaises(ValidationError):
+            bookmark.full_clean()
+
+    def test_bookmark_model_should_not_allow_empty_url(self):
+        bookmark = Bookmark(
+            url='',
+            date_added=datetime.datetime.now(),
+            date_modified=datetime.datetime.now(),
+            owner=self.user
+        )
+
+        with self.assertRaises(ValidationError):
+            bookmark.full_clean()
+
     @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)
diff --git a/bookmarks/tests/test_importer.py b/bookmarks/tests/test_importer.py
new file mode 100644
index 0000000..5cb2283
--- /dev/null
+++ b/bookmarks/tests/test_importer.py
@@ -0,0 +1,33 @@
+from django.test import TestCase
+
+from bookmarks.services.importer import import_netscape_html
+from bookmarks.tests.helpers import BookmarkFactoryMixin, disable_logging
+
+
+class ImporterTestCase(TestCase, BookmarkFactoryMixin):
+
+    def create_import_html(self, bookmark_tags_string: str):
+        return f'''
+<!DOCTYPE NETSCAPE-Bookmark-file-1>
+<META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=UTF-8">
+<TITLE>Bookmarks</TITLE>
+<H1>Bookmarks</H1>
+<DL><p>
+{bookmark_tags_string}
+</DL><p>
+        '''
+
+    @disable_logging
+    def test_validate_empty_or_missing_bookmark_url(self):
+        test_html = self.create_import_html(f'''
+            <!-- Empty URL -->
+            <DT><A HREF="" ADD_DATE="1616337559" PRIVATE="0" TOREAD="0" TAGS="tag3">Empty URL</A>
+            <DD>Empty URL
+            <!-- Missing URL -->
+            <DT><A ADD_DATE="1616337559" PRIVATE="0" TOREAD="0" TAGS="tag3">Missing URL</A>
+            <DD>Missing URL
+        ''')
+
+        import_result = import_netscape_html(test_html, self.get_or_create_test_user())
+
+        self.assertEqual(import_result.success, 0)
diff --git a/bookmarks/tests/test_settings_import_view.py b/bookmarks/tests/test_settings_import_view.py
index b5c609a..1012f4a 100644
--- a/bookmarks/tests/test_settings_import_view.py
+++ b/bookmarks/tests/test_settings_import_view.py
@@ -1,7 +1,7 @@
 from django.test import TestCase
 from django.urls import reverse
 
-from bookmarks.tests.helpers import BookmarkFactoryMixin
+from bookmarks.tests.helpers import BookmarkFactoryMixin, disable_logging
 
 
 class SettingsImportViewTestCase(TestCase, BookmarkFactoryMixin):
@@ -52,6 +52,7 @@ class SettingsImportViewTestCase(TestCase, BookmarkFactoryMixin):
         self.assertNoFormSuccessHint(response)
         self.assertFormErrorHint(response, 'Please select a file to import.')
 
+    @disable_logging
     def test_should_show_hint_if_import_raises_exception(self):
         with open('bookmarks/tests/resources/invalid_import_file.png', 'rb') as import_file:
             response = self.client.post(
@@ -64,6 +65,7 @@ class SettingsImportViewTestCase(TestCase, BookmarkFactoryMixin):
             self.assertNoFormSuccessHint(response)
             self.assertFormErrorHint(response, 'An error occurred during bookmark import.')
 
+    @disable_logging
     def test_should_show_respective_hints_if_not_all_bookmarks_were_imported_successfully(self):
         with open('bookmarks/tests/resources/simple_valid_import_file_with_one_invalid_bookmark.html') as import_file:
             response = self.client.post(