From 5f8b4ae895ecb228c5f5d615818ffe0a06a30473 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Tue, 4 Dec 2012 09:57:56 +0100 Subject: [PATCH] make media_manager a property of MediaEntry in mixin.py In all cases where get_media_manager(_media_type_as_string) was called in our code base we ultimately passed in a "MediaEntry().media_type" to get the matching MEDIA_MANAGER. It so makes sense to make this a function of the MediaEntry rather than a global function in mediagoblin.media_types and passing around media_entry.media_type as arguments all the time. It saves a few import statements and arguments. I also made it so the Media_manager property is cached for subsequent calls, although I am not too sure that this is needed (there are other cases for which this would make more sense) Also add a get_media_manager test to the media submission tests. It submits an image and checks that both media.media_type and media.media_manager return the right thing. Not sure if these tests could not be merged with an existing submission test, but it can't hurt to have things explicit. TODO: Right now we iterate through all existing media_managers to find the right one based on the string of its module name. This should be made a simple dict lookup to avoid all the extra work. Signed-off-by: Sebastian Spaeth --- mediagoblin/db/mixin.py | 24 +++++++++++++++++++++--- mediagoblin/media_types/__init__.py | 19 ------------------- mediagoblin/processing/task.py | 5 +---- mediagoblin/tests/test_submission.py | 15 ++++++++++++++- mediagoblin/user_pages/views.py | 4 +--- 5 files changed, 37 insertions(+), 30 deletions(-) diff --git a/mediagoblin/db/mixin.py b/mediagoblin/db/mixin.py index a8436c70..3f395e90 100644 --- a/mediagoblin/db/mixin.py +++ b/mediagoblin/db/mixin.py @@ -27,9 +27,11 @@ These functions now live here and get "mixed in" into the real objects. """ +from werkzeug.utils import cached_property + from mediagoblin import mg_globals from mediagoblin.auth import lib as auth_lib -from mediagoblin.media_types import get_media_manager +from mediagoblin.media_types import get_media_managers, FileTypeNotSupported from mediagoblin.tools import common, licenses from mediagoblin.tools.text import cleaned_markdown_conversion from mediagoblin.tools.url import slugify @@ -99,6 +101,7 @@ class MediaEntryMixin(object): def slug_or_id(self): return (self.slug or self._id) + def url_for_self(self, urlgen, **extra_args): """ Generate an appropriate url for ourselves @@ -125,11 +128,26 @@ class MediaEntryMixin(object): else: # No thumbnail in media available. Get the media's # MEDIA_MANAGER for the fallback icon and return static URL - # Raise FileTypeNotSupported in case no such manager is enabled - manager = get_media_manager(self.media_type) + # Raises FileTypeNotSupported in case no such manager is enabled + manager = self.media_manager thumb_url = mg_globals.app.staticdirector(manager[u'default_thumb']) return thumb_url + @cached_property + def media_manager(self): + """Returns the MEDIA_MANAGER of the media's media_type + + Raises FileTypeNotSupported in case no such manager is enabled + """ + # TODO, we should be able to make this a simple lookup rather + # than iterating through all media managers. + for media_type, manager in get_media_managers(): + if media_type == self.media_type: + return manager + # Not found? Then raise an error + raise FileTypeNotSupported( + "MediaManager not in enabled types. Check media_types in config?") + def get_fail_exception(self): """ Get the exception that's appropriate for this error diff --git a/mediagoblin/media_types/__init__.py b/mediagoblin/media_types/__init__.py index 5bf0124c..06763510 100644 --- a/mediagoblin/media_types/__init__.py +++ b/mediagoblin/media_types/__init__.py @@ -78,25 +78,6 @@ def get_media_managers(): yield media_type, sys.modules[media_type].MEDIA_MANAGER -def get_media_manager(_media_type): - ''' - Get the MEDIA_MANAGER based on a media type string - - Example:: - get_media_type('mediagoblin.media_types.image') - ''' - if not _media_type: - return False - - for media_type, manager in get_media_managers(): - if media_type in _media_type: - return manager - - # Nope? Then raise an error - raise FileTypeNotSupported( - "MediaManager not in enabled types. Check media_types in config?") - - def get_media_type_and_manager(filename): ''' Try to find the media type based on the file name, extension diff --git a/mediagoblin/processing/task.py b/mediagoblin/processing/task.py index 187b893d..09abebb5 100644 --- a/mediagoblin/processing/task.py +++ b/mediagoblin/processing/task.py @@ -20,7 +20,6 @@ from celery.task import Task from mediagoblin import mg_globals as mgg from mediagoblin.db.util import ObjectId -from mediagoblin.media_types import get_media_manager from mediagoblin.processing import mark_entry_failed, BaseProcessingFail from mediagoblin.tools.processing import json_processing_callback @@ -47,14 +46,12 @@ class ProcessMedia(Task): # Try to process, and handle expected errors. try: - manager = get_media_manager(entry.media_type) - entry.state = u'processing' entry.save() _log.debug('Processing {0}'.format(entry)) - manager['processor'](entry) + entry.media_manager['processor'](entry) entry.state = u'processed' entry.save() diff --git a/mediagoblin/tests/test_submission.py b/mediagoblin/tests/test_submission.py index b7b0e574..b6fe0015 100644 --- a/mediagoblin/tests/test_submission.py +++ b/mediagoblin/tests/test_submission.py @@ -28,7 +28,7 @@ from mediagoblin.tests.tools import get_test_app, \ fixture_add_user from mediagoblin import mg_globals from mediagoblin.tools import template - +from mediagoblin.media_types.image import MEDIA_MANAGER as img_MEDIA_MANAGER def resource(filename): return resource_filename('mediagoblin.tests', 'test_submission/' + filename) @@ -197,6 +197,19 @@ class TestSubmission: assert 'Sorry, I don\'t support that file type :(' == \ str(form.file.errors[0]) + + def test_get_media_manager(self): + """Test if the get_media_manger function returns sensible things + """ + response, request = self.do_post({'title': u'Balanced Goblin'}, + *REQUEST_CONTEXT, do_follow=True, + **self.upload_data(GOOD_JPG)) + media = self.check_media(request, {'title': u'Balanced Goblin'}, 1) + + assert_equal(media.media_type, u'mediagoblin.media_types.image') + assert_equal(media.media_manager, img_MEDIA_MANAGER) + + def test_sniffing(self): ''' Test sniffing mechanism to assert that regular uploads work as intended diff --git a/mediagoblin/user_pages/views.py b/mediagoblin/user_pages/views.py index c26bd340..ec159b6e 100644 --- a/mediagoblin/user_pages/views.py +++ b/mediagoblin/user_pages/views.py @@ -33,7 +33,6 @@ from mediagoblin.decorators import (uses_pagination, get_user_media_entry, from werkzeug.contrib.atom import AtomFeed -from mediagoblin.media_types import get_media_manager _log = logging.getLogger(__name__) _log.setLevel(logging.DEBUG) @@ -128,8 +127,7 @@ def media_home(request, media, page, **kwargs): comment_form = user_forms.MediaCommentForm(request.form) - media_template_name = get_media_manager( - media.media_type)['display_template'] + media_template_name = media.media_manager['display_template'] return render_to_response( request, -- 2.25.1