diff --git a/.travis.yml b/.travis.yml index 53ee59973..3248a9caf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,9 +3,6 @@ jobs: - stage: test language: python python: '2.7' - before_install: - - sudo apt-get update -qq - - sudo apt-get install -qq libexempi3 install: - pip install codecov - python setup.py install @@ -22,8 +19,6 @@ jobs: language: python python: '2.7' before_install: - - sudo apt-get update -qq - - sudo apt-get install -qq libexempi3 - nvm install 6.9.5 - nvm use 6.9.5 install: diff --git a/dispatch/api/serializers.py b/dispatch/api/serializers.py index 352e5e55a..7a34706c0 100644 --- a/dispatch/api/serializers.py +++ b/dispatch/api/serializers.py @@ -213,7 +213,6 @@ class ImageSerializer(serializers.HyperlinkedModelSerializer): filename = serializers.CharField(source='get_filename', read_only=True) title = serializers.CharField(required=False, allow_null=True, allow_blank=True) - caption = serializers.CharField(required=False, allow_null=True, allow_blank=True) url = serializers.CharField(source='get_absolute_url', read_only=True) url_medium = serializers.CharField(source='get_medium_url', read_only=True) @@ -228,7 +227,7 @@ class ImageSerializer(serializers.HyperlinkedModelSerializer): tags = TagSerializer(many=True, read_only=True) tag_ids = serializers.ListField( write_only=True, - required=True, + required=False, child=serializers.IntegerField()) width = serializers.IntegerField(read_only=True) @@ -241,7 +240,6 @@ class Meta: 'img', 'filename', 'title', - 'caption', 'authors', 'author_ids', 'tags', @@ -259,21 +257,16 @@ def create(self, validated_data): return self.update(Image(), validated_data) def update(self, instance, validated_data): - is_new = instance.pk is None - instance = super(ImageSerializer, self).update(instance, validated_data) # Save authors - author_ids = validated_data.get('author_ids') - if author_ids: - instance.save_authors(author_ids) + authors = validated_data.get('author_ids') + if authors: + instance.save_authors(authors) - # Save_tags - tag_ids = validated_data.get('tag_ids') - if tag_ids: - # Do not clear tags for first save. This prevents deletion of EXIF tags - clear = not is_new - instance.save_tags(tag_ids, clear=clear) + tag_ids = validated_data.get('tag_ids', False) + if tag_ids != False: + instance.save_tags(tag_ids) return instance @@ -643,9 +636,9 @@ def update(self, instance, validated_data): if featured_video != False: instance.save_featured_video(featured_video) - author_ids = validated_data.get('author_ids') - if author_ids: - instance.save_authors(author_ids, is_publishable=True) + authors = validated_data.get('author_ids') + if authors: + instance.save_authors(authors, is_publishable=True) tag_ids = validated_data.get('tag_ids', False) if tag_ids != False: diff --git a/dispatch/migrations/0013_image_caption.py b/dispatch/migrations/0013_image_caption.py deleted file mode 100644 index c1da79f47..000000000 --- a/dispatch/migrations/0013_image_caption.py +++ /dev/null @@ -1,20 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.11 on 2018-05-31 06:31 -from __future__ import unicode_literals - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('dispatch', '0012_polls'), - ] - - operations = [ - migrations.AddField( - model_name='image', - name='caption', - field=models.CharField(blank=True, max_length=255, null=True), - ), - ] diff --git a/dispatch/migrations/0014_user_groups.py b/dispatch/migrations/0013_user_groups.py similarity index 97% rename from dispatch/migrations/0014_user_groups.py rename to dispatch/migrations/0013_user_groups.py index f07dd45c5..eebe74724 100644 --- a/dispatch/migrations/0014_user_groups.py +++ b/dispatch/migrations/0013_user_groups.py @@ -37,7 +37,7 @@ def remove_groups(apps, schema_editor): class Migration(migrations.Migration): dependencies = [ - ('dispatch', '0013_image_caption'), + ('dispatch', '0012_polls'), ] operations = [ diff --git a/dispatch/migrations/0015_user_management.py b/dispatch/migrations/0014_user_management.py similarity index 96% rename from dispatch/migrations/0015_user_management.py rename to dispatch/migrations/0014_user_management.py index 4c38588ce..b22a65012 100644 --- a/dispatch/migrations/0015_user_management.py +++ b/dispatch/migrations/0014_user_management.py @@ -11,7 +11,7 @@ class Migration(migrations.Migration): dependencies = [ - ('dispatch', '0014_user_groups'), + ('dispatch', '0013_user_groups'), ] operations = [ diff --git a/dispatch/modules/content/image.py b/dispatch/modules/content/image.py deleted file mode 100644 index 62655f407..000000000 --- a/dispatch/modules/content/image.py +++ /dev/null @@ -1,134 +0,0 @@ -import os -from StringIO import StringIO -from tempfile import TemporaryFile - -from libxmp import XMPMeta, XMPError -from PIL import Image as Img - -# EXIF tag list: https://www.sno.phy.queensu.ca/~phil/exiftool/TagNames/EXIF.html -EXIF_DESCRIPTION = 270 -EXIF_ARTIST = 315 - -EXIF_FORMATS = '.(jpg|JPEG|jpeg|JPG|tiff|tif)' - -XMP_TITLE = 'title' -XMP_DESCRIPTION = 'description' -XMP_SUBJECT = 'subject' -XMP_CREATOR = 'creator' - -def get_extension(img): - """Returns the extension of Django ImageField object.""" - - ext = os.path.splitext(img.name)[1] - if ext: - # Remove period from extension - return ext[1:] - return ext - -def parse_exif(img): - """Extract EXIF portion of data from Django ImageField object.""" - - image = Img.open(StringIO(img.read())) - img.seek(0) - metadata = {} - - # EXIF data is only read from files with TIFF or JPEG format, otherwise an error will occur - if get_extension(img) in EXIF_FORMATS: - exif = image._getexif() - - if exif is None: - return metadata, authors - - metadata['description'] = exif.get(EXIF_DESCRIPTION) - - authors = set() - - author_name = exif.get(EXIF_ARTIST) - if author_name is not None: - authors.add(author_name) - - metadata['authors'] = authors - - return metadata - -def parse_xmp(img, metadata={}): - """Extract XMP portion of data from Django ImageField object.""" - - buffer = img.read() - img.seek(0) - b = bytearray(buffer) - - with TemporaryFile() as f: - f.write(b) - f.seek(0) - - xmp_data = '' - xmp_started = False - - for line in f: - if not xmp_started: - xmp_started = ' -1: - break - else: - return metadata - - xmp_open_tag = xmp_data.find('') - xmp_str = xmp_data[xmp_open_tag:xmp_close_tag + 12] - - xmp = XMPMeta() - xmp.parse_from_str(xmp_str) - - if xmp is None: - return metadata - - try: - ns = xmp.get_namespace_for_prefix('dc') - - title = xmp.get_array_item(ns, XMP_TITLE, 1) - if title: - metadata['title'] = title - - description = xmp.get_array_item(ns, XMP_DESCRIPTION, 1) - if description: - metadata['description'] = description - - tags = set() - counter = 1 - tag_name = xmp.get_array_item(ns, XMP_SUBJECT, counter) - while tag_name != '': - tags.add(tag_name) - counter += 1 - - try: - tag_name = xmp.get_array_item(ns, XMP_SUBJECT, counter) - except XMPError: - break - - metadata['tags'] = tags - - authors = metadata.get('authors', set()) - - author_name = xmp.get_array_item(ns, XMP_CREATOR, 1) - if author_name: - authors.add(author_name) - - metadata['authors'] = authors - - except XMPError: - pass - - return metadata - -def parse_metadata(img): - """Extract metadata from Django ImageField object.""" - - metadata = parse_exif(img) - - # Overwrite EXIF keys with XMP data - metadata = parse_xmp(img, metadata) - - return metadata diff --git a/dispatch/modules/content/mixins.py b/dispatch/modules/content/mixins.py index 5542e998b..8394757cc 100644 --- a/dispatch/modules/content/mixins.py +++ b/dispatch/modules/content/mixins.py @@ -82,31 +82,3 @@ def get_author_url(self): """Returns list of authors (including hyperlinks) as a comma-separated string (with 'and' before last author).""" return self.get_author_string(True) - -class TagMixin(object): - def save_tags(self, tag_ids, clear=True): - """Save tags with given ids to this instance.""" - - if clear: - self.tags.clear() - - for tag_id in tag_ids: - try: - tag = self.TagModel.objects.get(id=int(tag_id)) - self.tags.add(tag) - except self.TagModel.DoesNotExist: - pass - -class TopicMixin(object): - def save_topic(self, topic_id): - """Save topic with given id to this instance.""" - - if topic_id is None: - self.topic = None - else: - try: - topic = self.TopicModel.objects.get(id=int(topic_id)) - topic.update_timestamp() - self.topic = topic - except self.TopicModel.DoesNotExist: - pass diff --git a/dispatch/modules/content/models.py b/dispatch/modules/content/models.py index 9ea967ad4..0bb6dcdce 100644 --- a/dispatch/modules/content/models.py +++ b/dispatch/modules/content/models.py @@ -2,10 +2,9 @@ import os import re import uuid -import io -from PIL import Image as Img from jsonfield import JSONField +from PIL import Image as Img from django.db import IntegrityError from django.db import transaction @@ -24,9 +23,8 @@ from dispatch.modules.content.managers import PublishableManager from dispatch.modules.content.render import content_to_html -from dispatch.modules.content.image import parse_metadata, get_extension -from dispatch.modules.content.mixins import AuthorMixin, TagMixin, TopicMixin -from dispatch.modules.auth.models import Person, User +from dispatch.modules.content.mixins import AuthorMixin +from dispatch.modules.auth.models import Person class Tag(Model): name = CharField(max_length=255, unique=True) @@ -52,7 +50,9 @@ class Meta: ordering = ['order'] class Publishable(Model): - """Base model for Article and Page models.""" + """ + Base model for Article and Page models. + """ preview_id = UUIDField(default=uuid.uuid4) revision_id = PositiveIntegerField(default=0, db_index=True) @@ -153,10 +153,12 @@ def unpublish(self): # Overriding def save(self, revision=True, *args, **kwargs): - """Handles the saving/updating of a Publishable instance. + """ + Handles the saving/updating of a Publishable instance. Arguments: - revision - if True, a new version of this Publishable will be created.""" + revision - if True, a new version of this Publishable will be created. + """ if revision: # If this is a revision, set it to be the head of the list and increment the revision id @@ -204,7 +206,8 @@ def save(self, revision=True, *args, **kwargs): return self def save_featured_image(self, data): - """Handles saving the featured image. + """ + Handles saving the featured image. If data is None, the featured image will be removed. @@ -291,7 +294,7 @@ def get_previous_revision(self): class Meta: abstract = True -class Article(Publishable, AuthorMixin, TagMixin, TopicMixin): +class Article(Publishable, AuthorMixin): parent = ForeignKey('Article', related_name='article_parent', blank=True, null=True) @@ -315,8 +318,6 @@ class Article(Publishable, AuthorMixin, TagMixin, TopicMixin): reading_time = CharField(max_length=100, choices=READING_CHOICES, default='anytime') AuthorModel = Author - TagModel = Tag - TopicModel = Topic @property def title(self): @@ -334,8 +335,30 @@ def get_reading_list(self, ref=None, dur=None): 'name': name } + def save_tags(self, tag_ids): + self.tags.clear() + for tag_id in tag_ids: + try: + tag = Tag.objects.get(id=int(tag_id)) + self.tags.add(tag) + except Tag.DoesNotExist: + pass + + def save_topic(self, topic_id): + if topic_id is None: + self.topic = None + else: + try: + topic = Topic.objects.get(id=int(topic_id)) + topic.update_timestamp() + self.topic = topic + except Topic.DoesNotExist: + pass + def get_absolute_url(self): - """Returns article URL.""" + """ + Returns article URL. + """ return "%s%s/%s/" % (settings.BASE_URL, self.section.slug, self.slug) class Page(Publishable): @@ -347,23 +370,23 @@ def get_author_string(self): return None def get_absolute_url(self): - """Returns page URL.""" + """ + Returns page URL. + """ return "%s%s/" % (settings.BASE_URL, self.slug) class Video(Model): title = CharField(max_length=255) url = CharField(max_length=500) -class Image(Model, AuthorMixin, TagMixin): - +class Image(Model, AuthorMixin): img = ImageField(upload_to='images/%Y/%m') title = CharField(max_length=255, blank=True, null=True) - caption = CharField(max_length=255, blank=True, null=True) width = PositiveIntegerField(blank=True, null=True) height = PositiveIntegerField(blank=True, null=True) authors = ManyToManyField(Author, related_name='image_authors') - tags = ManyToManyField(Tag) + tags = ManyToManyField('Tag') created_at = DateTimeField(auto_now_add=True) updated_at = DateTimeField(auto_now=True) @@ -382,7 +405,6 @@ class Image(Model, AuthorMixin, TagMixin): THUMBNAIL_SIZE = 'square' AuthorModel = Author - TagModel = Tag def is_gif(self): """Returns true if image is a gif.""" @@ -398,7 +420,11 @@ def get_name(self): def get_extension(self): """Returns the file extension.""" - return get_extension(self.img) + ext = os.path.splitext(self.img.name)[1] + if ext: + # Remove period from extension + return ext[1:] + return ext def get_absolute_url(self): """Returns the full size image URL.""" @@ -427,28 +453,13 @@ def save(self, **kwargs): super(Image, self).save(**kwargs) if is_new and self.img: - metadata = parse_metadata(self.img) image = Img.open(StringIO.StringIO(self.img.read())) - self.width, self.height = image.size - # Save metadata - self.title = metadata.get('title') - self.caption = metadata.get('description') - - for tag_name in metadata.get('tags', set()): - tag, created = Tag.objects.get_or_create(name=tag_name) - self.tags.add(tag) - - for n, name in enumerate(metadata.get('authors', set())): - person, created = Person.objects.get_or_create(full_name=name) - author = Author.objects.create(person=person, order=n, type='photographer') - self.authors.add(author) - super(Image, self).save() - ext = self.get_extension() name = self.get_name() + ext = self.get_extension() for size in self.SIZES.keys(): self.save_thumbnail(image, self.SIZES[size], name, size, ext) @@ -479,6 +490,15 @@ def save_thumbnail(self, image, size, name, label, file_type): # Save the new file to the default storage system default_storage.save(name, thumb_file) + def save_tags(self, tag_ids): + self.tags.clear() + for tag_id in tag_ids: + try: + tag = Tag.objects.get(id=int(tag_id)) + self.tags.add(tag) + except Tag.DoesNotExist: + pass + class VideoAttachment(Model): article = ForeignKey(Article, blank=True, null=True, related_name='video_article') page = ForeignKey(Page, blank=True, null=True, related_name='video_page') @@ -523,7 +543,9 @@ class File(Model): updated_at = DateTimeField(auto_now=True) def get_absolute_url(self): - """Returns the absolute file URL.""" + """ + Returns the absolute file URL. + """ return settings.MEDIA_URL + str(self.file) class Issue(Model): diff --git a/dispatch/modules/content/render.py b/dispatch/modules/content/render.py index d7de9ee23..593a11b6a 100644 --- a/dispatch/modules/content/render.py +++ b/dispatch/modules/content/render.py @@ -3,11 +3,10 @@ from dispatch.modules.content.embeds import embeds, EmbedException def content_to_html(content, article_id): - """Returns article/page content as HTML.""" + """Returns artilce/page content as HTML""" def render_node(html, node, index): - """Renders node as HTML.""" - + """Renders node as HTML""" if node['type'] == 'paragraph': return html + '

%s

' % node['data'] else: @@ -35,14 +34,14 @@ def render_node(html, node, index): html = render_node(html, node, index) if (node['type'] == 'ad'): index += 1 - + # return mark_safe(reduce(render_node, content, '')) return mark_safe(html) def content_to_json(content): - """Returns article/page content as JSON.""" - + """Returns article/page content as JSON""" + def render_node(node): - """Renders node as JSON.""" + """Renders node as JSON""" if node['type'] == 'paragraph': return node diff --git a/dispatch/tests/input/test_exif_xmp.jpg b/dispatch/tests/input/test_exif_xmp.jpg deleted file mode 100644 index e6ca574f5..000000000 Binary files a/dispatch/tests/input/test_exif_xmp.jpg and /dev/null differ diff --git a/dispatch/tests/input/test_exif_xmp_different.jpg b/dispatch/tests/input/test_exif_xmp_different.jpg deleted file mode 100644 index ef2eca689..000000000 Binary files a/dispatch/tests/input/test_exif_xmp_different.jpg and /dev/null differ diff --git a/dispatch/tests/test_api_images.py b/dispatch/tests/test_api_images.py index ad40b54d6..0b7428915 100644 --- a/dispatch/tests/test_api_images.py +++ b/dispatch/tests/test_api_images.py @@ -9,7 +9,7 @@ from django.core.urlresolvers import reverse -from dispatch.models import Image, Person, Tag, Author +from dispatch.models import Image, Person, Tag from dispatch.tests.cases import DispatchAPITestCase, DispatchMediaTestMixin from dispatch.tests.helpers import DispatchTestHelpers @@ -123,49 +123,6 @@ def test_upload_duplicate_filenames(self): # Check that filenames are different self.assertTrue(image_1.data['url'] != image_2.data['url']) - def test_create_image_jpeg_with_meta(self): - """Should be able to read XMP and EXIF data from a JPEG image.""" - - url = reverse('api-images-list') - file = 'test_exif_xmp.jpg' - - with open(self.get_input_file(file)) as test_image: - response = self.client.post(url, { 'img': test_image }, format='multipart') - - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertTrue(self.fileExists(response.data['url'])) - - # Assert that image metadata was read - self.assertEqual(response.data['title'], 'Test Image') - self.assertEqual(response.data['caption'], 'This is a test image.') - self.assertEquals(response.data['authors'][0]['person']['full_name'], 'Person A') - - expected_tags = ('climb', 'joshua', 'tree', 'park') - - for test_tag in response.data['tags']: - if test_tag['name'] not in expected_tags: - self.fail('%s tag is expected.' % test_tag['name']) - - def test_create_image_jpeg_with_differing_meta(self): - """Should preferentially use XMP data over EXIF.""" - - url = reverse('api-images-list') - file = 'test_exif_xmp_different.jpg' - - with open(self.get_input_file(file)) as test_image: - response = self.client.post(url, { 'img': test_image }, format='multipart') - - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertTrue(self.fileExists(response.data['url'])) - - # Assert that image XMP metadata was stored instead of EXIF - self.assertEqual(response.data['caption'], 'This is another test image.') - - # Assert that author names were pulled from both XMP and EXIF - self.assertIn(response.data['authors'][0]['person']['full_name'], ('Person A', 'Person B')) - self.assertIn(response.data['authors'][1]['person']['full_name'], ('Person A', 'Person B')) - self.assertNotEqual(response.data['authors'][0]['person']['full_name'], response.data['authors'][1]['person']['full_name']) - def test_create_image_invalid_filename(self): """Should not be able to upload image with non-ASCII characters in filename.""" diff --git a/setup.py b/setup.py index 8c5c0ff51..d6f8e56c3 100644 --- a/setup.py +++ b/setup.py @@ -16,7 +16,6 @@ 'pillow', 'requests == 2.6.0', 'jsonfield', - 'python-xmp-toolkit', ], extras_require={ 'dev': [