launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/file-delete2 into lp:maas with lp:~rvb/maas/file-delete as a prerequisite.

Commit message:
Add support for reading an individual FileStorage object.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1125006 in MAAS: "No API support for deleting or listing files (FileStorage objects)"
  https://bugs.launchpad.net/maas/+bug/1125006

For more details, see:
https://code.launchpad.net/~rvb/maas/file-delete2/+merge/148382
-- 
https://code.launchpad.net/~rvb/maas/file-delete2/+merge/148382
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/file-delete2 into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2013-02-14 08:16:23 +0000
+++ src/maasserver/api.py	2013-02-14 08:16:23 +0000
@@ -190,6 +190,8 @@
     CommissioningScript,
     NodeCommissionResult,
     )
+from piston.emitters import JSONEmitter
+from piston.handler import typemapper
 from piston.utils import rc
 from provisioningserver.enum import POWER_TYPE
 from provisioningserver.kernel_opts import KernelParameters
@@ -810,7 +812,12 @@
         return ('files_handler', [])
 
 
+# DISPLAYED_FILES_FIELDS_OBJECT is the list of fields used when dumping
+# lists of FileStorage objects.
 DISPLAYED_FILES_FIELDS = ('filename', )
+# DISPLAYED_FILES_FIELDS_OBJECT is the list of fields used when dumping
+# individual FileStorage objects.
+DISPLAYED_FILES_FIELDS_OBJECT = DISPLAYED_FILES_FIELDS + ('content', )
 
 
 class FileHandler(OperationsHandler):
@@ -820,7 +827,21 @@
     """
     model = FileStorage
     fields = DISPLAYED_FILES_FIELDS
-    create = read = update = delete = None
+    create = update = delete = None
+
+    def read(self, request, filename):
+        """GET a FileStorage object."""
+        stored_file = get_object_or_404(FileStorage, filename=filename)
+        # Emit the json for this object manually because, no matter what the
+        # piston documentation says, once an type is associated with a list
+        # a fields by piston's typemapper mechanism, there is no way to
+        # override that in a specific handler with 'fields' or 'exclude'.
+        emitter = JSONEmitter(
+            stored_file, typemapper, None, DISPLAYED_FILES_FIELDS_OBJECT)
+        stream = emitter.render(request)
+        return HttpResponse(
+            stream, mimetype='application/json; charset=utf-8',
+            status=httplib.OK)
 
     @classmethod
     def resource_uri(cls, stored_file=None):

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2013-02-14 08:16:23 +0000
+++ src/maasserver/tests/test_api.py	2013-02-14 08:16:23 +0000
@@ -2634,6 +2634,14 @@
         # The 'list' operation is not available to anon users.
         self.assertEqual(httplib.BAD_REQUEST, response.status_code)
 
+    def test_anon_cannot_get_file(self):
+        filename = factory.make_name("file")
+        factory.make_file_storage(
+            filename=filename, content=b"test file content")
+        response = self.client.get(
+            reverse('file_handler', args=[filename]))
+        self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
+
 
 class FileStorageAPITest(FileStorageAPITestMixin, APITestCase):
 
@@ -2752,6 +2760,17 @@
         # The url-escaped name of the file is part of the resource uri.
         self.assertIn(urlquote_plus(filename), resource_uri_elements)
 
+    def test_get_file_returns_file_object(self):
+        filename = factory.make_name("file")
+        content = b"test file content"
+        factory.make_file_storage(filename=filename, content=content)
+        response = self.client.get(
+            reverse('file_handler', args=[filename]))
+        parsed_result = json.loads(response.content)
+        self.assertEqual(
+            (filename, content),
+            (parsed_result['filename'], parsed_result['content']))
+
 
 class TestTagAPI(APITestCase):
     """Tests for /api/1.0/tags/<tagname>/."""