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 <Sebastian@SSpaeth.de>
This commit is contained in:
Sebastian Spaeth 2012-12-04 09:57:56 +01:00
parent c39b9afc83
commit 5f8b4ae895
5 changed files with 37 additions and 30 deletions

View File

@ -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

View File

@ -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

View File

@ -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()

View File

@ -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

View File

@ -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,