launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #09210
[Merge] lp:~rvb/maas/js-combo into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/js-combo into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/js-combo/+merge/112048
This branch's main goal is to add the ability to load raphaelJS (the other JS library besides YUI that we use on the dashboard) from the package (which is in the works).
Instead of fixing this in an ad-hoc fashion, I did something I wanted to do for a very long time: use the combo loader to load all the JS/CSS assets we use (external libraries as well as our own code).
This gives us two things:
- all the asserts (per library) are loaded in one request as opposed to one file at a time (This is the log the I get when I load the home page (the dashboard): Before: http://paste.ubuntu.com/1060488/, after: http://paste.ubuntu.com/1060487/)
- it's easy to switch between serving the files from an absolute location (the files from a package for instance) or a relative location present in our own tree.
= Pre-imp =
I had a short pre-imp call with Gavin about that.
= Notes =
Don't let the stats (+260/-95) fool you, this branch adds quite a few tests for things that were not tested before (the fact that our JS code is included by the template for instance). Also, the templates have been greatly simplified and the complexity has moved to python code where things are more clearly expressed and more easy to test.
I removed YUI_COMBO_URL from the settings as it's really an implementation detail; not something that a user would want to configure.
The only real trick is in src/maasserver/views/combo.py where I had to change the way we load the files (using convoy's library) to load UTF8 encoded files. The test for that is test_raphael_load_js which loads raphael-min.js which contains non-ascii characters.
--
https://code.launchpad.net/~rvb/maas/js-combo/+merge/112048
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/js-combo into lp:maas.
=== modified file 'contrib/maas_local_settings_sample.py'
--- contrib/maas_local_settings_sample.py 2012-06-13 11:02:58 +0000
+++ contrib/maas_local_settings_sample.py 2012-06-26 10:04:41 +0000
@@ -20,6 +20,9 @@
# Use the (libjs-yui) package's files to serve YUI3.
YUI_LOCATION = '/usr/share/javascript/yui/'
+# Use the package's files to serve RaphaelJS.
+RAPHAELJS_LOCATION = '/usr/share/javascript/raphael/'
+
# RabbitMQ settings.
RABBITMQ_HOST = 'localhost'
RABBITMQ_USERID = 'maas_longpoll'
=== modified file 'src/maas/settings.py'
--- src/maas/settings.py 2012-06-21 20:49:18 +0000
+++ src/maas/settings.py 2012-06-26 10:04:41 +0000
@@ -72,13 +72,16 @@
API_URL_REGEXP = '^/api/1[.]0/'
METADATA_URL_REGEXP = '^/metadata/'
-YUI_COMBO_URL = "combo/"
# We handle exceptions ourselves (in
# maasserver.middleware.APIErrorsMiddleware)
PISTON_DISPLAY_ERRORS = False
TEMPLATE_DEBUG = DEBUG
+# Set this to where Raphael JS's files can be found to serve these files
+# from a custom location.
+RAPHAELJS_LOCATION = None
+
YUI_DEBUG = DEBUG
# Set this to where YUI3 files can be found to serve these files from a
@@ -211,6 +214,7 @@
"django.contrib.messages.context_processors.messages",
"maasserver.context_processors.yui",
"maasserver.context_processors.global_options",
+ "maasserver.context_processors.static_ressources",
)
MIDDLEWARE_CLASSES = (
=== modified file 'src/maasserver/context_processors.py'
--- src/maasserver/context_processors.py 2012-06-13 11:02:58 +0000
+++ src/maasserver/context_processors.py 2012-06-26 10:04:41 +0000
@@ -25,8 +25,42 @@
def yui(context):
return {
'YUI_DEBUG': settings.YUI_DEBUG,
- 'YUI_COMBO_URL': settings.YUI_COMBO_URL,
- 'FORCE_SCRIPT_NAME': settings.FORCE_SCRIPT_NAME,
+ }
+
+
+def static_ressources(context):
+ return {
+ 'CSS_LIST': [
+ 'css/base.css',
+ 'css/typography.css',
+ 'css/forms.css',
+ 'css/layout.css',
+ 'css/modifiers.css',
+ 'css/components/flash_messages.css',
+ 'css/components/pagination.css',
+ 'css/components/table_list.css',
+ 'css/components/title_form.css',
+ 'css/components/blocks.css',
+ 'css/components/yui_panel.css',
+ 'css/components/yui_overlay.css',
+ 'css/components/yui_node_add.css',
+ 'css/components/data_list.css',
+ 'css/components/search_box.css',
+ 'css/ubuntu-webfonts.css',
+ ],
+ 'JS_LIST': [
+ 'js/morph.js',
+ 'js/user_panel.js',
+ 'js/node_add.js',
+ 'js/node.js',
+ 'js/prefs.js',
+ 'js/utils.js',
+ 'js/node_views.js',
+ 'js/longpoll.js',
+ 'js/enums.js',
+ 'js/power_parameters.js',
+ 'js/nodes_chart.js',
+ ],
}
=== modified file 'src/maasserver/middleware.py'
--- src/maasserver/middleware.py 2012-04-25 16:19:07 +0000
+++ src/maasserver/middleware.py 2012-06-26 10:04:41 +0000
@@ -74,8 +74,10 @@
public_url_roots = [
# Login page: must be visible to anonymous users.
reverse('login'),
- # The combo loader is publicly accessible.
- '^/' + re.escape(settings.YUI_COMBO_URL),
+ # The combo loaders are publicly accessible.
+ reverse('combo-yui'),
+ reverse('combo-maas'),
+ reverse('combo-raphael'),
# Static resources are publicly visible.
settings.STATIC_URL,
reverse('favicon'),
=== removed file 'src/maasserver/static/css/import.css'
--- src/maasserver/static/css/import.css 2012-03-29 04:21:34 +0000
+++ src/maasserver/static/css/import.css 1970-01-01 00:00:00 +0000
@@ -1,16 +0,0 @@
-@import url("base.css");
-@import url("typography.css");
-@import url("forms.css");
-@import url("layout.css");
-@import url("modifiers.css");
-/* components */
-@import url("components/flash_messages.css");
-@import url("components/pagination.css");
-@import url("components/table_list.css");
-@import url("components/title_form.css");
-@import url("components/blocks.css");
-@import url("components/yui_panel.css");
-@import url("components/yui_overlay.css");
-@import url("components/yui_node_add.css");
-@import url("components/data_list.css");
-@import url("components/search_box.css");
=== modified file 'src/maasserver/templates/maasserver/base.html'
--- src/maasserver/templates/maasserver/base.html 2012-04-18 02:39:13 +0000
+++ src/maasserver/templates/maasserver/base.html 2012-06-26 10:04:41 +0000
@@ -3,8 +3,7 @@
<head>
<meta http-equiv="Content-type" content="text/html; charset=utf-8" />
<meta http-equiv="Content-Language" content="en-us" />
- <link rel="stylesheet" href="{{ STATIC_URL }}css/import.css" />
- <link rel="stylesheet" href="{{ STATIC_URL }}css/ubuntu-webfonts.css" />
+ <link rel="stylesheet" href="{% url "combo-maas" %}?{{ CSS_LIST|join:"&" }}" />
<title>{% block title %}{% endblock %} | {% include "maasserver/site_title.html" %}</title>
{% block js-conf %}{% include "maasserver/js-conf.html" %}{% endblock %}
{% block head %}{% endblock %}
=== modified file 'src/maasserver/templates/maasserver/index.html'
--- src/maasserver/templates/maasserver/index.html 2012-06-18 07:45:17 +0000
+++ src/maasserver/templates/maasserver/index.html 2012-06-26 10:04:41 +0000
@@ -8,9 +8,9 @@
{% endblock %}
{% block head %}
- <script type="text/javascript" src="{{ STATIC_URL }}jslibs/raphael/raphael-min.js"></script>
- <script type="text/javascript" src="{{ STATIC_URL }}js/nodes_chart.js"></script>
- <script type="text/javascript" src="{{ STATIC_URL }}js/enums.js"></script>
+ <script type="text/javascript"
+ src="{% url "combo-raphael" %}?raphael-min.js">
+ </script>
<script type="text/javascript">
<!--
YUI().use(
@@ -27,7 +27,7 @@
reservedNode: '#reserved-nodes',
retiredNode: '#retired-nodes'});
view.render();
-
+
Y.one('#addnode').on('click', function(e) {
e.preventDefault();
Y.maas.node_add.showAddNodeWidget({targetNode: '#dashboard'});
=== modified file 'src/maasserver/templates/maasserver/js-conf.html'
--- src/maasserver/templates/maasserver/js-conf.html 2012-06-13 11:02:58 +0000
+++ src/maasserver/templates/maasserver/js-conf.html 2012-06-26 10:04:41 +0000
@@ -5,8 +5,8 @@
combine: true,
filter: {% if YUI_DEBUG %}'raw'{% else %}'min'{% endif %},
root: '',
- base: '{% if FORCE_SCRIPT_NAME %}{{ FORCE_SCRIPT_NAME }}{% endif%}/{{YUI_COMBO_URL }}?',
- comboBase: '{% if FORCE_SCRIPT_NAME %}{{ FORCE_SCRIPT_NAME }}{% endif%}/{{ YUI_COMBO_URL }}?',
+ base: '{% url "combo-yui" %}?',
+ comboBase: '{% url "combo-yui" %}?',
};
var MAAS_config = {
uris: {
@@ -21,15 +21,8 @@
// -->
</script>
<script type="text/javascript"
- src="{% if FORCE_SCRIPT_NAME %}{{ FORCE_SCRIPT_NAME }}{% endif%}/{{ YUI_COMBO_URL }}?yui-base/yui-base-min.js">
-</script>
-<script type="text/javascript" src="{{ STATIC_URL }}js/morph.js"></script>
-<script type="text/javascript" src="{{ STATIC_URL }}js/user_panel.js"></script>
-<script type="text/javascript" src="{{ STATIC_URL }}js/node_add.js"></script>
-<script type="text/javascript" src="{{ STATIC_URL }}js/node.js"></script>
-<script type="text/javascript" src="{{ STATIC_URL }}js/prefs.js"></script>
-<script type="text/javascript" src="{{ STATIC_URL }}js/utils.js"></script>
-<script type="text/javascript" src="{{ STATIC_URL }}js/node_views.js"></script>
-<script type="text/javascript" src="{{ STATIC_URL }}js/longpoll.js"></script>
-<script type="text/javascript" src="{{ STATIC_URL }}js/enums.js"></script>
-<script type="text/javascript" src="{{ STATIC_URL }}js/power_parameters.js"></script>
+ src="{% url "combo-yui" %}?yui-base/yui-base-min.js">
+</script>
+<script type="text/javascript"
+ src="{% url "combo-maas" %}?{{ JS_LIST|join:"&" }}">
+</script>
=== modified file 'src/maasserver/tests/test_views_combo.py'
--- src/maasserver/tests/test_views_combo.py 2012-06-13 11:02:58 +0000
+++ src/maasserver/tests/test_views_combo.py 2012-06-26 10:04:41 +0000
@@ -12,72 +12,138 @@
__metaclass__ = type
__all__ = []
+from functools import partial
import httplib
import os
from django.conf import settings
+from django.core.urlresolvers import reverse
+from django.test.client import RequestFactory
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase
-from maasserver.views.combo import get_yui_location
+from maasserver.views.combo import (
+ get_combo_view,
+ get_location,
+ )
class TestUtilities(TestCase):
- def test_get_yui_location_if_yui_location_set(self):
- yui_location = factory.getRandomString()
- self.patch(settings, 'YUI_LOCATION', yui_location)
- self.assertEqual(yui_location, get_yui_location())
-
- def test_get_yui_location_if_static_root_is_none(self):
- self.patch(settings, 'STATIC_ROOT', None)
- yui_location = os.path.join(
- os.path.dirname(os.path.dirname(__file__)),
- 'static', 'jslibs', 'yui')
- self.assertEqual(yui_location, get_yui_location())
-
- def test_get_yui_location(self):
+ def test_get_location_returns_absolute_location_if_not_None(self):
+ abs_location = factory.getRandomString()
+ self.assertEqual(
+ abs_location, get_location(
+ abs_location=abs_location,
+ rel_location=factory.getRandomString()))
+
+ def test_get_location_returns_rel_location_if_static_root_not_none(self):
static_root = factory.getRandomString()
self.patch(settings, 'STATIC_ROOT', static_root)
- yui_location = os.path.join(static_root, 'jslibs', 'yui')
- self.assertEqual(yui_location, get_yui_location())
+ rel_location = [factory.getRandomString(), factory.getRandomString()]
+ expected_location = os.path.join(static_root, *rel_location)
+ self.assertEqual(
+ expected_location, get_location(rel_location=rel_location))
+
+ def test_yui_location_returns_rel_location_if_static_root_is_none(self):
+ self.patch(settings, 'STATIC_ROOT', None)
+ rel_location = [factory.getRandomString(), factory.getRandomString()]
+ rel_location_base = os.path.join(
+ os.path.dirname(os.path.dirname(__file__)), 'static')
+ expected_location = os.path.join(rel_location_base, *rel_location)
+ self.assertEqual(
+ expected_location, get_location(rel_location=rel_location))
+
+ def test_get_combo_view_returns_partial(self):
+ rel_location = [factory.getRandomString(), factory.getRandomString()]
+ view = get_combo_view(factory.getRandomString(), rel_location)
+ self.assertIsInstance(view, partial)
+
+ def test_get_combo_view_loads_from_disk(self):
+ test_file_contents = factory.getRandomString()
+ # Create a valid file with a proper extension (the combo loader only
+ # serves JS or CSS files)
+ test_file_name = "%s.js" % factory.getRandomString()
+ test_file = self.make_file(
+ name=test_file_name, contents=test_file_contents)
+ directory = os.path.dirname(test_file)
+ rel_location = [factory.getRandomString(), factory.getRandomString()]
+ view = get_combo_view(directory, rel_location)
+ # Create a request for test file.
+ rf = RequestFactory()
+ request = rf.get("/test/?%s" % test_file_name)
+ response = view(request)
+ expected_content = '/* %s */\n%s\n' % (
+ test_file_name, test_file_contents)
+ self.assertEqual(
+ (httplib.OK, expected_content),
+ (response.status_code, response.content))
+
+
+# String used by convoy to replace missing files.
+CONVOY_MISSING_FILE = "/* [missing] */"
class TestComboLoaderView(TestCase):
- """Test combo loader view."""
+ """Test combo loader views."""
- def test_load_js(self):
+ def test_yui_load_js(self):
requested_files = [
'oop/oop.js',
'event-custom-base/event-custom-base.js'
]
- response = self.client.get('/combo/?%s' % '&'.join(requested_files))
+ url = '%s?%s' % (reverse('combo-yui'), '&'.join(requested_files))
+ response = self.client.get(url)
self.assertIn('text/javascript', response['Content-Type'])
for requested_file in requested_files:
self.assertIn(requested_file, response.content)
# No sign of a missing js file.
- self.assertNotIn("/* [missing] */", response.content)
+ self.assertNotIn(CONVOY_MISSING_FILE, response.content)
# The file contains a link to YUI's licence.
self.assertIn('http://yuilibrary.com/license/', response.content)
- def test_load_css(self):
+ def test_yui_load_css(self):
requested_files = [
'widget-base/assets/skins/sam/widget-base.css',
'widget-stack/assets/skins/sam/widget-stack.css',
]
- response = self.client.get('/combo/?%s' % '&'.join(requested_files))
+ url = '%s?%s' % (reverse('combo-yui'), '&'.join(requested_files))
+ response = self.client.get(url)
self.assertIn('text/css', response['Content-Type'])
for requested_file in requested_files:
self.assertIn(requested_file, response.content)
# No sign of a missing css file.
- self.assertNotIn("/* [missing] */", response.content)
+ self.assertNotIn(CONVOY_MISSING_FILE, response.content)
# The file contains a link to YUI's licence.
self.assertIn('http://yuilibrary.com/license/', response.content)
- def test_combo_no_file_returns_not_found(self):
- response = self.client.get('/combo/')
+ def test_yui_combo_no_file_returns_not_found(self):
+ response = self.client.get(reverse('combo-yui'))
self.assertEqual(httplib.NOT_FOUND, response.status_code)
- def test_combo_wrong_file_extension_returns_bad_request(self):
- response = self.client.get('/combo/?file.wrongextension')
- self.assertEqual(httplib.BAD_REQUEST, response.status_code)
- self.assertEqual("Invalid file type requested.", response.content)
+ def test_yui_combo_wrong_file_extension_returns_bad_request(self):
+ url = '%s?%s' % (reverse('combo-yui'), 'file.wrongextension')
+ response = self.client.get(url)
+ self.assertEqual(
+ (httplib.BAD_REQUEST, "Invalid file type requested."),
+ (response.status_code, response.content))
+
+ def test_maas_load_js(self):
+ requested_files = ['js/node.js', 'js/enums.js']
+ url = '%s?%s' % (reverse('combo-maas'), '&'.join(requested_files))
+ response = self.client.get(url)
+ # No sign of a missing js file.
+ self.assertNotIn(CONVOY_MISSING_FILE, response.content)
+
+ def test_maas_load_css(self):
+ requested_files = ['css/base.css', 'css/forms.css']
+ url = '%s?%s' % (reverse('combo-maas'), '&'.join(requested_files))
+ response = self.client.get(url)
+ # No sign of a missing css file.
+ self.assertNotIn(CONVOY_MISSING_FILE, response.content)
+
+ def test_raphael_load_js(self):
+ requested_files = ['raphael-min.js']
+ url = '%s?%s' % (reverse('combo-raphael'), '&'.join(requested_files))
+ response = self.client.get(url)
+ # No sign of a missing js file.
+ self.assertNotIn(CONVOY_MISSING_FILE, response.content.decode('utf8'))
=== modified file 'src/maasserver/urls.py'
--- src/maasserver/urls.py 2012-06-22 20:24:20 +0000
+++ src/maasserver/urls.py 2012-06-26 10:04:41 +0000
@@ -12,9 +12,7 @@
__metaclass__ = type
__all__ = []
-import re
-from django.conf import settings as django_settings
from django.conf.urls.defaults import (
include,
patterns,
@@ -30,7 +28,6 @@
login,
logout,
)
-from maasserver.views.combo import combo_view
from maasserver.views.nodes import (
enlist_preseed_view,
MacAdd,
@@ -62,10 +59,13 @@
## URLs accessible to anonymous users.
-urlpatterns = patterns('maasserver.views',
- url(
- r'^%s' % re.escape(django_settings.YUI_COMBO_URL), combo_view,
- name='yui-combo'),
+# Combo URLs.
+urlpatterns = patterns('',
+ (r'combo/', include('maasserver.urls_combo'))
+)
+
+# Anonymous views.
+urlpatterns += patterns('maasserver.views',
url(r'^accounts/login/$', login, name='login'),
url(
r'^robots\.txt$', direct_to_template,
=== added file 'src/maasserver/urls_combo.py'
--- src/maasserver/urls_combo.py 1970-01-01 00:00:00 +0000
+++ src/maasserver/urls_combo.py 2012-06-26 10:04:41 +0000
@@ -0,0 +1,40 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""URL combo routing configuration."""
+
+from __future__ import (
+ absolute_import,
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = []
+
+
+from django.conf import settings as django_settings
+from django.conf.urls.defaults import (
+ patterns,
+ url,
+ )
+from maasserver.views.combo import get_combo_view
+
+
+urlpatterns = patterns('',
+ url(
+ r'^maas/', get_combo_view(rel_location=None),
+ name='combo-maas'),
+ url(
+ r'^raphael/',
+ get_combo_view(
+ abs_location=django_settings.RAPHAELJS_LOCATION,
+ rel_location=['jslibs', 'raphael']),
+ name='combo-raphael'),
+ url(
+ r'^yui/',
+ get_combo_view(
+ abs_location=django_settings.YUI_LOCATION,
+ rel_location=['jslibs', 'yui']),
+ name='combo-yui'),
+)
=== modified file 'src/maasserver/views/combo.py'
--- src/maasserver/views/combo.py 2012-06-13 11:02:58 +0000
+++ src/maasserver/views/combo.py 2012-06-26 10:04:41 +0000
@@ -11,9 +11,10 @@
__metaclass__ = type
__all__ = [
- 'combo_view',
+ 'get_combo_view',
]
+from functools import partial
import os
from convoy.combo import (
@@ -27,23 +28,63 @@
HttpResponseNotFound,
)
-
-def get_yui_location():
- if django_settings.YUI_LOCATION is not None:
- return django_settings.YUI_LOCATION
+# Static root computed from this very file.
+LOCAL_STATIC_ROOT = os.path.join(
+ os.path.dirname(os.path.dirname(__file__)), 'static')
+
+
+def get_location(abs_location=None, rel_location=None):
+ """Return the absolute location of a static ressource.
+
+ If the given absolute location (abs_location) is not None: return it.
+ If the STATIC_ROOT setting is not null, use it as the root for the given
+ relative location.
+ Otherwise, use LOCAL_STATIC_ROOT as the root for the given relative
+ location.
+
+ :param abs_location: An optional absolute location.
+ :type abs_location: basestring
+ :param rel_location: A list of path elements composing the relative
+ location where the static ressources can be found.
+ :param rel_location: A list of path elements or None.
+ :type rel_location: list
+ :return: The absolute path.
+ :rtype: basestring
+ """
+ if rel_location is None:
+ rel_location = ['']
+ if abs_location is not None:
+ return abs_location
elif django_settings.STATIC_ROOT:
return os.path.join(
- django_settings.STATIC_ROOT, 'jslibs', 'yui')
+ django_settings.STATIC_ROOT, *rel_location)
else:
- return os.path.join(
- os.path.dirname(os.path.dirname(__file__)), 'static',
- 'jslibs', 'yui')
-
-
-def combo_view(request):
- """Handle a request for combining a set of files."""
+ return os.path.join(LOCAL_STATIC_ROOT, *rel_location)
+
+
+def get_combo_view(abs_location=None, rel_location=None):
+ """Return a Django view to serve static ressources using a combo loader.
+
+ :param abs_location: An optional absolute location.
+ :type abs_location: basestring or None
+ :param rel_location: A list of path elements composing the relative
+ location where the static ressources can be found.
+ :type rel_location: list or None
+ :return: A Django view method.
+ :rtype: callable
+ """
+ location = get_location(
+ abs_location=abs_location, rel_location=rel_location)
+ return partial(combo_view, location=location)
+
+
+def combo_view(request, location, encoding='utf8'):
+ """Handle a request for combining a set of files.
+
+ The files are searched in the absolute location `abs_location` (if
+ defined) or in the relative location `rel_location`.
+ """
fnames = parse_qs(request.META.get("QUERY_STRING", ""))
- YUI_LOCATION = get_yui_location()
if fnames:
if fnames[0].endswith('.js'):
@@ -52,10 +93,9 @@
content_type = 'text/css'
else:
return HttpResponseBadRequest("Invalid file type requested.")
- content = b"".join(
- combine_files(
- fnames, YUI_LOCATION, resource_prefix='/',
- rewrite_urls=True))
+ content = "".join(
+ [content.decode(encoding) for content in combine_files(
+ fnames, location, resource_prefix='/', rewrite_urls=True)])
return HttpResponse(
content_type=content_type, status=200, content=content)
