Merge pull request #1247 from joachimesque/image-absolute-url-getter

Add get_absolute_url to ImageField

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):
@@ -387,8 +389,10 @@ def image_serializer(value, alt):
         url = value.url
     else:
         return None
-    if not url[:4] == "http":
+    if url is not None:
-        url = f"https://{DOMAIN}{url}"
+        url = url.lstrip("/")
+    url = urljoin(MEDIA_FULL_URL, url)
+
     return activitypub.Document(url=url, name=alt)
 
 
@@ -424,7 +428,12 @@ 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)
+
+        if not url:
+            return None
+
+        return activitypub.Document(url=url, name=alt)
 
     def field_from_activity(self, value):
         image_slug = value
@@ -460,6 +469,20 @@ 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")
+        if not name:
+            return None
+
+        url = filepath_to_uri(name)
+        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"""

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
 @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay")
@@ -424,21 +425,18 @@ class ModelFields(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",
@@ -452,7 +450,7 @@ class ModelFields(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")

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.DOMAIN}{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.DOMAIN}{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,
-            f"https://{settings.DOMAIN}{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.DOMAIN}{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.DOMAIN}{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.DOMAIN}{self.book.cover.url}",
+            urljoin(settings.MEDIA_FULL_URL, self.book.cover.url.lstrip("/")),
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")