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>

diff --git a/mediagoblin/db/mixin.py b/mediagoblin/db/mixin.py
index a8436c7097398566554f507cbde201222cc08b70..3f395e90b68a1a257786ee397165954c493d7b49 100644 (file)
--- 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 5bf0124ca6db5272d4796bcda21f0177925d42d4..0676351053eb0af8e658cf32337a3f8c139eb6a0 100644 (file)
--- 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 187b893dc47c9f2ccc3e081962dc544ea13be2d8..09abebb53e53f43a62e7f1837596c31a736f9363 100644 (file)
--- 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 b7b0e5748f849ad7f4cdea26ad841bf9893f2661..b6fe00157b632b4806f0349a3f632f4ce20655cd 100644 (file)
--- 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 c26bd340bae6ce2e50f57824084113b40736b535..ec159b6e222705e9eddead259a2398872be59c5e 100644 (file)
--- 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,