Merge pull request #1075 from bookwyrm-social/disable-connectors

Disable related connector when an instance is blocked

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",
@@ -102,13 +101,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

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)
 
 

bookwyrm/migrations/0074_auto_20210511_1829.py

@@ -0,0 +1,48 @@
+# Generated by Django 3.2 on 2021-05-11 18:29
+
+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),
+        ),
+        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,
+            ),
+        ),
+    ]

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"""
 

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)
@@ -17,6 +17,10 @@ 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)
+    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)
@@ -24,13 +28,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,

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_ready=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_ready=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"""

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"""
 

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)

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

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"""