From b8b491bbf2d488655f83263b7b3bb4096a72180a Mon Sep 17 00:00:00 2001 From: Joachim Date: Wed, 4 Aug 2021 16:55:12 +0200 Subject: [PATCH 01/11] Add get_absolute_url to ImageField --- bookwyrm/models/fields.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 3b369e84..e57374d5 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -3,6 +3,7 @@ from dataclasses import MISSING import imghdr import re from uuid import uuid4 +from urllib.parse import urljoin import dateutil.parser from dateutil.parser import ParserError @@ -13,11 +14,12 @@ from django.db import models from django.forms import ClearableFileInput, ImageField as DjangoImageField from django.utils import timezone from django.utils.translation import gettext_lazy as _ +from django.utils.encoding import filepath_to_uri from bookwyrm import activitypub from bookwyrm.connectors import get_image from bookwyrm.sanitize_html import InputHtmlParser -from bookwyrm.settings import DOMAIN +from bookwyrm.settings import MEDIA_FULL_URL def validate_remote_id(value): @@ -355,8 +357,6 @@ def image_serializer(value, alt): url = value.url else: return None - if not url[:4] == "http": - url = "https://{:s}{:s}".format(DOMAIN, url) return activitypub.Document(url=url, name=alt) @@ -423,6 +423,19 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): } ) + def get_absolute_url(self, instance): + """returns an absolute URL for the image""" + value = getattr(instance, self.name) + if value is None: + return + + url = filepath_to_uri(value) + if url is not None: + url = url.lstrip('/') + url = urljoin(MEDIA_FULL_URL, url) + + return url + class DateTimeField(ActivitypubFieldMixin, models.DateTimeField): """activitypub-aware datetime field""" From 7a716db48aad0093b1429bd66f360fb22ff258c4 Mon Sep 17 00:00:00 2001 From: Joachim Date: Wed, 4 Aug 2021 16:56:07 +0200 Subject: [PATCH 02/11] lint --- bookwyrm/models/fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index e57374d5..3baf3734 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -431,7 +431,7 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): url = filepath_to_uri(value) if url is not None: - url = url.lstrip('/') + url = url.lstrip("/") url = urljoin(MEDIA_FULL_URL, url) return url From 60e805ac2b1faaeb977d03397880d3939dc17674 Mon Sep 17 00:00:00 2001 From: Joachim Date: Wed, 4 Aug 2021 17:39:46 +0200 Subject: [PATCH 03/11] Fix tests --- bookwyrm/models/fields.py | 8 ++++---- bookwyrm/tests/models/test_fields.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 3baf3734..1a8c082a 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -423,13 +423,13 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): } ) - def get_absolute_url(self, instance): + def get_absolute_url(self, value): """returns an absolute URL for the image""" - value = getattr(instance, self.name) - if value is None: + name = getattr(value, self.name) + if name is None: return - url = filepath_to_uri(value) + url = filepath_to_uri(name) if url is not None: url = url.lstrip("/") url = urljoin(MEDIA_FULL_URL, url) diff --git a/bookwyrm/tests/models/test_fields.py b/bookwyrm/tests/models/test_fields.py index c234ffd0..083e8a1b 100644 --- a/bookwyrm/tests/models/test_fields.py +++ b/bookwyrm/tests/models/test_fields.py @@ -429,7 +429,7 @@ class ActivitypubFields(TestCase): def test_image_serialize(self): """make sure we're creating sensible image paths""" ValueMock = namedtuple("ValueMock", ("url")) - value_mock = ValueMock("/images/fish.jpg") + value_mock = ValueMock("https://your.domain.here/images/fish.jpg") result = fields.image_serializer(value_mock, "hello") self.assertEqual(result.type, "Document") self.assertEqual(result.url, "https://your.domain.here/images/fish.jpg") From bc7710a4a71e115d9eb6b7f88793acdf66f09d9c Mon Sep 17 00:00:00 2001 From: Joachim Date: Wed, 4 Aug 2021 18:18:18 +0200 Subject: [PATCH 04/11] Update Status Model Test --- bookwyrm/models/fields.py | 2 +- bookwyrm/tests/models/test_status_model.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 1a8c082a..0e2a42e5 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -427,7 +427,7 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): """returns an absolute URL for the image""" name = getattr(value, self.name) if name is None: - return + return None url = filepath_to_uri(name) if url is not None: diff --git a/bookwyrm/tests/models/test_status_model.py b/bookwyrm/tests/models/test_status_model.py index 355caab9..a01d8679 100644 --- a/bookwyrm/tests/models/test_status_model.py +++ b/bookwyrm/tests/models/test_status_model.py @@ -177,7 +177,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - "https://%s%s" % (settings.DOMAIN, self.book.cover.url), + "https://%s%s" % (settings.MEDIA_FULL_URL, self.book.cover.url), ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -208,7 +208,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - "https://%s%s" % (settings.DOMAIN, self.book.cover.url), + "https://%s%s" % (settings.MEDIA_FULL_URL, self.book.cover.url), ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -246,7 +246,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - "https://%s%s" % (settings.DOMAIN, self.book.cover.url), + "https://%s%s" % (settings.MEDIA_FULL_URL, self.book.cover.url), ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -287,7 +287,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - "https://%s%s" % (settings.DOMAIN, self.book.cover.url), + "https://%s%s" % (settings.MEDIA_FULL_URL, self.book.cover.url), ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -309,7 +309,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - "https://%s%s" % (settings.DOMAIN, self.book.cover.url), + "https://%s%s" % (settings.MEDIA_FULL_URL, self.book.cover.url), ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -331,7 +331,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - "https://%s%s" % (settings.DOMAIN, self.book.cover.url), + "https://%s%s" % (settings.MEDIA_FULL_URL, self.book.cover.url), ) self.assertEqual(activity["attachment"][0].name, "Test Edition") From c6f8236b07806edc596713cd82dfb172f6aea6a6 Mon Sep 17 00:00:00 2001 From: Joachim Date: Wed, 4 Aug 2021 19:11:57 +0200 Subject: [PATCH 05/11] Fix tests --- bookwyrm/models/fields.py | 5 +++-- bookwyrm/tests/models/test_fields.py | 14 ++++++-------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 0e2a42e5..2069ed1b 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -387,7 +387,8 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): activity[key] = formatted def field_to_activity(self, value, alt=None): - return image_serializer(value, alt) + url = self.get_absolute_url(value) + return activitypub.Document(url=url, name=alt) def field_from_activity(self, value): image_slug = value @@ -425,7 +426,7 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): def get_absolute_url(self, value): """returns an absolute URL for the image""" - name = getattr(value, self.name) + name = getattr(value, "name") if name is None: return None diff --git a/bookwyrm/tests/models/test_fields.py b/bookwyrm/tests/models/test_fields.py index 083e8a1b..d9c470e0 100644 --- a/bookwyrm/tests/models/test_fields.py +++ b/bookwyrm/tests/models/test_fields.py @@ -22,6 +22,7 @@ from bookwyrm.activitypub.base_activity import ActivityObject from bookwyrm.models import fields, User, Status from bookwyrm.models.base_model import BookWyrmModel from bookwyrm.models.activitypub_mixin import ActivitypubMixin +from bookwyrm.settings import DOMAIN # pylint: disable=too-many-public-methods class ActivitypubFields(TestCase): @@ -401,21 +402,18 @@ class ActivitypubFields(TestCase): image.save(output, format=image.format) user.avatar.save("test.jpg", ContentFile(output.getvalue())) - output = fields.image_serializer(user.avatar, alt="alt text") + instance = fields.ImageField() + + output = instance.field_to_activity(user.avatar) self.assertIsNotNone( re.match( - r".*\.jpg", + fr"https:\/\/{DOMAIN}\/.*\.jpg", output.url, ) ) - self.assertEqual(output.name, "alt text") + self.assertEqual(output.name, "") self.assertEqual(output.type, "Document") - instance = fields.ImageField() - - output = fields.image_serializer(user.avatar, alt=None) - self.assertEqual(instance.field_to_activity(user.avatar), output) - responses.add( responses.GET, "http://www.example.com/image.jpg", From ee39e8c036299cc3670f154324192b3131586880 Mon Sep 17 00:00:00 2001 From: Joachim Date: Wed, 4 Aug 2021 19:16:24 +0200 Subject: [PATCH 06/11] Fix R0201: Method could be a function (no-self-use) --- bookwyrm/models/fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 2069ed1b..cc9e0ec4 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -424,7 +424,7 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): } ) - def get_absolute_url(self, value): + def get_absolute_url(value): """returns an absolute URL for the image""" name = getattr(value, "name") if name is None: From 0db3512eb3deca2f4be422d4274412d1794b35b3 Mon Sep 17 00:00:00 2001 From: Joachim Date: Wed, 4 Aug 2021 19:21:56 +0200 Subject: [PATCH 07/11] Revert previous commit --- bookwyrm/models/fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index cc9e0ec4..2069ed1b 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -424,7 +424,7 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): } ) - def get_absolute_url(value): + def get_absolute_url(self, value): """returns an absolute URL for the image""" name = getattr(value, "name") if name is None: From c1673ef7174381ddfdccb1e8d33bd07db9ca5c4e Mon Sep 17 00:00:00 2001 From: Joachim Date: Wed, 4 Aug 2021 19:25:19 +0200 Subject: [PATCH 08/11] Update fields.py --- bookwyrm/models/fields.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 2069ed1b..57b364c9 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -424,6 +424,7 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): } ) + # pylint: disable=no-self-use def get_absolute_url(self, value): """returns an absolute URL for the image""" name = getattr(value, "name") From 35bd4a4071a13047c2c52ef51ef42616b48447d1 Mon Sep 17 00:00:00 2001 From: Joachim Date: Wed, 27 Oct 2021 18:13:47 +0200 Subject: [PATCH 09/11] Apply review suggestion --- bookwyrm/models/fields.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 7d4981e3..489ed061 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -425,6 +425,10 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): def field_to_activity(self, value, alt=None): url = self.get_absolute_url(value) + + if not url: + return None + return activitypub.Document(url=url, name=alt) def field_from_activity(self, value): @@ -465,7 +469,7 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): def get_absolute_url(self, value): """returns an absolute URL for the image""" name = getattr(value, "name") - if name is None: + if not name: return None url = filepath_to_uri(name) From b956b79bd03dc844ac7d94755edd7e0e8ee9ae90 Mon Sep 17 00:00:00 2001 From: Joachim Date: Wed, 27 Oct 2021 18:56:37 +0200 Subject: [PATCH 10/11] Add full URL generation to image_serializer --- bookwyrm/models/fields.py | 4 ++++ bookwyrm/tests/models/test_status_model.py | 13 +++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 489ed061..a490c133 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -389,6 +389,10 @@ def image_serializer(value, alt): url = value.url else: return None + if url is not None: + url = url.lstrip("/") + url = urljoin(MEDIA_FULL_URL, url) + return activitypub.Document(url=url, name=alt) diff --git a/bookwyrm/tests/models/test_status_model.py b/bookwyrm/tests/models/test_status_model.py index cbec151b..a46abe3b 100644 --- a/bookwyrm/tests/models/test_status_model.py +++ b/bookwyrm/tests/models/test_status_model.py @@ -2,6 +2,7 @@ from unittest.mock import patch from io import BytesIO import pathlib +from urllib.parse import urljoin from django.http import Http404 from django.core.files.base import ContentFile @@ -192,7 +193,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - f"https://{settings.MEDIA_FULL_URL}{self.book.cover.url}", + urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")), ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -222,7 +223,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - f"https://{settings.MEDIA_FULL_URL}{self.book.cover.url}", + urljoin(settings.MEDIA_FULL_URL,self.book.cover.url.lstrip("/")), ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -259,7 +260,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - "https://{settings.MEDIA_FULL_URL}{self.book.cover.url}", + urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")), ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -300,7 +301,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - f"https://{settings.MEDIA_FULL_URL}{self.book.cover.url}", + urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")) ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -322,7 +323,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - f"https://{settings.MEDIA_FULL_URL}{self.book.cover.url}", + urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")), ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -343,7 +344,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - f"https://{settings.MEDIA_FULL_URL}{self.book.cover.url}", + urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")), ) self.assertEqual(activity["attachment"][0].name, "Test Edition") From 56fd147c883a6f2d5823edd5dc8a18ecc8e87145 Mon Sep 17 00:00:00 2001 From: Joachim Date: Wed, 27 Oct 2021 19:00:09 +0200 Subject: [PATCH 11/11] Update test_status_model.py --- bookwyrm/tests/models/test_status_model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/tests/models/test_status_model.py b/bookwyrm/tests/models/test_status_model.py index a46abe3b..f5013422 100644 --- a/bookwyrm/tests/models/test_status_model.py +++ b/bookwyrm/tests/models/test_status_model.py @@ -223,7 +223,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - urljoin(settings.MEDIA_FULL_URL,self.book.cover.url.lstrip("/")), + urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")), ) self.assertEqual(activity["attachment"][0].name, "Test Edition") @@ -301,7 +301,7 @@ class Status(TestCase): self.assertEqual(activity["attachment"][0].type, "Document") self.assertEqual( activity["attachment"][0].url, - urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")) + urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")), ) self.assertEqual(activity["attachment"][0].name, "Test Edition")