MAAS

Merge lp:~rvb/maas/use-reverse into lp:~maas-committers/maas/trunk

  1. use-reverse
  2. Merge into trunk
Proposed by Raphaël Badin
Status: Merged
Approved by: Raphaël Badin
Approved revision: no longer in the source branch.
Merged at revision: 659
Proposed branch: lp:~rvb/maas/use-reverse
Merge into: lp:~maas-committers/maas/trunk
Prerequisite: lp:~rvb/maas/pxe-off-preseed
Diff against target: 286 lines (+60/-60)
2 files modified
src/maas/urls.py (+1/-1)
src/metadataserver/tests/test_api.py (+59/-59)
To merge this branch: bzr merge lp:~rvb/maas/use-reverse
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Francesco Banconi (community) code* Approve
Review via email: mp+111240@code.launchpad.net

Commit message

Use Django's reverse in src/metadataserver/tests/test_api.py

Description of the change

This branch fixes a problem I spotted when I was working on the pre-req branch.

The metadata application should be hooked up (in src/maas/urls.py) like this:

urlpatterns = patterns('',
    url(r'^', include('maasserver.urls')),
    url(r'^metadata/', include('metadataserver.urls')),
)

Note the trailing slash at the end of '^metadata/'. Otherwise, reverse(...) does not return a correct path.

See https://docs.djangoproject.com/en/dev/topics/http/urls/#naming-url-patterns for details.

I refactored src/metadataserver/tests/test_api.py to use Django's reverse method everywhere. The main benefit is that now we exercise the name=... part in all the urls definitions in src/metadataserver/urls.py (augmented test coverage). Also, note that this whole branch is neutral in terms of added/removed lines.

To post a comment you must log in.
Revision history for this message
Francesco Banconi (frankban) wrote :

Looks good.

review: Approve (code*)
Revision history for this message
Gavin Panella (allenap) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/maas/urls.py'
--- src/maas/urls.py 2012-04-16 10:00:51 +0000
+++ src/maas/urls.py 2012-06-20 16:33:18 +0000
@@ -23,7 +23,7 @@
2323
24urlpatterns = patterns('',24urlpatterns = patterns('',
25 url(r'^', include('maasserver.urls')),25 url(r'^', include('maasserver.urls')),
26 url(r'^metadata', include('metadataserver.urls')),26 url(r'^metadata/', include('metadataserver.urls')),
27)27)
2828
29if settings.STATIC_LOCAL_SERVE:29if settings.STATIC_LOCAL_SERVE:
3030
=== modified file 'src/metadataserver/tests/test_api.py'
--- src/metadataserver/tests/test_api.py 2012-06-20 16:33:18 +0000
+++ src/metadataserver/tests/test_api.py 2012-06-20 16:33:18 +0000
@@ -18,6 +18,7 @@
1818
19from django.conf import settings19from django.conf import settings
20from django.core.exceptions import PermissionDenied20from django.core.exceptions import PermissionDenied
21from django.core.urlresolvers import reverse
21from maasserver.enum import NODE_STATUS22from maasserver.enum import NODE_STATUS
22from maasserver.exceptions import (23from maasserver.exceptions import (
23 MAASAPINotFound,24 MAASAPINotFound,
@@ -128,33 +129,6 @@
128 token = NodeKey.objects.get_token_for_node(node)129 token = NodeKey.objects.get_token_for_node(node)
129 return OAuthAuthenticatedClient(get_node_init_user(), token)130 return OAuthAuthenticatedClient(get_node_init_user(), token)
130131
131 def make_url(self, path):
132 """Create an absolute URL for `path` on the metadata API.
133
134 :param path: Path within the metadata API to access. Should start
135 with a slash.
136 :return: An absolute URL for the given path within the metadata
137 service tree.
138 """
139 assert path.startswith('/'), "Give absolute metadata API path."
140 # Root of the metadata API service.
141 metadata_root = "/metadata"
142 return metadata_root + path
143
144 def get(self, path, client=None):
145 """GET a resource from the metadata API.
146
147 :param path: Path within the metadata API to access. Should start
148 with a slash.
149 :param token: If given, authenticate the request using this token.
150 :type token: oauth.oauth.OAuthToken
151 :return: A response to the GET request.
152 :rtype: django.http.HttpResponse
153 """
154 if client is None:
155 client = self.client
156 return client.get(self.make_url(path))
157
158 def call_signal(self, client=None, version='latest', files={}, **kwargs):132 def call_signal(self, client=None, version='latest', files={}, **kwargs):
159 """Call the API's signal method.133 """Call the API's signal method.
160134
@@ -177,37 +151,43 @@
177 for name, content in files.items():151 for name, content in files.items():
178 params[name] = BytesIO(content)152 params[name] = BytesIO(content)
179 params[name].name = name153 params[name].name = name
180 return client.post(self.make_url('/%s/' % version), params)154 url = reverse('metadata-version', args=[version])
155 return client.post(url, params)
181156
182 def test_no_anonymous_access(self):157 def test_no_anonymous_access(self):
183 self.assertEqual(httplib.UNAUTHORIZED, self.get('/').status_code)158 self.assertEqual(
159 httplib.UNAUTHORIZED,
160 self.client.get(reverse('metadata')).status_code)
184161
185 def test_metadata_index_shows_latest(self):162 def test_metadata_index_shows_latest(self):
186 client = self.make_node_client()163 client = self.make_node_client()
187 self.assertIn('latest', self.get('/', client).content)164 self.assertIn('latest', client.get(reverse('metadata')).content)
188165
189 def test_metadata_index_shows_only_known_versions(self):166 def test_metadata_index_shows_only_known_versions(self):
190 client = self.make_node_client()167 client = self.make_node_client()
191 for item in self.get('/', client).content.splitlines():168 for item in client.get(reverse('metadata')).content.splitlines():
192 check_version(item)169 check_version(item)
193 # The test is that we get here without exception.170 # The test is that we get here without exception.
194 pass171 pass
195172
196 def test_version_index_shows_meta_data(self):173 def test_version_index_shows_meta_data(self):
197 client = self.make_node_client()174 client = self.make_node_client()
198 items = self.get('/latest/', client).content.splitlines()175 url = reverse('metadata-version', args=['latest'])
176 items = client.get(url).content.splitlines()
199 self.assertIn('meta-data', items)177 self.assertIn('meta-data', items)
200178
201 def test_version_index_does_not_show_user_data_if_not_available(self):179 def test_version_index_does_not_show_user_data_if_not_available(self):
202 client = self.make_node_client()180 client = self.make_node_client()
203 items = self.get('/latest/', client).content.splitlines()181 url = reverse('metadata-version', args=['latest'])
182 items = client.get(url).content.splitlines()
204 self.assertNotIn('user-data', items)183 self.assertNotIn('user-data', items)
205184
206 def test_version_index_shows_user_data_if_available(self):185 def test_version_index_shows_user_data_if_available(self):
207 node = factory.make_node()186 node = factory.make_node()
208 NodeUserData.objects.set_user_data(node, b"User data for node")187 NodeUserData.objects.set_user_data(node, b"User data for node")
209 client = self.make_node_client(node)188 client = self.make_node_client(node)
210 items = self.get('/latest/', client).content.splitlines()189 url = reverse('metadata-version', args=['latest'])
190 items = client.get(url).content.splitlines()
211 self.assertIn('user-data', items)191 self.assertIn('user-data', items)
212192
213 def test_meta_data_view_lists_fields(self):193 def test_meta_data_view_lists_fields(self):
@@ -215,20 +195,23 @@
215 user, _ = factory.make_user_with_keys(n_keys=2, username='my-user')195 user, _ = factory.make_user_with_keys(n_keys=2, username='my-user')
216 node = factory.make_node(owner=user)196 node = factory.make_node(owner=user)
217 client = self.make_node_client(node=node)197 client = self.make_node_client(node=node)
218 response = self.get('/latest/meta-data/', client)198 url = reverse('metadata-meta-data', args=['latest', ''])
199 response = client.get(url)
219 self.assertIn('text/plain', response['Content-Type'])200 self.assertIn('text/plain', response['Content-Type'])
220 self.assertItemsEqual(201 self.assertItemsEqual(
221 MetaDataHandler.fields, response.content.split())202 MetaDataHandler.fields, response.content.split())
222203
223 def test_meta_data_view_is_sorted(self):204 def test_meta_data_view_is_sorted(self):
224 client = self.make_node_client()205 client = self.make_node_client()
225 response = self.get('/latest/meta-data/', client)206 url = reverse('metadata-meta-data', args=['latest', ''])
207 response = client.get(url)
226 attributes = response.content.split()208 attributes = response.content.split()
227 self.assertEqual(sorted(attributes), attributes)209 self.assertEqual(sorted(attributes), attributes)
228210
229 def test_meta_data_unknown_item_is_not_found(self):211 def test_meta_data_unknown_item_is_not_found(self):
230 client = self.make_node_client()212 client = self.make_node_client()
231 response = self.get('/latest/meta-data/UNKNOWN-ITEM-HA-HA-HA', client)213 url = reverse('metadata-meta-data', args=['latest', 'UNKNOWN-ITEM'])
214 response = client.get(url)
232 self.assertEqual(httplib.NOT_FOUND, response.status_code)215 self.assertEqual(httplib.NOT_FOUND, response.status_code)
233216
234 def test_get_attribute_producer_supports_all_fields(self):217 def test_get_attribute_producer_supports_all_fields(self):
@@ -239,7 +222,8 @@
239 def test_meta_data_local_hostname_returns_hostname(self):222 def test_meta_data_local_hostname_returns_hostname(self):
240 hostname = factory.getRandomString()223 hostname = factory.getRandomString()
241 client = self.make_node_client(factory.make_node(hostname=hostname))224 client = self.make_node_client(factory.make_node(hostname=hostname))
242 response = self.get('/latest/meta-data/local-hostname', client)225 url = reverse('metadata-meta-data', args=['latest', 'local-hostname'])
226 response = client.get(url)
243 self.assertEqual(227 self.assertEqual(
244 (httplib.OK, hostname),228 (httplib.OK, hostname),
245 (response.status_code, response.content.decode('ascii')))229 (response.status_code, response.content.decode('ascii')))
@@ -248,7 +232,8 @@
248 def test_meta_data_instance_id_returns_system_id(self):232 def test_meta_data_instance_id_returns_system_id(self):
249 node = factory.make_node()233 node = factory.make_node()
250 client = self.make_node_client(node)234 client = self.make_node_client(node)
251 response = self.get('/latest/meta-data/instance-id', client)235 url = reverse('metadata-meta-data', args=['latest', 'instance-id'])
236 response = client.get(url)
252 self.assertEqual(237 self.assertEqual(
253 (httplib.OK, node.system_id),238 (httplib.OK, node.system_id),
254 (response.status_code, response.content.decode('ascii')))239 (response.status_code, response.content.decode('ascii')))
@@ -259,39 +244,45 @@
259 node = factory.make_node()244 node = factory.make_node()
260 NodeUserData.objects.set_user_data(node, data)245 NodeUserData.objects.set_user_data(node, data)
261 client = self.make_node_client(node)246 client = self.make_node_client(node)
262 response = self.get('/latest/user-data', client)247 response = client.get(reverse('metadata-user-data', args=['latest']))
263 self.assertEqual('application/octet-stream', response['Content-Type'])248 self.assertEqual('application/octet-stream', response['Content-Type'])
264 self.assertIsInstance(response.content, str)249 self.assertIsInstance(response.content, str)
265 self.assertEqual(250 self.assertEqual(
266 (httplib.OK, data), (response.status_code, response.content))251 (httplib.OK, data), (response.status_code, response.content))
267252
268 def test_user_data_for_node_without_user_data_returns_not_found(self):253 def test_user_data_for_node_without_user_data_returns_not_found(self):
269 response = self.get('/latest/user-data', self.make_node_client())254 client = self.make_node_client()
255 response = client.get(reverse('metadata-user-data', args=['latest']))
270 self.assertEqual(httplib.NOT_FOUND, response.status_code)256 self.assertEqual(httplib.NOT_FOUND, response.status_code)
271257
272 def test_public_keys_not_listed_for_node_without_public_keys(self):258 def test_public_keys_not_listed_for_node_without_public_keys(self):
273 response = self.get('/latest/meta-data/', self.make_node_client())259 url = reverse('metadata-meta-data', args=['latest', ''])
260 client = self.make_node_client()
261 response = client.get(url)
274 self.assertNotIn(262 self.assertNotIn(
275 'public-keys', response.content.decode('ascii').split('\n'))263 'public-keys', response.content.decode('ascii').split('\n'))
276264
277 def test_public_keys_listed_for_node_with_public_keys(self):265 def test_public_keys_listed_for_node_with_public_keys(self):
278 user, _ = factory.make_user_with_keys(n_keys=2, username='my-user')266 user, _ = factory.make_user_with_keys(n_keys=2, username='my-user')
279 node = factory.make_node(owner=user)267 node = factory.make_node(owner=user)
280 response = self.get(268 url = reverse('metadata-meta-data', args=['latest', ''])
281 '/latest/meta-data/', self.make_node_client(node=node))269 client = self.make_node_client(node=node)
270 response = client.get(url)
282 self.assertIn(271 self.assertIn(
283 'public-keys', response.content.decode('ascii').split('\n'))272 'public-keys', response.content.decode('ascii').split('\n'))
284273
285 def test_public_keys_for_node_without_public_keys_returns_not_found(self):274 def test_public_keys_for_node_without_public_keys_returns_not_found(self):
286 response = self.get(275 url = reverse('metadata-meta-data', args=['latest', 'public-keys'])
287 '/latest/meta-data/public-keys', self.make_node_client())276 client = self.make_node_client()
277 response = client.get(url)
288 self.assertEqual(httplib.NOT_FOUND, response.status_code)278 self.assertEqual(httplib.NOT_FOUND, response.status_code)
289279
290 def test_public_keys_for_node_returns_list_of_keys(self):280 def test_public_keys_for_node_returns_list_of_keys(self):
291 user, _ = factory.make_user_with_keys(n_keys=2, username='my-user')281 user, _ = factory.make_user_with_keys(n_keys=2, username='my-user')
292 node = factory.make_node(owner=user)282 node = factory.make_node(owner=user)
293 response = self.get(283 url = reverse('metadata-meta-data', args=['latest', 'public-keys'])
294 '/latest/meta-data/public-keys', self.make_node_client(node=node))284 client = self.make_node_client(node=node)
285 response = client.get(url)
295 self.assertEqual(httplib.OK, response.status_code)286 self.assertEqual(httplib.OK, response.status_code)
296 keys = SSHKey.objects.filter(user=user).values_list('key', flat=True)287 keys = SSHKey.objects.filter(user=user).values_list('key', flat=True)
297 expected_response = '\n'.join(keys)288 expected_response = '\n'.join(keys)
@@ -320,7 +311,8 @@
320 def test_signaling_requires_status_code(self):311 def test_signaling_requires_status_code(self):
321 node = factory.make_node(status=NODE_STATUS.COMMISSIONING)312 node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
322 client = self.make_node_client(node=node)313 client = self.make_node_client(node=node)
323 response = client.post(self.make_url('/latest/'), {'op': 'signal'})314 url = reverse('metadata-version', args=['latest'])
315 response = client.post(url, {'op': 'signal'})
324 self.assertEqual(httplib.BAD_REQUEST, response.status_code)316 self.assertEqual(httplib.BAD_REQUEST, response.status_code)
325317
326 def test_signaling_rejects_unknown_status_code(self):318 def test_signaling_rejects_unknown_status_code(self):
@@ -505,8 +497,10 @@
505497
506 def test_api_retrieves_node_metadata_by_mac(self):498 def test_api_retrieves_node_metadata_by_mac(self):
507 mac = factory.make_mac_address()499 mac = factory.make_mac_address()
508 response = self.get(500 url = reverse(
509 '/latest/by-mac/%s/meta-data/instance-id/' % mac.mac_address)501 'metadata-meta-data-by-mac',
502 args=['latest', mac.mac_address, 'instance-id'])
503 response = self.client.get(url)
510 self.assertEqual(504 self.assertEqual(
511 (httplib.OK, mac.node.system_id),505 (httplib.OK, mac.node.system_id),
512 (response.status_code, response.content))506 (response.status_code, response.content))
@@ -515,7 +509,9 @@
515 mac = factory.make_mac_address()509 mac = factory.make_mac_address()
516 user_data = factory.getRandomString().encode('ascii')510 user_data = factory.getRandomString().encode('ascii')
517 NodeUserData.objects.set_user_data(mac.node, user_data)511 NodeUserData.objects.set_user_data(mac.node, user_data)
518 response = self.get('/latest/by-mac/%s/user-data' % mac.mac_address)512 url = reverse(
513 'metadata-user-data-by-mac', args=['latest', mac.mac_address])
514 response = self.client.get(url)
519 self.assertEqual(515 self.assertEqual(
520 (httplib.OK, user_data),516 (httplib.OK, user_data),
521 (response.status_code, response.content))517 (response.status_code, response.content))
@@ -523,29 +519,33 @@
523 def test_api_normally_disallows_anonymous_node_metadata_access(self):519 def test_api_normally_disallows_anonymous_node_metadata_access(self):
524 self.patch(settings, 'ALLOW_ANONYMOUS_METADATA_ACCESS', False)520 self.patch(settings, 'ALLOW_ANONYMOUS_METADATA_ACCESS', False)
525 mac = factory.make_mac_address()521 mac = factory.make_mac_address()
526 response = self.get(522 url = reverse(
527 '/latest/by-mac/%s/meta-data/instance-id/' % mac.mac_address)523 'metadata-meta-data-by-mac',
524 args=['latest', mac.mac_address, 'instance-id'])
525 response = self.client.get(url)
528 self.assertEqual(httplib.FORBIDDEN, response.status_code)526 self.assertEqual(httplib.FORBIDDEN, response.status_code)
529527
530 def test_netboot_off(self):528 def test_netboot_off(self):
531 node = factory.make_node(netboot=True)529 node = factory.make_node(netboot=True)
532 client = self.make_node_client(node=node)530 client = self.make_node_client(node=node)
533 response = client.post(531 url = reverse('metadata-version', args=['latest'])
534 self.make_url('/latest/'), {'op': 'netboot_off'})532 response = client.post(url, {'op': 'netboot_off'})
535 node = reload_object(node)533 node = reload_object(node)
536 self.assertFalse(node.netboot, response)534 self.assertFalse(node.netboot, response)
537535
538 def test_netboot_on(self):536 def test_netboot_on(self):
539 node = factory.make_node(netboot=False)537 node = factory.make_node(netboot=False)
540 client = self.make_node_client(node=node)538 client = self.make_node_client(node=node)
541 response = client.post(self.make_url('/latest/'), {'op': 'netboot_on'})539 url = reverse('metadata-version', args=['latest'])
540 response = client.post(url, {'op': 'netboot_on'})
542 node = reload_object(node)541 node = reload_object(node)
543 self.assertTrue(node.netboot, response)542 self.assertTrue(node.netboot, response)
544543
545 def test_anonymous_netboot_off(self):544 def test_anonymous_netboot_off(self):
546 node = factory.make_node(netboot=True)545 node = factory.make_node(netboot=True)
547 anon_netboot_off_url = self.make_url(546 anon_netboot_off_url = reverse(
548 '/latest/%s/edit/' % node.system_id)547 'metadata-anon-node-edit', args=['latest',
548 node.system_id])
549 response = self.client.post(549 response = self.client.post(
550 anon_netboot_off_url, {'op': 'netboot_off'})550 anon_netboot_off_url, {'op': 'netboot_off'})
551 node = reload_object(node)551 node = reload_object(node)