From 2fd7792f3415d0ac105da7cf26c8569adc13c765 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 3 May 2020 17:53:14 -0700 Subject: [PATCH 01/28] Remove fedireads_key field we have ID --- fedireads/activitypub/book.py | 1 - fedireads/books_manager.py | 27 +++++-------------- fedireads/connectors/abstract_connector.py | 5 ++-- fedireads/connectors/fedireads_connector.py | 20 +++++++------- fedireads/connectors/self_connector.py | 10 +++---- fedireads/models/book.py | 2 -- fedireads/outgoing.py | 2 +- fedireads/templates/book.html | 6 ++--- fedireads/templates/book_results.html | 2 +- fedireads/templates/edit_book.html | 6 ++--- fedireads/templates/editions.html | 2 +- fedireads/templates/snippets/authors.html | 2 +- .../templates/snippets/book_titleby.html | 2 +- .../templates/snippets/covers_shelf.html | 2 +- .../templates/snippets/create_status.html | 6 ++--- fedireads/templates/snippets/rate_action.html | 2 +- fedireads/templates/snippets/shelf.html | 2 +- fedireads/templates/snippets/tag.html | 4 +-- fedireads/urls.py | 4 +-- fedireads/view_actions.py | 4 +-- fedireads/views.py | 24 +++++++++++------ 21 files changed, 65 insertions(+), 70 deletions(-) diff --git a/fedireads/activitypub/book.py b/fedireads/activitypub/book.py index cbaa7a047..80d8ff0fe 100644 --- a/fedireads/activitypub/book.py +++ b/fedireads/activitypub/book.py @@ -11,7 +11,6 @@ def get_book(book): 'oclc_number', 'openlibrary_key', 'librarything_key', - 'fedireads_key', 'lccn', 'oclc_number', 'pages', diff --git a/fedireads/books_manager.py b/fedireads/books_manager.py index 28effe7e8..6f0d78cb6 100644 --- a/fedireads/books_manager.py +++ b/fedireads/books_manager.py @@ -5,18 +5,17 @@ from fedireads import models from fedireads.tasks import app -def get_or_create_book(key): +def get_or_create_book(value, key='id', connector_id=None): ''' pull up a book record by whatever means possible ''' try: - book = models.Book.objects.select_subclasses().get( - fedireads_key=key - ) + book = models.Book.objects.select_subclasses().get(**{key: value}) return book except models.Book.DoesNotExist: pass - connector = get_connector() - book = connector.get_or_create_book(key) + connector_info = models.Connector.objects.get(id=connector_id) + connector = load_connector(connector_info) + book = connector.get_or_create_book(value) load_more_data.delay(book.id) return book @@ -25,7 +24,7 @@ def get_or_create_book(key): 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 = get_connector(book) + connector = load_connector(book.connector) connector.expand_book_data(book) @@ -60,7 +59,7 @@ def first_search_result(query): def update_book(book): ''' re-sync with the original data source ''' - connector = get_connector(book) + connector = load_connector(book.connector) connector.update_book(book) @@ -70,18 +69,6 @@ def get_connectors(): return [load_connector(c) for c in connectors_info] -def get_connector(book=None): - ''' pick a book data connector ''' - if book and book.connector: - connector_info = book.connector - else: - # only select from external connectors - connector_info = models.Connector.objects.filter( - local=False - ).order_by('priority').first() - return load_connector(connector_info) - - def load_connector(connector_info): ''' instantiate the connector class ''' connector = importlib.import_module( diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index bba5cda8e..fb28c3fc1 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -22,12 +22,13 @@ class AbstractConnector(ABC): self.max_query_count = info.max_query_count self.name = info.name self.local = info.local + self.id = info.id def is_available(self): ''' check if you're allowed to use this connector ''' - if self.connector.max_query_count is not None: - if self.connector.query_count >= self.connector.max_query_count: + if self.max_query_count is not None: + if self.connector.query_count >= self.max_query_count: return False return True diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index ea06dbbaf..a83d2c930 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -9,6 +9,8 @@ from .abstract_connector import update_from_mappings, get_date class Connector(AbstractConnector): + ''' interact with other instances ''' + def search(self, query): ''' right now you can't search fedireads, but... ''' resp = requests.get( @@ -23,11 +25,11 @@ class Connector(AbstractConnector): return resp.json() - def get_or_create_book(self, fedireads_key): + def get_or_create_book(self, remote_id): ''' pull up a book record by whatever means possible ''' try: book = models.Book.objects.select_subclasses().get( - fedireads_key=fedireads_key + remote_id=remote_id ) return book except ObjectDoesNotExist: @@ -35,14 +37,14 @@ class Connector(AbstractConnector): # we can't load a book from a remote server, this is it return None # no book was found, so we start creating a new one - book = models.Book(fedireads_key=fedireads_key) + book = models.Book(remote_id=remote_id) def update_book(self, book): ''' add remote data to a local book ''' - fedireads_key = book.fedireads_key + remote_id = book.remote_id response = requests.get( - '%s/%s' % (self.base_url, fedireads_key), + '%s/%s' % (self.base_url, remote_id), headers={ 'Accept': 'application/activity+json; charset=utf-8', }, @@ -81,21 +83,21 @@ class Connector(AbstractConnector): return book - def get_or_create_author(self, fedireads_key): + def get_or_create_author(self, remote_id): ''' load that author ''' try: - return models.Author.objects.get(fedireads_key=fedireads_key) + return models.Author.objects.get(remote_id=remote_id) except ObjectDoesNotExist: pass - resp = requests.get('%s/authors/%s.json' % (self.url, fedireads_key)) + resp = requests.get('%s/authors/%s.json' % (self.url, remote_id)) if not resp.ok: resp.raise_for_status() data = resp.json() # ingest a new author - author = models.Author(fedireads_key=fedireads_key) + author = models.Author(remote_id=remote_id) mappings = { 'born': ('born', get_date), 'died': ('died', get_date), diff --git a/fedireads/connectors/self_connector.py b/fedireads/connectors/self_connector.py index e82b4f08c..a91e8207b 100644 --- a/fedireads/connectors/self_connector.py +++ b/fedireads/connectors/self_connector.py @@ -36,7 +36,7 @@ class Connector(AbstractConnector): search_results.append( SearchResult( book.title, - book.fedireads_key, + book.id, book.author_text, book.published_date.year if book.published_date else None, None @@ -45,21 +45,21 @@ class Connector(AbstractConnector): return search_results - def get_or_create_book(self, fedireads_key): + def get_or_create_book(self, book_id): ''' since this is querying its own data source, it can only get a book, not load one from an external source ''' try: return models.Book.objects.select_subclasses().get( - fedireads_key=fedireads_key + id=book_id ) except ObjectDoesNotExist: return None - def get_or_create_author(self, fedireads_key): + def get_or_create_author(self, author_id): ''' load that author ''' try: - return models.Author.objects.get(fedreads_key=fedireads_key) + return models.Author.objects.get(id=author_id) except ObjectDoesNotExist: pass diff --git a/fedireads/models/book.py b/fedireads/models/book.py index 2585d92f0..5f39ea97e 100644 --- a/fedireads/models/book.py +++ b/fedireads/models/book.py @@ -51,7 +51,6 @@ class Connector(FedireadsModel): class Book(FedireadsModel): ''' a generic book, which can mean either an edition or a work ''' # these identifiers apply to both works and editions - fedireads_key = models.CharField(max_length=255, unique=True, default=uuid4) openlibrary_key = models.CharField(max_length=255, blank=True, null=True) librarything_key = models.CharField(max_length=255, blank=True, null=True) goodreads_key = models.CharField(max_length=255, blank=True, null=True) @@ -151,7 +150,6 @@ class Edition(Book): class Author(FedireadsModel): ''' copy of an author from OL ''' - fedireads_key = models.CharField(max_length=255, unique=True, default=uuid4) openlibrary_key = models.CharField(max_length=255, blank=True, null=True) wikipedia_link = models.CharField(max_length=255, blank=True, null=True) # idk probably other keys would be useful here? diff --git a/fedireads/outgoing.py b/fedireads/outgoing.py index 717053a8c..fc0e542c4 100644 --- a/fedireads/outgoing.py +++ b/fedireads/outgoing.py @@ -286,7 +286,7 @@ def handle_tag(user, book, name): def handle_untag(user, book, name): ''' tag a book ''' - book = models.Book.objects.get(fedireads_key=book) + book = models.Book.objects.get(id=book) tag = models.Tag.objects.get(name=name, book=book, user=user) tag_activity = activitypub.get_remove_tag(tag) tag.delete() diff --git a/fedireads/templates/book.html b/fedireads/templates/book.html index f4bd13ee8..87a2f027f 100644 --- a/fedireads/templates/book.html +++ b/fedireads/templates/book.html @@ -6,7 +6,7 @@ {% include 'snippets/book_titleby.html' with book=book %} {% if request.user.is_authenticated %} - edit + edit Edit Book @@ -22,7 +22,7 @@ {% include 'snippets/shelve_button.html' %} {% if request.user.is_authenticated and not book.cover %} -
+ {% csrf_token %} {{ cover_form.as_p }} @@ -57,7 +57,7 @@

Tags

{% csrf_token %} - +
diff --git a/fedireads/templates/book_results.html b/fedireads/templates/book_results.html index fdba919b6..ed0c9b856 100644 --- a/fedireads/templates/book_results.html +++ b/fedireads/templates/book_results.html @@ -12,7 +12,7 @@ {% for result in result_set.results %}
- {{ result.title }} by {{ result.author }} ({{ result.year }}) + {{ result.title }} by {{ result.author }} ({{ result.year }})
{% endfor %} diff --git a/fedireads/templates/edit_book.html b/fedireads/templates/edit_book.html index 053cecd6c..d53409c92 100644 --- a/fedireads/templates/edit_book.html +++ b/fedireads/templates/edit_book.html @@ -4,7 +4,7 @@

Edit "{{ book.title }}" - + Close @@ -40,8 +40,8 @@

Book Identifiers

-

{{ form.isbn_13 }}

-

{{ form.fedireads_key }}

+

{{ form.isbn_13 }}

+

{{ form.isbn_10 }}

{{ form.openlibrary_key }}

{{ form.librarything_key }}

{{ form.goodreads_key }}

diff --git a/fedireads/templates/editions.html b/fedireads/templates/editions.html index d916ac11e..21091b5ff 100644 --- a/fedireads/templates/editions.html +++ b/fedireads/templates/editions.html @@ -2,7 +2,7 @@ {% load fr_display %} {% block content %}
-

Editions of "{{ work.title }}"

+

Editions of "{{ work.title }}"

    {% for book in editions %}
  1. diff --git a/fedireads/templates/snippets/authors.html b/fedireads/templates/snippets/authors.html index 35fe6a03c..e8106f5d6 100644 --- a/fedireads/templates/snippets/authors.html +++ b/fedireads/templates/snippets/authors.html @@ -1 +1 @@ -{{ book.authors.first.name }} +{{ book.authors.first.name }} diff --git a/fedireads/templates/snippets/book_titleby.html b/fedireads/templates/snippets/book_titleby.html index 69d3da109..543c64c5e 100644 --- a/fedireads/templates/snippets/book_titleby.html +++ b/fedireads/templates/snippets/book_titleby.html @@ -1,5 +1,5 @@ - {{ book.title }} + {{ book.title }} {% if book.authors %} diff --git a/fedireads/templates/snippets/covers_shelf.html b/fedireads/templates/snippets/covers_shelf.html index f12cfcfd9..0907ed0f6 100644 --- a/fedireads/templates/snippets/covers_shelf.html +++ b/fedireads/templates/snippets/covers_shelf.html @@ -37,7 +37,7 @@

    {% include 'snippets/avatar.html' with user=user %} Your thoughts on - a {{ book.title }} + a {{ book.title }} by {% include 'snippets/authors.html' with book=book %}

    diff --git a/fedireads/templates/snippets/create_status.html b/fedireads/templates/snippets/create_status.html index 99a934461..0ae6e2f8e 100644 --- a/fedireads/templates/snippets/create_status.html +++ b/fedireads/templates/snippets/create_status.html @@ -21,7 +21,7 @@ {% endif %}
    {% csrf_token %} - + {% include 'snippets/rate_form.html' with book=book %} {{ review_form.as_p }} @@ -29,14 +29,14 @@ {% csrf_token %} - + {{ comment_form.as_p }}
    diff --git a/fedireads/templates/snippets/rate_action.html b/fedireads/templates/snippets/rate_action.html index e6d1163a9..50c44e832 100644 --- a/fedireads/templates/snippets/rate_action.html +++ b/fedireads/templates/snippets/rate_action.html @@ -4,7 +4,7 @@ {% for i in '12345'|make_list %}
    {% csrf_token %} - +
    {% else %}
    {% csrf_token %} - +
    diff --git a/fedireads/urls.py b/fedireads/urls.py index 6574bbaa5..f6a02bc8b 100644 --- a/fedireads/urls.py +++ b/fedireads/urls.py @@ -11,7 +11,7 @@ localname_regex = r'(?P[\w\-_]+)' user_path = r'^user/%s' % username_regex local_user_path = r'^user/%s' % localname_regex status_path = r'%s/(status|review|comment)/(?P\d+)' % local_user_path -book_path = r'^book/(?P[\w\-]+)' +book_path = r'^book/(?P\d+)' handler404 = 'fedireads.views.not_found_page' handler500 = 'fedireads.views.server_error_page' @@ -58,7 +58,7 @@ urlpatterns = [ re_path(r'%s/replies(.json)?/?$' % status_path, views.replies_page), # books - re_path(r'%s(.json)?/?$' % book_path, views.book_page), + re_path(r'book/(?P[\w_:\d]+)(.json)?/?$', views.book_page), re_path(r'%s/(?Pfriends|local|federated)?$' % book_path, views.book_page), re_path(r'%s/edit/?$' % book_path, views.edit_book_page), re_path(r'^editions/(?P\d+)/?$', views.editions_page), diff --git a/fedireads/view_actions.py b/fedireads/view_actions.py index bf7918a39..98c71a9f8 100644 --- a/fedireads/view_actions.py +++ b/fedireads/view_actions.py @@ -133,7 +133,7 @@ def edit_book(request, book_id): form.save() outgoing.handle_update_book(request.user, book) - return redirect('/book/%s' % book.fedireads_key) + return redirect('/book/%s' % book.id) @login_required @@ -157,7 +157,7 @@ def upload_cover(request, book_id): book.save() outgoing.handle_update_book(request.user, book) - return redirect('/book/%s' % book.fedireads_key) + return redirect('/book/%s' % book.id) @login_required diff --git a/fedireads/views.py b/fedireads/views.py index 287ce0955..463696725 100644 --- a/fedireads/views.py +++ b/fedireads/views.py @@ -8,8 +8,9 @@ from django.template.response import TemplateResponse from django.views.decorators.csrf import csrf_exempt from fedireads import activitypub -from fedireads import forms, models, books_manager +from fedireads import forms, models from fedireads import goodreads_import +from fedireads.books_manager import get_or_create_book from fedireads.tasks import app @@ -363,9 +364,16 @@ def edit_profile_page(request): return TemplateResponse(request, 'edit_user.html', data) -def book_page(request, book_identifier, tab='friends'): +def book_page(request, book_id, tab='friends'): ''' info about a book ''' - book = books_manager.get_or_create_book(book_identifier) + key = 'id' + connector_id = None + if ':' in book_id: + try: + connector_id, key, book_id = book_id.split(':') + except ValueError: + return HttpResponseNotFound() + book = get_or_create_book(book_id, key=key, connector_id=connector_id) if is_api_request(request): return JsonResponse(activitypub.get_book(book)) @@ -430,7 +438,7 @@ def book_page(request, book_identifier, tab='friends'): {'id': 'federated', 'display': 'Federated'} ], 'active_tab': tab, - 'path': '/book/%s' % book_identifier, + 'path': '/book/%s' % book_id, 'cover_form': forms.CoverForm(instance=book), 'info_fields': [ {'name': 'ISBN', 'value': book.isbn_13}, @@ -445,9 +453,9 @@ def book_page(request, book_identifier, tab='friends'): @login_required -def edit_book_page(request, book_identifier): +def edit_book_page(request, book_id): ''' info about a book ''' - book = books_manager.get_or_create_book(book_identifier) + book = get_or_create_book(book_id) if not book.description: book.description = book.parent_work.description data = { @@ -468,10 +476,10 @@ def editions_page(request, work_id): return TemplateResponse(request, 'editions.html', data) -def author_page(request, author_identifier): +def author_page(request, author_id): ''' landing page for an author ''' try: - author = models.Author.objects.get(fedireads_key=author_identifier) + author = models.Author.objects.get(id=author_id) except ValueError: return HttpResponseNotFound() From 1dffe425e0be9729c3d23d268a7d21f3d672bb79 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 3 May 2020 18:07:06 -0700 Subject: [PATCH 02/28] Don't use book absolute ids for nav --- .../migrations/0037_auto_20200504_0049.py | 21 +++++++++++++++++++ fedireads/models/book.py | 5 +++-- fedireads/templates/author.html | 2 +- fedireads/templates/books.html | 2 +- fedireads/templates/editions.html | 6 ++++++ fedireads/templates/import_status.html | 2 +- .../templates/snippets/create_status.html | 6 +++--- .../templates/snippets/status_header.html | 8 +++---- fedireads/templates/tag.html | 2 +- fedireads/views.py | 7 ++++--- 10 files changed, 45 insertions(+), 16 deletions(-) create mode 100644 fedireads/migrations/0037_auto_20200504_0049.py diff --git a/fedireads/migrations/0037_auto_20200504_0049.py b/fedireads/migrations/0037_auto_20200504_0049.py new file mode 100644 index 000000000..039fac6e5 --- /dev/null +++ b/fedireads/migrations/0037_auto_20200504_0049.py @@ -0,0 +1,21 @@ +# Generated by Django 3.0.3 on 2020-05-04 00:49 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('fedireads', '0036_auto_20200503_2007'), + ] + + operations = [ + migrations.RemoveField( + model_name='author', + name='fedireads_key', + ), + migrations.RemoveField( + model_name='book', + name='fedireads_key', + ), + ] diff --git a/fedireads/models/book.py b/fedireads/models/book.py index 5f39ea97e..3e6e6c1e9 100644 --- a/fedireads/models/book.py +++ b/fedireads/models/book.py @@ -94,9 +94,10 @@ class Book(FedireadsModel): @property def absolute_id(self): ''' constructs the absolute reference to any db object ''' + if self.sync and self.remote_id: + return self.remote_id base_path = 'https://%s' % DOMAIN - model_name = type(self).__name__.lower() - return '%s/book/%s' % (base_path, self.openlibrary_key) + return '%s/book/%d' % (base_path, self.id) def save(self, *args, **kwargs): ''' can't be abstract for query reasons, but you shouldn't USE it ''' diff --git a/fedireads/templates/author.html b/fedireads/templates/author.html index cc06cccf3..f9da61d07 100644 --- a/fedireads/templates/author.html +++ b/fedireads/templates/author.html @@ -16,7 +16,7 @@
    {% for book in books %}
    - + {% include 'snippets/book_cover.html' with book=book %} {% include 'snippets/shelve_button.html' with book=book %} diff --git a/fedireads/templates/books.html b/fedireads/templates/books.html index af5221003..21aefd84c 100644 --- a/fedireads/templates/books.html +++ b/fedireads/templates/books.html @@ -6,7 +6,7 @@
    {% for book in books %}
    - + {% include 'snippets/book_cover.html' with book=book %} {% include 'snippets/shelve_button.html' with book=book %} diff --git a/fedireads/templates/editions.html b/fedireads/templates/editions.html index 21091b5ff..3a16026c9 100644 --- a/fedireads/templates/editions.html +++ b/fedireads/templates/editions.html @@ -5,9 +5,15 @@

    Editions of "{{ work.title }}"

      {% for book in editions %} +<<<<<<< HEAD
    1. {% include 'snippets/book_cover.html' with book=book %} +======= +
    2. diff --git a/fedireads/templates/import_status.html b/fedireads/templates/import_status.html index 97b5dbffe..a3c1eb113 100644 --- a/fedireads/templates/import_status.html +++ b/fedireads/templates/import_status.html @@ -44,7 +44,7 @@ {% if item.book %} - + {% include 'snippets/book_cover.html' with book=item.book size='small' %} {% endif %} diff --git a/fedireads/templates/snippets/create_status.html b/fedireads/templates/snippets/create_status.html index 0ae6e2f8e..1bbad7005 100644 --- a/fedireads/templates/snippets/create_status.html +++ b/fedireads/templates/snippets/create_status.html @@ -3,13 +3,13 @@ diff --git a/fedireads/templates/snippets/status_header.html b/fedireads/templates/snippets/status_header.html index 5c58064f1..2cc224bd5 100644 --- a/fedireads/templates/snippets/status_header.html +++ b/fedireads/templates/snippets/status_header.html @@ -6,13 +6,13 @@ {% if status.status_type == 'Update' %} {{ status.content | safe }} {% elif status.status_type == 'Review' and not status.name and not status.content%} - rated {{ status.book.title }} + rated {{ status.book.title }} {% elif status.status_type == 'Review' %} - reviewed {{ status.book.title }} + reviewed {{ status.book.title }} {% elif status.status_type == 'Comment' %} - commented on {{ status.book.title }} + commented on {{ status.book.title }} {% elif status.status_type == 'Quotation' %} - quoted {{ status.book.title }} + quoted {{ status.book.title }} {% elif status.status_type == 'Boost' %} boosted {% elif status.reply_parent %} diff --git a/fedireads/templates/tag.html b/fedireads/templates/tag.html index 3f77941c0..f9e890930 100644 --- a/fedireads/templates/tag.html +++ b/fedireads/templates/tag.html @@ -6,7 +6,7 @@
      {% for book in books.all %}
      - + {% include 'snippets/book_cover.html' with book=book %} {% include 'snippets/rate_action.html' with user=request.user book=book %} diff --git a/fedireads/views.py b/fedireads/views.py index 463696725..0bd3b7df6 100644 --- a/fedireads/views.py +++ b/fedireads/views.py @@ -4,6 +4,7 @@ from django.db.models import Avg, Q from django.http import HttpResponseBadRequest, HttpResponseNotFound,\ JsonResponse from django.core.exceptions import PermissionDenied +from django.shortcuts import redirect from django.template.response import TemplateResponse from django.views.decorators.csrf import csrf_exempt @@ -366,15 +367,15 @@ def edit_profile_page(request): def book_page(request, book_id, tab='friends'): ''' info about a book ''' - key = 'id' - connector_id = None if ':' in book_id: try: connector_id, key, book_id = book_id.split(':') except ValueError: return HttpResponseNotFound() - book = get_or_create_book(book_id, key=key, connector_id=connector_id) + book = get_or_create_book(book_id, key=key, connector_id=connector_id) + return redirect('/book/%d' % book.id) + book = get_or_create_book(book_id) if is_api_request(request): return JsonResponse(activitypub.get_book(book)) From de4f78389112cad619ab8e134491389e3d151ec1 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 3 May 2020 18:15:03 -0700 Subject: [PATCH 03/28] Fixes rebase merge error --- fedireads/templates/editions.html | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/fedireads/templates/editions.html b/fedireads/templates/editions.html index 3a16026c9..535e651e2 100644 --- a/fedireads/templates/editions.html +++ b/fedireads/templates/editions.html @@ -5,15 +5,9 @@

      Editions of "{{ work.title }}"

        {% for book in editions %} -<<<<<<< HEAD
      1. - - {% include 'snippets/book_cover.html' with book=book %} -======= -
      2. From 3a4a194160f72f75e0924505d9454ea261706c40 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 3 May 2020 18:56:29 -0700 Subject: [PATCH 04/28] Path to update books --- fedireads/books_manager.py | 4 +- fedireads/connectors/abstract_connector.py | 2 +- fedireads/connectors/openlibrary.py | 2 +- fedireads/connectors/self_connector.py | 1 - fedireads/incoming.py | 19 ++++++++- .../migrations/0037_auto_20200504_0049.py | 21 ---------- .../migrations/0037_auto_20200504_0154.py | 41 +++++++++++++++++++ fedireads/models/book.py | 7 ++-- init_db.py | 4 +- 9 files changed, 68 insertions(+), 33 deletions(-) delete mode 100644 fedireads/migrations/0037_auto_20200504_0049.py create mode 100644 fedireads/migrations/0037_auto_20200504_0154.py diff --git a/fedireads/books_manager.py b/fedireads/books_manager.py index 6f0d78cb6..fe00aa7dc 100644 --- a/fedireads/books_manager.py +++ b/fedireads/books_manager.py @@ -57,10 +57,10 @@ def first_search_result(query): return None -def update_book(book): +def update_book(book, data=None): ''' re-sync with the original data source ''' connector = load_connector(book.connector) - connector.update_book(book) + connector.update_book(book, data=data) def get_connectors(): diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index fb28c3fc1..bb3df27cf 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -57,7 +57,7 @@ class AbstractConnector(ABC): @abstractmethod - def update_book(self, book_obj): + def update_book(self, book_obj, data=None): ''' sync a book with the canonical remote copy ''' # return book model obj diff --git a/fedireads/connectors/openlibrary.py b/fedireads/connectors/openlibrary.py index 72be68fd5..1f3b3e43a 100644 --- a/fedireads/connectors/openlibrary.py +++ b/fedireads/connectors/openlibrary.py @@ -140,7 +140,7 @@ class Connector(AbstractConnector): return book - def update_book(self, book): + def update_book(self, book, data=None): ''' load new data ''' if not book.sync and not book.sync_cover: return diff --git a/fedireads/connectors/self_connector.py b/fedireads/connectors/self_connector.py index a91e8207b..f1d5b24bd 100644 --- a/fedireads/connectors/self_connector.py +++ b/fedireads/connectors/self_connector.py @@ -23,7 +23,6 @@ class Connector(AbstractConnector): SearchVector('isbn_10', weight='A') +\ SearchVector('openlibrary_key', weight='B') +\ SearchVector('goodreads_key', weight='B') +\ - SearchVector('source_url', weight='B') +\ SearchVector('asin', weight='B') +\ SearchVector('oclc_number', weight='B') +\ SearchVector('description', weight='C') +\ diff --git a/fedireads/incoming.py b/fedireads/incoming.py index 74218fbb9..62fd3d435 100644 --- a/fedireads/incoming.py +++ b/fedireads/incoming.py @@ -10,7 +10,7 @@ from django.http import HttpResponseBadRequest, HttpResponseNotFound from django.views.decorators.csrf import csrf_exempt import requests -from fedireads import models, outgoing +from fedireads import books_manager, models, outgoing from fedireads import status as status_builder from fedireads.remote_user import get_or_create_remote_user from fedireads.tasks import app @@ -63,7 +63,7 @@ def shared_inbox(request): }, 'Update': { 'Person': None,# TODO: handle_update_user - 'Document': None# TODO: handle_update_book + 'Document': handle_update_book, }, } activity_type = activity['type'] @@ -320,3 +320,18 @@ def handle_tag(activity): if not user.local: book = activity['target']['id'].split('/')[-1] status_builder.create_tag(user, book, activity['object']['name']) + + +@app.task +def handle_update_book(activity): + ''' a remote instance changed a book (Document) ''' + document = activity['object'] + # check if we have their copy and care about their updates + book = models.Book.objects.select_subclasses().filter( + remote_id=document['url'], + sync=True, + ).first() + if not book: + return + + books_manager.update_book(book, data=document) diff --git a/fedireads/migrations/0037_auto_20200504_0049.py b/fedireads/migrations/0037_auto_20200504_0049.py deleted file mode 100644 index 039fac6e5..000000000 --- a/fedireads/migrations/0037_auto_20200504_0049.py +++ /dev/null @@ -1,21 +0,0 @@ -# Generated by Django 3.0.3 on 2020-05-04 00:49 - -from django.db import migrations - - -class Migration(migrations.Migration): - - dependencies = [ - ('fedireads', '0036_auto_20200503_2007'), - ] - - operations = [ - migrations.RemoveField( - model_name='author', - name='fedireads_key', - ), - migrations.RemoveField( - model_name='book', - name='fedireads_key', - ), - ] diff --git a/fedireads/migrations/0037_auto_20200504_0154.py b/fedireads/migrations/0037_auto_20200504_0154.py new file mode 100644 index 000000000..5807580d7 --- /dev/null +++ b/fedireads/migrations/0037_auto_20200504_0154.py @@ -0,0 +1,41 @@ +# Generated by Django 3.0.3 on 2020-05-04 01:54 + +from django.db import migrations, models +import django.utils.timezone + + +class Migration(migrations.Migration): + + dependencies = [ + ('fedireads', '0036_auto_20200503_2007'), + ] + + operations = [ + migrations.RemoveField( + model_name='author', + name='fedireads_key', + ), + migrations.RemoveField( + model_name='book', + name='fedireads_key', + ), + migrations.RemoveField( + model_name='book', + name='source_url', + ), + migrations.AddField( + model_name='author', + name='last_sync_date', + field=models.DateTimeField(default=django.utils.timezone.now), + ), + migrations.AddField( + model_name='author', + name='sync', + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name='book', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + ] diff --git a/fedireads/models/book.py b/fedireads/models/book.py index 3e6e6c1e9..d79449845 100644 --- a/fedireads/models/book.py +++ b/fedireads/models/book.py @@ -1,6 +1,4 @@ ''' database schema for books and shelves ''' -from uuid import uuid4 - from django.utils import timezone from django.db import models from model_utils.managers import InheritanceManager @@ -50,6 +48,7 @@ class Connector(FedireadsModel): class Book(FedireadsModel): ''' a generic book, which can mean either an edition or a work ''' + remote_id = models.CharField(max_length=255, null=True) # these identifiers apply to both works and editions openlibrary_key = models.CharField(max_length=255, blank=True, null=True) librarything_key = models.CharField(max_length=255, blank=True, null=True) @@ -57,7 +56,6 @@ class Book(FedireadsModel): misc_identifiers = JSONField(null=True) # info about where the data comes from and where/if to sync - source_url = models.CharField(max_length=255, unique=True, null=True) sync = models.BooleanField(default=True) sync_cover = models.BooleanField(default=True) last_sync_date = models.DateTimeField(default=timezone.now) @@ -130,6 +128,7 @@ class Edition(Book): ''' an edition of a book ''' # default -> this is what gets displayed for a work default = models.BooleanField(default=False) + # these identifiers only apply to editions, not works isbn_10 = models.CharField(max_length=255, blank=True, null=True) isbn_13 = models.CharField(max_length=255, blank=True, null=True) @@ -152,6 +151,8 @@ class Edition(Book): class Author(FedireadsModel): ''' copy of an author from OL ''' openlibrary_key = models.CharField(max_length=255, blank=True, null=True) + sync = models.BooleanField(default=True) + last_sync_date = models.DateTimeField(default=timezone.now) wikipedia_link = models.CharField(max_length=255, blank=True, null=True) # idk probably other keys would be useful here? born = models.DateTimeField(blank=True, null=True) diff --git a/init_db.py b/init_db.py index 2b6a09ace..62e0d5713 100644 --- a/init_db.py +++ b/init_db.py @@ -36,5 +36,5 @@ Connector.objects.create( ) -get_or_create_book('OL1715344W') -get_or_create_book('OL102749W') +get_or_create_book('OL1715344W', key='openlibrary_key', connector_id=1) +get_or_create_book('OL102749W', key='openlibrary_key', connector_id=1) From b8568a76c81723f8cc70bc6de395ec77bf3c4a53 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 3 May 2020 18:58:12 -0700 Subject: [PATCH 05/28] Remove references to source_url --- fedireads/activitypub/book.py | 2 -- fedireads/connectors/fedireads_connector.py | 4 ++-- fedireads/forms.py | 1 - 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/fedireads/activitypub/book.py b/fedireads/activitypub/book.py index 80d8ff0fe..98a71f962 100644 --- a/fedireads/activitypub/book.py +++ b/fedireads/activitypub/book.py @@ -17,8 +17,6 @@ def get_book(book): 'physical_format', 'misc_identifiers', - 'source_url', - 'description', 'languages', 'series', diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index a83d2c930..3093f5a29 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -61,8 +61,8 @@ class Connector(AbstractConnector): } book = update_from_mappings(book, data, mappings) - if not book.source_url: - book.source_url = response.url + if not book.remote_id: + book.remote_id = response.url if not book.connector: book.connector = self.connector book.save() diff --git a/fedireads/forms.py b/fedireads/forms.py index 45ee4f142..a7501bf5f 100644 --- a/fedireads/forms.py +++ b/fedireads/forms.py @@ -109,7 +109,6 @@ class EditionForm(ModelForm): 'subjects',# TODO 'subject_places',# TODO - 'source_url', 'connector', ] From 7594bed5d30171fc2185666a819b8d991ed7898f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 3 May 2020 19:49:32 -0700 Subject: [PATCH 06/28] Use status source as connector --- fedireads/connectors/fedireads_connector.py | 21 +++++------ fedireads/status.py | 40 +++++++++++++++++++-- init_db.py | 2 +- 3 files changed, 50 insertions(+), 13 deletions(-) diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index 3093f5a29..fc354b181 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -40,19 +40,20 @@ class Connector(AbstractConnector): book = models.Book(remote_id=remote_id) - def update_book(self, book): + def update_book(self, book, data=None): ''' add remote data to a local book ''' remote_id = book.remote_id - response = requests.get( - '%s/%s' % (self.base_url, remote_id), - headers={ - 'Accept': 'application/activity+json; charset=utf-8', - }, - ) - if not response.ok: - response.raise_for_status() + if not data: + response = requests.get( + '%s/%s' % (self.base_url, remote_id), + headers={ + 'Accept': 'application/activity+json; charset=utf-8', + }, + ) + if not response.ok: + response.raise_for_status() - data = response.json() + data = response.json() # great, we can update our book. mappings = { diff --git a/fedireads/status.py b/fedireads/status.py index 558d8a70f..a0fa34f97 100644 --- a/fedireads/status.py +++ b/fedireads/status.py @@ -1,8 +1,9 @@ ''' Handle user activity ''' +from urllib.parse import urlparse + from django.db import IntegrityError -from fedireads import models -from fedireads.books_manager import get_or_create_book +from fedireads import books_manager, models from fedireads.sanitize_html import InputHtmlParser @@ -25,6 +26,41 @@ def create_review_from_activity(author, activity): return review +def get_or_create_book(remote_id): + ''' try the remote id and then figure out the right connector ''' + book = get_by_absolute_id(remote_id, models.Book) + if book: + return book + + connector = get_or_create_connector(remote_id) + return books_manager.get_or_create_book( + remote_id, + key=connector.key_name, + connector_id=connector.id + ) + + +def get_or_create_connector(remote_id): + ''' get the connector related to the author's server ''' + url = urlparse(remote_id) + identifier = url.netloc + try: + connector_info = models.Connector.objects.get(identifier=identifier) + except models.Connector.DoesNotExist: + models.Connector.objects.create( + identifier=identifier, + connector_file='fedireads_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, + key_name='remote_id', + priority=3 + ) + + return books_manager.load_connector(connector_info) + + def create_rating(user, book, rating): ''' a review that's just a rating ''' if not rating or rating < 1 or rating > 5: diff --git a/init_db.py b/init_db.py index 62e0d5713..6ed9f3871 100644 --- a/init_db.py +++ b/init_db.py @@ -31,7 +31,7 @@ Connector.objects.create( books_url='https://%s/book' % DOMAIN, covers_url='https://%s/images/covers' % DOMAIN, search_url='https://%s/search?q=' % DOMAIN, - key_name='openlibrary_key', + key_name='id', priority=1, ) From 3c3afed6b33e67e1cc779ee8b00705e48c43c92f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 3 May 2020 19:59:05 -0700 Subject: [PATCH 07/28] Adds remote id to self connector search --- fedireads/connectors/self_connector.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fedireads/connectors/self_connector.py b/fedireads/connectors/self_connector.py index f1d5b24bd..4cf47f73c 100644 --- a/fedireads/connectors/self_connector.py +++ b/fedireads/connectors/self_connector.py @@ -25,6 +25,7 @@ class Connector(AbstractConnector): SearchVector('goodreads_key', weight='B') +\ SearchVector('asin', weight='B') +\ SearchVector('oclc_number', weight='B') +\ + SearchVector('remote_id', weight='B') +\ SearchVector('description', weight='C') +\ SearchVector('series', weight='C') ).filter(search=query) @@ -63,7 +64,7 @@ class Connector(AbstractConnector): pass - def update_book(self, book_obj): + def update_book(self, book_obj, data=None): pass From 07aab3806b4ed1c6d4c936a3caf66a93febd8975 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 3 May 2020 21:00:25 -0700 Subject: [PATCH 08/28] Expand matching books on keys like isbn --- fedireads/connectors/abstract_connector.py | 56 +++++++++++++++++-- fedireads/connectors/fedireads_connector.py | 45 +++++++--------- fedireads/connectors/openlibrary.py | 59 ++++++++++----------- fedireads/connectors/self_connector.py | 17 +++--- 4 files changed, 109 insertions(+), 68 deletions(-) diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index bb3df27cf..0eb18c2d9 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -2,6 +2,7 @@ from abc import ABC, abstractmethod from dateutil import parser import pytz +import requests from fedireads import models @@ -33,10 +34,27 @@ class AbstractConnector(ABC): return True - @abstractmethod def search(self, query): ''' free text search ''' - # return list of search result objs + resp = requests.get( + '%s%s' % (self.search_url, query), + headers={ + 'Accept': 'application/json; charset=utf-8', + }, + ) + if not resp.ok: + resp.raise_for_status() + data = resp.json() + results = [] + + for doc in data['docs'][:10]: + results.append(self.format_search_result(doc)) + return results + + + @abstractmethod + def format_search_result(self, search_result): + ''' create a SearchResult obj from json ''' @abstractmethod @@ -82,6 +100,37 @@ def update_from_mappings(obj, data, mappings): return obj +def match_from_mappings(data, mappings): + ''' try to find existing copies of this book using various keys ''' + keys = [ + ('openlibrary_key', models.Book), + ('librarything_key', models.Book), + ('goodreads_key', models.Book), + ('lccn', models.Work), + ('isbn_10', models.Edition), + ('isbn_13', models.Edition), + ('oclc_number', models.Edition), + ('asin', models.Edition), + ] + noop = lambda x: x + for key, model in keys: + formatter = None + if key in mappings: + key, formatter = mappings[key] + if not formatter: + formatter = noop + + value = data.get(key) + if not value: + continue + value = formatter(value) + + match = model.objects.select_subclasses().filter( + **{key: value}).first() + if match: + return match + + def has_attr(obj, key): ''' helper function to check if a model object has a key ''' try: @@ -100,12 +149,11 @@ def get_date(date_string): class SearchResult: ''' standardized search result object ''' - def __init__(self, title, key, author, year, raw_data): + def __init__(self, title, key, author, year): self.title = title self.key = key self.author = author self.year = year - self.raw_data = raw_data def __repr__(self): return "".format( diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index fc354b181..7fbc4f49b 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -4,48 +4,37 @@ from django.core.files.base import ContentFile import requests from fedireads import models -from .abstract_connector import AbstractConnector -from .abstract_connector import update_from_mappings, get_date +from .abstract_connector import AbstractConnector, SearchResult, get_date +from .abstract_connector import match_from_mappings, update_from_mappings class Connector(AbstractConnector): ''' interact with other instances ''' - def search(self, query): - ''' right now you can't search fedireads, but... ''' - resp = requests.get( - '%s%s' % (self.search_url, query), - headers={ - 'Accept': 'application/activity+json; charset=utf-8', - }, - ) - if not resp.ok: - resp.raise_for_status() - - return resp.json() + def format_search_result(self, search_result): + return SearchResult(**search_result) def get_or_create_book(self, remote_id): ''' pull up a book record by whatever means possible ''' - try: - book = models.Book.objects.select_subclasses().get( - remote_id=remote_id - ) + book = models.Book.objects.select_subclasses().filter( + remote_id=remote_id + ).first() + if book: + if isinstance(book, models.Work): + return book.default_edition return book - except ObjectDoesNotExist: - if self.model.is_self: - # we can't load a book from a remote server, this is it - return None - # no book was found, so we start creating a new one - book = models.Book(remote_id=remote_id) + + # no book was found, so we start creating a new one + book = models.Book(remote_id=remote_id) + self.update_book(book) def update_book(self, book, data=None): ''' add remote data to a local book ''' - remote_id = book.remote_id if not data: response = requests.get( - '%s/%s' % (self.base_url, remote_id), + book.remote_id, headers={ 'Accept': 'application/activity+json; charset=utf-8', }, @@ -55,6 +44,10 @@ class Connector(AbstractConnector): data = response.json() + match = match_from_mappings(data, {}) + if match: + return match + # great, we can update our book. mappings = { 'published_date': ('published_date', get_date), diff --git a/fedireads/connectors/openlibrary.py b/fedireads/connectors/openlibrary.py index 1f3b3e43a..4efe9ab3a 100644 --- a/fedireads/connectors/openlibrary.py +++ b/fedireads/connectors/openlibrary.py @@ -7,7 +7,8 @@ from django.db import transaction from fedireads import models from .abstract_connector import AbstractConnector, SearchResult -from .abstract_connector import update_from_mappings, get_date +from .abstract_connector import match_from_mappings, update_from_mappings +from .abstract_connector import get_date from .openlibrary_languages import languages @@ -15,45 +16,34 @@ class Connector(AbstractConnector): ''' instantiate a connector for OL ''' def __init__(self, identifier): get_first = lambda a: a[0] - self.book_mappings = { - 'publish_date': ('published_date', get_date), - 'first_publish_date': ('first_published_date', get_date), - 'description': ('description', get_description), + self.key_mappings = { 'isbn_13': ('isbn_13', get_first), 'oclc_numbers': ('oclc_number', get_first), 'lccn': ('lccn', get_first), + } + + self.book_mappings = self.key_mappings.copy() + self.book_mappings.update({ + 'publish_date': ('published_date', get_date), + 'first_publish_date': ('first_published_date', get_date), + 'description': ('description', get_description), 'languages': ('languages', get_languages), 'number_of_pages': ('pages', None), 'series': ('series', get_first), - } + }) super().__init__(identifier) - def search(self, query): - ''' query openlibrary search ''' - resp = requests.get( - '%s%s' % (self.search_url, query), - headers={ - 'Accept': 'application/json; charset=utf-8', - }, + def format_search_result(self, doc): + key = doc['key'] + key = key.split('/')[-1] + author = doc.get('author_name') or ['Unknown'] + return SearchResult( + doc.get('title'), + key, + author[0], + doc.get('first_publish_year'), ) - if not resp.ok: - resp.raise_for_status() - data = resp.json() - results = [] - - for doc in data['docs'][:5]: - key = doc['key'] - key = key.split('/')[-1] - author = doc.get('author_name') or ['Unknown'] - results.append(SearchResult( - doc.get('title'), - key, - author[0], - doc.get('first_publish_year'), - doc - )) - return results def get_or_create_book(self, olkey): @@ -115,6 +105,11 @@ class Connector(AbstractConnector): def create_book(self, key, data, model): ''' create a work or edition from data ''' + # we really would rather use an existing book than make a new one + match = match_from_mappings(data, self.key_mappings) + if match: + return match + book = model.objects.create( openlibrary_key=key, title=data['title'], @@ -145,7 +140,9 @@ class Connector(AbstractConnector): if not book.sync and not book.sync_cover: return - data = self.load_book_data(book.openlibrary_key) + if not data: + data = self.load_book_data(book.openlibrary_key) + if book.sync_cover and data.get('covers'): book.cover.save(*self.get_cover(data['covers'][0]), save=True) if book.sync: diff --git a/fedireads/connectors/self_connector.py b/fedireads/connectors/self_connector.py index 4cf47f73c..dfcd36459 100644 --- a/fedireads/connectors/self_connector.py +++ b/fedireads/connectors/self_connector.py @@ -34,17 +34,20 @@ class Connector(AbstractConnector): search_results = [] for book in results[:10]: search_results.append( - SearchResult( - book.title, - book.id, - book.author_text, - book.published_date.year if book.published_date else None, - None - ) + self.format_search_result(book) ) return search_results + def format_search_result(self, book): + return SearchResult( + book.title, + book.id, + book.author_text, + book.published_date.year if book.published_date else None, + ) + + def get_or_create_book(self, book_id): ''' since this is querying its own data source, it can only get a book, not load one from an external source ''' From 7454fb745456f6704cc7554b4a3c6fbf03a718a2 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 3 May 2020 21:13:43 -0700 Subject: [PATCH 09/28] Search is a view not an action --- fedireads/urls.py | 3 ++- fedireads/view_actions.py | 19 +------------------ fedireads/views.py | 22 ++++++++++++++++++++-- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/fedireads/urls.py b/fedireads/urls.py index f6a02bc8b..bd43a72d4 100644 --- a/fedireads/urls.py +++ b/fedireads/urls.py @@ -68,13 +68,14 @@ urlpatterns = [ re_path(r'^shelf/%s/(?P[\w-]+)(.json)?/?$' % username_regex, views.shelf_page), re_path(r'^shelf/%s/(?P[\w-]+)(.json)?/?$' % localname_regex, views.shelf_page), + re_path(r'^search/?$', views.search), + # internal action endpoints re_path(r'^logout/?$', actions.user_logout), re_path(r'^user-login/?$', actions.user_login), re_path(r'^register/?$', actions.register), re_path(r'^edit_profile/?$', actions.edit_profile), - re_path(r'^search/?$', actions.search), re_path(r'^import_data/?', actions.import_data), re_path(r'^edit_book/(?P\d+)/?', actions.edit_book), re_path(r'^upload_cover/(?P\d+)/?', actions.upload_cover), diff --git a/fedireads/view_actions.py b/fedireads/view_actions.py index 98c71a9f8..4c7e65b9f 100644 --- a/fedireads/view_actions.py +++ b/fedireads/view_actions.py @@ -1,6 +1,5 @@ ''' views for actions you can take in the application ''' from io import BytesIO, TextIOWrapper -import re from PIL import Image from django.contrib.auth import authenticate, login, logout @@ -10,7 +9,7 @@ from django.http import HttpResponseBadRequest, HttpResponseNotFound from django.shortcuts import redirect from django.template.response import TemplateResponse -from fedireads import forms, models, books_manager, outgoing +from fedireads import forms, models, outgoing from fedireads import goodreads_import from fedireads.settings import DOMAIN from fedireads.views import get_user_from_username @@ -346,22 +345,6 @@ def unfollow(request): return redirect('/user/%s' % user_slug) -@login_required -def search(request): - ''' that search bar up top ''' - query = request.GET.get('q') - if re.match(r'\w+@\w+.\w+', query): - # if something looks like a username, search with webfinger - results = [outgoing.handle_account_search(query)] - template = 'user_results.html' - else: - # just send the question over to book search - results = books_manager.search(query) - template = 'book_results.html' - - return TemplateResponse(request, template, {'results': results}) - - @login_required def clear_notifications(request): ''' permanently delete notification for user ''' diff --git a/fedireads/views.py b/fedireads/views.py index 0bd3b7df6..020137408 100644 --- a/fedireads/views.py +++ b/fedireads/views.py @@ -1,4 +1,6 @@ ''' views for pages you can go to in the application ''' +import re + from django.contrib.auth.decorators import login_required from django.db.models import Avg, Q from django.http import HttpResponseBadRequest, HttpResponseNotFound,\ @@ -8,8 +10,8 @@ from django.shortcuts import redirect from django.template.response import TemplateResponse from django.views.decorators.csrf import csrf_exempt -from fedireads import activitypub -from fedireads import forms, models +from fedireads import activitypub, outgoing +from fedireads import forms, models, books_manager from fedireads import goodreads_import from fedireads.books_manager import get_or_create_book from fedireads.tasks import app @@ -141,6 +143,22 @@ def get_activity_feed(user, filter_level, model=models.Status): return activities +@login_required +def search(request): + ''' that search bar up top ''' + query = request.GET.get('q') + if re.match(r'\w+@\w+.\w+', query): + # if something looks like a username, search with webfinger + results = [outgoing.handle_account_search(query)] + template = 'user_results.html' + else: + # just send the question over to book search + results = books_manager.search(query) + template = 'book_results.html' + + return TemplateResponse(request, template, {'results': results}) + + def books_page(request): ''' discover books ''' recent_books = models.Work.objects From d990a5effd2f576c3648140bf614da9d720d6a30 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 4 May 2020 10:15:41 -0700 Subject: [PATCH 10/28] Json serialize search results --- fedireads/books_manager.py | 7 +++++++ fedireads/views.py | 18 ++++++++++++------ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/fedireads/books_manager.py b/fedireads/books_manager.py index fe00aa7dc..1489c5be1 100644 --- a/fedireads/books_manager.py +++ b/fedireads/books_manager.py @@ -48,6 +48,13 @@ def search(query): return results +def local_search(query): + ''' only look at local search results ''' + connector = load_connector(models.Connector.objects.get(local=True)) + return connector.search(query) + + + def first_search_result(query): ''' search until you find a result that fits ''' for connector in get_connectors(): diff --git a/fedireads/views.py b/fedireads/views.py index 020137408..6cfe7c38b 100644 --- a/fedireads/views.py +++ b/fedireads/views.py @@ -150,13 +150,19 @@ def search(request): if re.match(r'\w+@\w+.\w+', query): # if something looks like a username, search with webfinger results = [outgoing.handle_account_search(query)] - template = 'user_results.html' - else: - # just send the question over to book search - results = books_manager.search(query) - template = 'book_results.html' + return TemplateResponse( + request, 'user_results.html', {'results': results} + ) - return TemplateResponse(request, template, {'results': results}) + # or just send the question over to book search + + if is_api_request(request): + # only return local results via json so we don't cause a cascade + results = books_manager.local_search(query) + return JsonResponse([r.__dict__ for r in results], safe=False) + + results = books_manager.search(query) + return TemplateResponse(request, 'book_results.html', {'results': results}) def books_page(request): From 2c1e7b8ecc1fbcb26b4006e7c72fa2e80269cde9 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 4 May 2020 10:42:48 -0700 Subject: [PATCH 11/28] fixes mapping for openlibrary isbn10 field --- fedireads/connectors/openlibrary.py | 1 + 1 file changed, 1 insertion(+) diff --git a/fedireads/connectors/openlibrary.py b/fedireads/connectors/openlibrary.py index 4efe9ab3a..5d383ab8f 100644 --- a/fedireads/connectors/openlibrary.py +++ b/fedireads/connectors/openlibrary.py @@ -18,6 +18,7 @@ class Connector(AbstractConnector): get_first = lambda a: a[0] self.key_mappings = { 'isbn_13': ('isbn_13', get_first), + 'isbn_10': ('isbn_10', get_first), 'oclc_numbers': ('oclc_number', get_first), 'lccn': ('lccn', get_first), } From 1f2de18d4203e5fbb1ef3878aeb844ea1a539163 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 4 May 2020 12:36:55 -0700 Subject: [PATCH 12/28] Cleans up status creation book lookup flow --- fedireads/books_manager.py | 60 ++++++++++++++ fedireads/connectors/fedireads_connector.py | 4 + fedireads/outgoing.py | 5 +- fedireads/status.py | 89 +++------------------ fedireads/view_actions.py | 49 +++++------- 5 files changed, 99 insertions(+), 108 deletions(-) diff --git a/fedireads/books_manager.py b/fedireads/books_manager.py index 1489c5be1..209a9d500 100644 --- a/fedireads/books_manager.py +++ b/fedireads/books_manager.py @@ -1,5 +1,6 @@ ''' select and call a connector for whatever book task needs doing ''' import importlib +from urllib.parse import urlparse from fedireads import models from fedireads.tasks import app @@ -13,6 +14,13 @@ def get_or_create_book(value, key='id', connector_id=None): except models.Book.DoesNotExist: pass + if key == 'remote_id': + book = get_by_absolute_id(value, models.Book) + if book: + return book + connector = get_or_create_connector(value) + return connector.get_or_create_book(value) + connector_info = models.Connector.objects.get(id=connector_id) connector = load_connector(connector_info) book = connector.get_or_create_book(value) @@ -20,6 +28,58 @@ def get_or_create_book(value, key='id', connector_id=None): 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) + + try: + connector_info = models.Connector.objects.get(identifier=identifier) + except models.Connector.DoesNotExist: + connector_info = models.Connector.objects.create( + identifier=identifier, + connector_file='fedireads_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, + key_name='remote_id', + priority=3 + ) + + return load_connector(connector_info) + + +def get_by_absolute_id(absolute_id, model): + ''' generalized function to get from a model with a remote_id field ''' + if not absolute_id: + return None + + # check if it's a remote status + try: + return model.objects.get(remote_id=absolute_id) + except model.DoesNotExist: + pass + + # try finding a local status with that id + local_id = absolute_id.split('/')[-1] + try: + if hasattr(model.objects, 'select_subclasses'): + possible_match = model.objects.select_subclasses().get(id=local_id) + else: + possible_match = model.objects.get(id=local_id) + except model.DoesNotExist: + return None + + # make sure it's not actually a remote status with an id that + # clashes with a local id + if possible_match.absolute_id == absolute_id: + return possible_match + return None + + @app.task def load_more_data(book_id): ''' background the work of getting all 10,000 editions of LoTR ''' diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index 7fbc4f49b..84d804f69 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -102,6 +102,10 @@ class Connector(AbstractConnector): return author + def expand_book_data(self, book): + pass + + def get_cover(cover_url): ''' ask openlibrary for the cover ''' image_name = cover_url.split('/')[-1] diff --git a/fedireads/outgoing.py b/fedireads/outgoing.py index fc0e542c4..4fb307388 100644 --- a/fedireads/outgoing.py +++ b/fedireads/outgoing.py @@ -7,7 +7,7 @@ from django.http import HttpResponseNotFound, JsonResponse from django.views.decorators.csrf import csrf_exempt import requests -from fedireads import activitypub +from fedireads import activitypub, books_manager from fedireads import models from fedireads.broadcast import broadcast from fedireads.status import create_review, create_status @@ -260,9 +260,10 @@ def handle_comment(user, book, content): user, book, builder, fr_serializer, ap_serializer, content) -def handle_status(user, book, \ +def handle_status(user, book_id, \ builder, fr_serializer, ap_serializer, *args): ''' generic handler for statuses ''' + book = books_manager.get_or_create_book(book_id) status = builder(user, book, *args) activity = fr_serializer(status) diff --git a/fedireads/status.py b/fedireads/status.py index a0fa34f97..44901983e 100644 --- a/fedireads/status.py +++ b/fedireads/status.py @@ -1,24 +1,21 @@ ''' Handle user activity ''' -from urllib.parse import urlparse - from django.db import IntegrityError -from fedireads import books_manager, models +from fedireads import models +from fedireads.books_manager import get_or_create_book, get_by_absolute_id from fedireads.sanitize_html import InputHtmlParser def create_review_from_activity(author, activity): ''' parse an activity json blob into a status ''' book_id = activity['inReplyToBook'] - book_id = book_id.split('/')[-1] + book = get_or_create_book(book_id, key='remote_id') name = activity.get('name') rating = activity.get('rating') content = activity.get('content') published = activity.get('published') remote_id = activity['id'] - book = get_or_create_book(book_id) - review = create_review(author, book, name, content, rating) review.published_date = published review.remote_id = remote_id @@ -26,41 +23,6 @@ def create_review_from_activity(author, activity): return review -def get_or_create_book(remote_id): - ''' try the remote id and then figure out the right connector ''' - book = get_by_absolute_id(remote_id, models.Book) - if book: - return book - - connector = get_or_create_connector(remote_id) - return books_manager.get_or_create_book( - remote_id, - key=connector.key_name, - connector_id=connector.id - ) - - -def get_or_create_connector(remote_id): - ''' get the connector related to the author's server ''' - url = urlparse(remote_id) - identifier = url.netloc - try: - connector_info = models.Connector.objects.get(identifier=identifier) - except models.Connector.DoesNotExist: - models.Connector.objects.create( - identifier=identifier, - connector_file='fedireads_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, - key_name='remote_id', - priority=3 - ) - - return books_manager.load_connector(connector_info) - - def create_rating(user, book, rating): ''' a review that's just a rating ''' if not rating or rating < 1 or rating > 5: @@ -94,8 +56,8 @@ def create_review(user, book, name, content, rating): def create_quotation_from_activity(author, activity): ''' parse an activity json blob into a status ''' - book = activity['inReplyToBook'] - book = book.split('/')[-1] + book_id = activity['inReplyToBook'] + book = get_or_create_book(book_id, key='remote_id') quote = activity.get('quote') content = activity.get('content') published = activity.get('published') @@ -108,10 +70,9 @@ def create_quotation_from_activity(author, activity): return quotation -def create_quotation(user, possible_book, content, quote): +def create_quotation(user, book, content, quote): ''' a quotation has been added ''' # throws a value error if the book is not found - book = get_or_create_book(possible_book) content = sanitize(content) quote = sanitize(quote) @@ -123,11 +84,10 @@ def create_quotation(user, possible_book, content, quote): ) - def create_comment_from_activity(author, activity): ''' parse an activity json blob into a status ''' - book = activity['inReplyToBook'] - book = book.split('/')[-1] + book_id = activity['inReplyToBook'] + book = get_or_create_book(book_id, key='remote_id') content = activity.get('content') published = activity.get('published') remote_id = activity['id'] @@ -139,10 +99,9 @@ def create_comment_from_activity(author, activity): return comment -def create_comment(user, possible_book, content): +def create_comment(user, book, content): ''' a book comment has been added ''' # throws a value error if the book is not found - book = get_or_create_book(possible_book) content = sanitize(content) return models.Comment.objects.create( @@ -206,34 +165,6 @@ def get_favorite(absolute_id): return get_by_absolute_id(absolute_id, models.Favorite) -def get_by_absolute_id(absolute_id, model): - ''' generalized function to get from a model with a remote_id field ''' - if not absolute_id: - return None - - # check if it's a remote status - try: - return model.objects.get(remote_id=absolute_id) - except model.DoesNotExist: - pass - - # try finding a local status with that id - local_id = absolute_id.split('/')[-1] - try: - if hasattr(model.objects, 'select_subclasses'): - possible_match = model.objects.select_subclasses().get(id=local_id) - else: - possible_match = model.objects.get(id=local_id) - except model.DoesNotExist: - return None - - # make sure it's not actually a remote status with an id that - # clashes with a local id - if possible_match.absolute_id == absolute_id: - return possible_match - return None - - def create_status(user, content, reply_parent=None, mention_books=None, remote_id=None): ''' a status update ''' @@ -260,7 +191,7 @@ def create_status(user, content, reply_parent=None, mention_books=None, def create_tag(user, possible_book, name): ''' add a tag to a book ''' - book = get_or_create_book(possible_book) + book = get_or_create_book(possible_book, key='remote_id') try: tag = models.Tag.objects.create(name=name, book=book, user=user) diff --git a/fedireads/view_actions.py b/fedireads/view_actions.py index 4c7e65b9f..eaa493041 100644 --- a/fedireads/view_actions.py +++ b/fedireads/view_actions.py @@ -189,27 +189,25 @@ def shelve(request): def rate(request): ''' just a star rating for a book ''' form = forms.RatingForm(request.POST) - book_identifier = request.POST.get('book') + book_id = request.POST.get('book') # TODO: better failure behavior if not form.is_valid(): - return redirect('/book/%s' % book_identifier) + return redirect('/book/%s' % book_id) rating = form.cleaned_data.get('rating') # throws a value error if the book is not found - book = get_or_create_book(book_identifier) - outgoing.handle_rate(request.user, book, rating) - return redirect('/book/%s' % book_identifier) + outgoing.handle_rate(request.user, book_id, rating) + return redirect('/book/%s' % book_id) @login_required def review(request): ''' create a book review ''' form = forms.ReviewForm(request.POST) - book_identifier = request.POST.get('book') - # TODO: better failure behavior + book_id = request.POST.get('book') if not form.is_valid(): - return redirect('/book/%s' % book_identifier) + return redirect('/book/%s' % book_id) # TODO: validation, htmlification name = form.cleaned_data.get('name') @@ -220,42 +218,39 @@ def review(request): except ValueError: rating = None - # throws a value error if the book is not found - book = get_or_create_book(book_identifier) - - outgoing.handle_review(request.user, book, name, content, rating) - return redirect('/book/%s' % book_identifier) + outgoing.handle_review(request.user, book_id, name, content, rating) + return redirect('/book/%s' % book_id) @login_required def quotate(request): ''' create a book quotation ''' form = forms.QuotationForm(request.POST) - book_identifier = request.POST.get('book') + book_id = request.POST.get('book') if not form.is_valid(): - return redirect('/book/%s' % book_identifier) + return redirect('/book/%s' % book_id) quote = form.cleaned_data.get('quote') content = form.cleaned_data.get('content') - outgoing.handle_quotation(request.user, book_identifier, content, quote) - return redirect('/book/%s' % book_identifier) + outgoing.handle_quotation(request.user, book_id, content, quote) + return redirect('/book/%s' % book_id) @login_required def comment(request): ''' create a book comment ''' form = forms.CommentForm(request.POST) - book_identifier = request.POST.get('book') + book_id = request.POST.get('book') # TODO: better failure behavior if not form.is_valid(): - return redirect('/book/%s' % book_identifier) + return redirect('/book/%s' % book_id) # TODO: validation, htmlification content = form.data.get('content') - outgoing.handle_comment(request.user, book_identifier, content) - return redirect('/book/%s' % book_identifier) + outgoing.handle_comment(request.user, book_id, content) + return redirect('/book/%s' % book_id) @login_required @@ -264,20 +259,20 @@ def tag(request): # I'm not using a form here because sometimes "name" is sent as a hidden # field which doesn't validate name = request.POST.get('name') - book_identifier = request.POST.get('book') + book_id = request.POST.get('book') - outgoing.handle_tag(request.user, book_identifier, name) - return redirect('/book/%s' % book_identifier) + outgoing.handle_tag(request.user, book_id, name) + return redirect('/book/%s' % book_id) @login_required def untag(request): ''' untag a book ''' name = request.POST.get('name') - book_identifier = request.POST.get('book') + book_id = request.POST.get('book') - outgoing.handle_untag(request.user, book_identifier, name) - return redirect('/book/%s' % book_identifier) + outgoing.handle_untag(request.user, book_id, name) + return redirect('/book/%s' % book_id) @login_required From 3681e7955996d244d7b9aba07d67db69a9f78b18 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 4 May 2020 14:06:53 -0700 Subject: [PATCH 13/28] Fixes recipient list for following --- fedireads/outgoing.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fedireads/outgoing.py b/fedireads/outgoing.py index 4fb307388..0baa5b09a 100644 --- a/fedireads/outgoing.py +++ b/fedireads/outgoing.py @@ -84,7 +84,7 @@ def handle_account_search(query): def handle_follow(user, to_follow): ''' someone local wants to follow someone ''' activity = activitypub.get_follow_request(user, to_follow) - broadcast(user, activity, [to_follow.inbox]) + broadcast(user, activity, direct_recipients=[to_follow]) def handle_unfollow(user, to_unfollow): @@ -94,7 +94,7 @@ def handle_unfollow(user, to_unfollow): user_object=to_unfollow ) activity = activitypub.get_unfollow(relationship) - broadcast(user, activity, [to_unfollow.inbox]) + broadcast(user, activity, direct_recipients=[to_unfollow]) to_unfollow.followers.remove(user) @@ -340,7 +340,7 @@ def handle_unfavorite(user, status): return fav_activity = activitypub.get_unfavorite(favorite) - broadcast(user, fav_activity, [status.user]) + broadcast(user, fav_activity, direct_recipients=[status.user]) def handle_boost(user, status): From 1b91fb375fdf99335bd0a4348b15098b703d2728 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 4 May 2020 14:15:16 -0700 Subject: [PATCH 14/28] Fixes recipient logic --- fedireads/broadcast.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fedireads/broadcast.py b/fedireads/broadcast.py index f1a29b43f..995df2cf5 100644 --- a/fedireads/broadcast.py +++ b/fedireads/broadcast.py @@ -36,7 +36,7 @@ def broadcast(sender, activity, software=None, \ recipients = [u.inbox for u in direct_recipients or []] # TODO: other kinds of privacy if privacy == 'public': - recipients = get_public_recipients(sender, software=software) + recipients += get_public_recipients(sender, software=software) broadcast_task.delay(sender.id, activity, recipients) From 0edb9688cb0b2dee04bf94c080f3f0a2c007a37c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 8 May 2020 16:56:49 -0700 Subject: [PATCH 15/28] Adds create_book functionality for fedireads conn --- fedireads/activitypub/book.py | 12 +++-- fedireads/books_manager.py | 12 +++-- fedireads/connectors/abstract_connector.py | 27 +++++++++++ fedireads/connectors/fedireads_connector.py | 53 +++++++++++++++++++-- fedireads/connectors/openlibrary.py | 20 +------- 5 files changed, 95 insertions(+), 29 deletions(-) diff --git a/fedireads/activitypub/book.py b/fedireads/activitypub/book.py index 98a71f962..d3b7ca8d8 100644 --- a/fedireads/activitypub/book.py +++ b/fedireads/activitypub/book.py @@ -5,6 +5,7 @@ def get_book(book): ''' activitypub serialize a book ''' fields = [ + 'title', 'sort_title', 'subtitle', 'isbn_13', @@ -27,10 +28,11 @@ def get_book(book): 'physical_format', ] + book_type = type(book).__name__ activity = { '@context': 'https://www.w3.org/ns/activitystreams', 'type': 'Document', - 'book_type': type(book).__name__, + 'book_type': book_type, 'name': book.title, 'url': book.absolute_id, @@ -39,9 +41,13 @@ def get_book(book): book.first_published_date else None, 'published_date': book.published_date.isoformat() if \ book.published_date else None, - 'parent_work': book.parent_work.absolute_id if \ - hasattr(book, 'parent_work') else None, } + if book_type == 'Edition': + activity['work'] = book.parent_work.absolute_id + else: + editions = book.edition_set.order_by('default') + activity['editions'] = [get_book(b) for b in editions] + for field in fields: if hasattr(book, field): activity[field] = book.__getattribute__(field) diff --git a/fedireads/books_manager.py b/fedireads/books_manager.py index 209a9d500..e82aaeb61 100644 --- a/fedireads/books_manager.py +++ b/fedireads/books_manager.py @@ -18,11 +18,13 @@ def get_or_create_book(value, key='id', connector_id=None): book = get_by_absolute_id(value, models.Book) if book: return book - connector = get_or_create_connector(value) - return connector.get_or_create_book(value) - connector_info = models.Connector.objects.get(id=connector_id) - connector = load_connector(connector_info) + if connector_id: + connector_info = models.Connector.objects.get(id=connector_id) + connector = load_connector(connector_info) + else: + connector = get_or_create_connector(value) + book = connector.get_or_create_book(value) load_more_data.delay(book.id) return book @@ -33,7 +35,7 @@ def get_or_create_connector(remote_id): url = urlparse(remote_id) identifier = url.netloc if not identifier: - raise(ValueError) + raise ValueError('Invalid remote id') try: connector_info = models.Connector.objects.get(identifier=identifier) diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index 0eb18c2d9..8828f0eaf 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -52,6 +52,33 @@ class AbstractConnector(ABC): return results + def create_book(self, key, data, model): + ''' create a work or edition from data ''' + # we really would rather use an existing book than make a new one + match = match_from_mappings(data, self.key_mappings) + if match: + if not isinstance(match, model): + if type(match).__name__ == 'Edition': + return match.parent_work + else: + return match.default_edition + return match + + kwargs = { + self.key_name: key, + 'title': data['title'], + 'connector': self.connector + } + book = model.objects.create(**kwargs) + return self.update_book_from_data(book, data) + + + def update_book_from_data(self, book, data): + ''' simple function to save data to a book ''' + update_from_mappings(book, data, self.book_mappings) + book.save() + + @abstractmethod def format_search_result(self, search_result): ''' create a SearchResult obj from json ''' diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index 84d804f69..c6abe8f29 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -1,7 +1,9 @@ ''' using another fedireads instance as a source of book data ''' +import requests + from django.core.exceptions import ObjectDoesNotExist from django.core.files.base import ContentFile -import requests +from django.db import transaction from fedireads import models from .abstract_connector import AbstractConnector, SearchResult, get_date @@ -10,6 +12,15 @@ from .abstract_connector import match_from_mappings, update_from_mappings class Connector(AbstractConnector): ''' interact with other instances ''' + def __init__(self, identifier): + self.key_mappings = { + 'isbn_13': ('isbn_13', None), + 'isbn_10': ('isbn_10', None), + 'oclc_numbers': ('oclc_number', None), + 'lccn': ('lccn', None), + } + super().__init__(identifier) + def format_search_result(self, search_result): return SearchResult(**search_result) @@ -26,8 +37,44 @@ class Connector(AbstractConnector): return book # no book was found, so we start creating a new one - book = models.Book(remote_id=remote_id) - self.update_book(book) + response = requests.get( + remote_id, + headers={ + 'Accept': 'application/activity+json; charset=utf-8', + }, + ) + if not response.ok: + response.raise_for_status() + data = response.json() + + if data['book_type'] == 'work': + work_data = data + try: + edition_data = data['editions'][0] + except KeyError: + # hack: re-use the work data as the edition data + edition_data = data + else: + edition_data = data + try: + work_data = data['work'] + except KeyError: + # hack: re-use the work data as the edition data + work_data = data + + with transaction.atomic(): + # create both work and a default edition + work_key = edition_data.get('url') + work = self.create_book(work_key, work_data, models.Work) + + ed_key = edition_data.get('url') + edition = self.create_book(ed_key, edition_data, models.Edition) + edition.default = True + edition.parent_work = work + edition.save() + + print(work, edition) + return edition def update_book(self, book, data=None): diff --git a/fedireads/connectors/openlibrary.py b/fedireads/connectors/openlibrary.py index 5d383ab8f..5d4b272fa 100644 --- a/fedireads/connectors/openlibrary.py +++ b/fedireads/connectors/openlibrary.py @@ -7,7 +7,7 @@ from django.db import transaction from fedireads import models from .abstract_connector import AbstractConnector, SearchResult -from .abstract_connector import match_from_mappings, update_from_mappings +from .abstract_connector import update_from_mappings from .abstract_connector import get_date from .openlibrary_languages import languages @@ -104,26 +104,10 @@ class Connector(AbstractConnector): return edition - def create_book(self, key, data, model): - ''' create a work or edition from data ''' - # we really would rather use an existing book than make a new one - match = match_from_mappings(data, self.key_mappings) - if match: - return match - - book = model.objects.create( - openlibrary_key=key, - title=data['title'], - connector=self.connector, - ) - return self.update_book_from_data(book, data) - - def update_book_from_data(self, book, data): ''' updaet a book model instance from ol data ''' # populate the simple data fields - update_from_mappings(book, data, self.book_mappings) - book.save() + super().update_book_from_data(book, data) authors = self.get_authors_from_data(data) for author in authors: From 9beae9cfcd39bff19a95c4c2e3b10398bbea4233 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 8 May 2020 17:36:02 -0700 Subject: [PATCH 16/28] Include all relevent book data without infinite recusion --- fedireads/activitypub/book.py | 14 ++++++++------ fedireads/connectors/abstract_connector.py | 1 + 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/fedireads/activitypub/book.py b/fedireads/activitypub/book.py index d3b7ca8d8..e3d2cefd8 100644 --- a/fedireads/activitypub/book.py +++ b/fedireads/activitypub/book.py @@ -1,7 +1,7 @@ ''' federate book data ''' from fedireads.settings import DOMAIN -def get_book(book): +def get_book(book, recursive=True): ''' activitypub serialize a book ''' fields = [ @@ -42,11 +42,13 @@ def get_book(book): 'published_date': book.published_date.isoformat() if \ book.published_date else None, } - if book_type == 'Edition': - activity['work'] = book.parent_work.absolute_id - else: - editions = book.edition_set.order_by('default') - activity['editions'] = [get_book(b) for b in editions] + if recursive: + if book_type == 'Edition': + activity['work'] = get_book(book.parent_work, recursive=False) + else: + editions = book.edition_set.order_by('default') + activity['editions'] = [ + get_book(b, recursive=False) for b in editions] for field in fields: if hasattr(book, field): diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index 8828f0eaf..0fff20b53 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -55,6 +55,7 @@ class AbstractConnector(ABC): def create_book(self, key, data, model): ''' create a work or edition from data ''' # we really would rather use an existing book than make a new one + print(data) match = match_from_mappings(data, self.key_mappings) if match: if not isinstance(match, model): From c8bb7176346b7bbcbf34b4e7cdd5f484c052e40f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 8 May 2020 17:41:23 -0700 Subject: [PATCH 17/28] Fixes small logic errors --- fedireads/connectors/abstract_connector.py | 4 +++- fedireads/connectors/fedireads_connector.py | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index 0fff20b53..9e1ac6ca3 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -15,6 +15,8 @@ class AbstractConnector(ABC): info = models.Connector.objects.get(identifier=identifier) self.connector = info + self.book_mappings = {} + self.base_url = info.base_url self.books_url = info.books_url self.covers_url = info.covers_url @@ -55,7 +57,6 @@ class AbstractConnector(ABC): def create_book(self, key, data, model): ''' create a work or edition from data ''' # we really would rather use an existing book than make a new one - print(data) match = match_from_mappings(data, self.key_mappings) if match: if not isinstance(match, model): @@ -78,6 +79,7 @@ class AbstractConnector(ABC): ''' simple function to save data to a book ''' update_from_mappings(book, data, self.book_mappings) book.save() + return book @abstractmethod diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index c6abe8f29..1fa49ebcd 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -73,7 +73,6 @@ class Connector(AbstractConnector): edition.parent_work = work edition.save() - print(work, edition) return edition From bb01834a31970c121e4c248dea10dc74abd12a68 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 8 May 2020 17:56:24 -0700 Subject: [PATCH 18/28] Parser for search results in connectors --- fedireads/connectors/abstract_connector.py | 7 ++++++- fedireads/connectors/fedireads_connector.py | 4 ++++ fedireads/connectors/openlibrary.py | 4 ++++ fedireads/connectors/self_connector.py | 4 ++++ fedireads/views.py | 1 - 5 files changed, 18 insertions(+), 2 deletions(-) diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index 9e1ac6ca3..9b9cc4293 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -49,7 +49,7 @@ class AbstractConnector(ABC): data = resp.json() results = [] - for doc in data['docs'][:10]: + for doc in self.parse_search_data(data)[:10]: results.append(self.format_search_result(doc)) return results @@ -82,6 +82,11 @@ class AbstractConnector(ABC): return book + @abstractmethod + def parse_search_data(self, data): + ''' turn the result json from a search into a list ''' + + @abstractmethod def format_search_result(self, search_result): ''' create a SearchResult obj from json ''' diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index 1fa49ebcd..a156f1e61 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -26,6 +26,10 @@ class Connector(AbstractConnector): return SearchResult(**search_result) + def parse_search_data(self, data): + return data + + def get_or_create_book(self, remote_id): ''' pull up a book record by whatever means possible ''' book = models.Book.objects.select_subclasses().filter( diff --git a/fedireads/connectors/openlibrary.py b/fedireads/connectors/openlibrary.py index 5d4b272fa..94f13f6d4 100644 --- a/fedireads/connectors/openlibrary.py +++ b/fedireads/connectors/openlibrary.py @@ -47,6 +47,10 @@ class Connector(AbstractConnector): ) + def parse_search_data(self, data): + return data.get('docs') + + def get_or_create_book(self, olkey): ''' pull up a book record by whatever means possible. if you give a work key, it should give you the default edition, diff --git a/fedireads/connectors/self_connector.py b/fedireads/connectors/self_connector.py index dfcd36459..8e09e3131 100644 --- a/fedireads/connectors/self_connector.py +++ b/fedireads/connectors/self_connector.py @@ -39,6 +39,10 @@ class Connector(AbstractConnector): return search_results + def parse_search_data(self, data): + return data + + def format_search_result(self, book): return SearchResult( book.title, diff --git a/fedireads/views.py b/fedireads/views.py index 6cfe7c38b..74bd6b8f6 100644 --- a/fedireads/views.py +++ b/fedireads/views.py @@ -143,7 +143,6 @@ def get_activity_feed(user, filter_level, model=models.Status): return activities -@login_required def search(request): ''' that search bar up top ''' query = request.GET.get('q') From 093945e7fb867319aa40e56b085f0d166f21007c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 9 May 2020 12:09:40 -0700 Subject: [PATCH 19/28] Load author data with fedireads connector --- fedireads/activitypub/__init__.py | 2 +- fedireads/activitypub/book.py | 20 ++++++++++++++--- fedireads/books_manager.py | 7 +++++- fedireads/connectors/abstract_connector.py | 21 +++++++++++++++++ fedireads/connectors/fedireads_connector.py | 12 ++++++++++ fedireads/connectors/openlibrary.py | 25 +++++---------------- fedireads/connectors/self_connector.py | 6 +++++ fedireads/urls.py | 2 +- fedireads/views.py | 3 +++ 9 files changed, 73 insertions(+), 25 deletions(-) diff --git a/fedireads/activitypub/__init__.py b/fedireads/activitypub/__init__.py index 4be7ca53f..b75eca7ee 100644 --- a/fedireads/activitypub/__init__.py +++ b/fedireads/activitypub/__init__.py @@ -1,6 +1,6 @@ ''' bring activitypub functions into the namespace ''' from .actor import get_actor -from .book import get_book +from .book import get_book, get_author from .create import get_create, get_update from .follow import get_following, get_followers from .follow import get_follow_request, get_unfollow, get_accept, get_reject diff --git a/fedireads/activitypub/book.py b/fedireads/activitypub/book.py index e3d2cefd8..04e8d1931 100644 --- a/fedireads/activitypub/book.py +++ b/fedireads/activitypub/book.py @@ -36,7 +36,7 @@ def get_book(book, recursive=True): 'name': book.title, 'url': book.absolute_id, - 'authors': [get_author(a) for a in book.authors.all()], + 'authors': [a.absolute_id for a in book.authors.all()], 'first_published_date': book.first_published_date.isoformat() if \ book.first_published_date else None, 'published_date': book.published_date.isoformat() if \ @@ -68,7 +68,21 @@ def get_book(book, recursive=True): def get_author(author): ''' serialize an author ''' - return { - 'name': author.name, + fields = [ + 'name', + 'born', + 'died', + 'aliases', + 'bio' + 'openlibrary_key', + 'wikipedia_link', + ] + activity = { + '@context': 'https://www.w3.org/ns/activitystreams', 'url': author.absolute_id, + 'type': 'Person', } + for field in fields: + if hasattr(author, field): + activity[field] = author.__getattribute__(field) + return activity diff --git a/fedireads/books_manager.py b/fedireads/books_manager.py index e82aaeb61..fceb73aff 100644 --- a/fedireads/books_manager.py +++ b/fedireads/books_manager.py @@ -1,4 +1,6 @@ ''' select and call a connector for whatever book task needs doing ''' +from requests import HTTPError + import importlib from urllib.parse import urlparse @@ -96,7 +98,10 @@ def search(query): dedup_slug = lambda r: '%s/%s/%s' % (r.title, r.author, r.year) result_index = set() for connector in get_connectors(): - result_set = connector.search(query) + try: + result_set = connector.search(query) + except HTTPError: + continue result_set = [r for r in result_set \ if dedup_slug(r) not in result_index] diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index 9b9cc4293..9cdfd86fa 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -79,9 +79,30 @@ class AbstractConnector(ABC): ''' simple function to save data to a book ''' update_from_mappings(book, data, self.book_mappings) book.save() + + authors = self.get_authors_from_data(data) + for author in authors: + book.authors.add(author) + if authors: + book.author_text = ', '.join(a.name for a in authors) + book.save() + + cover = self.get_cover_from_data(data) + if cover: + book.cover.save(*cover, save=True) return book + @abstractmethod + def get_authors_from_data(self, data): + ''' load author data ''' + + + @abstractmethod + def get_cover_from_data(self, data): + ''' load cover ''' + + @abstractmethod def parse_search_data(self, data): ''' turn the result json from a search into a list ''' diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index a156f1e61..d00a20601 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -80,6 +80,18 @@ class Connector(AbstractConnector): return edition + def get_cover_from_data(self, data): + return None + + + def get_authors_from_data(self, data): + authors = [] + + for author_url in data.get('authors', []): + authors.append(self.get_or_create_author(author_url)) + return authors + + def update_book(self, book, data=None): ''' add remote data to a local book ''' if not data: diff --git a/fedireads/connectors/openlibrary.py b/fedireads/connectors/openlibrary.py index 94f13f6d4..cb974b685 100644 --- a/fedireads/connectors/openlibrary.py +++ b/fedireads/connectors/openlibrary.py @@ -108,22 +108,6 @@ class Connector(AbstractConnector): return edition - def update_book_from_data(self, book, data): - ''' updaet a book model instance from ol data ''' - # populate the simple data fields - super().update_book_from_data(book, data) - - authors = self.get_authors_from_data(data) - for author in authors: - book.authors.add(author) - if authors: - book.author_text = ', '.join(a.name for a in authors) - - if data.get('covers'): - book.cover.save(*self.get_cover(data['covers'][0]), save=True) - return book - - def update_book(self, book, data=None): ''' load new data ''' if not book.sync and not book.sync_cover: @@ -133,7 +117,7 @@ class Connector(AbstractConnector): data = self.load_book_data(book.openlibrary_key) if book.sync_cover and data.get('covers'): - book.cover.save(*self.get_cover(data['covers'][0]), save=True) + book.cover.save(*self.get_cover_from_data(data, save=True)) if book.sync: book = self.update_book_from_data(book, data) return book @@ -217,9 +201,12 @@ class Connector(AbstractConnector): return author - def get_cover(self, cover_id): + def get_cover_from_data(self, data): ''' ask openlibrary for the cover ''' - # TODO: get medium and small versions + if not data.get('covers'): + return None + + cover_id = data.get('covers')[0] image_name = '%s-M.jpg' % cover_id url = '%s/b/id/%s' % (self.covers_url, image_name) response = requests.get(url) diff --git a/fedireads/connectors/self_connector.py b/fedireads/connectors/self_connector.py index 8e09e3131..96c0f8c3b 100644 --- a/fedireads/connectors/self_connector.py +++ b/fedireads/connectors/self_connector.py @@ -39,6 +39,12 @@ class Connector(AbstractConnector): return search_results + def get_authors_from_data(self, data): + return None + + def get_cover_from_data(self, data): + return None + def parse_search_data(self, data): return data diff --git a/fedireads/urls.py b/fedireads/urls.py index bd43a72d4..d838fc2d5 100644 --- a/fedireads/urls.py +++ b/fedireads/urls.py @@ -63,7 +63,7 @@ urlpatterns = [ re_path(r'%s/edit/?$' % book_path, views.edit_book_page), re_path(r'^editions/(?P\d+)/?$', views.editions_page), - re_path(r'^author/(?P[\w\-]+)/?$', views.author_page), + re_path(r'^author/(?P[\w\-]+)(.json)?/?$', views.author_page), re_path(r'^tag/(?P.+)/?$', views.tag_page), re_path(r'^shelf/%s/(?P[\w-]+)(.json)?/?$' % username_regex, views.shelf_page), re_path(r'^shelf/%s/(?P[\w-]+)(.json)?/?$' % localname_regex, views.shelf_page), diff --git a/fedireads/views.py b/fedireads/views.py index 74bd6b8f6..2d2ca9ae2 100644 --- a/fedireads/views.py +++ b/fedireads/views.py @@ -507,6 +507,9 @@ def author_page(request, author_id): except ValueError: return HttpResponseNotFound() + if is_api_request(request): + return JsonResponse(activitypub.get_author(author)) + books = models.Work.objects.filter(authors=author) data = { 'author': author, From 4741ada418900bb29099e5ac40b2c2c01a5e736c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 9 May 2020 12:16:59 -0700 Subject: [PATCH 20/28] Cleans up search results page --- fedireads/connectors/abstract_connector.py | 1 + fedireads/templates/book_results.html | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index 9cdfd86fa..27cd87eb9 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -26,6 +26,7 @@ class AbstractConnector(ABC): self.name = info.name self.local = info.local self.id = info.id + self.identifier = info.identifier def is_available(self): diff --git a/fedireads/templates/book_results.html b/fedireads/templates/book_results.html index ed0c9b856..99e079e62 100644 --- a/fedireads/templates/book_results.html +++ b/fedireads/templates/book_results.html @@ -3,10 +3,11 @@

        Search results

        {% for result_set in results %} + {% if result_set.results %}
        {% if not result_set.connector.local %}

        - Results from {{ result_set.connector.name }} + Results from {% if result_set.connector.name %}{{ result_set.connector.name }}{% else %}{{ result_set.connector.identifier }}{% endif %}

        {% endif %} @@ -16,6 +17,7 @@
        {% endfor %} + {% endif %} {% endfor %}
      {% endblock %} From e9393ede28d507beb0ae73cff30546e831f251b0 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 9 May 2020 12:29:23 -0700 Subject: [PATCH 21/28] Adds remote ID to authors --- fedireads/connectors/fedireads_connector.py | 6 +++++- fedireads/migrations/0038_author_remote_id.py | 18 ++++++++++++++++++ fedireads/models/book.py | 1 + 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 fedireads/migrations/0038_author_remote_id.py diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index d00a20601..d5cec38ec 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -1,4 +1,5 @@ ''' using another fedireads instance as a source of book data ''' +import re import requests from django.core.exceptions import ObjectDoesNotExist @@ -32,6 +33,9 @@ class Connector(AbstractConnector): def get_or_create_book(self, remote_id): ''' pull up a book record by whatever means possible ''' + # re-construct a remote id from the int and books_url + if re.match(r'^\d+$', remote_id): + remote_id = self.books_url + '/' + remote_id book = models.Book.objects.select_subclasses().filter( remote_id=remote_id ).first() @@ -146,7 +150,7 @@ class Connector(AbstractConnector): except ObjectDoesNotExist: pass - resp = requests.get('%s/authors/%s.json' % (self.url, remote_id)) + resp = requests.get('%s/authors/%s.json' % (self.base_url, remote_id)) if not resp.ok: resp.raise_for_status() diff --git a/fedireads/migrations/0038_author_remote_id.py b/fedireads/migrations/0038_author_remote_id.py new file mode 100644 index 000000000..7a367c90d --- /dev/null +++ b/fedireads/migrations/0038_author_remote_id.py @@ -0,0 +1,18 @@ +# Generated by Django 3.0.3 on 2020-05-09 19:27 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('fedireads', '0037_auto_20200504_0154'), + ] + + operations = [ + migrations.AddField( + model_name='author', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + ] diff --git a/fedireads/models/book.py b/fedireads/models/book.py index d79449845..a3510397a 100644 --- a/fedireads/models/book.py +++ b/fedireads/models/book.py @@ -150,6 +150,7 @@ class Edition(Book): class Author(FedireadsModel): ''' copy of an author from OL ''' + remote_id = models.CharField(max_length=255, null=True) openlibrary_key = models.CharField(max_length=255, blank=True, null=True) sync = models.BooleanField(default=True) last_sync_date = models.DateTimeField(default=timezone.now) From 3a8d84e9b1e33df2ac083282cb9f2aed29422575 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 9 May 2020 12:33:02 -0700 Subject: [PATCH 22/28] A couple bugs in loading authors --- fedireads/connectors/abstract_connector.py | 2 ++ fedireads/connectors/fedireads_connector.py | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index 27cd87eb9..aa36bc52f 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -198,6 +198,8 @@ def has_attr(obj, key): def get_date(date_string): ''' helper function to try to interpret dates ''' + if not date_string: + return None try: return pytz.utc.localize(parser.parse(date_string)) except ValueError: diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index d5cec38ec..74fe4914c 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -150,7 +150,12 @@ class Connector(AbstractConnector): except ObjectDoesNotExist: pass - resp = requests.get('%s/authors/%s.json' % (self.base_url, remote_id)) + resp = requests.get( + remote_id, + headers={ + 'Accept': 'application/activity+json; charset=utf-8', + }, + ) if not resp.ok: resp.raise_for_status() From 5924e8ed63434f6569285c948478d61f793d4df4 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 9 May 2020 12:39:58 -0700 Subject: [PATCH 23/28] Generalizes http request for json data in connectors --- fedireads/connectors/abstract_connector.py | 14 +++++++++ fedireads/connectors/fedireads_connector.py | 34 +++------------------ fedireads/connectors/openlibrary.py | 23 +++++--------- 3 files changed, 25 insertions(+), 46 deletions(-) diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index aa36bc52f..fd0b184eb 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -206,6 +206,20 @@ def get_date(date_string): return None +def get_data(url): + ''' wrapper for request.get ''' + resp = requests.get( + url, + headers={ + 'Accept': 'application/activity+json; charset=utf-8', + }, + ) + if not resp.ok: + resp.raise_for_status() + data = response.json() + return data + + class SearchResult: ''' standardized search result object ''' def __init__(self, title, key, author, year): diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index 74fe4914c..8308233da 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -7,7 +7,7 @@ from django.core.files.base import ContentFile from django.db import transaction from fedireads import models -from .abstract_connector import AbstractConnector, SearchResult, get_date +from .abstract_connector import AbstractConnector, SearchResult, get_date, get_data from .abstract_connector import match_from_mappings, update_from_mappings @@ -45,15 +45,7 @@ class Connector(AbstractConnector): return book # no book was found, so we start creating a new one - response = requests.get( - remote_id, - headers={ - 'Accept': 'application/activity+json; charset=utf-8', - }, - ) - if not response.ok: - response.raise_for_status() - data = response.json() + data = get_data(remote_id) if data['book_type'] == 'work': work_data = data @@ -99,16 +91,7 @@ class Connector(AbstractConnector): def update_book(self, book, data=None): ''' add remote data to a local book ''' if not data: - response = requests.get( - book.remote_id, - headers={ - 'Accept': 'application/activity+json; charset=utf-8', - }, - ) - if not response.ok: - response.raise_for_status() - - data = response.json() + data = get_data(book.remote_id) match = match_from_mappings(data, {}) if match: @@ -150,16 +133,7 @@ class Connector(AbstractConnector): except ObjectDoesNotExist: pass - resp = requests.get( - remote_id, - headers={ - 'Accept': 'application/activity+json; charset=utf-8', - }, - ) - if not resp.ok: - resp.raise_for_status() - - data = resp.json() + data = get_data(remote_id) # ingest a new author author = models.Author(remote_id=remote_id) diff --git a/fedireads/connectors/openlibrary.py b/fedireads/connectors/openlibrary.py index cb974b685..7d836e7d0 100644 --- a/fedireads/connectors/openlibrary.py +++ b/fedireads/connectors/openlibrary.py @@ -8,7 +8,7 @@ from django.db import transaction from fedireads import models from .abstract_connector import AbstractConnector, SearchResult from .abstract_connector import update_from_mappings -from .abstract_connector import get_date +from .abstract_connector import get_date, get_data from .openlibrary_languages import languages @@ -136,21 +136,14 @@ class Connector(AbstractConnector): def load_book_data(self, olkey): ''' query openlibrary for data on a book ''' - response = requests.get('%s/works/%s.json' % (self.books_url, olkey)) - if not response.ok: - response.raise_for_status() - data = response.json() - return data + url = '%s/works/%s.json' % (self.books_url, olkey) + return get_data(url) def load_edition_data(self, olkey): ''' query openlibrary for editions of a work ''' - response = requests.get( - '%s/works/%s/editions.json' % (self.books_url, olkey)) - if not response.ok: - response.raise_for_status() - data = response.json() - return data + url = '%s/works/%s/editions.json' % (self.books_url, olkey) + return get_data(url) def expand_book_data(self, book): @@ -179,11 +172,9 @@ class Connector(AbstractConnector): except models.Author.DoesNotExist: pass - response = requests.get('%s/authors/%s.json' % (self.base_url, olkey)) - if not response.ok: - response.raise_for_status() + url = '%s/authors/%s.json' % (self.base_url, olkey) + data = get_data(url) - data = response.json() author = models.Author(openlibrary_key=olkey) mappings = { 'birth_date': ('born', get_date), From 8c3e2082823b5278afbf25dd4bb854cedffff886 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 9 May 2020 12:53:55 -0700 Subject: [PATCH 24/28] Load covers in fedireads connector --- fedireads/connectors/abstract_connector.py | 27 +++++++-- fedireads/connectors/fedireads_connector.py | 61 +++++++-------------- fedireads/connectors/openlibrary.py | 15 ----- 3 files changed, 41 insertions(+), 62 deletions(-) diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index fd0b184eb..e0a1a20ba 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -94,6 +94,27 @@ class AbstractConnector(ABC): return book + def update_book(self, book, data=None): + ''' load new data ''' + if not book.sync and not book.sync_cover: + return + + if not data: + key = getattr(book, self.key_name) + data = self.load_book_data(key) + + if book.sync_cover: + book.cover.save(*self.get_cover_from_data(data), save=True) + if book.sync: + book = self.update_book_from_data(book, data) + return book + + + def load_book_data(self, remote_id): + ''' default method for loading book data ''' + return get_data(remote_id) + + @abstractmethod def get_authors_from_data(self, data): ''' load author data ''' @@ -131,12 +152,6 @@ class AbstractConnector(ABC): # return book model obj - @abstractmethod - def update_book(self, book_obj, data=None): - ''' sync a book with the canonical remote copy ''' - # return book model obj - - def update_from_mappings(obj, data, mappings): ''' assign data to model with mappings ''' noop = lambda x: x diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index 8308233da..8f99ee947 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -1,13 +1,15 @@ ''' using another fedireads instance as a source of book data ''' import re -import requests +from uuid import uuid4 from django.core.exceptions import ObjectDoesNotExist from django.core.files.base import ContentFile from django.db import transaction +import requests from fedireads import models -from .abstract_connector import AbstractConnector, SearchResult, get_date, get_data +from .abstract_connector import AbstractConnector, SearchResult +from .abstract_connector import get_date, get_data from .abstract_connector import match_from_mappings, update_from_mappings @@ -20,6 +22,11 @@ class Connector(AbstractConnector): 'oclc_numbers': ('oclc_number', None), 'lccn': ('lccn', None), } + self.book_mappings = self.key_mappings.copy() + self.book_mappings.update({ + 'published_date': ('published_date', get_date), + 'first_published_date': ('first_published_date', get_date), + }) super().__init__(identifier) @@ -77,7 +84,17 @@ class Connector(AbstractConnector): def get_cover_from_data(self, data): - return None + cover_url = data.get('cover') + if not cover_url: + return None + response = requests.get(cover_url) + if not response.ok: + response.raise_for_status() + + image_name = uuid4() + cover_url.split('.')[-1] + image_content = ContentFile(response.content) + return [image_name, image_content] + def get_authors_from_data(self, data): @@ -88,44 +105,6 @@ class Connector(AbstractConnector): return authors - def update_book(self, book, data=None): - ''' add remote data to a local book ''' - if not data: - data = get_data(book.remote_id) - - match = match_from_mappings(data, {}) - if match: - return match - - # great, we can update our book. - mappings = { - 'published_date': ('published_date', get_date), - 'first_published_date': ('first_published_date', get_date), - } - book = update_from_mappings(book, data, mappings) - - if not book.remote_id: - book.remote_id = response.url - if not book.connector: - book.connector = self.connector - book.save() - - if data.get('parent_work'): - work = self.get_or_create_book(data.get('parent_work')) - book.parent_work = work - - for author_blob in data.get('authors', []): - author_blob = author_blob.get('author', author_blob) - author_id = author_blob['key'] - author_id = author_id.split('/')[-1] - book.authors.add(self.get_or_create_author(author_id)) - - if book.sync_cover and data.get('covers') and data['covers']: - book.cover.save(*get_cover(data['covers'][0]), save=True) - - return book - - def get_or_create_author(self, remote_id): ''' load that author ''' try: diff --git a/fedireads/connectors/openlibrary.py b/fedireads/connectors/openlibrary.py index 7d836e7d0..ee73f47d9 100644 --- a/fedireads/connectors/openlibrary.py +++ b/fedireads/connectors/openlibrary.py @@ -108,21 +108,6 @@ class Connector(AbstractConnector): return edition - def update_book(self, book, data=None): - ''' load new data ''' - if not book.sync and not book.sync_cover: - return - - if not data: - data = self.load_book_data(book.openlibrary_key) - - if book.sync_cover and data.get('covers'): - book.cover.save(*self.get_cover_from_data(data, save=True)) - if book.sync: - book = self.update_book_from_data(book, data) - return book - - def get_authors_from_data(self, data): ''' parse author json and load or create authors ''' authors = [] From 277c1a80fd10d870062ec1c56ab6eb3f20409e67 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 9 May 2020 12:59:06 -0700 Subject: [PATCH 25/28] Fixes loading covers --- fedireads/connectors/abstract_connector.py | 2 +- fedireads/connectors/fedireads_connector.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index e0a1a20ba..83729bb3f 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -231,7 +231,7 @@ def get_data(url): ) if not resp.ok: resp.raise_for_status() - data = response.json() + data = resp.json() return data diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index 8f99ee947..7ead431b7 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -84,14 +84,15 @@ class Connector(AbstractConnector): def get_cover_from_data(self, data): - cover_url = data.get('cover') - if not cover_url: + cover_data = data.get('attachment') + if not cover_data: return None + cover_url = cover_data[0].get('url') response = requests.get(cover_url) if not response.ok: response.raise_for_status() - image_name = uuid4() + cover_url.split('.')[-1] + image_name = str(uuid4()) + cover_url.split('.')[-1] image_content = ContentFile(response.content) return [image_name, image_content] From 7220a1784022cbd3fc65baedc4f15d8aa38f8650 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 9 May 2020 13:11:14 -0700 Subject: [PATCH 26/28] Fixes bug that uses edition remote_id for work --- fedireads/books_manager.py | 10 ++++++---- fedireads/connectors/fedireads_connector.py | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/fedireads/books_manager.py b/fedireads/books_manager.py index fceb73aff..d105da75e 100644 --- a/fedireads/books_manager.py +++ b/fedireads/books_manager.py @@ -10,11 +10,13 @@ from fedireads.tasks import app def get_or_create_book(value, key='id', connector_id=None): ''' pull up a book record by whatever means possible ''' - try: - book = models.Book.objects.select_subclasses().get(**{key: value}) + book = models.Book.objects.select_subclasses().filter( + **{key: value} + ).first() + if book: + if not isinstance(book, models.Edition): + return book.default_edition return book - except models.Book.DoesNotExist: - pass if key == 'remote_id': book = get_by_absolute_id(value, models.Book) diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index 7ead431b7..4827e6aac 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -71,7 +71,7 @@ class Connector(AbstractConnector): with transaction.atomic(): # create both work and a default edition - work_key = edition_data.get('url') + work_key = work_data.get('url') work = self.create_book(work_key, work_data, models.Work) ed_key = edition_data.get('url') From 2a98093ebe46fd8ccf3db2189b6f467b1d01c161 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 9 May 2020 13:36:10 -0700 Subject: [PATCH 27/28] Tidy up self connector --- fedireads/connectors/fedireads_connector.py | 5 ++--- fedireads/connectors/self_connector.py | 21 ++++++++++----------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index 4827e6aac..83705e661 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -9,8 +9,7 @@ import requests from fedireads import models from .abstract_connector import AbstractConnector, SearchResult -from .abstract_connector import get_date, get_data -from .abstract_connector import match_from_mappings, update_from_mappings +from .abstract_connector import update_from_mappings, get_date, get_data class Connector(AbstractConnector): @@ -97,7 +96,6 @@ class Connector(AbstractConnector): return [image_name, image_content] - def get_authors_from_data(self, data): authors = [] @@ -128,6 +126,7 @@ class Connector(AbstractConnector): def expand_book_data(self, book): + # TODO pass diff --git a/fedireads/connectors/self_connector.py b/fedireads/connectors/self_connector.py index 96c0f8c3b..c603af92e 100644 --- a/fedireads/connectors/self_connector.py +++ b/fedireads/connectors/self_connector.py @@ -39,16 +39,6 @@ class Connector(AbstractConnector): return search_results - def get_authors_from_data(self, data): - return None - - def get_cover_from_data(self, data): - return None - - def parse_search_data(self, data): - return data - - def format_search_result(self, book): return SearchResult( book.title, @@ -77,9 +67,18 @@ class Connector(AbstractConnector): pass + def parse_search_data(self, data): + ''' it's already in the right format, don't even worry about it ''' + return data + + def get_authors_from_data(self, data): + return None + + def get_cover_from_data(self, data): + return None + def update_book(self, book_obj, data=None): pass - def expand_book_data(self, book): pass From 2ef87c21314f149f57819846540ce177fa573b46 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 10 May 2020 12:56:59 -0700 Subject: [PATCH 28/28] Refactors get_or_create_book --- fedireads/books_manager.py | 6 +- fedireads/connectors/abstract_connector.py | 244 +++++++++++++------- fedireads/connectors/fedireads_connector.py | 90 ++------ fedireads/connectors/openlibrary.py | 152 +++++------- fedireads/connectors/self_connector.py | 22 +- fedireads/templates/book_results.html | 6 +- fedireads/urls.py | 3 +- fedireads/view_actions.py | 7 + fedireads/views.py | 8 - 9 files changed, 258 insertions(+), 280 deletions(-) diff --git a/fedireads/books_manager.py b/fedireads/books_manager.py index d105da75e..2b20fe0c2 100644 --- a/fedireads/books_manager.py +++ b/fedireads/books_manager.py @@ -4,7 +4,7 @@ from requests import HTTPError import importlib from urllib.parse import urlparse -from fedireads import models +from fedireads import models, settings from fedireads.tasks import app @@ -69,6 +69,10 @@ def get_by_absolute_id(absolute_id, model): except model.DoesNotExist: pass + url = urlparse(absolute_id) + if url.netloc != settings.DOMAIN: + return None + # try finding a local status with that id local_id = absolute_id.split('/')[-1] try: diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py index 83729bb3f..1068fdeb3 100644 --- a/fedireads/connectors/abstract_connector.py +++ b/fedireads/connectors/abstract_connector.py @@ -4,6 +4,8 @@ from dateutil import parser import pytz import requests +from django.db import transaction + from fedireads import models @@ -16,18 +18,26 @@ class AbstractConnector(ABC): self.connector = info self.book_mappings = {} + self.key_mappings = { + 'isbn_13': ('isbn_13', None), + 'isbn_10': ('isbn_10', None), + 'oclc_numbers': ('oclc_number', None), + 'lccn': ('lccn', None), + } - self.base_url = info.base_url - self.books_url = info.books_url - self.covers_url = info.covers_url - self.search_url = info.search_url - self.key_name = info.key_name - self.max_query_count = info.max_query_count - self.name = info.name - self.local = info.local - self.id = info.id - self.identifier = info.identifier - + fields = [ + 'base_url', + 'books_url', + 'covers_url', + 'search_url', + 'key_name', + 'max_query_count', + 'name', + 'identifier', + 'local' + ] + for field in fields: + setattr(self, field, getattr(info, field)) def is_available(self): ''' check if you're allowed to use this connector ''' @@ -55,38 +65,94 @@ class AbstractConnector(ABC): return results - def create_book(self, key, data, model): - ''' create a work or edition from data ''' - # we really would rather use an existing book than make a new one - match = match_from_mappings(data, self.key_mappings) - if match: - if not isinstance(match, model): - if type(match).__name__ == 'Edition': - return match.parent_work - else: - return match.default_edition - return match + def get_or_create_book(self, remote_id): + ''' pull up a book record by whatever means possible ''' + # try to load the book + book = models.Book.objects.select_subclasses().filter( + remote_id=remote_id + ).first() + if book: + if isinstance(book, models.Work): + return book.default_edition + return book - kwargs = { - self.key_name: key, - 'title': data['title'], - 'connector': self.connector - } - book = model.objects.create(**kwargs) + # no book was found, so we start creating a new one + data = get_data(remote_id) + + work = None + edition = None + if self.is_work_data(data): + work_data = data + # if we requested a work and there's already an edition, we're set + work = self.match_from_mappings(work_data) + if work and work.default_edition: + return work.default_edition + + # no such luck, we need more information. + try: + edition_data = self.get_edition_from_work_data(work_data) + except KeyError: + # hack: re-use the work data as the edition data + # this is why remote ids aren't necessarily unique + edition_data = data + else: + edition_data = data + edition = self.match_from_mappings(edition_data) + # no need to figure out about the work if we already know about it + if edition and edition.parent_work: + return edition + + # no such luck, we need more information. + try: + work_data = self.get_work_from_edition_date(edition_data) + except KeyError: + # remember this hack: re-use the work data as the edition data + work_data = data + + # at this point, we need to figure out the work, edition, or both + # atomic so that we don't save a work with no edition for vice versa + with transaction.atomic(): + if not work: + work_key = work_data.get('url') + work = self.create_book(work_key, work_data, models.Work) + + if not edition: + ed_key = edition_data.get('url') + edition = self.create_book(ed_key, edition_data, models.Edition) + edition.default = True + edition.parent_work = work + edition.save() + + # now's our change to fill in author gaps + if not edition.authors and work.authors: + edition.authors.set(work.authors.all()) + edition.author_text = work.author_text + edition.save() + + return edition + + + def create_book(self, remote_id, data, model): + ''' create a work or edition from data ''' + book = model.objects.create( + remote_id=remote_id, + title=data['title'], + connector=self.connector, + ) return self.update_book_from_data(book, data) - def update_book_from_data(self, book, data): - ''' simple function to save data to a book ''' - update_from_mappings(book, data, self.book_mappings) + def update_book_from_data(self, book, data, update_cover=True): + ''' for creating a new book or syncing with data ''' + book = update_from_mappings(book, data, self.book_mappings) + + for author in self.get_authors_from_data(data): + book.authors.add(author) + book.author_text = ', '.join(a.name for a in book.authors.all()) book.save() - authors = self.get_authors_from_data(data) - for author in authors: - book.authors.add(author) - if authors: - book.author_text = ', '.join(a.name for a in authors) - book.save() + if not update_cover: + return book cover = self.get_cover_from_data(data) if cover: @@ -103,16 +169,61 @@ class AbstractConnector(ABC): key = getattr(book, self.key_name) data = self.load_book_data(key) - if book.sync_cover: - book.cover.save(*self.get_cover_from_data(data), save=True) if book.sync: - book = self.update_book_from_data(book, data) + book = self.update_book_from_data( + book, data, update_cover=book.sync_cover) + else: + cover = self.get_cover_from_data(data) + if cover: + book.cover.save(*cover, save=True) + return book - def load_book_data(self, remote_id): - ''' default method for loading book data ''' - return get_data(remote_id) + def match_from_mappings(self, data): + ''' try to find existing copies of this book using various keys ''' + keys = [ + ('openlibrary_key', models.Book), + ('librarything_key', models.Book), + ('goodreads_key', models.Book), + ('lccn', models.Work), + ('isbn_10', models.Edition), + ('isbn_13', models.Edition), + ('oclc_number', models.Edition), + ('asin', models.Edition), + ] + noop = lambda x: x + for key, model in keys: + formatter = None + if key in self.key_mappings: + key, formatter = self.key_mappings[key] + if not formatter: + formatter = noop + + value = data.get(key) + if not value: + continue + value = formatter(value) + + match = model.objects.select_subclasses().filter( + **{key: value}).first() + if match: + return match + + + @abstractmethod + def is_work_data(self, data): + ''' differentiate works and editions ''' + + + @abstractmethod + def get_edition_from_work_data(self, data): + ''' every work needs at least one edition ''' + + + @abstractmethod + def get_work_from_edition_date(self, data): + ''' every edition needs a work ''' @abstractmethod @@ -135,23 +246,11 @@ class AbstractConnector(ABC): ''' create a SearchResult obj from json ''' - @abstractmethod - def get_or_create_book(self, book_id): - ''' request and format a book given an identifier ''' - # return book model obj - - @abstractmethod def expand_book_data(self, book): ''' get more info on a book ''' - @abstractmethod - def get_or_create_author(self, book_id): - ''' request and format a book given an identifier ''' - # return book model obj - - def update_from_mappings(obj, data, mappings): ''' assign data to model with mappings ''' noop = lambda x: x @@ -172,37 +271,6 @@ def update_from_mappings(obj, data, mappings): return obj -def match_from_mappings(data, mappings): - ''' try to find existing copies of this book using various keys ''' - keys = [ - ('openlibrary_key', models.Book), - ('librarything_key', models.Book), - ('goodreads_key', models.Book), - ('lccn', models.Work), - ('isbn_10', models.Edition), - ('isbn_13', models.Edition), - ('oclc_number', models.Edition), - ('asin', models.Edition), - ] - noop = lambda x: x - for key, model in keys: - formatter = None - if key in mappings: - key, formatter = mappings[key] - if not formatter: - formatter = noop - - value = data.get(key) - if not value: - continue - value = formatter(value) - - match = model.objects.select_subclasses().filter( - **{key: value}).first() - if match: - return match - - def has_attr(obj, key): ''' helper function to check if a model object has a key ''' try: @@ -226,7 +294,7 @@ def get_data(url): resp = requests.get( url, headers={ - 'Accept': 'application/activity+json; charset=utf-8', + 'Accept': 'application/json; charset=utf-8', }, ) if not resp.ok: @@ -235,7 +303,7 @@ def get_data(url): return data -class SearchResult: +class SearchResult(object): ''' standardized search result object ''' def __init__(self, title, key, author, year): self.title = title diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py index 83705e661..f1a5d68fd 100644 --- a/fedireads/connectors/fedireads_connector.py +++ b/fedireads/connectors/fedireads_connector.py @@ -1,10 +1,8 @@ ''' using another fedireads instance as a source of book data ''' -import re from uuid import uuid4 from django.core.exceptions import ObjectDoesNotExist from django.core.files.base import ContentFile -from django.db import transaction import requests from fedireads import models @@ -15,71 +13,29 @@ from .abstract_connector import update_from_mappings, get_date, get_data class Connector(AbstractConnector): ''' interact with other instances ''' def __init__(self, identifier): - self.key_mappings = { - 'isbn_13': ('isbn_13', None), - 'isbn_10': ('isbn_10', None), - 'oclc_numbers': ('oclc_number', None), - 'lccn': ('lccn', None), - } + super().__init__(identifier) self.book_mappings = self.key_mappings.copy() self.book_mappings.update({ 'published_date': ('published_date', get_date), 'first_published_date': ('first_published_date', get_date), }) - super().__init__(identifier) - def format_search_result(self, search_result): - return SearchResult(**search_result) + def is_work_data(self, data): + return data['book_type'] == 'Work' - def parse_search_data(self, data): - return data + def get_edition_from_work_data(self, data): + return data['editions'][0] - def get_or_create_book(self, remote_id): - ''' pull up a book record by whatever means possible ''' - # re-construct a remote id from the int and books_url - if re.match(r'^\d+$', remote_id): - remote_id = self.books_url + '/' + remote_id - book = models.Book.objects.select_subclasses().filter( - remote_id=remote_id - ).first() - if book: - if isinstance(book, models.Work): - return book.default_edition - return book + def get_work_from_edition_date(self, data): + return data['work'] - # no book was found, so we start creating a new one - data = get_data(remote_id) - if data['book_type'] == 'work': - work_data = data - try: - edition_data = data['editions'][0] - except KeyError: - # hack: re-use the work data as the edition data - edition_data = data - else: - edition_data = data - try: - work_data = data['work'] - except KeyError: - # hack: re-use the work data as the edition data - work_data = data - - with transaction.atomic(): - # create both work and a default edition - work_key = work_data.get('url') - work = self.create_book(work_key, work_data, models.Work) - - ed_key = edition_data.get('url') - edition = self.create_book(ed_key, edition_data, models.Edition) - edition.default = True - edition.parent_work = work - edition.save() - - return edition + def get_authors_from_data(self, data): + for author_url in data.get('authors', []): + yield self.get_or_create_author(author_url) def get_cover_from_data(self, data): @@ -96,14 +52,6 @@ class Connector(AbstractConnector): return [image_name, image_content] - def get_authors_from_data(self, data): - authors = [] - - for author_url in data.get('authors', []): - authors.append(self.get_or_create_author(author_url)) - return authors - - def get_or_create_author(self, remote_id): ''' load that author ''' try: @@ -125,16 +73,14 @@ class Connector(AbstractConnector): return author + def parse_search_data(self, data): + return data + + + def format_search_result(self, search_result): + return SearchResult(**search_result) + + def expand_book_data(self, book): # TODO pass - - -def get_cover(cover_url): - ''' ask openlibrary for the cover ''' - image_name = cover_url.split('/')[-1] - response = requests.get(cover_url) - if not response.ok: - response.raise_for_status() - image_content = ContentFile(response.content) - return [image_name, image_content] diff --git a/fedireads/connectors/openlibrary.py b/fedireads/connectors/openlibrary.py index ee73f47d9..40fe63f85 100644 --- a/fedireads/connectors/openlibrary.py +++ b/fedireads/connectors/openlibrary.py @@ -3,7 +3,6 @@ import re import requests from django.core.files.base import ContentFile -from django.db import transaction from fedireads import models from .abstract_connector import AbstractConnector, SearchResult @@ -15,6 +14,7 @@ from .openlibrary_languages import languages class Connector(AbstractConnector): ''' instantiate a connector for OL ''' def __init__(self, identifier): + super().__init__(identifier) get_first = lambda a: a[0] self.key_mappings = { 'isbn_13': ('isbn_13', get_first), @@ -32,12 +32,63 @@ class Connector(AbstractConnector): 'number_of_pages': ('pages', None), 'series': ('series', get_first), }) - super().__init__(identifier) + + + def is_work_data(self, data): + return not re.match(r'^OL\d+M$', data['key']) + + + def get_edition_from_work_data(self, data): + try: + key = data['key'] + except KeyError: + return False + url = '%s/%s/editions' % (self.books_url, key) + data = get_data(url) + return pick_default_edition(data['entries']) + + + def get_work_from_edition_date(self, data): + try: + key = data['works'][0]['key'] + except (IndexError, KeyError): + return False + url = '%s/%s' % (self.books_url, key) + return get_data(url) + + + def get_authors_from_data(self, data): + ''' parse author json and load or create authors ''' + for author_blob in data.get('authors', []): + author_blob = author_blob.get('author', author_blob) + # this id is "/authors/OL1234567A" and we want just "OL1234567A" + author_id = author_blob['key'].split('/')[-1] + yield self.get_or_create_author(author_id) + + + def get_cover_from_data(self, data): + ''' ask openlibrary for the cover ''' + if not data.get('covers'): + return None + + cover_id = data.get('covers')[0] + image_name = '%s-M.jpg' % cover_id + url = '%s/b/id/%s' % (self.covers_url, image_name) + response = requests.get(url) + if not response.ok: + response.raise_for_status() + image_content = ContentFile(response.content) + return [image_name, image_content] + + + def parse_search_data(self, data): + return data.get('docs') def format_search_result(self, doc): key = doc['key'] - key = key.split('/')[-1] + # build the absolute id from the openlibrary key + key = self.books_url + key author = doc.get('author_name') or ['Unknown'] return SearchResult( doc.get('title'), @@ -47,84 +98,6 @@ class Connector(AbstractConnector): ) - def parse_search_data(self, data): - return data.get('docs') - - - def get_or_create_book(self, olkey): - ''' pull up a book record by whatever means possible. - if you give a work key, it should give you the default edition, - annotated with work data. ''' - - book = models.Book.objects.select_subclasses().filter( - openlibrary_key=olkey - ).first() - if book: - if isinstance(book, models.Work): - return book.default_edition - return book - - # no book was found, so we start creating a new one - if re.match(r'^OL\d+W$', olkey): - with transaction.atomic(): - # create both work and a default edition - work_data = self.load_book_data(olkey) - work = self.create_book(olkey, work_data, models.Work) - - edition_options = self.load_edition_data(olkey).get('entries') - edition_data = pick_default_edition(edition_options) - if not edition_data: - # hack: re-use the work data as the edition data - edition_data = work_data - key = edition_data.get('key').split('/')[-1] - edition = self.create_book(key, edition_data, models.Edition) - edition.default = True - edition.parent_work = work - edition.save() - else: - with transaction.atomic(): - edition_data = self.load_book_data(olkey) - edition = self.create_book(olkey, edition_data, models.Edition) - - work_data = edition_data.get('works') - if not work_data: - # hack: we're re-using the edition data as the work data - work_key = olkey - else: - work_key = work_data[0]['key'].split('/')[-1] - - work = models.Work.objects.filter( - openlibrary_key=work_key - ).first() - if not work: - work_data = self.load_book_data(work_key) - work = self.create_book(work_key, work_data, models.Work) - edition.parent_work = work - edition.save() - if not edition.authors and work.authors: - edition.authors.set(work.authors.all()) - edition.author_text = ', '.join(a.name for a in edition.authors) - - return edition - - - def get_authors_from_data(self, data): - ''' parse author json and load or create authors ''' - authors = [] - for author_blob in data.get('authors', []): - # this id is "/authors/OL1234567A" and we want just "OL1234567A" - author_blob = author_blob.get('author', author_blob) - author_id = author_blob['key'].split('/')[-1] - authors.append(self.get_or_create_author(author_id)) - return authors - - - def load_book_data(self, olkey): - ''' query openlibrary for data on a book ''' - url = '%s/works/%s.json' % (self.books_url, olkey) - return get_data(url) - - def load_edition_data(self, olkey): ''' query openlibrary for editions of a work ''' url = '%s/works/%s/editions.json' % (self.books_url, olkey) @@ -167,8 +140,8 @@ class Connector(AbstractConnector): 'bio': ('bio', get_description), } author = update_from_mappings(author, data, mappings) - # TODO this is making some BOLD assumption name = data.get('name') + # TODO this is making some BOLD assumption if name: author.last_name = name.split(' ')[-1] author.first_name = ' '.join(name.split(' ')[:-1]) @@ -177,21 +150,6 @@ class Connector(AbstractConnector): return author - def get_cover_from_data(self, data): - ''' ask openlibrary for the cover ''' - if not data.get('covers'): - return None - - cover_id = data.get('covers')[0] - image_name = '%s-M.jpg' % cover_id - url = '%s/b/id/%s' % (self.covers_url, image_name) - response = requests.get(url) - if not response.ok: - response.raise_for_status() - image_content = ContentFile(response.content) - return [image_name, image_content] - - def get_description(description_blob): ''' descriptions can be a string or a dict ''' if isinstance(description_blob, dict): diff --git a/fedireads/connectors/self_connector.py b/fedireads/connectors/self_connector.py index c603af92e..cc68446f9 100644 --- a/fedireads/connectors/self_connector.py +++ b/fedireads/connectors/self_connector.py @@ -42,7 +42,7 @@ class Connector(AbstractConnector): def format_search_result(self, book): return SearchResult( book.title, - book.id, + book.absolute_id, book.author_text, book.published_date.year if book.published_date else None, ) @@ -59,17 +59,14 @@ class Connector(AbstractConnector): return None - def get_or_create_author(self, author_id): - ''' load that author ''' - try: - return models.Author.objects.get(id=author_id) - except ObjectDoesNotExist: - pass + def is_work_data(self, data): + pass + def get_edition_from_work_data(self, data): + pass - def parse_search_data(self, data): - ''' it's already in the right format, don't even worry about it ''' - return data + def get_work_from_edition_date(self, data): + pass def get_authors_from_data(self, data): return None @@ -77,8 +74,9 @@ class Connector(AbstractConnector): def get_cover_from_data(self, data): return None - def update_book(self, book_obj, data=None): - pass + def parse_search_data(self, data): + ''' it's already in the right format, don't even worry about it ''' + return data def expand_book_data(self, book): pass diff --git a/fedireads/templates/book_results.html b/fedireads/templates/book_results.html index 99e079e62..71d3a637c 100644 --- a/fedireads/templates/book_results.html +++ b/fedireads/templates/book_results.html @@ -13,7 +13,11 @@ {% for result in result_set.results %}
      - {{ result.title }} by {{ result.author }} ({{ result.year }}) +
      + {% csrf_token %} + + +
      {% endfor %} diff --git a/fedireads/urls.py b/fedireads/urls.py index d838fc2d5..5a43b2653 100644 --- a/fedireads/urls.py +++ b/fedireads/urls.py @@ -58,7 +58,7 @@ urlpatterns = [ re_path(r'%s/replies(.json)?/?$' % status_path, views.replies_page), # books - re_path(r'book/(?P[\w_:\d]+)(.json)?/?$', views.book_page), + re_path(r'%s(.json)?/?$' % book_path, views.book_page), re_path(r'%s/(?Pfriends|local|federated)?$' % book_path, views.book_page), re_path(r'%s/edit/?$' % book_path, views.edit_book_page), re_path(r'^editions/(?P\d+)/?$', views.editions_page), @@ -77,6 +77,7 @@ urlpatterns = [ re_path(r'^edit_profile/?$', actions.edit_profile), re_path(r'^import_data/?', actions.import_data), + re_path(r'^resolve_book/?', actions.resolve_book), re_path(r'^edit_book/(?P\d+)/?', actions.edit_book), re_path(r'^upload_cover/(?P\d+)/?', actions.upload_cover), diff --git a/fedireads/view_actions.py b/fedireads/view_actions.py index eaa493041..85cf40afd 100644 --- a/fedireads/view_actions.py +++ b/fedireads/view_actions.py @@ -115,6 +115,13 @@ def edit_profile(request): return redirect('/user/%s' % request.user.localname) +def resolve_book(request): + ''' figure out the local path to a book from a remote_id ''' + remote_id = request.POST.get('remote_id') + book = get_or_create_book(remote_id, key='remote_id') + return redirect('/book/%d' % book.id) + + @login_required def edit_book(request, book_id): ''' edit a book cool ''' diff --git a/fedireads/views.py b/fedireads/views.py index 2d2ca9ae2..828f2cffa 100644 --- a/fedireads/views.py +++ b/fedireads/views.py @@ -390,14 +390,6 @@ def edit_profile_page(request): def book_page(request, book_id, tab='friends'): ''' info about a book ''' - if ':' in book_id: - try: - connector_id, key, book_id = book_id.split(':') - except ValueError: - return HttpResponseNotFound() - book = get_or_create_book(book_id, key=key, connector_id=connector_id) - return redirect('/book/%d' % book.id) - book = get_or_create_book(book_id) if is_api_request(request): return JsonResponse(activitypub.get_book(book))