diff --git a/bookwyrm/connectors/__init__.py b/bookwyrm/connectors/__init__.py index 4eb91de4..cfafd286 100644 --- a/bookwyrm/connectors/__init__.py +++ b/bookwyrm/connectors/__init__.py @@ -2,3 +2,5 @@ from .settings import CONNECTORS from .abstract_connector import ConnectorException from .abstract_connector import get_data, get_image + +from .connector_manager import search, local_search, first_search_result diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index 3461e80f..82b99378 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -6,17 +6,13 @@ from urllib3.exceptions import RequestError from django.db import transaction import requests -from requests import HTTPError from requests.exceptions import SSLError from bookwyrm import activitypub, models, settings +from .connector_manager import load_more_data, ConnectorException logger = logging.getLogger(__name__) -class ConnectorException(HTTPError): - ''' when the connector can't do what was asked ''' - - class AbstractMinimalConnector(ABC): ''' just the bare bones, for other bookwyrm instances ''' def __init__(self, identifier): @@ -90,7 +86,6 @@ class AbstractConnector(AbstractMinimalConnector): return True - @transaction.atomic def get_or_create_book(self, remote_id): ''' translate arbitrary json into an Activitypub dataclass ''' # first, check if we have the origin_id saved @@ -123,13 +118,17 @@ class AbstractConnector(AbstractMinimalConnector): if not work_data or not edition_data: raise ConnectorException('Unable to load book data: %s' % remote_id) - # create activitypub object - work_activity = activitypub.Work(**work_data) - # this will dedupe automatically - work = work_activity.to_model(models.Work) - for author in self.get_authors_from_data(data): - work.authors.add(author) - return self.create_edition_from_data(work, edition_data) + with transaction.atomic(): + # create activitypub object + work_activity = activitypub.Work(**work_data) + # this will dedupe automatically + work = work_activity.to_model(models.Work) + for author in self.get_authors_from_data(data): + work.authors.add(author) + + edition = self.create_edition_from_data(work, edition_data) + load_more_data.delay(self.connector.id, work.id) + return edition def create_edition_from_data(self, work, edition_data): diff --git a/bookwyrm/books_manager.py b/bookwyrm/connectors/connector_manager.py similarity index 86% rename from bookwyrm/books_manager.py rename to bookwyrm/connectors/connector_manager.py index bc1fa723..d3b01f7a 100644 --- a/bookwyrm/books_manager.py +++ b/bookwyrm/connectors/connector_manager.py @@ -1,51 +1,15 @@ -''' select and call a connector for whatever book task needs doing ''' +''' interface with whatever connectors the app has ''' import importlib from urllib.parse import urlparse from requests import HTTPError from bookwyrm import models -from bookwyrm.connectors import ConnectorException from bookwyrm.tasks import app -def get_edition(book_id): - ''' look up a book in the db and return an edition ''' - book = models.Book.objects.select_subclasses().get(id=book_id) - if isinstance(book, models.Work): - book = book.default_edition - return book - - -def get_or_create_connector(remote_id): - ''' get the connector related to the author's server ''' - url = urlparse(remote_id) - identifier = url.netloc - if not identifier: - raise ValueError('Invalid remote id') - - try: - connector_info = models.Connector.objects.get(identifier=identifier) - except models.Connector.DoesNotExist: - connector_info = models.Connector.objects.create( - identifier=identifier, - connector_file='bookwyrm_connector', - base_url='https://%s' % identifier, - books_url='https://%s/book' % identifier, - covers_url='https://%s/images/covers' % identifier, - search_url='https://%s/search?q=' % identifier, - priority=2 - ) - - return load_connector(connector_info) - - -@app.task -def load_more_data(book_id): - ''' background the work of getting all 10,000 editions of LoTR ''' - book = models.Book.objects.select_subclasses().get(id=book_id) - connector = load_connector(book.connector) - connector.expand_book_data(book) +class ConnectorException(HTTPError): + ''' when the connector can't do what was asked ''' def search(query, min_confidence=0.1): @@ -92,6 +56,38 @@ def get_connectors(): yield load_connector(info) +def get_or_create_connector(remote_id): + ''' get the connector related to the author's server ''' + url = urlparse(remote_id) + identifier = url.netloc + if not identifier: + raise ValueError('Invalid remote id') + + try: + connector_info = models.Connector.objects.get(identifier=identifier) + except models.Connector.DoesNotExist: + connector_info = models.Connector.objects.create( + identifier=identifier, + connector_file='bookwyrm_connector', + base_url='https://%s' % identifier, + books_url='https://%s/book' % identifier, + covers_url='https://%s/images/covers' % identifier, + search_url='https://%s/search?q=' % identifier, + priority=2 + ) + + return load_connector(connector_info) + + +@app.task +def load_more_data(connector_id, book_id): + ''' background the work of getting all 10,000 editions of LoTR ''' + connector_info = models.Connector.objects.get(id=connector_id) + connector = load_connector(connector_info) + book = models.Book.objects.select_subclasses().get(id=book_id) + connector.expand_book_data(book) + + def load_connector(connector_info): ''' instantiate the connector class ''' connector = importlib.import_module( diff --git a/bookwyrm/connectors/openlibrary.py b/bookwyrm/connectors/openlibrary.py index a1155d2d..ee7c6271 100644 --- a/bookwyrm/connectors/openlibrary.py +++ b/bookwyrm/connectors/openlibrary.py @@ -3,7 +3,8 @@ import re from bookwyrm import models from .abstract_connector import AbstractConnector, SearchResult, Mapping -from .abstract_connector import ConnectorException, get_data +from .abstract_connector import get_data +from .connector_manager import ConnectorException from .openlibrary_languages import languages diff --git a/bookwyrm/models/import_job.py b/bookwyrm/models/import_job.py index 576dd07d..1ebe9b31 100644 --- a/bookwyrm/models/import_job.py +++ b/bookwyrm/models/import_job.py @@ -6,7 +6,7 @@ from django.contrib.postgres.fields import JSONField from django.db import models from django.utils import timezone -from bookwyrm import books_manager +from bookwyrm.connectors import connector_manager from bookwyrm.models import ReadThrough, User, Book from .fields import PrivacyLevels @@ -71,7 +71,7 @@ class ImportItem(models.Model): def get_book_from_isbn(self): ''' search by isbn ''' - search_result = books_manager.first_search_result( + search_result = connector_manager.first_search_result( self.isbn, min_confidence=0.999 ) if search_result: @@ -86,7 +86,7 @@ class ImportItem(models.Model): self.data['Title'], self.data['Author'] ) - search_result = books_manager.first_search_result( + search_result = connector_manager.first_search_result( search_term, min_confidence=0.999 ) if search_result: diff --git a/bookwyrm/tests/connectors/test_abstract_connector.py b/bookwyrm/tests/connectors/test_abstract_connector.py index 6bcfc76b..6e912858 100644 --- a/bookwyrm/tests/connectors/test_abstract_connector.py +++ b/bookwyrm/tests/connectors/test_abstract_connector.py @@ -1,4 +1,5 @@ ''' testing book data connectors ''' +from unittest.mock import patch from django.test import TestCase import responses @@ -104,8 +105,10 @@ class AbstractConnector(TestCase): 'https://example.com/book/abcd', json=self.edition_data ) - result = self.connector.get_or_create_book( - 'https://example.com/book/abcd') + with patch( + 'bookwyrm.connectors.abstract_connector.load_more_data.delay'): + result = self.connector.get_or_create_book( + 'https://example.com/book/abcd') self.assertEqual(result, self.book) self.assertEqual(models.Edition.objects.count(), 1) self.assertEqual(models.Edition.objects.count(), 1) diff --git a/bookwyrm/tests/test_books_manager.py b/bookwyrm/tests/connectors/test_connector_manager.py similarity index 59% rename from bookwyrm/tests/test_books_manager.py rename to bookwyrm/tests/connectors/test_connector_manager.py index 039bdfc5..783b5a27 100644 --- a/bookwyrm/tests/test_books_manager.py +++ b/bookwyrm/tests/connectors/test_connector_manager.py @@ -1,12 +1,18 @@ +''' interface between the app and various connectors ''' from django.test import TestCase -from bookwyrm import books_manager, models -from bookwyrm.connectors.bookwyrm_connector import Connector as BookWyrmConnector -from bookwyrm.connectors.self_connector import Connector as SelfConnector +from bookwyrm import models +from bookwyrm.connectors import connector_manager +from bookwyrm.connectors.bookwyrm_connector \ + import Connector as BookWyrmConnector +from bookwyrm.connectors.self_connector \ + import Connector as SelfConnector -class Book(TestCase): +class ConnectorManager(TestCase): + ''' interface between the app and various connectors ''' def setUp(self): + ''' we'll need some books and a connector info entry ''' self.work = models.Work.objects.create( title='Example Work' ) @@ -28,53 +34,50 @@ class Book(TestCase): covers_url='http://test.com/', ) - def test_get_edition(self): - edition = books_manager.get_edition(self.edition.id) - self.assertEqual(edition, self.edition) - - - def test_get_edition_work(self): - edition = books_manager.get_edition(self.work.id) - self.assertEqual(edition, self.edition) - def test_get_or_create_connector(self): + ''' loads a connector if the data source is known or creates one ''' remote_id = 'https://example.com/object/1' - connector = books_manager.get_or_create_connector(remote_id) + connector = connector_manager.get_or_create_connector(remote_id) self.assertIsInstance(connector, BookWyrmConnector) self.assertEqual(connector.identifier, 'example.com') self.assertEqual(connector.base_url, 'https://example.com') - same_connector = books_manager.get_or_create_connector(remote_id) + same_connector = connector_manager.get_or_create_connector(remote_id) self.assertEqual(connector.identifier, same_connector.identifier) def test_get_connectors(self): + ''' load all connectors ''' remote_id = 'https://example.com/object/1' - books_manager.get_or_create_connector(remote_id) - connectors = list(books_manager.get_connectors()) + connector_manager.get_or_create_connector(remote_id) + connectors = list(connector_manager.get_connectors()) self.assertEqual(len(connectors), 2) self.assertIsInstance(connectors[0], SelfConnector) self.assertIsInstance(connectors[1], BookWyrmConnector) def test_search(self): - results = books_manager.search('Example') + ''' search all connectors ''' + results = connector_manager.search('Example') self.assertEqual(len(results), 1) self.assertIsInstance(results[0]['connector'], SelfConnector) self.assertEqual(len(results[0]['results']), 1) self.assertEqual(results[0]['results'][0].title, 'Example Edition') def test_local_search(self): - results = books_manager.local_search('Example') + ''' search only the local database ''' + results = connector_manager.local_search('Example') self.assertEqual(len(results), 1) self.assertEqual(results[0].title, 'Example Edition') def test_first_search_result(self): - result = books_manager.first_search_result('Example') + ''' only get one search result ''' + result = connector_manager.first_search_result('Example') self.assertEqual(result.title, 'Example Edition') - no_result = books_manager.first_search_result('dkjfhg') + no_result = connector_manager.first_search_result('dkjfhg') self.assertIsNone(no_result) def test_load_connector(self): - connector = books_manager.load_connector(self.connector) + ''' load a connector object from the database entry ''' + connector = connector_manager.load_connector(self.connector) self.assertIsInstance(connector, SelfConnector) self.assertEqual(connector.identifier, 'test_connector') diff --git a/bookwyrm/tests/connectors/test_openlibrary_connector.py b/bookwyrm/tests/connectors/test_openlibrary_connector.py index 437b23dc..10e74770 100644 --- a/bookwyrm/tests/connectors/test_openlibrary_connector.py +++ b/bookwyrm/tests/connectors/test_openlibrary_connector.py @@ -12,7 +12,7 @@ from bookwyrm.connectors.openlibrary import get_languages, get_description from bookwyrm.connectors.openlibrary import pick_default_edition, \ get_openlibrary_key from bookwyrm.connectors.abstract_connector import SearchResult -from bookwyrm.connectors.abstract_connector import ConnectorException +from bookwyrm.connectors.connector_manager import ConnectorException class Openlibrary(TestCase): diff --git a/bookwyrm/tests/models/test_import_model.py b/bookwyrm/tests/models/test_import_model.py index a975f410..3d890141 100644 --- a/bookwyrm/tests/models/test_import_model.py +++ b/bookwyrm/tests/models/test_import_model.py @@ -8,7 +8,8 @@ from django.utils import timezone from django.test import TestCase import responses -from bookwyrm import books_manager, models +from bookwyrm import models +from bookwyrm.connectors import connector_manager from bookwyrm.connectors.abstract_connector import SearchResult @@ -134,7 +135,7 @@ class ImportJob(TestCase): search_url='https://openlibrary.org/search?q=', priority=3, ) - connector = books_manager.load_connector(connector_info) + connector = connector_manager.load_connector(connector_info) result = SearchResult( title='Test Result', key='https://openlibrary.org/works/OL1234W', @@ -163,8 +164,12 @@ class ImportJob(TestCase): 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() + with patch( + 'bookwyrm.connectors.abstract_connector.load_more_data.delay'): + with patch( + 'bookwyrm.connectors.connector_manager.first_search_result' + ) as search: + search.return_value = result + book = self.item_1.get_book_from_isbn() self.assertEqual(book.title, 'Sabriel') diff --git a/bookwyrm/tests/test_views.py b/bookwyrm/tests/test_views.py index 51c6f502..34321e83 100644 --- a/bookwyrm/tests/test_views.py +++ b/bookwyrm/tests/test_views.py @@ -39,6 +39,14 @@ class Views(TestCase): ) + def test_get_edition(self): + ''' given an edition or a work, returns an edition ''' + self.assertEqual( + views.get_edition(self.book.id), self.book) + self.assertEqual( + views.get_edition(self.work.id), self.book) + + def test_get_user_from_username(self): ''' works for either localname or username ''' self.assertEqual( @@ -193,7 +201,8 @@ class Views(TestCase): 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: + with patch( + 'bookwyrm.connectors.connector_manager.search') as manager: manager.return_value = [search_result] response = views.search(request) self.assertIsInstance(response, TemplateResponse) diff --git a/bookwyrm/view_actions.py b/bookwyrm/view_actions.py index 8a686bab..eb09a37a 100644 --- a/bookwyrm/view_actions.py +++ b/bookwyrm/view_actions.py @@ -17,11 +17,12 @@ from django.template.response import TemplateResponse from django.utils import timezone from django.views.decorators.http import require_GET, require_POST -from bookwyrm import books_manager, forms, models, outgoing, goodreads_import +from bookwyrm import forms, models, outgoing, goodreads_import +from bookwyrm.connectors import connector_manager from bookwyrm.broadcast import broadcast from bookwyrm.emailing import password_reset_email from bookwyrm.settings import DOMAIN -from bookwyrm.views import get_user_from_username +from bookwyrm.views import get_user_from_username, get_edition @require_POST @@ -210,10 +211,8 @@ def edit_profile(request): def resolve_book(request): ''' figure out the local path to a book from a remote_id ''' remote_id = request.POST.get('remote_id') - connector = books_manager.get_or_create_connector(remote_id) + connector = connector_manager.get_or_create_connector(remote_id) book = connector.get_or_create_book(remote_id) - if book.connector: - books_manager.load_more_data.delay(book.id) return redirect('/book/%d' % book.id) @@ -371,7 +370,7 @@ def delete_shelf(request, shelf_id): @require_POST def shelve(request): ''' put a on a user's shelf ''' - book = books_manager.get_edition(request.POST['book']) + book = get_edition(request.POST['book']) desired_shelf = models.Shelf.objects.filter( identifier=request.POST['shelf'], @@ -417,7 +416,7 @@ def unshelve(request): @require_POST def start_reading(request, book_id): ''' begin reading a book ''' - book = books_manager.get_edition(book_id) + book = get_edition(book_id) shelf = models.Shelf.objects.filter( identifier='reading', user=request.user @@ -453,7 +452,7 @@ def start_reading(request, book_id): @require_POST def finish_reading(request, book_id): ''' a user completed a book, yay ''' - book = books_manager.get_edition(book_id) + book = get_edition(book_id) shelf = models.Shelf.objects.filter( identifier='read', user=request.user diff --git a/bookwyrm/views.py b/bookwyrm/views.py index 27492940..9a0157b1 100644 --- a/bookwyrm/views.py +++ b/bookwyrm/views.py @@ -13,14 +13,22 @@ from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_GET from bookwyrm import outgoing -from bookwyrm.activitypub import ActivitypubResponse -from bookwyrm import forms, models, books_manager +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 from bookwyrm.tasks import app from bookwyrm.utils import regex +def get_edition(book_id): + ''' look up a book in the db and return an edition ''' + book = models.Book.objects.select_subclasses().get(id=book_id) + if isinstance(book, models.Work): + book = book.get_default_edition() + return book + 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 @@ -211,7 +219,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) + book_results = connector_manager.local_search(query) return JsonResponse([r.json() for r in book_results], safe=False) # use webfinger for mastodon style account@domain.com username @@ -225,7 +233,7 @@ def search(request): similarity__gt=0.5, ).order_by('-similarity')[:10] - book_results = books_manager.search(query) + book_results = connector_manager.search(query) data = { 'title': 'Search Results', 'book_results': book_results, @@ -645,7 +653,7 @@ def book_page(request, book_id): @require_GET def edit_book_page(request, book_id): ''' info about a book ''' - book = books_manager.get_edition(book_id) + book = get_edition(book_id) if not book.description: book.description = book.parent_work.description data = { diff --git a/celerywyrm/celery.py b/celerywyrm/celery.py index efa081ee..5a53dab5 100644 --- a/celerywyrm/celery.py +++ b/celerywyrm/celery.py @@ -20,8 +20,9 @@ app.config_from_object('django.conf:settings', namespace='CELERY') # Load task modules from all registered Django app configs. app.autodiscover_tasks() app.autodiscover_tasks(['bookwyrm'], related_name='activitypub.base_activity') -app.autodiscover_tasks(['bookwyrm'], related_name='books_manager') app.autodiscover_tasks(['bookwyrm'], related_name='broadcast') +app.autodiscover_tasks( + ['bookwyrm'], related_name='connectors.abstract_connector') app.autodiscover_tasks(['bookwyrm'], related_name='emailing') app.autodiscover_tasks(['bookwyrm'], related_name='goodreads_import') app.autodiscover_tasks(['bookwyrm'], related_name='incoming')