From e7c5b77b4c2bdceb68e6a675c0f72b223f72e6c6 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 11 May 2021 09:54:04 -0700 Subject: [PATCH 1/6] Removes unused connector fields and adds active boolean --- bookwyrm/connectors/abstract_connector.py | 7 ---- .../migrations/0074_auto_20210511_1651.py | 34 +++++++++++++++++++ bookwyrm/models/connector.py | 8 +---- .../connectors/test_abstract_connector.py | 7 ---- 4 files changed, 35 insertions(+), 21 deletions(-) create mode 100644 bookwyrm/migrations/0074_auto_20210511_1651.py diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index 18ccb942..f9496aa0 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -102,13 +102,6 @@ class AbstractConnector(AbstractMinimalConnector): # title we handle separately. self.book_mappings = [] - def is_available(self): - """check if you're allowed to use this connector""" - if self.max_query_count is not None: - if self.connector.query_count >= self.max_query_count: - return False - return True - def get_or_create_book(self, remote_id): """translate arbitrary json into an Activitypub dataclass""" # first, check if we have the origin_id saved diff --git a/bookwyrm/migrations/0074_auto_20210511_1651.py b/bookwyrm/migrations/0074_auto_20210511_1651.py new file mode 100644 index 00000000..51e9e09c --- /dev/null +++ b/bookwyrm/migrations/0074_auto_20210511_1651.py @@ -0,0 +1,34 @@ +# Generated by Django 3.2 on 2021-05-11 16:51 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0073_sitesettings_footer_item"), + ] + + operations = [ + migrations.RemoveField( + model_name="connector", + name="max_query_count", + ), + migrations.RemoveField( + model_name="connector", + name="politeness_delay", + ), + migrations.RemoveField( + model_name="connector", + name="query_count", + ), + migrations.RemoveField( + model_name="connector", + name="query_count_expiry", + ), + migrations.AddField( + model_name="connector", + name="active", + field=models.BooleanField(default=True), + ), + ] diff --git a/bookwyrm/models/connector.py b/bookwyrm/models/connector.py index 625cdbed..4debcc3d 100644 --- a/bookwyrm/models/connector.py +++ b/bookwyrm/models/connector.py @@ -17,6 +17,7 @@ class Connector(BookWyrmModel): local = models.BooleanField(default=False) connector_file = models.CharField(max_length=255, choices=ConnectorFiles.choices) api_key = models.CharField(max_length=255, null=True, blank=True) + active = models.BooleanField(default=True) base_url = models.CharField(max_length=255) books_url = models.CharField(max_length=255) @@ -24,13 +25,6 @@ class Connector(BookWyrmModel): search_url = models.CharField(max_length=255, null=True, blank=True) isbn_search_url = models.CharField(max_length=255, null=True, blank=True) - politeness_delay = models.IntegerField(null=True, blank=True) # seconds - max_query_count = models.IntegerField(null=True, blank=True) - # how many queries executed in a unit of time, like a day - query_count = models.IntegerField(default=0) - # when to reset the query count back to 0 (ie, after 1 day) - query_count_expiry = models.DateTimeField(auto_now_add=True, blank=True) - def __str__(self): return "{} ({})".format( self.identifier, diff --git a/bookwyrm/tests/connectors/test_abstract_connector.py b/bookwyrm/tests/connectors/test_abstract_connector.py index 4497b4e5..5c50e4b7 100644 --- a/bookwyrm/tests/connectors/test_abstract_connector.py +++ b/bookwyrm/tests/connectors/test_abstract_connector.py @@ -84,13 +84,6 @@ class AbstractConnector(TestCase): """barebones connector for search with defaults""" self.assertIsInstance(self.connector.book_mappings, list) - def test_is_available(self): - """this isn't used....""" - self.assertTrue(self.connector.is_available()) - self.connector.max_query_count = 1 - self.connector.connector.query_count = 2 - self.assertFalse(self.connector.is_available()) - def test_get_or_create_book_existing(self): """find an existing book by remote/origin id""" self.assertEqual(models.Book.objects.count(), 1) From 19f788b9aa36004e872aac3dd3adbb7b620241e5 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 11 May 2021 11:31:02 -0700 Subject: [PATCH 2/6] Deactivate connectors related to blocked federated servers --- ...10511_1651.py => 0074_auto_20210511_1829.py} | 16 +++++++++++++++- bookwyrm/models/base_model.py | 10 ++++++++++ bookwyrm/models/connector.py | 5 ++++- bookwyrm/models/federated_server.py | 17 +++++++++++++++++ bookwyrm/models/user.py | 12 +----------- 5 files changed, 47 insertions(+), 13 deletions(-) rename bookwyrm/migrations/{0074_auto_20210511_1651.py => 0074_auto_20210511_1829.py} (60%) diff --git a/bookwyrm/migrations/0074_auto_20210511_1651.py b/bookwyrm/migrations/0074_auto_20210511_1829.py similarity index 60% rename from bookwyrm/migrations/0074_auto_20210511_1651.py rename to bookwyrm/migrations/0074_auto_20210511_1829.py index 51e9e09c..287a51ad 100644 --- a/bookwyrm/migrations/0074_auto_20210511_1651.py +++ b/bookwyrm/migrations/0074_auto_20210511_1829.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2 on 2021-05-11 16:51 +# Generated by Django 3.2 on 2021-05-11 18:29 from django.db import migrations, models @@ -31,4 +31,18 @@ class Migration(migrations.Migration): name="active", field=models.BooleanField(default=True), ), + migrations.AddField( + model_name="connector", + name="deactivation_reason", + field=models.CharField( + blank=True, + choices=[ + ("self_deletion", "Self Deletion"), + ("moderator_deletion", "Moderator Deletion"), + ("domain_block", "Domain Block"), + ], + max_length=255, + null=True, + ), + ), ] diff --git a/bookwyrm/models/base_model.py b/bookwyrm/models/base_model.py index e85ff733..2cb7c036 100644 --- a/bookwyrm/models/base_model.py +++ b/bookwyrm/models/base_model.py @@ -6,6 +6,16 @@ from bookwyrm.settings import DOMAIN from .fields import RemoteIdField +DeactivationReason = models.TextChoices( + "DeactivationReason", + [ + "self_deletion", + "moderator_deletion", + "domain_block", + ], +) + + class BookWyrmModel(models.Model): """shared fields""" diff --git a/bookwyrm/models/connector.py b/bookwyrm/models/connector.py index 4debcc3d..2d671790 100644 --- a/bookwyrm/models/connector.py +++ b/bookwyrm/models/connector.py @@ -2,7 +2,7 @@ from django.db import models from bookwyrm.connectors.settings import CONNECTORS -from .base_model import BookWyrmModel +from .base_model import BookWyrmModel, DeactivationReason ConnectorFiles = models.TextChoices("ConnectorFiles", CONNECTORS) @@ -18,6 +18,9 @@ class Connector(BookWyrmModel): connector_file = models.CharField(max_length=255, choices=ConnectorFiles.choices) api_key = models.CharField(max_length=255, null=True, blank=True) active = models.BooleanField(default=True) + deactivation_reason = models.CharField( + max_length=255, choices=DeactivationReason.choices, null=True, blank=True + ) base_url = models.CharField(max_length=255) books_url = models.CharField(max_length=255) diff --git a/bookwyrm/models/federated_server.py b/bookwyrm/models/federated_server.py index 7d446ca0..86635ab0 100644 --- a/bookwyrm/models/federated_server.py +++ b/bookwyrm/models/federated_server.py @@ -1,5 +1,6 @@ """ connections to external ActivityPub servers """ from urllib.parse import urlparse +from django.apps import apps from django.db import models from .base_model import BookWyrmModel @@ -34,6 +35,13 @@ class FederatedServer(BookWyrmModel): is_active=False, deactivation_reason="domain_block" ) + # check for related connectors + if self.application_type == "bookwyrm": + connector_model = apps.get_model("bookwyrm.Connector", require_read=True) + connector_model.objects.filter( + identifier=self.server_name, active=True + ).update(active=False, deactivation_reason="domain_block") + def unblock(self): """unblock a server""" self.status = "federated" @@ -43,6 +51,15 @@ class FederatedServer(BookWyrmModel): is_active=True, deactivation_reason=None ) + # check for related connectors + if self.application_type == "bookwyrm": + connector_model = apps.get_model("bookwyrm.Connector", require_read=True) + connector_model.objects.filter( + identifier=self.server_name, + active=False, + deactivation_reason="domain_block", + ).update(active=True, deactivation_reason=None) + @classmethod def is_blocked(cls, url): """look up if a domain is blocked""" diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index 5f0e64e3..7d821c5b 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -19,21 +19,11 @@ from bookwyrm.signatures import create_key_pair from bookwyrm.tasks import app from bookwyrm.utils import regex from .activitypub_mixin import OrderedCollectionPageMixin, ActivitypubMixin -from .base_model import BookWyrmModel +from .base_model import BookWyrmModel, DeactivationReason from .federated_server import FederatedServer from . import fields, Review -DeactivationReason = models.TextChoices( - "DeactivationReason", - [ - "self_deletion", - "moderator_deletion", - "domain_block", - ], -) - - class User(OrderedCollectionPageMixin, AbstractUser): """a user who wants to read books""" From 9b42bba236d6ce1db6b6598084eb3a15cf8dc71b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 11 May 2021 11:34:58 -0700 Subject: [PATCH 3/6] Filter out inactive connectors --- bookwyrm/connectors/connector_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/connectors/connector_manager.py b/bookwyrm/connectors/connector_manager.py index 43489669..95c5959d 100644 --- a/bookwyrm/connectors/connector_manager.py +++ b/bookwyrm/connectors/connector_manager.py @@ -87,7 +87,7 @@ def first_search_result(query, min_confidence=0.1): def get_connectors(): """load all connectors""" - for info in models.Connector.objects.order_by("priority").all(): + for info in models.Connector.objects.filter(active=True).order_by("priority").all(): yield load_connector(info) From d7a8dd5e197f2570fd231af48eef6bd4c7b2aaf8 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 11 May 2021 13:20:17 -0700 Subject: [PATCH 4/6] Removes outdated field form abstract connector --- bookwyrm/connectors/abstract_connector.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index f9496aa0..60667815 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -30,7 +30,6 @@ class AbstractMinimalConnector(ABC): "covers_url", "search_url", "isbn_search_url", - "max_query_count", "name", "identifier", "local", From 3c7882b055e71448add7eadcdb4b4727d7f7956d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 14 May 2021 16:13:32 -0700 Subject: [PATCH 5/6] Fixes abstract minimal connector test --- bookwyrm/tests/connectors/test_abstract_minimal_connector.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bookwyrm/tests/connectors/test_abstract_minimal_connector.py b/bookwyrm/tests/connectors/test_abstract_minimal_connector.py index bc5625c9..84629139 100644 --- a/bookwyrm/tests/connectors/test_abstract_minimal_connector.py +++ b/bookwyrm/tests/connectors/test_abstract_minimal_connector.py @@ -53,7 +53,6 @@ class AbstractConnector(TestCase): self.assertEqual(connector.isbn_search_url, "https://example.com/isbn?q=") self.assertIsNone(connector.name) self.assertEqual(connector.identifier, "example.com") - self.assertIsNone(connector.max_query_count) self.assertFalse(connector.local) @responses.activate From 82117a7d28b593b03a8a29ccbf3c3d13b2aebfad Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 18 May 2021 12:23:36 -0700 Subject: [PATCH 6/6] Tests deactivating connectors --- bookwyrm/models/federated_server.py | 4 ++-- bookwyrm/tests/views/test_federation.py | 31 +++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/bookwyrm/models/federated_server.py b/bookwyrm/models/federated_server.py index 86635ab0..e297c46c 100644 --- a/bookwyrm/models/federated_server.py +++ b/bookwyrm/models/federated_server.py @@ -37,7 +37,7 @@ class FederatedServer(BookWyrmModel): # check for related connectors if self.application_type == "bookwyrm": - connector_model = apps.get_model("bookwyrm.Connector", require_read=True) + connector_model = apps.get_model("bookwyrm.Connector", require_ready=True) connector_model.objects.filter( identifier=self.server_name, active=True ).update(active=False, deactivation_reason="domain_block") @@ -53,7 +53,7 @@ class FederatedServer(BookWyrmModel): # check for related connectors if self.application_type == "bookwyrm": - connector_model = apps.get_model("bookwyrm.Connector", require_read=True) + connector_model = apps.get_model("bookwyrm.Connector", require_ready=True) connector_model.objects.filter( identifier=self.server_name, active=False, diff --git a/bookwyrm/tests/views/test_federation.py b/bookwyrm/tests/views/test_federation.py index f17f7624..4469040e 100644 --- a/bookwyrm/tests/views/test_federation.py +++ b/bookwyrm/tests/views/test_federation.py @@ -60,7 +60,12 @@ class FederationViews(TestCase): def test_server_page_block(self): """block a server""" - server = models.FederatedServer.objects.create(server_name="hi.there.com") + server = models.FederatedServer.objects.create( + server_name="hi.there.com", application_type="bookwyrm" + ) + connector = models.Connector.objects.get( + identifier="hi.there.com", + ) self.remote_user.federated_server = server self.remote_user.save() @@ -72,17 +77,32 @@ class FederationViews(TestCase): request.user.is_superuser = True view(request, server.id) + server.refresh_from_db() self.remote_user.refresh_from_db() self.assertEqual(server.status, "blocked") + # and the user was deactivated self.assertFalse(self.remote_user.is_active) + self.assertEqual(self.remote_user.deactivation_reason, "domain_block") + + # and the connector was disabled + connector.refresh_from_db() + self.assertFalse(connector.active) + self.assertEqual(connector.deactivation_reason, "domain_block") def test_server_page_unblock(self): """unblock a server""" server = models.FederatedServer.objects.create( - server_name="hi.there.com", status="blocked" + server_name="hi.there.com", status="blocked", application_type="bookwyrm" ) + connector = models.Connector.objects.get( + identifier="hi.there.com", + ) + connector.active = False + connector.deactivation_reason = "domain_block" + connector.save() + self.remote_user.federated_server = server self.remote_user.is_active = False self.remote_user.deactivation_reason = "domain_block" @@ -96,8 +116,15 @@ class FederationViews(TestCase): server.refresh_from_db() self.remote_user.refresh_from_db() self.assertEqual(server.status, "federated") + # and the user was re-activated self.assertTrue(self.remote_user.is_active) + self.assertIsNone(self.remote_user.deactivation_reason) + + # and the connector was re-enabled + connector.refresh_from_db() + self.assertTrue(connector.active) + self.assertIsNone(connector.deactivation_reason) def test_add_view_get(self): """there are so many views, this just makes sure it LOADS"""