From 19e940ded5dcd269e05ed17004016f5bb206298f Mon Sep 17 00:00:00 2001 From: Kalob Taulien Date: Mon, 23 Mar 2020 14:16:32 -0600 Subject: [PATCH] Use absolute URL for images --- CHANGELOG.txt | 1 + tests/settings.py | 2 +- tests/tests/test_api.py | 27 ++++++++++++++++++++++++--- wagtail_transfer/field_adapters.py | 15 ++++++++++++--- wagtail_transfer/files.py | 2 +- 5 files changed, 39 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 0c45bf4..f8e4c99 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -5,6 +5,7 @@ Changelog ~~~~~~~~~~~~~~~~~~ * Fix error when exporting empty non-required ChooserBlocks (Kalob Taulinen, Jacob Topp-Mugglestone) + * Ensure that file URLs are always absolute by falling back on BASE_URL when MEDIA_URL is local (Kalob Taulinen) 0.2 (17.03.2020) diff --git a/tests/settings.py b/tests/settings.py index d590c3c..378f28f 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -114,7 +114,7 @@ STATIC_URL = '/static/' MEDIA_ROOT = os.path.join(BASE_DIR, 'test-media') -MEDIA_URL = 'http://example.com/media/' +MEDIA_URL = 'http://media.example.com/media/' SECRET_KEY = 'not needed' diff --git a/tests/tests/test_api.py b/tests/tests/test_api.py index 505c8fe..16ec983 100644 --- a/tests/tests/test_api.py +++ b/tests/tests/test_api.py @@ -8,7 +8,7 @@ from django.core.files import File from django.core.files.images import ImageFile from django.contrib.contenttypes.models import ContentType -from django.test import TestCase +from django.test import TestCase, override_settings from wagtail.core.models import Page, Collection from wagtail.images.models import Image from wagtail.documents.models import Document @@ -364,6 +364,27 @@ def test_image(self): self.assertEqual(response.status_code, 200) data = json.loads(response.content) + self.assertEqual(len(data['objects']), 1) + obj = data['objects'][0] + self.assertEqual(obj['fields']['file']['download_url'], 'http://media.example.com/media/original_images/wagtail.jpg') + self.assertEqual(obj['fields']['file']['size'], 1160) + self.assertEqual(obj['fields']['file']['hash'], '45c5db99aea04378498883b008ee07528f5ae416') + + @override_settings(MEDIA_URL='/media/') + def test_image_with_local_media_url(self): + """File URLs should use BASE_URL to form an absolute URL if MEDIA_URL is relative""" + with open(os.path.join(FIXTURES_DIR, 'wagtail.jpg'), 'rb') as f: + image = Image.objects.create( + title="Wagtail", + file=ImageFile(f, name='wagtail.jpg') + ) + + response = self.get({ + 'wagtailimages.image': [image.pk] + }) + self.assertEqual(response.status_code, 200) + data = json.loads(response.content) + self.assertEqual(len(data['objects']), 1) obj = data['objects'][0] self.assertEqual(obj['fields']['file']['download_url'], 'http://example.com/media/original_images/wagtail.jpg') @@ -385,7 +406,7 @@ def test_document(self): self.assertEqual(len(data['objects']), 1) obj = data['objects'][0] - self.assertEqual(obj['fields']['file']['download_url'], 'http://example.com/media/documents/document.txt') + self.assertEqual(obj['fields']['file']['download_url'], 'http://media.example.com/media/documents/document.txt') self.assertEqual(obj['fields']['file']['size'], 33) self.assertEqual(obj['fields']['file']['hash'], '9b90daf19b6e1e8a4852c64f9ea7fec5bcc5f7fb') @@ -403,7 +424,7 @@ def test_custom_model_with_file_field(self): self.assertEqual(len(data['objects']), 1) obj = data['objects'][0] - self.assertEqual(obj['fields']['image']['download_url'], 'http://example.com/media/avatars/wagtail.jpg') + self.assertEqual(obj['fields']['image']['download_url'], 'http://media.example.com/media/avatars/wagtail.jpg') self.assertEqual(obj['fields']['image']['size'], 1160) self.assertEqual(obj['fields']['image']['hash'], '45c5db99aea04378498883b008ee07528f5ae416') diff --git a/wagtail_transfer/field_adapters.py b/wagtail_transfer/field_adapters.py index 43f658b..9912b9a 100644 --- a/wagtail_transfer/field_adapters.py +++ b/wagtail_transfer/field_adapters.py @@ -3,6 +3,7 @@ import pathlib from urllib.parse import urlparse +from django.conf import settings from django.contrib.contenttypes.fields import GenericRelation from django.db import models from django.db.models.fields.reverse_related import ManyToOneRel @@ -10,7 +11,7 @@ from taggit.managers import TaggableManager from wagtail.core.fields import RichTextField, StreamField -from .files import get_file_size, get_file_hash, File +from .files import get_file_size, get_file_hash, File, FileTransferError from .models import get_base_model from .richtext import get_reference_handler from .streamfield import get_object_references, update_object_ids @@ -161,8 +162,13 @@ def update_object_references(self, value, destination_ids_by_source): class FileAdapter(FieldAdapter): def serialize(self, instance): + url = self.field.value_from_object(instance).url + if settings.MEDIA_URL.startswith('/'): + # Using a relative media url. ie. /media/ + # Prepend the BASE_URL to turn this into an absolute URL + url = settings.BASE_URL.rstrip('/') + url return { - 'download_url': self.field.value_from_object(instance).url, + 'download_url': url, 'size': get_file_size(self.field, instance), 'hash': get_file_hash(self.field, instance), } @@ -184,7 +190,10 @@ def populate_field(self, instance, value, context): local_filename = self.field.generate_filename(instance, name) _file = File(local_filename, value['size'], value['hash'], value['download_url']) - imported_file = _file.transfer() + try: + imported_file = _file.transfer() + except FileTransferError: + return None context.imported_files_by_source_url[_file.source_url] = imported_file value = imported_file.file.name diff --git a/wagtail_transfer/files.py b/wagtail_transfer/files.py index ea3a68f..e7749b5 100644 --- a/wagtail_transfer/files.py +++ b/wagtail_transfer/files.py @@ -107,7 +107,7 @@ def transfer(self): response = requests.get(self.source_url) if response.status_code != 200: - raise FileTransferError # TODO + raise FileTransferError("Non-200 response from image URL") return ImportedFile.objects.create( file=ContentFile(response.content, name=self.local_filename),