Fix the theme_update service (bug 1055654)

apps/addons/tests/test_theme_update.py

@@ -11,6 +11,7 @@
 
 import amo.tests
 from addons.models import Addon
+from amo.helpers import user_media_path, user_media_url
 from versions.models import Version
 from services import theme_update
 
@@ -137,3 +138,19 @@ def test_get_json_good_ids(self):
 
         self.check_good(
             json.loads(self.get_update('en-US', 813, 'src=gp').get_json()))
+
+    def test_image_path(self):
+        up = self.get_update('en-US', 15663)
+        up.get_update()
+        image_path = up.image_path('foo.png')
+        # This is ugly. It's needed because services.theme_update imports
+        # settings_local, and settings_test is overriding MEDIA_ROOT.
+        import settings_local
+        with self.settings(MEDIA_ROOT=settings_local.MEDIA_ROOT):
+            assert user_media_path('addons') in image_path
+
+    def test_image_url(self):
+        up = self.get_update('en-US', 15663)
+        up.get_update()
+        image_url = up.image_url('foo.png')
+        assert user_media_url('addons') in image_url

apps/amo/helpers.py

@@ -662,6 +662,7 @@ def inline_css(bundle, media=False, debug=None):
                                  '</style>' % media)
 
 
+# A (temporary?) copy of this is in services/utils.py. See bug 1055654.
 def user_media_path(what):
     """Make it possible to override storage paths in settings.
 
@@ -675,6 +676,7 @@ def user_media_path(what):
     return getattr(settings, key, default)
 
 
+# A (temporary?) copy of this is in services/utils.py. See bug 1055654.
 def user_media_url(what):
     """
     Generate default media url, and make possible to override it from

services/theme_update.py

@@ -8,9 +8,8 @@
 
 from constants import base
 from utils import log_configure, log_exception, mypool
-from amo.helpers import user_media_path
 
-from services.utils import settings
+from services.utils import settings, user_media_path, user_media_url
 
 # Configure the log.
 log_configure()
@@ -183,7 +182,7 @@ def image_url(self, filename):
             elif filename == 'icon.png':
                 filename = 'preview_small.jpg'
 
-        image_url = posixpath.join(user_media_path('addons'),
+        image_url = posixpath.join(user_media_url('addons'),
                                    str(row['addon_id']), filename or '')
         modified = int(row['modified']) if row['modified'] else 0
         return '%s?%s' % (image_url, modified)

services/utils.py

@@ -37,6 +37,22 @@
                             STATUS_LITE_AND_NOMINATED)
 
 
+# This is not DRY: it's a copy of amo.helpers.user_media_path, to avoid an
+# import (which should triggers an import loop).
+# See bug 1055654.
+def user_media_path(what):
+    """Make it possible to override storage paths in settings.
+
+    By default, all storage paths are in the MEDIA_ROOT.
+
+    This is backwards compatible.
+
+    """
+    default = os.path.join(settings.MEDIA_ROOT, what)
+    key = "{0}_PATH".format(what.upper())
+    return getattr(settings, key, default)
+
+
 # This is not DRY: it's a copy of amo.helpers.user_media_url, to avoid an
 # import (which should be avoided, according to the comments above, and which
 # triggers an import loop).