From 313e389fc678e935fddee9646fea901b110b1452 Mon Sep 17 00:00:00 2001 From: Andrew Radev Date: Thu, 24 Dec 2020 12:56:22 +0200 Subject: [PATCH 01/79] Fix typo in Patreon link --- .github/FUNDING.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml index cfbe05241..5662d1d57 100644 --- a/.github/FUNDING.yml +++ b/.github/FUNDING.yml @@ -1,7 +1,7 @@ # These are supported funding model platforms github: # Replace with up to 4 GitHub Sponsors-enabled usernames e.g., [user1, user2] -patreon: bookwrym +patreon: bookwyrm open_collective: # Replace with a single Open Collective username ko_fi: # Replace with a single Ko-fi username tidelift: # Replace with a single Tidelift platform-name/package-name e.g., npm/babel From 9f74e95b00c07e2f4418870d26a8da6b79ab1a8b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 27 Dec 2020 13:32:27 -0800 Subject: [PATCH 02/79] stylistic cleanup of import model tests --- bookwyrm/tests/models/test_import_model.py | 28 ++++++++++++---------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/bookwyrm/tests/models/test_import_model.py b/bookwyrm/tests/models/test_import_model.py index c703d08a4..4833ed169 100644 --- a/bookwyrm/tests/models/test_import_model.py +++ b/bookwyrm/tests/models/test_import_model.py @@ -54,11 +54,11 @@ class ImportJob(TestCase): user = models.User.objects.create_user( 'mouse', 'mouse@mouse.mouse', 'mouseword', local=True) job = models.ImportJob.objects.create(user=user) - models.ImportItem.objects.create( + self.item_1 = models.ImportItem.objects.create( job=job, index=1, data=currently_reading_data) - models.ImportItem.objects.create( + self.item_2 = models.ImportItem.objects.create( job=job, index=2, data=read_data) - models.ImportItem.objects.create( + self.item_3 = models.ImportItem.objects.create( job=job, index=3, data=unknown_read_data) @@ -72,8 +72,7 @@ class ImportJob(TestCase): def test_shelf(self): ''' converts to the local shelf typology ''' expected = 'reading' - item = models.ImportItem.objects.get(index=1) - self.assertEqual(item.shelf, expected) + self.assertEqual(self.item_1.shelf, expected) def test_date_added(self): @@ -91,21 +90,26 @@ class ImportJob(TestCase): def test_currently_reading_reads(self): + ''' infer currently reading dates where available ''' expected = [models.ReadThrough( - start_date=datetime.datetime(2019, 4, 9, 0, 0, tzinfo=timezone.utc))] + start_date=datetime.datetime(2019, 4, 9, 0, 0, tzinfo=timezone.utc) + )] actual = models.ImportItem.objects.get(index=1) self.assertEqual(actual.reads[0].start_date, expected[0].start_date) self.assertEqual(actual.reads[0].finish_date, expected[0].finish_date) def test_read_reads(self): - actual = models.ImportItem.objects.get(index=2) - self.assertEqual(actual.reads[0].start_date, datetime.datetime(2019, 4, 9, 0, 0, tzinfo=timezone.utc)) - self.assertEqual(actual.reads[0].finish_date, datetime.datetime(2019, 4, 12, 0, 0, tzinfo=timezone.utc)) + ''' infer read dates where available ''' + actual = self.item_2 + self.assertEqual( + actual.reads[0].start_date, + datetime.datetime(2019, 4, 9, 0, 0, tzinfo=timezone.utc)) + self.assertEqual( + actual.reads[0].finish_date, + datetime.datetime(2019, 4, 12, 0, 0, tzinfo=timezone.utc)) def test_unread_reads(self): + ''' handle books with no read dates ''' expected = [] actual = models.ImportItem.objects.get(index=3) self.assertEqual(actual.reads, expected) - - - From 97a5364b70df6361a41116b7ad0663e04f270628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anton=20Str=C3=B6mkvist?= Date: Sun, 27 Dec 2020 23:29:43 +0100 Subject: [PATCH 03/79] Fix docstring for to_reject_activity --- bookwyrm/models/relationship.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/models/relationship.py b/bookwyrm/models/relationship.py index debe2ace7..0f3c1dab9 100644 --- a/bookwyrm/models/relationship.py +++ b/bookwyrm/models/relationship.py @@ -54,7 +54,7 @@ class UserRelationship(ActivitypubMixin, BookWyrmModel): def to_reject_activity(self): - ''' generate an Accept for this follow request ''' + ''' generate a Reject for this follow request ''' return activitypub.Reject( id=self.get_remote_id(status='rejects'), actor=self.user_object.remote_id, From ac261d7b1a641952978855f2717dff4688b3f521 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 27 Dec 2020 14:27:18 -0800 Subject: [PATCH 04/79] Send connector with search result also fix typo in get_work_from_edition_data function --- bookwyrm/connectors/abstract_connector.py | 3 +- bookwyrm/connectors/bookwyrm_connector.py | 1 + bookwyrm/connectors/openlibrary.py | 3 +- bookwyrm/connectors/self_connector.py | 9 +--- bookwyrm/models/import_job.py | 4 +- bookwyrm/tests/models/test_import_model.py | 57 +++++++++++++++++++++- 6 files changed, 65 insertions(+), 12 deletions(-) diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index 86ac74353..ce1184d8c 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -168,7 +168,7 @@ class AbstractConnector(AbstractMinimalConnector): ''' every work needs at least one edition ''' @abstractmethod - def get_work_from_edition_date(self, data): + def get_work_from_edition_data(self, data): ''' every edition needs a work ''' @abstractmethod @@ -228,6 +228,7 @@ class SearchResult: key: str author: str year: str + connector: object confidence: int = 1 def __repr__(self): diff --git a/bookwyrm/connectors/bookwyrm_connector.py b/bookwyrm/connectors/bookwyrm_connector.py index e4d32fd33..3c6f46145 100644 --- a/bookwyrm/connectors/bookwyrm_connector.py +++ b/bookwyrm/connectors/bookwyrm_connector.py @@ -13,4 +13,5 @@ class Connector(AbstractMinimalConnector): return data def format_search_result(self, search_result): + search_result['connector'] = self return SearchResult(**search_result) diff --git a/bookwyrm/connectors/openlibrary.py b/bookwyrm/connectors/openlibrary.py index 3b60c3073..c59829d68 100644 --- a/bookwyrm/connectors/openlibrary.py +++ b/bookwyrm/connectors/openlibrary.py @@ -85,7 +85,7 @@ class Connector(AbstractConnector): return pick_default_edition(data['entries']) - def get_work_from_edition_date(self, data): + def get_work_from_edition_data(self, data): try: key = data['works'][0]['key'] except (IndexError, KeyError): @@ -123,6 +123,7 @@ class Connector(AbstractConnector): title=search_result.get('title'), key=key, author=', '.join(author), + connector=self, year=search_result.get('first_publish_year'), ) diff --git a/bookwyrm/connectors/self_connector.py b/bookwyrm/connectors/self_connector.py index 8d31c8a1a..cad982493 100644 --- a/bookwyrm/connectors/self_connector.py +++ b/bookwyrm/connectors/self_connector.py @@ -51,28 +51,23 @@ class Connector(AbstractConnector): author=search_result.author_text, year=search_result.published_date.year if \ search_result.published_date else None, + connector=self, confidence=search_result.rank, ) - def get_remote_id_from_data(self, data): - pass - def is_work_data(self, data): pass def get_edition_from_work_data(self, data): pass - def get_work_from_edition_date(self, data): + def get_work_from_edition_data(self, data): pass def get_authors_from_data(self, data): return None - def get_cover_from_data(self, data): - return None - def parse_search_data(self, data): ''' it's already in the right format, don't even worry about it ''' return data diff --git a/bookwyrm/models/import_job.py b/bookwyrm/models/import_job.py index 835094cd7..576dd07d1 100644 --- a/bookwyrm/models/import_job.py +++ b/bookwyrm/models/import_job.py @@ -76,7 +76,7 @@ class ImportItem(models.Model): ) if search_result: # raises ConnectorException - return books_manager.get_or_create_book(search_result.key) + return search_result.connector.get_or_create_book(search_result.key) return None @@ -91,7 +91,7 @@ class ImportItem(models.Model): ) if search_result: # raises ConnectorException - return books_manager.get_or_create_book(search_result.key) + return search_result.connector.get_or_create_book(search_result.key) return None diff --git a/bookwyrm/tests/models/test_import_model.py b/bookwyrm/tests/models/test_import_model.py index 4833ed169..d393d968c 100644 --- a/bookwyrm/tests/models/test_import_model.py +++ b/bookwyrm/tests/models/test_import_model.py @@ -1,9 +1,15 @@ ''' testing models ''' import datetime +import json +import pathlib +from unittest.mock import patch + from django.utils import timezone from django.test import TestCase +import responses -from bookwyrm import models +from bookwyrm import books_manager, models +from bookwyrm.connectors.abstract_connector import SearchResult class ImportJob(TestCase): @@ -113,3 +119,52 @@ class ImportJob(TestCase): expected = [] actual = models.ImportItem.objects.get(index=3) self.assertEqual(actual.reads, expected) + + + @responses.activate + def test_get_book_from_isbn(self): + ''' search and load books by isbn (9780356506999) ''' + connector_info = models.Connector.objects.create( + identifier='openlibrary.org', + name='OpenLibrary', + connector_file='openlibrary', + base_url='https://openlibrary.org', + books_url='https://openlibrary.org', + covers_url='https://covers.openlibrary.org', + search_url='https://openlibrary.org/search?q=', + priority=3, + ) + connector = books_manager.load_connector(connector_info) + result = SearchResult( + title='Test Result', + key='https://openlibrary.org/works/OL1234W', + author='An Author', + year='1980', + connector=connector, + ) + + + datafile = pathlib.Path(__file__).parent.joinpath( + '../data/ol_edition.json') + bookdata = json.loads(datafile.read_bytes()) + responses.add( + responses.GET, + 'https://openlibrary.org/works/OL1234W', + json=bookdata, + status=200) + responses.add( + responses.GET, + 'https://openlibrary.org//works/OL15832982W', + json=bookdata, + status=200) + responses.add( + responses.GET, + 'https://openlibrary.org//authors/OL382982A.json', + json={'name': 'test author'}, + status=200) + + with patch('bookwyrm.books_manager.first_search_result') as search: + search.return_value = result + book = self.item_1.get_book_from_isbn() + + self.assertEqual(book.title, 'Sabriel') From fb10cb35ad13da4272f1f75a5f8b05a9fc174761 Mon Sep 17 00:00:00 2001 From: "Renato \"Lond\" Cerqueira" Date: Wed, 30 Dec 2020 12:35:11 +0100 Subject: [PATCH 05/79] Add BookWyrm user-agent to http requests This allows other software to identify BookWyrm in calls, as well as will allow BookWyrm to differentiate between calls done from other fediverse software and BookWyrm to answer with specific BookWyrm data. --- bookwyrm/broadcast.py | 3 ++- bookwyrm/connectors/abstract_connector.py | 11 +++++++++-- bookwyrm/settings.py | 6 ++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/bookwyrm/broadcast.py b/bookwyrm/broadcast.py index a98b6774a..f4186c4d0 100644 --- a/bookwyrm/broadcast.py +++ b/bookwyrm/broadcast.py @@ -3,7 +3,7 @@ import json from django.utils.http import http_date import requests -from bookwyrm import models +from bookwyrm import models, settings from bookwyrm.activitypub import ActivityEncoder from bookwyrm.tasks import app from bookwyrm.signatures import make_signature, make_digest @@ -79,6 +79,7 @@ def sign_and_send(sender, data, destination): 'Digest': digest, 'Signature': make_signature(sender, destination, now, digest), 'Content-Type': 'application/activity+json; charset=utf-8', + 'User-Agent': settings.USER_AGENT, }, ) if not response.ok: diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index ce1184d8c..706f5bb9e 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -8,7 +8,7 @@ import requests from requests import HTTPError from requests.exceptions import SSLError -from bookwyrm import activitypub, models +from bookwyrm import activitypub, models, settings class ConnectorException(HTTPError): @@ -42,6 +42,7 @@ class AbstractMinimalConnector(ABC): '%s%s' % (self.search_url, query), headers={ 'Accept': 'application/json; charset=utf-8', + 'User-Agent': settings.USER_AGENT, }, ) if not resp.ok: @@ -196,6 +197,7 @@ def get_data(url): url, headers={ 'Accept': 'application/json; charset=utf-8', + 'User-Agent': settings.USER_AGENT, }, ) except RequestError: @@ -213,7 +215,12 @@ def get_data(url): def get_image(url): ''' wrapper for requesting an image ''' try: - resp = requests.get(url) + resp = requests.get( + url, + headers={ + 'User-Agent': settings.USER_AGENT, + }, + ) except (RequestError, SSLError): return None if not resp.ok: diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index c42215b40..f598cd3c6 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -3,6 +3,8 @@ import os from environs import Env +import requests + env = Env() DOMAIN = env('DOMAIN') @@ -150,3 +152,7 @@ STATIC_URL = '/static/' STATIC_ROOT = os.path.join(BASE_DIR, env('STATIC_ROOT', 'static')) MEDIA_URL = '/images/' MEDIA_ROOT = os.path.join(BASE_DIR, env('MEDIA_ROOT', 'images')) + +USER_AGENT = "%s (BookWyrm/%s; +https://%s/)" % (requests.utils.default_user_agent(), + "0.1", # TODO: change 0.1 into actual version + DOMAIN) From 21f67c9e28b2760926da496e8d4bef14e3dbf52b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 09:11:00 -0800 Subject: [PATCH 06/79] Catch error response decoding json in search connector --- bookwyrm/books_manager.py | 3 ++- bookwyrm/connectors/abstract_connector.py | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/bookwyrm/books_manager.py b/bookwyrm/books_manager.py index 3b8657686..bc1fa7231 100644 --- a/bookwyrm/books_manager.py +++ b/bookwyrm/books_manager.py @@ -5,6 +5,7 @@ from urllib.parse import urlparse from requests import HTTPError from bookwyrm import models +from bookwyrm.connectors import ConnectorException from bookwyrm.tasks import app @@ -55,7 +56,7 @@ def search(query, min_confidence=0.1): for connector in get_connectors(): try: result_set = connector.search(query, min_confidence=min_confidence) - except HTTPError: + except (HTTPError, ConnectorException): continue result_set = [r for r in result_set \ diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index ce1184d8c..221c59c0c 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -46,7 +46,10 @@ class AbstractMinimalConnector(ABC): ) if not resp.ok: resp.raise_for_status() - data = resp.json() + try: + data = resp.json() + except ValueError as e: + raise ConnectorException('Unable to parse json response', e) results = [] for doc in self.parse_search_data(data)[:10]: From d7db6d50ba4d3480d0167b1f0c9f80d3289cb766 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 09:14:07 -0800 Subject: [PATCH 07/79] Log errors in conenctor search --- bookwyrm/connectors/abstract_connector.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index 221c59c0c..fd2f8b3db 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -1,6 +1,7 @@ ''' functionality outline for a book data connector ''' from abc import ABC, abstractmethod from dataclasses import dataclass +import logging from urllib3.exceptions import RequestError from django.db import transaction @@ -11,6 +12,7 @@ from requests.exceptions import SSLError from bookwyrm import activitypub, models +logger = logging.getLogger(__name__) class ConnectorException(HTTPError): ''' when the connector can't do what was asked ''' @@ -49,6 +51,7 @@ class AbstractMinimalConnector(ABC): try: data = resp.json() except ValueError as e: + logger.exception(e) raise ConnectorException('Unable to parse json response', e) results = [] From 6a8353de096b4919fe6bf9913a437b3f189bd3b3 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 09:26:02 -0800 Subject: [PATCH 08/79] Adds test for SearchResult dataclass also just cleans up the styles in the test file for linting --- .../connectors/test_abstract_connector.py | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/bookwyrm/tests/connectors/test_abstract_connector.py b/bookwyrm/tests/connectors/test_abstract_connector.py index d6e02960e..bec9b11d5 100644 --- a/bookwyrm/tests/connectors/test_abstract_connector.py +++ b/bookwyrm/tests/connectors/test_abstract_connector.py @@ -2,12 +2,14 @@ from django.test import TestCase from bookwyrm import models -from bookwyrm.connectors.abstract_connector import Mapping +from bookwyrm.connectors.abstract_connector import Mapping, SearchResult from bookwyrm.connectors.openlibrary import Connector class AbstractConnector(TestCase): + ''' generic code for connecting to outside data sources ''' def setUp(self): + ''' we need an example connector ''' self.book = models.Edition.objects.create(title='Example Edition') models.Connector.objects.create( @@ -38,6 +40,7 @@ class AbstractConnector(TestCase): def test_create_mapping(self): + ''' maps remote fields for book data to bookwyrm activitypub fields ''' mapping = Mapping('isbn') self.assertEqual(mapping.local_field, 'isbn') self.assertEqual(mapping.remote_field, 'isbn') @@ -45,6 +48,7 @@ class AbstractConnector(TestCase): def test_create_mapping_with_remote(self): + ''' the remote field is different than the local field ''' mapping = Mapping('isbn', remote_field='isbn13') self.assertEqual(mapping.local_field, 'isbn') self.assertEqual(mapping.remote_field, 'isbn13') @@ -52,9 +56,24 @@ class AbstractConnector(TestCase): def test_create_mapping_with_formatter(self): + ''' a function is provided to modify the data ''' formatter = lambda x: 'aa' + x mapping = Mapping('isbn', formatter=formatter) self.assertEqual(mapping.local_field, 'isbn') self.assertEqual(mapping.remote_field, 'isbn') self.assertEqual(mapping.formatter, formatter) self.assertEqual(mapping.formatter('bb'), 'aabb') + + + def test_search_result(self): + ''' a class that stores info about a search result ''' + result = SearchResult( + title='Title', + key='https://example.com/book/1', + author='Author Name', + year='1850', + connector=self.connector, + ) + # there's really not much to test here, it's just a dataclass + self.assertEqual(result.confidence, 1) + self.assertEqual(result.title, 'Title') From c8d031e311d9cec540a7fac681152d13a8280fc2 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 09:48:37 -0800 Subject: [PATCH 09/79] Tests connector search --- .../connectors/test_abstract_connector.py | 83 +++++++++++++++---- 1 file changed, 66 insertions(+), 17 deletions(-) diff --git a/bookwyrm/tests/connectors/test_abstract_connector.py b/bookwyrm/tests/connectors/test_abstract_connector.py index bec9b11d5..cab2f834c 100644 --- a/bookwyrm/tests/connectors/test_abstract_connector.py +++ b/bookwyrm/tests/connectors/test_abstract_connector.py @@ -1,7 +1,9 @@ ''' testing book data connectors ''' from django.test import TestCase +import responses from bookwyrm import models +from bookwyrm.connectors import abstract_connector from bookwyrm.connectors.abstract_connector import Mapping, SearchResult from bookwyrm.connectors.openlibrary import Connector @@ -12,12 +14,12 @@ class AbstractConnector(TestCase): ''' we need an example connector ''' self.book = models.Edition.objects.create(title='Example Edition') - models.Connector.objects.create( + self.connector_info = models.Connector.objects.create( identifier='example.com', connector_file='openlibrary', base_url='https://example.com', - books_url='https:/example.com', - covers_url='https://example.com', + books_url='https://example.com/books', + covers_url='https://example.com/covers', search_url='https://example.com/search?q=', ) self.connector = Connector('example.com') @@ -39,6 +41,67 @@ class AbstractConnector(TestCase): ] + def test_abstract_minimal_connector_init(self): + ''' barebones connector for search with defaults ''' + class TestConnector(abstract_connector.AbstractMinimalConnector): + ''' nothing added here ''' + def format_search_result(): + pass + def get_or_create_book(): + pass + def parse_search_data(): + pass + + connector = TestConnector('example.com') + self.assertEqual(connector.connector, self.connector_info) + self.assertEqual(connector.base_url, 'https://example.com') + self.assertEqual(connector.books_url, 'https://example.com/books') + self.assertEqual(connector.covers_url, 'https://example.com/covers') + self.assertEqual(connector.search_url, 'https://example.com/search?q=') + self.assertIsNone(connector.name), + self.assertEqual(connector.identifier, 'example.com'), + self.assertIsNone(connector.max_query_count) + self.assertFalse(connector.local) + + + @responses.activate + def test_abstract_minimal_connector_search(self): + ''' makes an http request to the outside service ''' + class TestConnector(abstract_connector.AbstractMinimalConnector): + ''' nothing added here ''' + def format_search_result(self, data): + return data + def get_or_create_book(self, data): + pass + def parse_search_data(self, data): + return data + connector = TestConnector('example.com') + responses.add( + responses.GET, + 'https://example.com/search?q=a%20book%20title', + json=['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l'], + status=200) + results = connector.search('a book title') + self.assertEqual(len(results), 10) + self.assertEqual(results[0], 'a') + self.assertEqual(results[1], 'b') + self.assertEqual(results[2], 'c') + + + def test_search_result(self): + ''' a class that stores info about a search result ''' + result = SearchResult( + title='Title', + key='https://example.com/book/1', + author='Author Name', + year='1850', + connector=self.connector, + ) + # there's really not much to test here, it's just a dataclass + self.assertEqual(result.confidence, 1) + self.assertEqual(result.title, 'Title') + + def test_create_mapping(self): ''' maps remote fields for book data to bookwyrm activitypub fields ''' mapping = Mapping('isbn') @@ -63,17 +126,3 @@ class AbstractConnector(TestCase): self.assertEqual(mapping.remote_field, 'isbn') self.assertEqual(mapping.formatter, formatter) self.assertEqual(mapping.formatter('bb'), 'aabb') - - - def test_search_result(self): - ''' a class that stores info about a search result ''' - result = SearchResult( - title='Title', - key='https://example.com/book/1', - author='Author Name', - year='1850', - connector=self.connector, - ) - # there's really not much to test here, it's just a dataclass - self.assertEqual(result.confidence, 1) - self.assertEqual(result.title, 'Title') From 8ffb22291a68e2f1ad2ba3240bff348fb355ab58 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 09:51:37 -0800 Subject: [PATCH 10/79] Add connector to tested fields on search result --- bookwyrm/tests/connectors/test_bookwyrm_connector.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bookwyrm/tests/connectors/test_bookwyrm_connector.py b/bookwyrm/tests/connectors/test_bookwyrm_connector.py index 8d866ca25..1833ed4e3 100644 --- a/bookwyrm/tests/connectors/test_bookwyrm_connector.py +++ b/bookwyrm/tests/connectors/test_bookwyrm_connector.py @@ -31,6 +31,7 @@ class BookWyrmConnector(TestCase): def test_format_search_result(self): + ''' create a SearchResult object from search response json ''' datafile = pathlib.Path(__file__).parent.joinpath( '../data/fr_search.json') search_data = json.loads(datafile.read_bytes()) @@ -43,3 +44,4 @@ class BookWyrmConnector(TestCase): self.assertEqual(result.key, 'https://example.com/book/122') self.assertEqual(result.author, 'Susanna Clarke') self.assertEqual(result.year, 2017) + self.assertEqual(result.connector, self.connector) From e5a914c3c211d6125c66d1a871c5fe641b6559da Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 11:37:26 -0800 Subject: [PATCH 11/79] Fixes API search causing 500 error --- bookwyrm/connectors/abstract_connector.py | 8 +++++++- bookwyrm/views.py | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index fd2f8b3db..ae73f75e1 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -1,6 +1,6 @@ ''' functionality outline for a book data connector ''' from abc import ABC, abstractmethod -from dataclasses import dataclass +from dataclasses import asdict, dataclass import logging from urllib3.exceptions import RequestError @@ -241,6 +241,12 @@ class SearchResult: return "".format( self.key, self.title, self.author) + def json(self): + ''' serialize a connector for json response ''' + serialized = asdict(self) + del serialized['connector'] + return serialized + class Mapping: ''' associate a local database field with a field in an external dataset ''' diff --git a/bookwyrm/views.py b/bookwyrm/views.py index 1afb82669..bd8948062 100644 --- a/bookwyrm/views.py +++ b/bookwyrm/views.py @@ -210,7 +210,7 @@ def search(request): if is_api_request(request): # only return local book results via json so we don't cause a cascade book_results = books_manager.local_search(query) - return JsonResponse([r.__dict__ for r in book_results], safe=False) + return JsonResponse([r.json() for r in book_results], safe=False) # use webfinger for mastodon style account@domain.com username if re.match(regex.full_username, query): From 490591623f796da4b4791ca5048d301c2a7b77b3 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 11:37:49 -0800 Subject: [PATCH 12/79] Adds test for search view --- bookwyrm/tests/test_views.py | 81 ++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 bookwyrm/tests/test_views.py diff --git a/bookwyrm/tests/test_views.py b/bookwyrm/tests/test_views.py new file mode 100644 index 000000000..ba4531f69 --- /dev/null +++ b/bookwyrm/tests/test_views.py @@ -0,0 +1,81 @@ +''' test for app action functionality ''' +import json +from unittest.mock import patch + +from django.http import JsonResponse +from django.template.response import TemplateResponse +from django.test import TestCase +from django.test.client import RequestFactory + +from bookwyrm import models, views +from bookwyrm.connectors import abstract_connector +from bookwyrm.settings import DOMAIN + + +class Views(TestCase): + ''' every response to a get request, html or json ''' + def setUp(self): + ''' we need basic test data and mocks ''' + self.factory = RequestFactory() + self.book = models.Edition.objects.create(title='Test Book') + models.Connector.objects.create( + identifier='self', + connector_file='self_connector', + local=True + ) + + + def test_search_json_response(self): + ''' searches local data only and returns book data in json format ''' + # we need a connector for this, sorry + request = self.factory.get('', {'q': 'Test Book'}) + with patch('bookwyrm.views.is_api_request') as is_api: + is_api.return_value = True + response = views.search(request) + self.assertIsInstance(response, JsonResponse) + + data = json.loads(response.content) + self.assertEqual(len(data), 1) + self.assertEqual(data[0]['title'], 'Test Book') + self.assertEqual( + data[0]['key'], 'https://%s/book/%d' % (DOMAIN, self.book.id)) + + + def test_search_html_response(self): + ''' searches remote connectors ''' + class TestConnector(abstract_connector.AbstractMinimalConnector): + ''' nothing added here ''' + def format_search_result(self, search_result): + pass + def get_or_create_book(self, remote_id): + pass + def parse_search_data(self, data): + pass + models.Connector.objects.create( + identifier='example.com', + connector_file='openlibrary', + base_url='https://example.com', + books_url='https://example.com/books', + covers_url='https://example.com/covers', + search_url='https://example.com/search?q=', + ) + connector = TestConnector('example.com') + + search_result = abstract_connector.SearchResult( + key='http://www.example.com/book/1', + title='Gideon the Ninth', + author='Tamsyn Muir', + year='2019', + connector=connector + ) + + request = self.factory.get('', {'q': 'Test Book'}) + with patch('bookwyrm.views.is_api_request') as is_api: + is_api.return_value = False + with patch('bookwyrm.books_manager.search') as manager: + manager.return_value = [search_result] + response = views.search(request) + self.assertIsInstance(response, TemplateResponse) + self.assertEqual(response.template_name, 'search_results.html') + self.assertEqual( + response.context_data['book_results'][0].title, 'Gideon the Ninth') From d3161ea3619677ba228839640d5427ae33d71a24 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 11:42:07 -0800 Subject: [PATCH 13/79] Send appropriate error codes back with error pages --- bookwyrm/views.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bookwyrm/views.py b/bookwyrm/views.py index bd8948062..2c30c16fa 100644 --- a/bookwyrm/views.py +++ b/bookwyrm/views.py @@ -38,12 +38,14 @@ def is_api_request(request): def server_error_page(request): ''' 500 errors ''' - return TemplateResponse(request, 'error.html', {'title': 'Oops!'}) + return TemplateResponse( + request, 'error.html', {'title': 'Oops!'}, status=500) def not_found_page(request, _): ''' 404s ''' - return TemplateResponse(request, 'notfound.html', {'title': 'Not found'}) + return TemplateResponse( + request, 'notfound.html', {'title': 'Not found'}, status=404) @login_required From 5623c268d69b65c6332ba430cf0f7e7b5cf8651c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 11:55:13 -0800 Subject: [PATCH 14/79] Set application version number in settings --- bookwyrm/settings.py | 6 +++--- bookwyrm/wellknown.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index f598cd3c6..46c38b5af 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -7,6 +7,7 @@ import requests env = Env() DOMAIN = env('DOMAIN') +VERSION = '0.0.1' PAGE_LENGTH = env('PAGE_LENGTH', 15) @@ -153,6 +154,5 @@ STATIC_ROOT = os.path.join(BASE_DIR, env('STATIC_ROOT', 'static')) MEDIA_URL = '/images/' MEDIA_ROOT = os.path.join(BASE_DIR, env('MEDIA_ROOT', 'images')) -USER_AGENT = "%s (BookWyrm/%s; +https://%s/)" % (requests.utils.default_user_agent(), - "0.1", # TODO: change 0.1 into actual version - DOMAIN) +USER_AGENT = "%s (BookWyrm/%s; +https://%s/)" % ( + requests.utils.default_user_agent(), VERSION, DOMAIN) diff --git a/bookwyrm/wellknown.py b/bookwyrm/wellknown.py index ec8557ddf..1f6d4ccfb 100644 --- a/bookwyrm/wellknown.py +++ b/bookwyrm/wellknown.py @@ -6,7 +6,7 @@ from django.http import JsonResponse from django.utils import timezone from bookwyrm import models -from bookwyrm.settings import DOMAIN +from bookwyrm.settings import DOMAIN, VERSION def webfinger(request): @@ -76,7 +76,7 @@ def nodeinfo(request): 'version': '2.0', 'software': { 'name': 'bookwyrm', - 'version': '0.0.1' + 'version': VERSION }, 'protocols': [ 'activitypub' From e0adb3307b653519fe79e689008bc23e35928e69 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 12:01:15 -0800 Subject: [PATCH 15/79] Gracefully handle unknown status types Fixes #432 --- bookwyrm/incoming.py | 8 ++++++-- bookwyrm/tests/test_incoming.py | 6 ++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/bookwyrm/incoming.py b/bookwyrm/incoming.py index ddf99f97b..5e42fe45a 100644 --- a/bookwyrm/incoming.py +++ b/bookwyrm/incoming.py @@ -185,11 +185,15 @@ def handle_create(activity): ''' someone did something, good on them ''' # deduplicate incoming activities activity = activity['object'] - status_id = activity['id'] + status_id = activity.get('id') if models.Status.objects.filter(remote_id=status_id).count(): return - serializer = activitypub.activity_objects[activity['type']] + try: + serializer = activitypub.activity_objects[activity['type']] + except KeyError: + return + activity = serializer(**activity) try: model = models.activity_models[activity.type] diff --git a/bookwyrm/tests/test_incoming.py b/bookwyrm/tests/test_incoming.py index 0269c64ca..b8e3508e5 100644 --- a/bookwyrm/tests/test_incoming.py +++ b/bookwyrm/tests/test_incoming.py @@ -272,6 +272,12 @@ class Incoming(TestCase): incoming.handle_create(activity) self.assertEqual(models.Status.objects.count(), 2) + def test_handle_create_unknown_type(self): + ''' folks send you all kinds of things ''' + activity = {'object': {'id': 'hi'}, 'type': 'Fish'} + result = incoming.handle_create(activity) + self.assertIsNone(result) + def test_handle_create_remote_note_with_mention(self): ''' should only create it under the right circumstances ''' self.assertEqual(models.Status.objects.count(), 1) From 8bb7a081c21ee8daabdb723ca89b56e574a9269e Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 12:12:32 -0800 Subject: [PATCH 16/79] Makes follow request button a different color than an unread notification Fixes #420 --- bookwyrm/templates/snippets/follow_request_buttons.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/templates/snippets/follow_request_buttons.html b/bookwyrm/templates/snippets/follow_request_buttons.html index b5a64cc8d..17b3484b6 100644 --- a/bookwyrm/templates/snippets/follow_request_buttons.html +++ b/bookwyrm/templates/snippets/follow_request_buttons.html @@ -3,7 +3,7 @@
{% csrf_token %} - +
{% csrf_token %} From 44a8184f7257436e0aac4f310a5df1fb44d1caf6 Mon Sep 17 00:00:00 2001 From: "Renato \"Lond\" Cerqueira" Date: Wed, 30 Dec 2020 11:12:04 +0100 Subject: [PATCH 17/79] Create a ActivitypubResponse class and use it in the views This improves compatibility with other fediverse software by using the expected content type for any api calls. --- bookwyrm/activitypub/__init__.py | 1 + bookwyrm/activitypub/response.py | 18 ++++++++++++++++++ bookwyrm/views.py | 29 +++++++++++------------------ 3 files changed, 30 insertions(+), 18 deletions(-) create mode 100644 bookwyrm/activitypub/response.py diff --git a/bookwyrm/activitypub/__init__.py b/bookwyrm/activitypub/__init__.py index b5b124ec0..a4fef41e5 100644 --- a/bookwyrm/activitypub/__init__.py +++ b/bookwyrm/activitypub/__init__.py @@ -11,6 +11,7 @@ from .note import Tombstone from .interaction import Boost, Like from .ordered_collection import OrderedCollection, OrderedCollectionPage from .person import Person, PublicKey +from .response import ActivitypubResponse from .book import Edition, Work, Author from .verbs import Create, Delete, Undo, Update from .verbs import Follow, Accept, Reject diff --git a/bookwyrm/activitypub/response.py b/bookwyrm/activitypub/response.py new file mode 100644 index 000000000..bbc44c4de --- /dev/null +++ b/bookwyrm/activitypub/response.py @@ -0,0 +1,18 @@ +from django.http import JsonResponse + +from .base_activity import ActivityEncoder + +class ActivitypubResponse(JsonResponse): + """ + A class to be used in any place that's serializing responses for + Activitypub enabled clients. Uses JsonResponse under the hood, but already + configures some stuff beforehand. Made to be a drop-in replacement of + JsonResponse. + """ + def __init__(self, data, encoder=ActivityEncoder, safe=True, + json_dumps_params=None, **kwargs): + + if 'content_type' not in kwargs: + kwargs['content_type'] = 'application/activity+json' + + super().__init__(data, encoder, safe, json_dumps_params, **kwargs) diff --git a/bookwyrm/views.py b/bookwyrm/views.py index 1afb82669..5ed683c02 100644 --- a/bookwyrm/views.py +++ b/bookwyrm/views.py @@ -13,7 +13,7 @@ from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_GET from bookwyrm import outgoing -from bookwyrm.activitypub import ActivityEncoder +from bookwyrm.activitypub import ActivityEncoder, ActivitypubResponse from bookwyrm import forms, models, books_manager from bookwyrm import goodreads_import from bookwyrm.settings import PAGE_LENGTH @@ -378,7 +378,7 @@ def user_page(request, username): if is_api_request(request): # we have a json request - return JsonResponse(user.to_activity(), encoder=ActivityEncoder) + return ActivitypubResponse(user.to_activity()) # otherwise we're at a UI view try: @@ -446,7 +446,7 @@ def followers_page(request, username): return HttpResponseNotFound() if is_api_request(request): - return JsonResponse(user.to_followers_activity(**request.GET)) + return ActivitypubResponse(user.to_followers_activity(**request.GET)) data = { 'title': '%s: followers' % user.name, @@ -467,7 +467,7 @@ def following_page(request, username): return HttpResponseNotFound() if is_api_request(request): - return JsonResponse(user.to_following_activity(**request.GET)) + return ActivitypubResponse(user.to_following_activity(**request.GET)) data = { 'title': '%s: following' % user.name, @@ -497,7 +497,7 @@ def status_page(request, username, status_id): return HttpResponseNotFound() if is_api_request(request): - return JsonResponse(status.to_activity(), encoder=ActivityEncoder) + return ActivitypubResponse(status.to_activity()) data = { 'title': 'Status by %s' % user.username, @@ -530,10 +530,7 @@ def replies_page(request, username, status_id): if status.user.localname != username: return HttpResponseNotFound() - return JsonResponse( - status.to_replies(**request.GET), - encoder=ActivityEncoder - ) + return ActivitypubResponse(status.to_replies(**request.GET)) @login_required @@ -565,7 +562,7 @@ def book_page(request, book_id): return HttpResponseNotFound() if is_api_request(request): - return JsonResponse(book.to_activity(), encoder=ActivityEncoder) + return ActivitypubResponse(book.to_activity()) if isinstance(book, models.Work): book = book.get_default_edition() @@ -677,10 +674,7 @@ def editions_page(request, book_id): work = get_object_or_404(models.Work, id=book_id) if is_api_request(request): - return JsonResponse( - work.to_edition_list(**request.GET), - encoder=ActivityEncoder - ) + return ActivitypubResponse(work.to_edition_list(**request.GET)) data = { 'title': 'Editions of %s' % work.title, @@ -696,7 +690,7 @@ def author_page(request, author_id): author = get_object_or_404(models.Author, id=author_id) if is_api_request(request): - return JsonResponse(author.to_activity(), encoder=ActivityEncoder) + return ActivitypubResponse(author.to_activity()) books = models.Work.objects.filter( Q(authors=author) | Q(editions__authors=author)).distinct() @@ -716,8 +710,7 @@ def tag_page(request, tag_id): return HttpResponseNotFound() if is_api_request(request): - return JsonResponse( - tag_obj.to_activity(**request.GET), encoder=ActivityEncoder) + return ActivitypubResponse(tag_obj.to_activity(**request.GET)) books = models.Edition.objects.filter( usertag__tag__identifier=tag_id @@ -768,7 +761,7 @@ def shelf_page(request, username, shelf_identifier): if is_api_request(request): - return JsonResponse(shelf.to_activity(**request.GET)) + return ActivitypubResponse(shelf.to_activity(**request.GET)) data = { 'title': '%s\'s %s shelf' % (user.display_name, shelf.name), From 88d8b6e5776abb5d28484b8543c7b022e71c1b3f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 12:38:34 -0800 Subject: [PATCH 18/79] Cleans up outbox function --- bookwyrm/outgoing.py | 13 +++++-------- bookwyrm/tests/test_outgoing.py | 26 +++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/bookwyrm/outgoing.py b/bookwyrm/outgoing.py index 00154cf44..73c1f92a9 100644 --- a/bookwyrm/outgoing.py +++ b/bookwyrm/outgoing.py @@ -2,8 +2,10 @@ import re from django.db import IntegrityError, transaction -from django.http import HttpResponseNotFound, JsonResponse +from django.http import JsonResponse +from django.shortcuts import get_object_or_404 from django.views.decorators.csrf import csrf_exempt +from django.views.decorators.http import require_GET from markdown import markdown from requests import HTTPError @@ -20,15 +22,10 @@ from bookwyrm.utils import regex @csrf_exempt +@require_GET def outbox(request, username): ''' outbox for the requested user ''' - if request.method != 'GET': - return HttpResponseNotFound() - - try: - user = models.User.objects.get(localname=username) - except models.User.DoesNotExist: - return HttpResponseNotFound() + user = get_object_or_404(models.User, localname=username) # collection overview return JsonResponse( diff --git a/bookwyrm/tests/test_outgoing.py b/bookwyrm/tests/test_outgoing.py index 2c1d119cc..03bcb99b9 100644 --- a/bookwyrm/tests/test_outgoing.py +++ b/bookwyrm/tests/test_outgoing.py @@ -3,7 +3,9 @@ import json import pathlib from unittest.mock import patch +from django.http import JsonResponse from django.test import TestCase +from django.test.client import RequestFactory import responses from bookwyrm import models, outgoing @@ -14,6 +16,7 @@ class Outgoing(TestCase): ''' sends out activities ''' def setUp(self): ''' we'll need some data ''' + self.factory = RequestFactory() with patch('bookwyrm.models.user.set_remote_server'): self.remote_user = models.User.objects.create_user( 'rat', 'rat@rat.com', 'ratword', @@ -24,7 +27,7 @@ class Outgoing(TestCase): ) self.local_user = models.User.objects.create_user( 'mouse', 'mouse@mouse.com', 'mouseword', local=True, - remote_id='https://example.com/users/mouse', + localname='mouse', remote_id='https://example.com/users/mouse', ) datafile = pathlib.Path(__file__).parent.joinpath( @@ -46,6 +49,27 @@ class Outgoing(TestCase): ) + def test_outbox(self): + ''' returns user's statuses ''' + request = self.factory.get('') + result = outgoing.outbox(request, 'mouse') + self.assertIsInstance(result, JsonResponse) + + def test_outbox_bad_method(self): + ''' can't POST to outbox ''' + request = self.factory.post('') + result = outgoing.outbox(request, 'mouse') + self.assertEqual(result.status_code, 405) + + def test_outbox_unknown_user(self): + ''' should 404 for unknown and remote users ''' + request = self.factory.post('') + result = outgoing.outbox(request, 'beepboop') + self.assertEqual(result.status_code, 405) + result = outgoing.outbox(request, 'rat') + self.assertEqual(result.status_code, 405) + + def test_handle_follow(self): ''' send a follow request ''' self.assertEqual(models.UserFollowRequest.objects.count(), 0) From babc604397e95278f767bc5eb764ba7e1273897f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 12:41:19 -0800 Subject: [PATCH 19/79] Fixes outbox privacy --- bookwyrm/models/user.py | 1 + bookwyrm/outgoing.py | 1 - bookwyrm/tests/test_outgoing.py | 18 ++++++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index 30eeffbc8..cf6dd3b2b 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -111,6 +111,7 @@ class User(OrderedCollectionPageMixin, AbstractUser): queryset = Status.objects.filter( user=self, deleted=False, + privacy__in=['public', 'unlisted'], ).select_subclasses().order_by('-published_date') return self.to_ordered_collection(queryset, \ remote_id=self.outbox, **kwargs) diff --git a/bookwyrm/outgoing.py b/bookwyrm/outgoing.py index 73c1f92a9..de2a4cbe9 100644 --- a/bookwyrm/outgoing.py +++ b/bookwyrm/outgoing.py @@ -27,7 +27,6 @@ def outbox(request, username): ''' outbox for the requested user ''' user = get_object_or_404(models.User, localname=username) - # collection overview return JsonResponse( user.to_outbox(**request.GET), encoder=activitypub.ActivityEncoder diff --git a/bookwyrm/tests/test_outgoing.py b/bookwyrm/tests/test_outgoing.py index 03bcb99b9..b89e75f69 100644 --- a/bookwyrm/tests/test_outgoing.py +++ b/bookwyrm/tests/test_outgoing.py @@ -69,6 +69,24 @@ class Outgoing(TestCase): result = outgoing.outbox(request, 'rat') self.assertEqual(result.status_code, 405) + def test_outbox_privacy(self): + ''' don't show dms et cetera in outbox ''' + models.Status.objects.create( + content='PRIVATE!!', user=self.local_user, privacy='direct') + models.Status.objects.create( + content='bffs ONLY', user=self.local_user, privacy='followers') + models.Status.objects.create( + content='unlisted status', user=self.local_user, privacy='unlisted') + models.Status.objects.create( + content='look at this', user=self.local_user, privacy='public') + + request = self.factory.get('') + result = outgoing.outbox(request, 'mouse') + self.assertIsInstance(result, JsonResponse) + data = json.loads(result.content) + self.assertEqual(data['type'], 'OrderedCollection') + self.assertEqual(data['totalItems'], 2) + def test_handle_follow(self): ''' send a follow request ''' From c1243b5c21ff64ffb520e1a0f61684d55a70ee5b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 13:14:16 -0800 Subject: [PATCH 20/79] Makes outbox filter-able --- bookwyrm/models/__init__.py | 3 +++ bookwyrm/models/base_model.py | 4 +++- bookwyrm/models/user.py | 15 +++++++++++++-- bookwyrm/outgoing.py | 5 ++++- bookwyrm/tests/test_outgoing.py | 22 ++++++++++++++++++++++ 5 files changed, 45 insertions(+), 4 deletions(-) diff --git a/bookwyrm/models/__init__.py b/bookwyrm/models/__init__.py index 0c3bf33e8..762904166 100644 --- a/bookwyrm/models/__init__.py +++ b/bookwyrm/models/__init__.py @@ -25,3 +25,6 @@ from .site import SiteSettings, SiteInvite, PasswordReset cls_members = inspect.getmembers(sys.modules[__name__], inspect.isclass) activity_models = {c[1].activity_serializer.__name__: c[1] \ for c in cls_members if hasattr(c[1], 'activity_serializer')} + +status_models = [ + c.__name__ for (_, c) in activity_models.items() if issubclass(c, Status)] diff --git a/bookwyrm/models/base_model.py b/bookwyrm/models/base_model.py index 0de61fd1b..469a69720 100644 --- a/bookwyrm/models/base_model.py +++ b/bookwyrm/models/base_model.py @@ -237,7 +237,9 @@ class OrderedCollectionPageMixin(ActivitypubMixin): ).serialize() -def to_ordered_collection_page(queryset, remote_id, id_only=False, page=1): +# pylint: disable=unused-argument +def to_ordered_collection_page( + queryset, remote_id, id_only=False, page=1, **kwargs): ''' serialize and pagiante a queryset ''' paginated = Paginator(queryset, PAGE_LENGTH) diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index cf6dd3b2b..b27ce4475 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -1,6 +1,7 @@ ''' database schema for user data ''' from urllib.parse import urlparse +from django.apps import apps from django.contrib.auth.models import AbstractUser from django.db import models from django.dispatch import receiver @@ -106,9 +107,19 @@ class User(OrderedCollectionPageMixin, AbstractUser): activity_serializer = activitypub.Person - def to_outbox(self, **kwargs): + def to_outbox(self, filter_type=None, **kwargs): ''' an ordered collection of statuses ''' - queryset = Status.objects.filter( + if filter_type: + filter_class = apps.get_model( + 'bookwyrm.%s' % filter_type, require_ready=True) + if not issubclass(filter_class, Status): + raise TypeError( + 'filter_status_class must be a subclass of models.Status') + queryset = filter_class.objects + else: + queryset = Status.objects + + queryset = queryset.filter( user=self, deleted=False, privacy__in=['public', 'unlisted'], diff --git a/bookwyrm/outgoing.py b/bookwyrm/outgoing.py index de2a4cbe9..aee463c90 100644 --- a/bookwyrm/outgoing.py +++ b/bookwyrm/outgoing.py @@ -26,9 +26,12 @@ from bookwyrm.utils import regex def outbox(request, username): ''' outbox for the requested user ''' user = get_object_or_404(models.User, localname=username) + filter_type = request.GET.get('type') + if filter_type not in models.status_models: + filter_type = None return JsonResponse( - user.to_outbox(**request.GET), + user.to_outbox(**request.GET, filter_type=filter_type), encoder=activitypub.ActivityEncoder ) diff --git a/bookwyrm/tests/test_outgoing.py b/bookwyrm/tests/test_outgoing.py index b89e75f69..923600349 100644 --- a/bookwyrm/tests/test_outgoing.py +++ b/bookwyrm/tests/test_outgoing.py @@ -87,6 +87,28 @@ class Outgoing(TestCase): self.assertEqual(data['type'], 'OrderedCollection') self.assertEqual(data['totalItems'], 2) + def test_outbox_filter(self): + ''' if we only care about reviews, only get reviews ''' + models.Review.objects.create( + content='look at this', name='hi', rating=1, + book=self.book, user=self.local_user) + models.Status.objects.create( + content='look at this', user=self.local_user) + + request = self.factory.get('', {'type': 'bleh'}) + result = outgoing.outbox(request, 'mouse') + self.assertIsInstance(result, JsonResponse) + data = json.loads(result.content) + self.assertEqual(data['type'], 'OrderedCollection') + self.assertEqual(data['totalItems'], 2) + + request = self.factory.get('', {'type': 'Review'}) + result = outgoing.outbox(request, 'mouse') + self.assertIsInstance(result, JsonResponse) + data = json.loads(result.content) + self.assertEqual(data['type'], 'OrderedCollection') + self.assertEqual(data['totalItems'], 1) + def test_handle_follow(self): ''' send a follow request ''' From 45c13bd76c4b416096aa06538d96861794258002 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 13:16:09 -0800 Subject: [PATCH 21/79] Only get reviews when loading user data --- bookwyrm/models/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index b27ce4475..e70309a72 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -284,7 +284,7 @@ def get_or_create_remote_server(domain): @app.task def get_remote_reviews(outbox): ''' ingest reviews by a new remote bookwyrm user ''' - outbox_page = outbox + '?page=true' + outbox_page = outbox + '?page=true&type=Review' data = get_data(outbox_page) # TODO: pagination? From 48f002727aea18701a1d80378bf07509aa489f6d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 13:59:51 -0800 Subject: [PATCH 22/79] small change and test to get_user_from_username --- bookwyrm/tests/test_views.py | 12 ++++++++++++ bookwyrm/views.py | 8 ++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/bookwyrm/tests/test_views.py b/bookwyrm/tests/test_views.py index ba4531f69..2c4353b7b 100644 --- a/bookwyrm/tests/test_views.py +++ b/bookwyrm/tests/test_views.py @@ -23,6 +23,18 @@ class Views(TestCase): connector_file='self_connector', local=True ) + self.local_user = models.User.objects.create_user( + 'mouse', 'mouse@mouse.mouse', 'password', local=True) + + + def test_get_user_from_username(self): + ''' works for either localname or username ''' + self.assertEqual( + views.get_user_from_username('mouse'), self.local_user) + self.assertEqual( + views.get_user_from_username('mouse@%s' % DOMAIN), self.local_user) + with self.assertRaises(models.User.DoesNotExist): + views.get_user_from_username('mojfse@example.com') def test_search_json_response(self): diff --git a/bookwyrm/views.py b/bookwyrm/views.py index 946c061ba..df08ab18f 100644 --- a/bookwyrm/views.py +++ b/bookwyrm/views.py @@ -13,7 +13,7 @@ from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_GET from bookwyrm import outgoing -from bookwyrm.activitypub import ActivityEncoder, ActivitypubResponse +from bookwyrm.activitypub import ActivitypubResponse from bookwyrm import forms, models, books_manager from bookwyrm import goodreads_import from bookwyrm.settings import PAGE_LENGTH @@ -23,11 +23,11 @@ from bookwyrm.utils import regex def get_user_from_username(username): ''' helper function to resolve a localname or a username to a user ''' + # raises DoesNotExist if user is now found try: - user = models.User.objects.get(localname=username) + return models.User.objects.get(localname=username) except models.User.DoesNotExist: - user = models.User.objects.get(username=username) - return user + return models.User.objects.get(username=username) def is_api_request(request): From 4e413a3779435c901751bfdb3880611faa23ea8a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 14:57:57 -0800 Subject: [PATCH 23/79] Adds tests for generating feeds --- bookwyrm/tests/test_views.py | 85 ++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/bookwyrm/tests/test_views.py b/bookwyrm/tests/test_views.py index 2c4353b7b..1dff049ae 100644 --- a/bookwyrm/tests/test_views.py +++ b/bookwyrm/tests/test_views.py @@ -25,6 +25,14 @@ class Views(TestCase): ) self.local_user = models.User.objects.create_user( 'mouse', 'mouse@mouse.mouse', 'password', local=True) + with patch('bookwyrm.models.user.set_remote_server.delay'): + self.remote_user = models.User.objects.create_user( + 'rat', 'rat@rat.com', 'ratword', + local=False, + remote_id='https://example.com/users/rat', + inbox='https://example.com/users/rat/inbox', + outbox='https://example.com/users/rat/outbox', + ) def test_get_user_from_username(self): @@ -37,6 +45,83 @@ class Views(TestCase): views.get_user_from_username('mojfse@example.com') + def test_is_api_request(self): + ''' should it return html or json ''' + request = self.factory.get('/path') + request.headers = {'Accept': 'application/json'} + self.assertTrue(views.is_api_request(request)) + + request = self.factory.get('/path.json') + request.headers = {'Accept': 'Praise'} + self.assertTrue(views.is_api_request(request)) + + request = self.factory.get('/path') + request.headers = {'Accept': 'Praise'} + self.assertFalse(views.is_api_request(request)) + + + def test_get_activity_feed(self): + ''' loads statuses ''' + rat = models.User.objects.create_user( + 'rat', 'rat@rat.rat', 'password', local=True) + + public_status = models.Comment.objects.create( + content='public status', book=self.book, user=self.local_user) + direct_status = models.Status.objects.create( + content='direct', user=self.local_user, privacy='direct') + + rat_public = models.Status.objects.create( + content='blah blah', user=rat) + rat_unlisted = models.Status.objects.create( + content='blah blah', user=rat, privacy='unlisted') + remote_status = models.Status.objects.create( + content='blah blah', user=self.remote_user) + followers_status = models.Status.objects.create( + content='blah', user=rat, privacy='followers') + rat_mention = models.Status.objects.create( + content='blah blah blah', user=rat, privacy='followers') + rat_mention.mention_users.set([self.local_user]) + + statuses = views.get_activity_feed(self.local_user, 'home') + self.assertEqual(len(statuses), 2) + self.assertEqual(statuses[1], public_status) + self.assertEqual(statuses[0], rat_mention) + + statuses = views.get_activity_feed( + self.local_user, 'home', model=models.Comment) + self.assertEqual(len(statuses), 1) + self.assertEqual(statuses[0], public_status) + + statuses = views.get_activity_feed(self.local_user, 'local') + self.assertEqual(len(statuses), 2) + self.assertEqual(statuses[1], public_status) + self.assertEqual(statuses[0], rat_public) + + statuses = views.get_activity_feed(self.local_user, 'direct') + self.assertEqual(len(statuses), 1) + self.assertEqual(statuses[0], direct_status) + + statuses = views.get_activity_feed(self.local_user, 'federated') + self.assertEqual(len(statuses), 3) + self.assertEqual(statuses[2], public_status) + self.assertEqual(statuses[1], rat_public) + self.assertEqual(statuses[0], remote_status) + + statuses = views.get_activity_feed(self.local_user, 'friends') + self.assertEqual(len(statuses), 2) + self.assertEqual(statuses[1], public_status) + self.assertEqual(statuses[0], rat_mention) + + rat.followers.add(self.local_user) + statuses = views.get_activity_feed(self.local_user, 'friends') + self.assertEqual(len(statuses), 5) + self.assertEqual(statuses[4], public_status) + self.assertEqual(statuses[3], rat_public) + self.assertEqual(statuses[2], rat_unlisted) + self.assertEqual(statuses[1], followers_status) + self.assertEqual(statuses[0], rat_mention) + + def test_search_json_response(self): ''' searches local data only and returns book data in json format ''' # we need a connector for this, sorry From 670036f8a91e4a07f093a3306d792a1d1b82ae63 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 15:21:14 -0800 Subject: [PATCH 24/79] Fixes user preview on rating display --- bookwyrm/templates/book.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/templates/book.html b/bookwyrm/templates/book.html index 506ee3db0..4bbc8d103 100644 --- a/bookwyrm/templates/book.html +++ b/bookwyrm/templates/book.html @@ -166,10 +166,10 @@ {% for rating in ratings %}
-
{% include 'snippets/avatar.html' %}
+
{% include 'snippets/avatar.html' with user=rating.user %}
- {% include 'snippets/username.html' %} + {% include 'snippets/username.html' with user=rating.user %}
rated it
From 885bb023a3e882e2449cf0e2afbdb5d4faf50302 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 16:07:29 -0800 Subject: [PATCH 25/79] Sort shelves by shelfbook updated date --- bookwyrm/templates/shelf.html | 2 +- bookwyrm/templates/snippets/shelf.html | 4 ++-- bookwyrm/views.py | 5 +++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/bookwyrm/templates/shelf.html b/bookwyrm/templates/shelf.html index 390b9fc66..4b27e3a49 100644 --- a/bookwyrm/templates/shelf.html +++ b/bookwyrm/templates/shelf.html @@ -122,7 +122,7 @@
- {% include 'snippets/shelf.html' with shelf=shelf ratings=ratings %} + {% include 'snippets/shelf.html' with shelf=shelf books=books ratings=ratings %}
{% endblock %} diff --git a/bookwyrm/templates/snippets/shelf.html b/bookwyrm/templates/snippets/shelf.html index 0d05e661e..d5cdd003f 100644 --- a/bookwyrm/templates/snippets/shelf.html +++ b/bookwyrm/templates/snippets/shelf.html @@ -1,6 +1,6 @@ {% load humanize %} {% load bookwyrm_tags %} -{% if shelf.books.all|length > 0 %} +{% if books|length > 0 %} @@ -34,7 +34,7 @@ {% endif %} -{% for book in shelf.books.all %} +{% for book in books %}
{% include 'snippets/book_cover.html' with book=book size="small" %} diff --git a/bookwyrm/views.py b/bookwyrm/views.py index df08ab18f..0d7726b63 100644 --- a/bookwyrm/views.py +++ b/bookwyrm/views.py @@ -765,12 +765,17 @@ def shelf_page(request, username, shelf_identifier): if is_api_request(request): return ActivitypubResponse(shelf.to_activity(**request.GET)) + books = models.ShelfBook.objects.filter( + added_by=user, shelf=shelf + ).order_by('-updated_date').all() + data = { 'title': '%s\'s %s shelf' % (user.display_name, shelf.name), 'user': user, 'is_self': is_self, 'shelves': shelves.all(), 'shelf': shelf, + 'books': [b.book for b in books], } return TemplateResponse(request, 'shelf.html', data) From d42ebbaf4a83f790e9942bfb1f7f1220b180e94e Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 16:10:32 -0800 Subject: [PATCH 26/79] Link to local versions of statuses in notifications --- bookwyrm/templates/notifications.html | 4 ++-- bookwyrm/urls.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bookwyrm/templates/notifications.html b/bookwyrm/templates/notifications.html index ddcbc0fda..a50d67af2 100644 --- a/bookwyrm/templates/notifications.html +++ b/bookwyrm/templates/notifications.html @@ -26,10 +26,10 @@ {% elif notification.notification_type == 'MENTION' %} mentioned you in a - status + status {% elif notification.notification_type == 'REPLY' %} - replied + replied to your status {% elif notification.notification_type == 'FOLLOW' %} diff --git a/bookwyrm/urls.py b/bookwyrm/urls.py index 22edd38a8..3eea012ab 100644 --- a/bookwyrm/urls.py +++ b/bookwyrm/urls.py @@ -20,7 +20,7 @@ status_types = [ 'generatednote' ] status_path = r'%s/(%s)/(?P\d+)' % \ - (local_user_path, '|'.join(status_types)) + (user_path, '|'.join(status_types)) book_path = r'^book/(?P\d+)' From d821a08cffb5cd96406dd7358db1a529e9d11c22 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 16:33:04 -0800 Subject: [PATCH 27/79] include tags in replies I was against this but apparently it helps the replies actually WORK --- bookwyrm/templates/snippets/reply_form.html | 2 +- bookwyrm/templatetags/bookwyrm_tags.py | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/bookwyrm/templates/snippets/reply_form.html b/bookwyrm/templates/snippets/reply_form.html index 787a3ac54..8150b8a5e 100644 --- a/bookwyrm/templates/snippets/reply_form.html +++ b/bookwyrm/templates/snippets/reply_form.html @@ -9,7 +9,7 @@ {% include 'snippets/content_warning_field.html' with parent_status=activity %}
- +
diff --git a/bookwyrm/templatetags/bookwyrm_tags.py b/bookwyrm/templatetags/bookwyrm_tags.py index 61b097bd2..89c1690a4 100644 --- a/bookwyrm/templatetags/bookwyrm_tags.py +++ b/bookwyrm/templatetags/bookwyrm_tags.py @@ -110,7 +110,7 @@ def get_uuid(identifier): return '%s%s' % (identifier, uuid4()) -@register.filter(name="post_date") +@register.filter(name='post_date') def time_since(date): ''' concise time ago function ''' if not isinstance(date, datetime): @@ -133,13 +133,20 @@ def time_since(date): return '%ds' % delta.seconds -@register.filter(name="to_markdown") +@register.filter(name='to_markdown') def get_markdown(content): ''' convert markdown to html ''' if content: return to_markdown(content) return None +@register.filter(name='mentions') +def get_mentions(status, user): + ''' anyone tagged or replied to in this status ''' + mentions = set([status.user] + list(status.mention_users.all())) + return ' '.join( + '@' + get_user_identifier(m) for m in mentions if not m == user) + @register.simple_tag(takes_context=True) def active_shelf(context, book): ''' check what shelf a user has a book on, if any ''' From dc68fdd53e09f7bbac162bc175ed16242c1f4612 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 30 Dec 2020 17:36:35 -0800 Subject: [PATCH 28/79] User generated local paths --- bookwyrm/models/base_model.py | 5 +++++ bookwyrm/models/user.py | 5 +++++ bookwyrm/templates/author.html | 2 +- bookwyrm/templates/following.html | 2 +- bookwyrm/templates/notifications.html | 12 +++++----- .../snippets/follow_request_buttons.html | 22 ++++++++++--------- bookwyrm/templates/snippets/reply_form.html | 10 ++++----- bookwyrm/templates/snippets/tag.html | 2 +- bookwyrm/templates/snippets/user_header.html | 6 ++--- bookwyrm/templates/snippets/username.html | 2 +- bookwyrm/templates/user.html | 6 ++--- bookwyrm/urls.py | 7 +++--- bookwyrm/utils/regex.py | 5 +++-- bookwyrm/views.py | 2 +- 14 files changed, 50 insertions(+), 38 deletions(-) diff --git a/bookwyrm/models/base_model.py b/bookwyrm/models/base_model.py index 469a69720..d82334e1a 100644 --- a/bookwyrm/models/base_model.py +++ b/bookwyrm/models/base_model.py @@ -35,6 +35,11 @@ class BookWyrmModel(models.Model): ''' this is just here to provide default fields for other models ''' abstract = True + @property + def local_path(self): + ''' how to link to this object in the local app ''' + return self.get_remote_id().replace('https://%s' % DOMAIN, '') + @receiver(models.signals.post_save) #pylint: disable=unused-argument diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index e70309a72..686af24da 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -179,6 +179,11 @@ class User(OrderedCollectionPageMixin, AbstractUser): return super().save(*args, **kwargs) + @property + def local_path(self): + ''' this model doesn't inherit bookwyrm model, so here we are ''' + return '/user/%s' % (self.localname or self.username) + class KeyPair(ActivitypubMixin, BookWyrmModel): ''' public and private keys for a user ''' diff --git a/bookwyrm/templates/author.html b/bookwyrm/templates/author.html index e51ef3021..4235b266a 100644 --- a/bookwyrm/templates/author.html +++ b/bookwyrm/templates/author.html @@ -8,7 +8,7 @@
{% if request.user.is_authenticated and perms.bookwyrm.edit_book %} {% endfor %} {% if not following.count %} -
No one is following {{ user|username }}
+
{{ user|username }} isn't following any users
{% endif %} diff --git a/bookwyrm/templates/notifications.html b/bookwyrm/templates/notifications.html index a50d67af2..50b4946f6 100644 --- a/bookwyrm/templates/notifications.html +++ b/bookwyrm/templates/notifications.html @@ -22,16 +22,16 @@ {% include 'snippets/username.html' with user=notification.related_user %} {% if notification.notification_type == 'FAVORITE' %} favorited your -
status + status {% elif notification.notification_type == 'MENTION' %} mentioned you in a - status + status {% elif notification.notification_type == 'REPLY' %} - replied + replied to your - status + status {% elif notification.notification_type == 'FOLLOW' %} followed you {% elif notification.notification_type == 'FOLLOW_REQUEST' %} @@ -41,7 +41,7 @@ {% elif notification.notification_type == 'BOOST' %} - boosted your status + boosted your status {% endif %} {% else %} your import completed. @@ -54,7 +54,7 @@
{{ notification.related_status.published_date | post_date }} diff --git a/bookwyrm/templates/snippets/follow_request_buttons.html b/bookwyrm/templates/snippets/follow_request_buttons.html index 17b3484b6..a6f585c74 100644 --- a/bookwyrm/templates/snippets/follow_request_buttons.html +++ b/bookwyrm/templates/snippets/follow_request_buttons.html @@ -1,13 +1,15 @@ {% load bookwyrm_tags %} {% if request.user|follow_request_exists:user %} - - {% csrf_token %} - - - -
- {% csrf_token %} - - -
+
+
+ {% csrf_token %} + + +
+
+ {% csrf_token %} + + +
+
{% endif %} diff --git a/bookwyrm/templates/snippets/reply_form.html b/bookwyrm/templates/snippets/reply_form.html index 8150b8a5e..b70a1f87c 100644 --- a/bookwyrm/templates/snippets/reply_form.html +++ b/bookwyrm/templates/snippets/reply_form.html @@ -1,20 +1,20 @@ {% load bookwyrm_tags %} -{% with activity.id|uuid as uuid %} +{% with status.id|uuid as uuid %}
{% csrf_token %} - +
- {% include 'snippets/content_warning_field.html' with parent_status=activity %} + {% include 'snippets/content_warning_field.html' with parent_status=status %}
- +
- {% include 'snippets/privacy_select.html' with current=activity.privacy %} + {% include 'snippets/privacy_select.html' with current=status.privacy %}
-

- Imports are limited in size, and only the first {{ limit }} items will be imported.

diff --git a/bookwyrm/views.py b/bookwyrm/views.py index 9a0157b11..10137684b 100644 --- a/bookwyrm/views.py +++ b/bookwyrm/views.py @@ -14,7 +14,6 @@ from django.views.decorators.http import require_GET from bookwyrm import outgoing from bookwyrm import forms, models -from bookwyrm import goodreads_import from bookwyrm.activitypub import ActivitypubResponse from bookwyrm.connectors import connector_manager from bookwyrm.settings import PAGE_LENGTH @@ -252,7 +251,6 @@ def import_page(request): 'import_form': forms.ImportForm(), 'jobs': models.ImportJob. objects.filter(user=request.user).order_by('-created_date'), - 'limit': goodreads_import.MAX_ENTRIES, }) From 3344eed3b978ca449f86bf4a75b052b304d4899c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 2 Jan 2021 11:29:50 -0800 Subject: [PATCH 55/79] Tests for goodreads import lookup --- bookwyrm/connectors/self_connector.py | 3 +- bookwyrm/goodreads_import.py | 4 +- bookwyrm/tests/test_goodreads_import.py | 69 ++++++++++++++++++++++++- 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/bookwyrm/connectors/self_connector.py b/bookwyrm/connectors/self_connector.py index cad982493..7d0298be7 100644 --- a/bookwyrm/connectors/self_connector.py +++ b/bookwyrm/connectors/self_connector.py @@ -9,8 +9,7 @@ from .abstract_connector import AbstractConnector, SearchResult class Connector(AbstractConnector): ''' instantiate a connector ''' def search(self, query, min_confidence=0.1): - ''' right now you can't search bookwyrm sorry, but when - that gets implemented it will totally rule ''' + ''' search your local database ''' vector = SearchVector('title', weight='A') +\ SearchVector('subtitle', weight='B') +\ SearchVector('authors__name', weight='C') +\ diff --git a/bookwyrm/goodreads_import.py b/bookwyrm/goodreads_import.py index b052ce99f..9b8a4f019 100644 --- a/bookwyrm/goodreads_import.py +++ b/bookwyrm/goodreads_import.py @@ -23,6 +23,7 @@ def create_job(user, csv_file, include_reviews, privacy): ImportItem(job=job, index=index, data=entry).save() return job + def create_retry_job(user, original_job, items): ''' retry items that didn't import ''' job = ImportJob.objects.create( @@ -35,6 +36,7 @@ def create_retry_job(user, original_job, items): ImportItem(job=job, index=item.index, data=item.data).save() return job + def start_import(job): ''' initalizes a csv import job ''' result = import_data.delay(job.id) @@ -47,7 +49,6 @@ def import_data(job_id): ''' does the actual lookup work in a celery task ''' job = ImportJob.objects.get(id=job_id) try: - results = [] for item in job.items.all(): try: item.resolve() @@ -59,7 +60,6 @@ def import_data(job_id): if item.book: item.save() - results.append(item) # shelves book and handles reviews outgoing.handle_imported_book( diff --git a/bookwyrm/tests/test_goodreads_import.py b/bookwyrm/tests/test_goodreads_import.py index 312538a49..2518ab7b4 100644 --- a/bookwyrm/tests/test_goodreads_import.py +++ b/bookwyrm/tests/test_goodreads_import.py @@ -1,10 +1,13 @@ ''' testing import ''' +from collections import namedtuple import pathlib +from unittest.mock import patch from django.test import TestCase import responses from bookwyrm import goodreads_import, models +from bookwyrm.settings import DOMAIN class GoodreadsImport(TestCase): ''' importing from goodreads csv ''' @@ -16,16 +19,29 @@ class GoodreadsImport(TestCase): self.user = models.User.objects.create_user( 'mouse', 'mouse@mouse.mouse', 'password', local=True) + models.Connector.objects.create( + identifier=DOMAIN, + name='Local', + local=True, + connector_file='self_connector', + base_url='https://%s' % DOMAIN, + books_url='https://%s/book' % DOMAIN, + covers_url='https://%s/images/covers' % DOMAIN, + search_url='https://%s/search?q=' % DOMAIN, + priority=1, + ) + def test_create_job(self): ''' creates the import job entry and checks csv ''' - goodreads_import.create_job(self.user, self.csv, False, 'public') - import_job = models.ImportJob.objects.get() + import_job = goodreads_import.create_job( + self.user, self.csv, False, 'public') self.assertEqual(import_job.user, self.user) self.assertEqual(import_job.include_reviews, False) self.assertEqual(import_job.privacy, 'public') import_items = models.ImportItem.objects.filter(job=import_job).all() + self.assertEqual(len(import_items), 3) self.assertEqual(import_items[0].index, 0) self.assertEqual(import_items[0].data['Book Id'], '42036538') self.assertEqual(import_items[1].index, 1) @@ -34,6 +50,55 @@ class GoodreadsImport(TestCase): self.assertEqual(import_items[2].data['Book Id'], '28694510') + def test_create_retry_job(self): + ''' trying again with items that didn't import ''' + import_job = goodreads_import.create_job( + self.user, self.csv, False, 'unlisted') + import_items = models.ImportItem.objects.filter( + job=import_job + ).all()[:2] + + retry = goodreads_import.create_retry_job( + self.user, import_job, import_items) + self.assertNotEqual(import_job, retry) + self.assertEqual(retry.user, self.user) + self.assertEqual(retry.include_reviews, False) + self.assertEqual(retry.privacy, 'unlisted') + + retry_items = models.ImportItem.objects.filter(job=retry).all() + self.assertEqual(len(retry_items), 2) + self.assertEqual(retry_items[0].index, 0) + self.assertEqual(retry_items[0].data['Book Id'], '42036538') + self.assertEqual(retry_items[1].index, 1) + self.assertEqual(retry_items[1].data['Book Id'], '52691223') + + + def test_start_import(self): + ''' begin loading books ''' + import_job = goodreads_import.create_job( + self.user, self.csv, False, 'unlisted') + MockTask = namedtuple('Task', ('id')) + mock_task = MockTask(7) + with patch('bookwyrm.goodreads_import.import_data.delay') as start: + start.return_value = mock_task + goodreads_import.start_import(import_job) + import_job.refresh_from_db() + self.assertEqual(import_job.task_id, '7') + + @responses.activate def test_import_data(self): ''' resolve entry ''' + import_job = goodreads_import.create_job( + self.user, self.csv, False, 'unlisted') + book = models.Edition.objects.create(title='Test Book') + + with patch( + 'bookwyrm.models.import_job.ImportItem.get_book_from_isbn' + ) as resolve: + resolve.return_value = book + with patch('bookwyrm.outgoing.handle_imported_book'): + goodreads_import.import_data(import_job.id) + + import_item = models.ImportItem.objects.get(job=import_job, index=0) + self.assertEqual(import_item.book.id, book.id) From 8c8aae2c92b0eb2bdb26f8ba71d79b339aa2aa92 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 2 Jan 2021 11:59:14 -0800 Subject: [PATCH 56/79] Check if a book is already shelved after import --- bookwyrm/connectors/abstract_connector.py | 2 +- bookwyrm/outgoing.py | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index 82b99378f..d63bd135e 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -205,7 +205,7 @@ def get_data(url): 'User-Agent': settings.USER_AGENT, }, ) - except RequestError: + except (RequestError, SSLError): raise ConnectorException() if not resp.ok: resp.raise_for_status() diff --git a/bookwyrm/outgoing.py b/bookwyrm/outgoing.py index fd4dff8f2..172563bbb 100644 --- a/bookwyrm/outgoing.py +++ b/bookwyrm/outgoing.py @@ -166,22 +166,24 @@ def handle_imported_book(user, item, include_reviews, privacy): if not item.book: return - if item.shelf: + existing_shelf = models.ShelfBook.objects.filter( + book=item.book, added_by=user).exists() + + # shelve the book if it hasn't been shelved already + if item.shelf and not existing_shelf: desired_shelf = models.Shelf.objects.get( identifier=item.shelf, user=user ) - # shelve the book if it hasn't been shelved already - shelf_book, created = models.ShelfBook.objects.get_or_create( + shelf_book = models.ShelfBook.objects.create( book=item.book, shelf=desired_shelf, added_by=user) - if created: - broadcast(user, shelf_book.to_add_activity(user), privacy=privacy) + broadcast(user, shelf_book.to_add_activity(user), privacy=privacy) - # only add new read-throughs if the item isn't already shelved - for read in item.reads: - read.book = item.book - read.user = user - read.save() + # only add new read-throughs if the item isn't already shelved + for read in item.reads: + read.book = item.book + read.user = user + read.save() if include_reviews and (item.rating or item.review): review_title = 'Review of {!r} on Goodreads'.format( From b2c22c5b7f77feb9ef430da78a6bece16641db6f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 2 Jan 2021 13:09:40 -0800 Subject: [PATCH 57/79] Tests handle import shelving --- bookwyrm/tests/test_outgoing.py | 55 +++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/bookwyrm/tests/test_outgoing.py b/bookwyrm/tests/test_outgoing.py index 14d67aeb3..f0125907e 100644 --- a/bookwyrm/tests/test_outgoing.py +++ b/bookwyrm/tests/test_outgoing.py @@ -1,4 +1,5 @@ ''' sending out activities ''' +import csv import json import pathlib from unittest.mock import patch @@ -258,6 +259,60 @@ class Outgoing(TestCase): self.assertEqual(self.shelf.books.count(), 0) + def test_handle_imported_book(self): + ''' goodreads import added a book, this adds related connections ''' + shelf = self.local_user.shelf_set.filter(identifier='read').first() + self.assertIsNone(shelf.books.first()) + + import_job = models.ImportJob.objects.create(user=self.local_user) + datafile = pathlib.Path(__file__).parent.joinpath('data/goodreads.csv') + csv_file = open(datafile, 'r') + for index, entry in enumerate(list(csv.DictReader(csv_file))): + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=index, data=entry, book=self.book) + break + + with patch('bookwyrm.broadcast.broadcast_task.delay'): + outgoing.handle_imported_book( + self.local_user, import_item, False, 'public') + + shelf.refresh_from_db() + self.assertEqual(shelf.books.first(), self.book) + + readthrough = models.ReadThrough.objects.get(user=self.local_user) + self.assertEqual(readthrough.book, self.book) + self.assertEqual(readthrough.start_date.year, 2020) + self.assertEqual(readthrough.start_date.month, 10) + self.assertEqual(readthrough.start_date.day, 21) + self.assertEqual(readthrough.finish_date.year, 2020) + self.assertEqual(readthrough.finish_date.month, 10) + self.assertEqual(readthrough.finish_date.day, 25) + + + def test_handle_imported_book_already_shelved(self): + ''' goodreads import added a book, this adds related connections ''' + shelf = self.local_user.shelf_set.filter(identifier='to-read').first() + models.ShelfBook.objects.create( + shelf=shelf, added_by=self.local_user, book=self.book) + + import_job = models.ImportJob.objects.create(user=self.local_user) + datafile = pathlib.Path(__file__).parent.joinpath('data/goodreads.csv') + csv_file = open(datafile, 'r') + for index, entry in enumerate(list(csv.DictReader(csv_file))): + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=index, data=entry, book=self.book) + break + + with patch('bookwyrm.broadcast.broadcast_task.delay'): + outgoing.handle_imported_book( + self.local_user, import_item, False, 'public') + + shelf.refresh_from_db() + self.assertEqual(shelf.books.first(), self.book) + self.assertIsNone( + self.local_user.shelf_set.get(identifier='read').books.first()) + + def test_handle_status(self): ''' create a status ''' form = forms.CommentForm({ From 22f5fa154caf2a03b90c144a1fbee328ed350d27 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 2 Jan 2021 13:26:42 -0800 Subject: [PATCH 58/79] Add readthroughs even when a book is already shelved --- bookwyrm/outgoing.py | 9 +++--- bookwyrm/tests/data/goodreads.csv | 2 +- bookwyrm/tests/test_outgoing.py | 47 +++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/bookwyrm/outgoing.py b/bookwyrm/outgoing.py index 172563bbb..88377d335 100644 --- a/bookwyrm/outgoing.py +++ b/bookwyrm/outgoing.py @@ -179,11 +179,10 @@ def handle_imported_book(user, item, include_reviews, privacy): book=item.book, shelf=desired_shelf, added_by=user) broadcast(user, shelf_book.to_add_activity(user), privacy=privacy) - # only add new read-throughs if the item isn't already shelved - for read in item.reads: - read.book = item.book - read.user = user - read.save() + for read in item.reads: + read.book = item.book + read.user = user + read.save() if include_reviews and (item.rating or item.review): review_title = 'Review of {!r} on Goodreads'.format( diff --git a/bookwyrm/tests/data/goodreads.csv b/bookwyrm/tests/data/goodreads.csv index b96a0c26d..5f124edc9 100644 --- a/bookwyrm/tests/data/goodreads.csv +++ b/bookwyrm/tests/data/goodreads.csv @@ -1,4 +1,4 @@ Book Id,Title,Author,Author l-f,Additional Authors,ISBN,ISBN13,My Rating,Average Rating,Publisher,Binding,Number of Pages,Year Published,Original Publication Year,Date Read,Date Added,Bookshelves,Bookshelves with positions,Exclusive Shelf,My Review,Spoiler,Private Notes,Read Count,Recommended For,Recommended By,Owned Copies,Original Purchase Date,Original Purchase Location,Condition,Condition Description,BCID 42036538,Gideon the Ninth (The Locked Tomb #1),Tamsyn Muir,"Muir, Tamsyn",,"=""1250313198""","=""9781250313195""",0,4.20,Tor,Hardcover,448,2019,2019,2020/10/25,2020/10/21,,,read,,,,1,,,0,,,,, 52691223,Subcutanean,Aaron A. Reed,"Reed, Aaron A.",,"=""""","=""""",0,4.45,,Paperback,232,2020,,2020/03/06,2020/03/05,,,read,,,,1,,,0,,,,, -28694510,Patisserie at Home,Mélanie Dupuis,"Dupuis, Mélanie",Anne Cazor,"=""0062445316""","=""9780062445315""",2,4.60,Harper Design,Hardcover,288,2016,,,2019/07/08,,,read,"The good:
- Well illustrated and good photographs
- I loved the organization, with sections for base recipes like pie crust, full recipes, and skills
- I loved the madeleines and sweet pie crust recipe
- Very precise measurements

The bad:
- I found the index very hard to use, I would have much preferred a regular, alphabetical index of everything instead of breaking it down in to categories like a table of contents
- The primary unit of measure is ounces, which I found very hard to work with, and in fraction form which my food scale definitely does not do. One recipe calls for 1/32 oz, which I have absolutely no way of measuring out
- Some of the measurements were baffling, like 1/3 tablespoon. 1/3 tablespoon is 1 teaspoon, why would you write 1/3 tablespoon??
- The croissant dough recipe said to allow the pastry to get to room temperature before rolling which meant the butter squirted out and made a huge mess. I don't know why it said to do this??? Rolling works just fine if it's chilled.
- The financier recipe just tells you to add egg whites and has no other raising agent so if you just add the egg whites it will obviously not rise. Either there should have been a raising agent or the egg whites should have been beaten? I don't know.",,,2,,,0,,,,, +28694510,Patisserie at Home,Mélanie Dupuis,"Dupuis, Mélanie",Anne Cazor,"=""0062445316""","=""9780062445315""",2,4.60,Harper Design,Hardcover,288,2016,,,2019/07/08,,,read,"mixed feelings",,,2,,,0,,,,, diff --git a/bookwyrm/tests/test_outgoing.py b/bookwyrm/tests/test_outgoing.py index f0125907e..5e585b1d2 100644 --- a/bookwyrm/tests/test_outgoing.py +++ b/bookwyrm/tests/test_outgoing.py @@ -281,6 +281,7 @@ class Outgoing(TestCase): readthrough = models.ReadThrough.objects.get(user=self.local_user) self.assertEqual(readthrough.book, self.book) + # I can't remember how to create dates and I don't want to look it up. self.assertEqual(readthrough.start_date.year, 2020) self.assertEqual(readthrough.start_date.month, 10) self.assertEqual(readthrough.start_date.day, 21) @@ -311,6 +312,52 @@ class Outgoing(TestCase): self.assertEqual(shelf.books.first(), self.book) self.assertIsNone( self.local_user.shelf_set.get(identifier='read').books.first()) + readthrough = models.ReadThrough.objects.get(user=self.local_user) + self.assertEqual(readthrough.book, self.book) + self.assertEqual(readthrough.start_date.year, 2020) + self.assertEqual(readthrough.start_date.month, 10) + self.assertEqual(readthrough.start_date.day, 21) + self.assertEqual(readthrough.finish_date.year, 2020) + self.assertEqual(readthrough.finish_date.month, 10) + self.assertEqual(readthrough.finish_date.day, 25) + + + def test_handle_imported_book_review(self): + ''' goodreads review import ''' + import_job = models.ImportJob.objects.create(user=self.local_user) + datafile = pathlib.Path(__file__).parent.joinpath('data/goodreads.csv') + csv_file = open(datafile, 'r') + entry = list(csv.DictReader(csv_file))[2] + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=0, data=entry, book=self.book) + + with patch('bookwyrm.broadcast.broadcast_task.delay'): + outgoing.handle_imported_book( + self.local_user, import_item, True, 'unlisted') + review = models.Review.objects.get(book=self.book, user=self.local_user) + self.assertEqual(review.content, 'mixed feelings') + self.assertEqual(review.rating, 2) + self.assertEqual(review.published_date.year, 2019) + self.assertEqual(review.published_date.month, 7) + self.assertEqual(review.published_date.day, 8) + self.assertEqual(review.privacy, 'unlisted') + + + def test_handle_imported_book_reviews_disabled(self): + ''' goodreads review import ''' + import_job = models.ImportJob.objects.create(user=self.local_user) + datafile = pathlib.Path(__file__).parent.joinpath('data/goodreads.csv') + csv_file = open(datafile, 'r') + entry = list(csv.DictReader(csv_file))[2] + import_item = models.ImportItem.objects.create( + job_id=import_job.id, index=0, data=entry, book=self.book) + + with patch('bookwyrm.broadcast.broadcast_task.delay'): + outgoing.handle_imported_book( + self.local_user, import_item, False, 'unlisted') + self.assertFalse(models.Review.objects.filter( + book=self.book, user=self.local_user + ).exists()) def test_handle_status(self): From 4c968c417bfd6f6e4758dc15ec51e2aada708ede Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 2 Jan 2021 14:20:39 -0800 Subject: [PATCH 59/79] cleans up search tests --- .../tests/connectors/test_self_connector.py | 88 +++++++++++-------- 1 file changed, 53 insertions(+), 35 deletions(-) diff --git a/bookwyrm/tests/connectors/test_self_connector.py b/bookwyrm/tests/connectors/test_self_connector.py index 91857def0..1cc5983c7 100644 --- a/bookwyrm/tests/connectors/test_self_connector.py +++ b/bookwyrm/tests/connectors/test_self_connector.py @@ -9,7 +9,9 @@ from bookwyrm.settings import DOMAIN class SelfConnector(TestCase): + ''' just uses local data ''' def setUp(self): + ''' creating the connector ''' models.Connector.objects.create( identifier=DOMAIN, name='Local', @@ -22,58 +24,74 @@ class SelfConnector(TestCase): priority=1, ) self.connector = Connector(DOMAIN) - self.work = models.Work.objects.create( - title='Example Work', - ) - author = models.Author.objects.create(name='Anonymous') - self.edition = models.Edition.objects.create( - title='Edition of Example Work', - published_date=datetime.datetime(1980, 5, 10, tzinfo=timezone.utc), - parent_work=self.work, - ) - self.edition.authors.add(author) - models.Edition.objects.create( - title='Another Edition', - parent_work=self.work, - series='Anonymous' - ) - models.Edition.objects.create( - title='More Editions', - subtitle='The Anonymous Edition', - parent_work=self.work, - ) - - edition = models.Edition.objects.create( - title='An Edition', - parent_work=self.work - ) - edition.authors.add(models.Author.objects.create(name='Fish')) def test_format_search_result(self): + ''' create a SearchResult ''' + author = models.Author.objects.create(name='Anonymous') + edition = models.Edition.objects.create( + title='Edition of Example Work', + published_date=datetime.datetime(1980, 5, 10, tzinfo=timezone.utc), + ) + edition.authors.add(author) result = self.connector.search('Edition of Example')[0] self.assertEqual(result.title, 'Edition of Example Work') - self.assertEqual(result.key, self.edition.remote_id) + self.assertEqual(result.key, edition.remote_id) self.assertEqual(result.author, 'Anonymous') self.assertEqual(result.year, 1980) + self.assertEqual(result.connector, self.connector) def test_search_rank(self): + ''' prioritize certain results ''' + author = models.Author.objects.create(name='Anonymous') + edition = models.Edition.objects.create( + title='Edition of Example Work', + published_date=datetime.datetime(1980, 5, 10, tzinfo=timezone.utc), + ) + # author text is rank C + edition.authors.add(author) + + # series is rank D + models.Edition.objects.create( + title='Another Edition', + series='Anonymous' + ) + # subtitle is rank B + models.Edition.objects.create( + title='More Editions', + subtitle='The Anonymous Edition', + ) + # title is rank A + models.Edition.objects.create(title='Anonymous') + # doesn't rank in this search + edition = models.Edition.objects.create( + title='An Edition', + ) + results = self.connector.search('Anonymous') - self.assertEqual(len(results), 2) - self.assertEqual(results[0].title, 'More Editions') - self.assertEqual(results[1].title, 'Edition of Example Work') + self.assertEqual(len(results), 3) + self.assertEqual(results[0].title, 'Anonymous') + self.assertEqual(results[1].title, 'More Editions') + self.assertEqual(results[2].title, 'Edition of Example Work') def test_search_default_filter(self): ''' it should get rid of duplicate editions for the same work ''' - self.work.default_edition = self.edition - self.work.save() + work = models.Work.objects.create(title='Work Title') + models.Edition.objects.create( + title='Edition 1 Title', parent_work=work) + edition_2 = models.Edition.objects.create( + title='Edition 2 Title', parent_work=work) + edition_3 = models.Edition.objects.create( + title='Fish', parent_work=work) + work.default_edition = edition_2 + work.save() - results = self.connector.search('Anonymous') + results = self.connector.search('Edition Title') self.assertEqual(len(results), 1) - self.assertEqual(results[0].title, 'Edition of Example Work') + self.assertEqual(results[0].key, edition_2.remote_id) results = self.connector.search('Fish') self.assertEqual(len(results), 1) - self.assertEqual(results[0].title, 'An Edition') + self.assertEqual(results[0].key, edition_3.remote_id) From afa1921968cf245cbe2da168526efc3130b72c2d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 2 Jan 2021 14:33:01 -0800 Subject: [PATCH 60/79] Deduplicates search results --- bookwyrm/connectors/self_connector.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/connectors/self_connector.py b/bookwyrm/connectors/self_connector.py index 7d0298be7..921f517f5 100644 --- a/bookwyrm/connectors/self_connector.py +++ b/bookwyrm/connectors/self_connector.py @@ -36,7 +36,7 @@ class Connector(AbstractConnector): or results search_results = [] - for book in results[:10]: + for book in set(results[:10]): search_results.append( self.format_search_result(book) ) From a2e8cf1993b81a074c1776e1e3105aa323458833 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 2 Jan 2021 15:15:25 -0800 Subject: [PATCH 61/79] Return best matching edition instead of default in search --- bookwyrm/connectors/self_connector.py | 38 ++++++++++--------- .../tests/connectors/test_self_connector.py | 11 +++++- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/bookwyrm/connectors/self_connector.py b/bookwyrm/connectors/self_connector.py index 921f517f5..957adafad 100644 --- a/bookwyrm/connectors/self_connector.py +++ b/bookwyrm/connectors/self_connector.py @@ -1,6 +1,6 @@ ''' using a bookwyrm instance as a source of book data ''' from django.contrib.postgres.search import SearchRank, SearchVector -from django.db.models import F +from django.db.models import Count, F from bookwyrm import models from .abstract_connector import AbstractConnector, SearchResult @@ -12,16 +12,7 @@ class Connector(AbstractConnector): ''' search your local database ''' vector = SearchVector('title', weight='A') +\ SearchVector('subtitle', weight='B') +\ - SearchVector('authors__name', weight='C') +\ - SearchVector('isbn_13', weight='A') +\ - SearchVector('isbn_10', weight='A') +\ - SearchVector('openlibrary_key', weight='C') +\ - SearchVector('goodreads_key', weight='C') +\ - SearchVector('asin', weight='C') +\ - SearchVector('oclc_number', weight='C') +\ - SearchVector('remote_id', weight='C') +\ - SearchVector('description', weight='D') +\ - SearchVector('series', weight='D') + SearchVector('authors__name', weight='C') results = models.Edition.objects.annotate( search=vector @@ -31,15 +22,26 @@ class Connector(AbstractConnector): rank__gt=min_confidence ).order_by('-rank') - # remove non-default editions, if possible - results = results.filter(parent_work__default_edition__id=F('id')) \ - or results + # when there are multiple editions of the same work, pick the closest + editions_of_work = results.values( + 'parent_work' + ).annotate( + Count('parent_work') + ).values_list('parent_work') search_results = [] - for book in set(results[:10]): - search_results.append( - self.format_search_result(book) - ) + for work_id in set(editions_of_work): + editions = results.filter(parent_work=work_id) + default = editions.filter(parent_work__default_edition=F('id')) + default_rank = default.first().rank if default.exists() else 0 + # if mutliple books have the top rank, pick the default edition + if default_rank == editions.first().rank: + selected = default.first() + else: + selected = editions.first() + search_results.append(self.format_search_result(selected)) + if len(search_results) >= 10: + break return search_results diff --git a/bookwyrm/tests/connectors/test_self_connector.py b/bookwyrm/tests/connectors/test_self_connector.py index 1cc5983c7..a28ca12a4 100644 --- a/bookwyrm/tests/connectors/test_self_connector.py +++ b/bookwyrm/tests/connectors/test_self_connector.py @@ -76,10 +76,10 @@ class SelfConnector(TestCase): self.assertEqual(results[2].title, 'Edition of Example Work') - def test_search_default_filter(self): + def test_search_multiple_editions(self): ''' it should get rid of duplicate editions for the same work ''' work = models.Work.objects.create(title='Work Title') - models.Edition.objects.create( + edition_1 = models.Edition.objects.create( title='Edition 1 Title', parent_work=work) edition_2 = models.Edition.objects.create( title='Edition 2 Title', parent_work=work) @@ -88,10 +88,17 @@ class SelfConnector(TestCase): work.default_edition = edition_2 work.save() + # pick the best edition + results = self.connector.search('Edition 1 Title') + self.assertEqual(len(results), 1) + self.assertEqual(results[0].key, edition_1.remote_id) + + # pick the default edition when no match is best results = self.connector.search('Edition Title') self.assertEqual(len(results), 1) self.assertEqual(results[0].key, edition_2.remote_id) + # only matches one edition, so no deduplication takes place results = self.connector.search('Fish') self.assertEqual(len(results), 1) self.assertEqual(results[0].key, edition_3.remote_id) From a413c879637e0210c12bbb98d6d41d580441dbaa Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 2 Jan 2021 15:48:59 -0800 Subject: [PATCH 62/79] Separate search for unique identifiers out from text search --- bookwyrm/connectors/self_connector.py | 91 ++++++++++++++++++--------- 1 file changed, 60 insertions(+), 31 deletions(-) diff --git a/bookwyrm/connectors/self_connector.py b/bookwyrm/connectors/self_connector.py index 957adafad..04fea7350 100644 --- a/bookwyrm/connectors/self_connector.py +++ b/bookwyrm/connectors/self_connector.py @@ -1,6 +1,9 @@ ''' using a bookwyrm instance as a source of book data ''' +from functools import reduce +import operator + from django.contrib.postgres.search import SearchRank, SearchVector -from django.db.models import Count, F +from django.db.models import Count, F, Q from bookwyrm import models from .abstract_connector import AbstractConnector, SearchResult @@ -10,36 +13,14 @@ class Connector(AbstractConnector): ''' instantiate a connector ''' def search(self, query, min_confidence=0.1): ''' search your local database ''' - vector = SearchVector('title', weight='A') +\ - SearchVector('subtitle', weight='B') +\ - SearchVector('authors__name', weight='C') - - results = models.Edition.objects.annotate( - search=vector - ).annotate( - rank=SearchRank(vector, query) - ).filter( - rank__gt=min_confidence - ).order_by('-rank') - - # when there are multiple editions of the same work, pick the closest - editions_of_work = results.values( - 'parent_work' - ).annotate( - Count('parent_work') - ).values_list('parent_work') - + # first, try searching unqiue identifiers + results = search_identifiers(query) + if not results: + # then try searching title/author + results = search_title_author(query, min_confidence) search_results = [] - for work_id in set(editions_of_work): - editions = results.filter(parent_work=work_id) - default = editions.filter(parent_work__default_edition=F('id')) - default_rank = default.first().rank if default.exists() else 0 - # if mutliple books have the top rank, pick the default edition - if default_rank == editions.first().rank: - selected = default.first() - else: - selected = editions.first() - search_results.append(self.format_search_result(selected)) + for result in results: + search_results.append(self.format_search_result(result)) if len(search_results) >= 10: break return search_results @@ -53,7 +34,8 @@ class Connector(AbstractConnector): year=search_result.published_date.year if \ search_result.published_date else None, connector=self, - confidence=search_result.rank, + confidence=search_result.rank if \ + hasattr(search_result, 'rank') else 1, ) @@ -75,3 +57,50 @@ class Connector(AbstractConnector): def expand_book_data(self, book): pass + + +def search_identifiers(query): + ''' tries remote_id, isbn; defined as dedupe fields on the model ''' + filters = [{f.name: query} for f in models.Edition._meta.get_fields() \ + if hasattr(f, 'deduplication_field') and f.deduplication_field] + results = models.Edition.objects.filter( + reduce(operator.or_, (Q(**f) for f in filters)) + ).distinct() + + # when there are multiple editions of the same work, pick the default. + # it would be odd for this to happen. + return results.filter(parent_work__default_edition__id=F('id')) \ + or results + + +def search_title_author(query, min_confidence): + ''' searches for title and author ''' + print('DON"T BOTHER') + vector = SearchVector('title', weight='A') +\ + SearchVector('subtitle', weight='B') +\ + SearchVector('authors__name', weight='C') + + results = models.Edition.objects.annotate( + search=vector + ).annotate( + rank=SearchRank(vector, query) + ).filter( + rank__gt=min_confidence + ).order_by('-rank') + + # when there are multiple editions of the same work, pick the closest + editions_of_work = results.values( + 'parent_work' + ).annotate( + Count('parent_work') + ).values_list('parent_work') + + for work_id in set(editions_of_work): + editions = results.filter(parent_work=work_id) + default = editions.filter(parent_work__default_edition=F('id')) + default_rank = default.first().rank if default.exists() else 0 + # if mutliple books have the top rank, pick the default edition + if default_rank == editions.first().rank: + yield default.first() + else: + yield editions.first() From d3c181cacbc4ac5283a20feb9da1fd4bc15ca2fb Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 2 Jan 2021 16:09:54 -0800 Subject: [PATCH 63/79] Re-sorts search results after deduplication --- bookwyrm/connectors/self_connector.py | 5 +++-- bookwyrm/tests/connectors/test_self_connector.py | 6 +++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/bookwyrm/connectors/self_connector.py b/bookwyrm/connectors/self_connector.py index 04fea7350..0c21e7bcc 100644 --- a/bookwyrm/connectors/self_connector.py +++ b/bookwyrm/connectors/self_connector.py @@ -23,6 +23,7 @@ class Connector(AbstractConnector): search_results.append(self.format_search_result(result)) if len(search_results) >= 10: break + search_results.sort(key=lambda r: r.confidence, reverse=True) return search_results @@ -75,10 +76,10 @@ def search_identifiers(query): def search_title_author(query, min_confidence): ''' searches for title and author ''' - print('DON"T BOTHER') vector = SearchVector('title', weight='A') +\ SearchVector('subtitle', weight='B') +\ - SearchVector('authors__name', weight='C') + SearchVector('authors__name', weight='C') +\ + SearchVector('series', weight='D') results = models.Edition.objects.annotate( search=vector diff --git a/bookwyrm/tests/connectors/test_self_connector.py b/bookwyrm/tests/connectors/test_self_connector.py index a28ca12a4..0fc789556 100644 --- a/bookwyrm/tests/connectors/test_self_connector.py +++ b/bookwyrm/tests/connectors/test_self_connector.py @@ -48,6 +48,7 @@ class SelfConnector(TestCase): edition = models.Edition.objects.create( title='Edition of Example Work', published_date=datetime.datetime(1980, 5, 10, tzinfo=timezone.utc), + parent_work=models.Work.objects.create(title='') ) # author text is rank C edition.authors.add(author) @@ -55,18 +56,21 @@ class SelfConnector(TestCase): # series is rank D models.Edition.objects.create( title='Another Edition', - series='Anonymous' + series='Anonymous', + parent_work=models.Work.objects.create(title='') ) # subtitle is rank B models.Edition.objects.create( title='More Editions', subtitle='The Anonymous Edition', + parent_work=models.Work.objects.create(title='') ) # title is rank A models.Edition.objects.create(title='Anonymous') # doesn't rank in this search edition = models.Edition.objects.create( title='An Edition', + parent_work=models.Work.objects.create(title='') ) results = self.connector.search('Anonymous') From b389cfb01363bb4747145b1acfcf6d2dd40cb225 Mon Sep 17 00:00:00 2001 From: "Renato \"Lond\" Cerqueira" Date: Sun, 3 Jan 2021 14:19:03 +0100 Subject: [PATCH 64/79] Use user-agent to determine whether to show pure representation Together with #434 and #435, this fixes #429. Use the user-agent to determine if the call is coming from a BookWyrm instance or not. If it's not, give a pure activitypub representation for the status. Otherwise, give a BookWyrm one, to allow for a complete integration between instances. --- bookwyrm/tests/test_views.py | 15 ++++++++++++++- bookwyrm/utils/regex.py | 2 ++ bookwyrm/views.py | 9 ++++++++- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/bookwyrm/tests/test_views.py b/bookwyrm/tests/test_views.py index 34321e832..bb8a5ff25 100644 --- a/bookwyrm/tests/test_views.py +++ b/bookwyrm/tests/test_views.py @@ -10,7 +10,7 @@ from django.test.client import RequestFactory from bookwyrm import models, views from bookwyrm.connectors import abstract_connector -from bookwyrm.settings import DOMAIN +from bookwyrm.settings import DOMAIN, USER_AGENT # pylint: disable=too-many-public-methods @@ -539,3 +539,16 @@ class Views(TestCase): request, self.local_user.username, shelf.identifier) self.assertIsInstance(result, JsonResponse) self.assertEqual(result.status_code, 200) + + + def test_is_bookwyrm_request(self): + ''' tests the function that checks if a request came from a bookwyrm instance ''' + request = self.factory.get('', {'q': 'Test Book'}) + self.assertFalse(views.is_bookworm_request(request)) + + request = self.factory.get('', {'q': 'Test Book'}, + HTTP_USER_AGENT="http.rb/4.4.1 (Mastodon/3.3.0; +https://mastodon.social/)") + self.assertFalse(views.is_bookworm_request(request)) + + request = self.factory.get('', {'q': 'Test Book'}, HTTP_USER_AGENT=USER_AGENT) + self.assertTrue(views.is_bookworm_request(request)) diff --git a/bookwyrm/utils/regex.py b/bookwyrm/utils/regex.py index 8045e7f3e..9553c913a 100644 --- a/bookwyrm/utils/regex.py +++ b/bookwyrm/utils/regex.py @@ -6,3 +6,5 @@ strict_localname = r'@[a-zA-Z_\-\.0-9]+' username = r'%s(@%s)?' % (localname, domain) strict_username = r'%s(@%s)?' % (strict_localname, domain) full_username = r'%s@%s' % (localname, domain) +# should match (BookWyrm/1.0.0; or (BookWyrm/99.1.2; +bookwyrm_user_agent = r'\(BookWyrm/[0-9]+\.[0-9]+\.[0-9]+;' diff --git a/bookwyrm/views.py b/bookwyrm/views.py index 10137684b..b518c4fac 100644 --- a/bookwyrm/views.py +++ b/bookwyrm/views.py @@ -42,6 +42,13 @@ def is_api_request(request): return 'json' in request.headers.get('Accept') or \ request.path[-5:] == '.json' +def is_bookworm_request(request): + ''' check if the request is coming from another bookworm instance ''' + user_agent = request.headers.get('User-Agent') + if user_agent is None or re.search(regex.bookwyrm_user_agent, user_agent) is None: + return False + + return True def server_error_page(request): ''' 500 errors ''' @@ -505,7 +512,7 @@ def status_page(request, username, status_id): return HttpResponseNotFound() if is_api_request(request): - return ActivitypubResponse(status.to_activity()) + return ActivitypubResponse(status.to_activity(pure=not is_bookworm_request(request))) data = { 'title': 'Status by %s' % user.username, From b5ce7a0c259580b8d1343c2d07bbfb0491c1afdf Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 3 Jan 2021 07:48:57 -0800 Subject: [PATCH 65/79] Search on username and localname in user search --- bookwyrm/views.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bookwyrm/views.py b/bookwyrm/views.py index 10137684b..40c2c9dca 100644 --- a/bookwyrm/views.py +++ b/bookwyrm/views.py @@ -5,6 +5,7 @@ from django.contrib.auth.decorators import login_required, permission_required from django.contrib.postgres.search import TrigramSimilarity from django.core.paginator import Paginator from django.db.models import Avg, Q +from django.db.models.functions import Greatest from django.http import HttpResponseNotFound, JsonResponse from django.core.exceptions import PermissionDenied from django.shortcuts import get_object_or_404, redirect @@ -227,7 +228,10 @@ def search(request): # do a local user search user_results = models.User.objects.annotate( - similarity=TrigramSimilarity('username', query), + similarity=Greatest( + TrigramSimilarity('username', query), + TrigramSimilarity('localname', query), + ) ).filter( similarity__gt=0.5, ).order_by('-similarity')[:10] From e54e1f245996737c5ebbd7dc9fb51a365324b974 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 3 Jan 2021 07:57:57 -0800 Subject: [PATCH 66/79] Tests user search --- bookwyrm/tests/test_views.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/bookwyrm/tests/test_views.py b/bookwyrm/tests/test_views.py index 34321e832..69188e2b7 100644 --- a/bookwyrm/tests/test_views.py +++ b/bookwyrm/tests/test_views.py @@ -211,6 +211,19 @@ class Views(TestCase): response.context_data['book_results'][0].title, 'Gideon the Ninth') + def test_search_html_response_users(self): + ''' searches remote connectors ''' + request = self.factory.get('', {'q': 'mouse'}) + with patch('bookwyrm.views.is_api_request') as is_api: + is_api.return_value = False + with patch('bookwyrm.connectors.connector_manager.search'): + response = views.search(request) + self.assertIsInstance(response, TemplateResponse) + self.assertEqual(response.template_name, 'search_results.html') + self.assertEqual( + response.context_data['user_results'][0], self.local_user) + + def test_import_page(self): ''' there are so many views, this just makes sure it LOADS ''' request = self.factory.get('') From 4d4b27e8447463a3992a3cff3456fd78866a5564 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 3 Jan 2021 08:17:00 -0800 Subject: [PATCH 67/79] Fixes replies showing up twice in feed --- bookwyrm/views.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/bookwyrm/views.py b/bookwyrm/views.py index b518c4fac..09f727f27 100644 --- a/bookwyrm/views.py +++ b/bookwyrm/views.py @@ -45,7 +45,8 @@ def is_api_request(request): def is_bookworm_request(request): ''' check if the request is coming from another bookworm instance ''' user_agent = request.headers.get('User-Agent') - if user_agent is None or re.search(regex.bookwyrm_user_agent, user_agent) is None: + if user_agent is None or \ + re.search(regex.bookwyrm_user_agent, user_agent) is None: return False return True @@ -180,7 +181,7 @@ def get_activity_feed(user, filter_level, model=models.Status): return activities.filter( Q(user=user) | Q(mention_users=user), privacy='direct' - ) + ).distinct() # never show DMs in the regular feed activities = activities.filter(~Q(privacy='direct')) @@ -195,7 +196,7 @@ def get_activity_feed(user, filter_level, model=models.Status): Q(user__in=following, privacy__in=[ 'public', 'unlisted', 'followers' ]) | Q(mention_users=user) | Q(user=user) - ) + ).distinct() elif filter_level == 'self': activities = activities.filter(user=user, privacy='public') elif filter_level == 'local': @@ -512,7 +513,8 @@ def status_page(request, username, status_id): return HttpResponseNotFound() if is_api_request(request): - return ActivitypubResponse(status.to_activity(pure=not is_bookworm_request(request))) + return ActivitypubResponse( + status.to_activity(pure=not is_bookworm_request(request))) data = { 'title': 'Status by %s' % user.username, From 04eb51863306a10313b1f4c4a417779b106a7765 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 3 Jan 2021 10:03:57 -0800 Subject: [PATCH 68/79] Make the top bar dropdown accessible to NVDA screenreaders plus, it's just better for this to be a ul --- bookwyrm/templates/layout.html | 54 +++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/bookwyrm/templates/layout.html b/bookwyrm/templates/layout.html index ab113ad09..3e1af28c2 100644 --- a/bookwyrm/templates/layout.html +++ b/bookwyrm/templates/layout.html @@ -63,33 +63,45 @@