Merge lp:~stevanr/linaro-license-protection/bug-1085007 into lp:~linaro-automation/linaro-license-protection/trunk

Proposed by Stevan Radaković
Status: Merged
Approved by: Данило Шеган
Approved revision: 174
Merged at revision: 171
Proposed branch: lp:~stevanr/linaro-license-protection/bug-1085007
Merge into: lp:~linaro-automation/linaro-license-protection/trunk
Diff against target: 197 lines (+105/-4)
6 files modified
license_protected_downloads/tests/helpers.py (+43/-0)
license_protected_downloads/tests/test_views.py (+29/-1)
license_protected_downloads/views.py (+17/-0)
settings.py (+6/-0)
templates/header.html (+6/-3)
urls.py (+4/-0)
To merge this branch: bzr merge lp:~stevanr/linaro-license-protection/bug-1085007
Reviewer Review Type Date Requested Status
Данило Шеган (community) Approve
Review via email: mp+150646@code.launchpad.net

Description of the change

Additional fix for bug 1085007.
When certificates for www.linaro.org are fixed, this code will become obsolete and may be removed.

To post a comment you must log in.
Revision history for this message
Данило Шеган (danilo) wrote :
Download full text (3.2 KiB)

Hi Stevan,

Thanks for working on this. It's very close to being ready to land :)

У уто, 26. 02 2013. у 19:43 +0000, Stevan Radaković пише:
> === modified file 'license_protected_downloads/views.py'

> @@ -479,3 +480,13 @@
> path = result[1]
>
> return HttpResponse(json.dumps(RenderTextFiles.find_and_render(path)))
> +
> +
> +def get_remote_static(request):
> +

Please provide a docstring explaining what this does. And replace this blank line with it.

> + name = request.GET.get("name")
> + if name not in settings.SUPPORTED_REMOTE_STATIC_FILES:
> + raise Http404

Please add a message to this exception: it's always a good practice to
do that.

> +
> + data = urllib2.urlopen(settings.SUPPORTED_REMOTE_STATIC_FILES[name])
> + return HttpResponse(data)
>
> === modified file 'settings.py'
> --- settings.py 2012-11-29 09:36:04 +0000
> +++ settings.py 2013-02-26 19:42:24 +0000
> @@ -191,3 +191,9 @@
> 'Building From Source',
> 'Firmware Update',
> 'RTSM']
> +
> +SUPPORTED_REMOTE_STATIC_FILES = {
> + "linarofamily.js": "http://www.linaro.org/remote/js/linarofamily.js",
> + "init.css": "http://www.linaro.org/remote/css/init.css",
> + "remote.css": "http://www.linaro.org/remote/css/remote.css",
> + }
>
> === modified file 'templates/header.html'
> --- templates/header.html 2013-02-21 08:48:00 +0000
> +++ templates/header.html 2013-02-26 19:42:24 +0000
> @@ -5,9 +5,9 @@
> {% endif %}
> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
> <title>Linaro Snapshots</title>
> - <link href="//www.linaro.org/remote/css/init.css" rel="stylesheet" type="text/css" >
> - <link href="//www.linaro.org/remote/css/remote.css" rel="stylesheet" type="text/css" >
> - <script language="javascript" type="text/javascript" src="//www.linaro.org/remote/js/linarofamily.js"></script>
> + <link href="/get-remote-static?name=init.css" rel="stylesheet" type="text/css" >
> + <link href="/get-remote-static?name=remote.css" rel="stylesheet" type="text/css" >
> + <script language="javascript" type="text/javascript" src="/get-remote-static?name=linarofamily.js"></script>

This seems to break the line length limit: please fix this by breaking
into multiple lines where needed to meet our 79-character maximum line
length.

>
> <script type="text/javascript" src="/js/jquery-1.7.2.js"></script>
>
> === modified file 'urls.py'
> --- urls.py 2012-10-22 12:48:48 +0000
> +++ urls.py 2013-02-26 19:42:24 +0000
> @@ -22,6 +22,10 @@
> url(r'^css/(?P<path>.*)$', 'django.views.static.serve',
> {'document_root': settings.CSS_PATH}),
>
> + url(r'^get-remote-static',
> + 'license_protected_downloads.views.get_remote_static',
> + name='get_remote_static'),
> +
> # The license page...
> url(r'^license$',
> 'license_protected_downloads.views.show_license',

Btw, any reason there're no tests for this? Seems pretty simple to test
the get_remote_static() view (you might want to override settings in a
test though). It would be good to test the "negative" case as well: the
fact that it throws a 404 wh...

Read more...

review: Needs Fixing
171. By Stevan Radaković

Add tests, docstrings and fix pep8 in html.

172. By Stevan Radaković

Add test server for urllib testing.

173. By Stevan Radaković

Add test server helper from danilos branch.

174. By Stevan Radaković

Add test for non-existing file. Add exception handling for HTTPError.

Revision history for this message
Данило Шеган (danilo) wrote :

Thanks for the improvements. Please extend the comment in get_remote_static to say how we should switch the error to Http404 once we start sending the email to infrastructure-errors.

review: Approve
175. By Stevan Radaković

Add additional comment for re-raising error.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'license_protected_downloads/tests/helpers.py'
2--- license_protected_downloads/tests/helpers.py 2012-11-29 09:36:04 +0000
3+++ license_protected_downloads/tests/helpers.py 2013-02-28 10:08:20 +0000
4@@ -4,6 +4,9 @@
5 import os
6 import shutil
7 import tempfile
8+import threading
9+import BaseHTTPServer
10+import SocketServer
11
12
13 class temporary_directory(object):
14@@ -39,3 +42,43 @@
15 target.write(data)
16 target.close()
17 return full_path
18+
19+
20+class HttpHandler(BaseHTTPServer.BaseHTTPRequestHandler):
21+ def do_GET(self):
22+ if self.path in self.urls:
23+ self.send_response(200, "OK")
24+ self.end_headers()
25+ self.request.sendall(self.urls[self.path])
26+ else:
27+ self.send_error(404, 'URL %s not found' % self.path)
28+ self.end_headers()
29+
30+
31+class ThreadedHTTPServer(SocketServer.ThreadingMixIn,
32+ BaseHTTPServer.HTTPServer):
33+ pass
34+
35+
36+class TestHttpServer(object):
37+ """Creates a context manager for a temporary directory."""
38+
39+ def __init__(self, urls):
40+ self.handler = HttpHandler
41+ self.handler.urls = urls
42+
43+ def __enter__(self):
44+ self.server = ThreadedHTTPServer(
45+ ("localhost", 0), self.handler)
46+ self.ip, self.port = self.server.server_address
47+ self.thread = threading.Thread(target=self.server.serve_forever)
48+ self.thread.daemon = True
49+ self.thread.start()
50+ return self
51+
52+ def __exit__(self, *args):
53+ self.server.shutdown()
54+
55+ @property
56+ def base_url(self):
57+ return "http://%s:%s" % (self.ip, self.port)
58
59=== modified file 'license_protected_downloads/tests/test_views.py'
60--- license_protected_downloads/tests/test_views.py 2013-02-26 08:38:32 +0000
61+++ license_protected_downloads/tests/test_views.py 2013-02-28 10:08:20 +0000
62@@ -6,18 +6,19 @@
63 import os
64 import tempfile
65 import unittest
66+import urllib2
67 import urlparse
68
69 from license_protected_downloads import bzr_version
70 from license_protected_downloads.buildinfo import BuildInfo
71 from license_protected_downloads.config import INTERNAL_HOSTS
72 from license_protected_downloads.tests.helpers import temporary_directory
73+from license_protected_downloads.tests.helpers import TestHttpServer
74 from license_protected_downloads.views import _insert_license_into_db
75 from license_protected_downloads.views import _process_include_tags
76 from license_protected_downloads.views import _sizeof_fmt
77 from license_protected_downloads.views import is_same_parent_dir
78
79-
80 THIS_DIRECTORY = os.path.dirname(os.path.abspath(__file__))
81 TESTSERVER_ROOT = os.path.join(THIS_DIRECTORY, "testserver_root")
82
83@@ -664,6 +665,33 @@
84 fname = os.path.join(TESTSERVER_ROOT, "../file")
85 self.assertFalse(is_same_parent_dir(TESTSERVER_ROOT, fname))
86
87+ def test_get_remote_static_unsupported_file(self):
88+ response = self.client.get('/get-remote-static?name=unsupported.css')
89+ self.assertEqual(response.status_code, 404)
90+
91+ def test_get_remote_static_nonexisting_file(self):
92+ pages = {"/": "index"}
93+
94+ with TestHttpServer(pages) as http_server:
95+ css_url = '%s/init.css' % http_server.base_url
96+ settings.SUPPORTED_REMOTE_STATIC_FILES = {
97+ 'init.css': css_url}
98+
99+ self.assertRaises(urllib2.HTTPError, self.client.get,
100+ '/get-remote-static?name=init.css')
101+
102+ def test_get_remote_static(self):
103+ pages = {"/": "index", "/init.css": "test CSS"}
104+
105+ with TestHttpServer(pages) as http_server:
106+ css_url = '%s/init.css' % http_server.base_url
107+ settings.SUPPORTED_REMOTE_STATIC_FILES = {
108+ 'init.css': css_url}
109+
110+ response = self.client.get('/get-remote-static?name=init.css')
111+ self.assertEqual(response.status_code, 200)
112+ self.assertContains(response, 'test CSS')
113+
114
115 class HowtoViewTests(BaseServeViewTest):
116 def test_no_howtos(self):
117
118=== modified file 'license_protected_downloads/views.py'
119--- license_protected_downloads/views.py 2013-02-21 12:09:12 +0000
120+++ license_protected_downloads/views.py 2013-02-28 10:08:20 +0000
121@@ -4,6 +4,7 @@
122 import mimetypes
123 import os
124 import re
125+import urllib2
126 from mimetypes import guess_type
127 from datetime import datetime
128
129@@ -479,3 +480,19 @@
130 path = result[1]
131
132 return HttpResponse(json.dumps(RenderTextFiles.find_and_render(path)))
133+
134+
135+def get_remote_static(request):
136+ """Fetches remote static files based on the dict map in settings.py."""
137+ name = request.GET.get("name")
138+ if name not in settings.SUPPORTED_REMOTE_STATIC_FILES:
139+ raise Http404("File name not supported.")
140+
141+ try:
142+ data = urllib2.urlopen(settings.SUPPORTED_REMOTE_STATIC_FILES[name])
143+ except urllib2.HTTPError:
144+ # TODO: send an email to infrastructure-errors list,
145+ # then implement raising of Http404 instead of HTTPError
146+ raise
147+
148+ return HttpResponse(data)
149
150=== modified file 'settings.py'
151--- settings.py 2012-11-29 09:36:04 +0000
152+++ settings.py 2013-02-28 10:08:20 +0000
153@@ -191,3 +191,9 @@
154 'Building From Source',
155 'Firmware Update',
156 'RTSM']
157+
158+SUPPORTED_REMOTE_STATIC_FILES = {
159+ "linarofamily.js": "http://www.linaro.org/remote/js/linarofamily.js",
160+ "init.css": "http://www.linaro.org/remote/css/init.css",
161+ "remote.css": "http://www.linaro.org/remote/css/remote.css",
162+ }
163
164=== modified file 'templates/header.html'
165--- templates/header.html 2013-02-21 08:48:00 +0000
166+++ templates/header.html 2013-02-28 10:08:20 +0000
167@@ -5,9 +5,12 @@
168 {% endif %}
169 <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
170 <title>Linaro Snapshots</title>
171- <link href="//www.linaro.org/remote/css/init.css" rel="stylesheet" type="text/css" >
172- <link href="//www.linaro.org/remote/css/remote.css" rel="stylesheet" type="text/css" >
173- <script language="javascript" type="text/javascript" src="//www.linaro.org/remote/js/linarofamily.js"></script>
174+ <link href="/get-remote-static?name=init.css"
175+ rel="stylesheet" type="text/css" >
176+ <link href="/get-remote-static?name=remote.css"
177+ rel="stylesheet" type="text/css" >
178+ <script language="javascript" type="text/javascript"
179+ src="/get-remote-static?name=linarofamily.js"></script>
180
181
182 <script type="text/javascript" src="/js/jquery-1.7.2.js"></script>
183
184=== modified file 'urls.py'
185--- urls.py 2012-10-22 12:48:48 +0000
186+++ urls.py 2013-02-28 10:08:20 +0000
187@@ -22,6 +22,10 @@
188 url(r'^css/(?P<path>.*)$', 'django.views.static.serve',
189 {'document_root': settings.CSS_PATH}),
190
191+ url(r'^get-remote-static',
192+ 'license_protected_downloads.views.get_remote_static',
193+ name='get_remote_static'),
194+
195 # The license page...
196 url(r'^license$',
197 'license_protected_downloads.views.show_license',

Subscribers

People subscribed via source and target branches