launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/maas/serve-files into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~julian-edwards/maas/serve-files/+merge/92764
-- 
https://code.launchpad.net/~julian-edwards/maas/serve-files/+merge/92764
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/maas/serve-files into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-02-13 10:40:45 +0000
+++ src/maasserver/api.py	2012-02-13 13:18:33 +0000
@@ -12,23 +12,36 @@
 __all__ = [
     "api_doc",
     "generate_api_doc",
+    "FilesHandler",
     "NodeHandler",
     "NodeMacsHandler",
     ]
 
+import httplib
 import types
 
-from django.core.exceptions import ValidationError
-from django.http import HttpResponseBadRequest
+from django.core.exceptions import (
+    ObjectDoesNotExist,
+    ValidationError,
+    )
+from django.http import (
+    HttpResponse,
+    HttpResponseBadRequest,
+    )
 from django.shortcuts import (
     get_object_or_404,
     render_to_response,
     )
 from django.template import RequestContext
 from docutils import core
+from maasserver.exceptions import (
+    MaasAPIBadRequest,
+    MaasAPINotFound,
+    )
 from maasserver.forms import NodeWithMACAddressesForm
 from maasserver.macaddress import validate_mac
 from maasserver.models import (
+    FileStorage,
     MACAddress,
     Node,
     )
@@ -315,6 +328,48 @@
 
 
 @api_operations
+class FilesHandler(BaseHandler):
+    """File management operations."""
+    allowed_methods = ('GET', 'POST',)
+
+    @api_exported('get', 'GET')
+    def get(self, request):
+        """Get a named file."""
+        filename = request.GET.get("filename", None)
+        #import pdb; pdb.set_trace()
+        if not filename:
+            raise MaasAPIBadRequest("Filename not supplied")
+        try:
+            db_file =  FileStorage.objects.get(filename=filename)
+        except ObjectDoesNotExist:
+            raise MaasAPINotFound("File not found")
+        return HttpResponse(db_file.data.read(), status=httplib.OK)
+
+    @api_exported('add', 'POST')
+    def add(self, request):
+        """Create a new Node."""
+        filename = request.data.get("filename", None)
+        if not filename:
+            raise MaasAPIBadRequest("Filename not supplied")
+        files = request.FILES
+        if not files:
+            raise MaasAPIBadRequest("File not supplied")
+        if len(files) != 1:
+            raise MaasAPIBadRequest("Exactly one file must be supplied")
+        uploaded_file = files['file']
+
+        # As per the comment in FileStorage, this ought to deal in
+        # chunks instead of reading the file into memory, but large
+        # files are not expected.
+        storage = FileStorage()
+        storage.save_file(filename, uploaded_file)
+        storage.save()
+
+    @classmethod
+    def resource_uri(cls, *args, **kwargs):
+        return ('files_handler', [])
+
+
 class AccountHandler(BaseHandler):
     """Manage the current logged-in user."""
     allowed_methods = ('POST',)

=== modified file 'src/maasserver/exceptions.py'
--- src/maasserver/exceptions.py	2012-02-13 11:53:47 +0000
+++ src/maasserver/exceptions.py	2012-02-13 13:18:33 +0000
@@ -11,7 +11,9 @@
 __metaclass__ = type
 __all__ = [
     "MaasException",
+    "MaasAPIBadRequest",
     "MaasAPIException",
+    "MaasAPINotFound",
     "PermissionDenied",
     ]
 
@@ -33,5 +35,13 @@
     api_error = httplib.INTERNAL_SERVER_ERROR
 
 
+class MaasAPIBadRequest(MaasAPIException):
+    api_error = httplib.BAD_REQUEST
+
+
+class MaasAPINotFound(MaasAPIException):
+    api_error = httplib.NOT_FOUND
+
+
 class PermissionDenied(MaasAPIException):
     api_error = httplib.UNAUTHORIZED

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-02-13 11:53:47 +0000
+++ src/maasserver/tests/test_api.py	2012-02-13 13:18:33 +0000
@@ -13,8 +13,12 @@
 
 import httplib
 import json
+import os
+import shutil
+from StringIO import StringIO
 import time
 
+from django.conf import settings
 from django.test.client import Client
 from maasserver.models import (
     MACAddress,
@@ -516,3 +520,109 @@
                 self.node.system_id, 'invalid-mac'))
 
         self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+
+
+class FileStorageAPITest(APITestCase):
+
+    def setUp(self):
+        super(FileStorageAPITest, self).setUp()
+        os.mkdir(settings.MEDIA_ROOT)
+        self.tmpdir = os.path.join(settings.MEDIA_ROOT, "testing")
+        os.mkdir(self.tmpdir)
+        self.addCleanup(shutil.rmtree, settings.MEDIA_ROOT)
+
+    def make_file(self, name="foo", contents="test file contents"):
+        """Make a temp file named `name` with contents `contents`.
+
+        :return: The full file path of the file that was created.
+        """
+        filepath = os.path.join(self.tmpdir, name)
+        with open(filepath, "w") as f:
+            f.write(contents)
+        return filepath
+
+    def _create_API_params(self, op=None, filename=None, fileObj=None):
+        params = {}
+        if op is not None:
+            params["op"] = op
+        if filename is not None:
+            params["filename"] = filename
+        if fileObj is not None:
+            params["file"] = fileObj
+        return params
+
+    def make_API_POST_request(self, op=None, filename=None, fileObj=None):
+        """Make an API POST request and return the response."""
+        params = self._create_API_params(op, filename, fileObj)
+        return self.client.post("/api/files/", params)
+
+    def make_API_GET_request(self, op=None, filename=None, fileObj=None):
+        """Make an API GET request and return the response."""
+        params = self._create_API_params(op, filename, fileObj)
+        return self.client.get("/api/files/", params)
+
+    def test_add_file_succeeds(self):
+        filepath = self.make_file()
+
+        with open(filepath) as f:
+            response = self.make_API_POST_request("add", "foo", f)
+
+        self.assertEqual(httplib.OK, response.status_code)
+
+    def test_add_file_fails_with_no_filename(self):
+        filepath = self.make_file()
+
+        with open(filepath) as f:
+            response = self.make_API_POST_request("add", fileObj=f)
+
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+        self.assertIn('text/plain', response['Content-Type'])
+        self.assertEqual("Filename not supplied", response.content)
+
+    def test_add_file_fails_with_no_file_attached(self):
+        response = self.make_API_POST_request("add", "foo")
+
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+        self.assertIn('text/plain', response['Content-Type'])
+        self.assertEqual("File not supplied", response.content)
+
+    def test_add_file_fails_with_too_many_files(self):
+        filepath = self.make_file(name="foo")
+        filepath2 = self.make_file(name="foo2")
+
+        with open(filepath) as f, open(filepath2) as f2:
+            response = self.client.post(
+                "/api/files/",
+                {
+                    "op": "add",
+                    "filename": "foo",
+                    "file": f,
+                    "file2": f2,
+                })
+
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+        self.assertIn('text/plain', response['Content-Type'])
+        self.assertEqual("Exactly one file must be supplied", response.content)
+
+    def test_get_file_succeeds(self):
+        storage = factory.make_file_storage(
+            filename="foofilers", data="give me rope")
+        response = self.make_API_GET_request("get", "foofilers")
+
+        self.assertEqual(httplib.OK, response.status_code)
+        self.assertEqual("give me rope", response.content)
+
+    def test_get_file_fails_with_no_filename(self):
+        response = self.make_API_GET_request("get")
+
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+        self.assertIn('text/plain', response['Content-Type'])
+        self.assertEqual("Filename not supplied", response.content)
+
+    def test_get_file_fails_with_missing_file(self):
+        #import pdb;pdb.set_trace()
+        response = self.make_API_GET_request("get", filename="missingfilename")
+
+        self.assertEqual(httplib.NOT_FOUND, response.status_code)
+        self.assertIn('text/plain', response['Content-Type'])
+        self.assertEqual("File not found", response.content)

=== modified file 'src/maasserver/urls.py'
--- src/maasserver/urls.py	2012-02-10 08:49:01 +0000
+++ src/maasserver/urls.py	2012-02-13 13:18:33 +0000
@@ -23,6 +23,7 @@
 from maasserver.api import (
     AccountHandler,
     api_doc,
+    FilesHandler,
     MaasAPIAuthentication,
     NodeHandler,
     NodeMacHandler,
@@ -66,11 +67,12 @@
 # API.
 auth = MaasAPIAuthentication(realm="MaaS API")
 
+account_handler = Resource(AccountHandler, authentication=auth)
+files_handler = Resource(FilesHandler, authentication=auth)
 node_handler = Resource(NodeHandler, authentication=auth)
 nodes_handler = Resource(NodesHandler, authentication=auth)
 node_mac_handler = Resource(NodeMacHandler, authentication=auth)
 node_macs_handler = Resource(NodeMacsHandler, authentication=auth)
-account_handler = Resource(AccountHandler, authentication=auth)
 
 # API URLs accessible to anonymous users.
 urlpatterns += patterns('',
@@ -90,5 +92,6 @@
         r'^api/nodes/(?P<system_id>[\w\-]+)/$', node_handler,
         name='node_handler'),
     url(r'^api/nodes/$', nodes_handler, name='nodes_handler'),
+    url(r'^api/files/$', files_handler, name='files_handler'),
     url(r'^api/account/$', account_handler, name='account_handler'),
 )