Merge pull request #12 from Cadasta/delete-file

Fix #9 -- Delete files from S3

README.rst

@@ -215,11 +215,14 @@ If you plan to use a custom widget in your forms, you can add a Django
 API
 -------------------------------------------------------------------------------
 
+Getting a signed URL
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
 If you are building an API-only application, you can get a signed URL by
 POSTing :code:`client_method` and :code:`http_method`.
 
 Request
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+`````````````
 
 .. code-block::
 
@@ -232,7 +235,7 @@ Request
   }
 
 Response
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+`````````````
 
 .. code-block::
 
@@ -296,3 +299,41 @@ response and include all :code:`fields` with the request payload.
     :target: https://travis-ci.org/Cadasta/django-buckets
 .. |pypi-version| image:: https://img.shields.io/pypi/v/django-buckets.svg
     :target: https://pypi.python.org/pypi/django-buckets
+
+
+Deleting a file
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Request
+`````````````
+
+.. code-block::
+
+  POST /s3/delete-resource/
+  Accept: application/json
+  Content-Type: application/json
+
+  {
+    "key": "file.txt"
+  }
+
+
+Response
+`````````````
+
+*When the file was deleted successfully:*
+
+.. code-block::
+
+  HTTP/1.1 204 No Content
+
+*When the file was not found:*
+
+.. code-block::
+
+  HTTP/1.1 400 Bad Request
+  Content-Type: application/json
+
+  {
+    "error": "S3 resource does not exist."
+  }

buckets/__init__.py

@@ -1 +1 @@
-__version__ = '0.1.16'
+__version__ = '0.1.17'

buckets/exceptions.py

@@ -2,3 +2,7 @@ class InvalidPayload(BaseException):
     def __init__(self, errors={}, *args, **kwargs):
         super(InvalidPayload, self).__init__(*args, **kwargs)
         self.errors = errors
+
+
+class S3ResourceNotFound(BaseException):
+    pass

buckets/fields.py

@@ -30,7 +30,12 @@ def _set_file(self, file):
         self.committed = False
 
     def _del_file(self):
-        self.storage.delete(self.url)
+        name = self.url.split('/')[-1]
+
+        if self.field.upload_to:
+            name = self.field.upload_to + '/' + name
+
+        self.storage.delete(name)
         if hasattr(self, '_file'):
             del self._file
 

buckets/static/buckets/js/script.js

@@ -156,9 +156,17 @@
         e.preventDefault();
 
         var el = e.target.parentElement.parentElement;
-        el.querySelector('.file-url').value = '';
-        el.querySelector('.file-input').value = '';
-        el.classList.remove('uploaded');
+        var url = el.getAttribute('data-upload-to') + '/' + el.querySelector('.file-link').innerHTML,
+            headers  = { // 'content-type': 'application/json',
+                        'X-CSRFToken': getCookie('csrftoken')},
+            form = new FormData();
+            form.append('key', url);
+
+        request('POST', '/s3/delete-resource/', form, headers, null, function() {
+            el.querySelector('.file-url').value = '';
+            el.querySelector('.file-input').value = '';
+            el.classList.remove('uploaded');
+        });
     }
 
     function addEventHandlers(el) {

buckets/storage.py

@@ -6,6 +6,7 @@
 import boto3
 from botocore.client import Config
 from botocore.exceptions import ClientError
+from buckets.exceptions import S3ResourceNotFound
 
 from .utils import validate_settings, random_id, ensure_dirs
 
@@ -57,6 +58,13 @@ def _save(self, name, content):
         return 'https://s3-{}.amazonaws.com/{}/{}'.format(
             self.region, self.bucket_name, name)
 
+    def delete(self, name):
+        s3 = self.get_boto_ressource()
+        if self.exists(name):
+            s3.Object(self.bucket_name, name).delete()
+        else:
+            raise S3ResourceNotFound()
+
     def exists(self, name):
         s3 = self.get_boto_ressource()
         try:

buckets/test/mocks.py

@@ -16,8 +16,11 @@ def teardown():
     request.addfinalizer(teardown)
 
 
-def create_file():
-    path = os.path.join(settings.MEDIA_ROOT, 's3', 'text.txt')
+def create_file(subdir=None, name='text.txt'):
+    path = 's3'
+    if subdir:
+        path += '/' + subdir
+    path = os.path.join(settings.MEDIA_ROOT, path, name)
     file = open(path, 'wb')
     file.write('Some content'.encode('utf-8'))
     file.close()

buckets/test/storage.py

@@ -3,6 +3,7 @@
 
 from django.conf import settings
 from buckets.utils import ensure_dirs, random_id
+from buckets.exceptions import S3ResourceNotFound
 
 
 class FakeS3Storage(object):
@@ -35,10 +36,15 @@ def save(self, name, content):
 
         return url
 
-    def delete(self, url):
-        name = os.path.basename(urllib.request.url2pathname(url))
+    def delete(self, name):
+        print(name)
+        print(self.dir)
         uploaded = os.path.join(self.dir, 'uploads', name)
-        os.remove(uploaded)
+        print(uploaded)
+        try:
+            os.remove(uploaded)
+        except:
+            raise S3ResourceNotFound()
 
     def exists(self, key):
         path = os.path.join(self.dir, 'uploads', key)

buckets/urls.py

@@ -3,4 +3,6 @@
 
 urlpatterns = [
     url(r'^s3/signed-url/$', views.signed_url, name='s3_signed_url'),
+    url(r'^s3/delete-resource/$',
+        views.delete_resource, name='s3_delete_resource'),
 ]

buckets/views.py

@@ -1,9 +1,9 @@
 from django.views.decorators.http import require_POST
-from django.http import JsonResponse
+from django.http import JsonResponse, HttpResponse
 from django.utils.translation import ugettext as _
 from django.core.files.storage import default_storage
 
-from buckets.exceptions import InvalidPayload
+from buckets.exceptions import InvalidPayload, S3ResourceNotFound
 
 
 def validate_payload(payload):
@@ -32,3 +32,13 @@ def signed_url(request):
             status = 400
 
     return JsonResponse(response, status=status)
+
+
+@require_POST
+def delete_resource(request):
+    try:
+        default_storage.delete(request.POST['key'])
+        return HttpResponse('', status=204)
+    except S3ResourceNotFound:
+        return JsonResponse({'error': _("S3 resource does not exist.")},
+                            status=400)

tests/test_fields.py

@@ -62,9 +62,9 @@ def test_delete_file(make_dirs):  # noqa
               's3', 'uploads', 'text.txt'), 'wb') as dest_file:
         dest_file.write(open(file.name, 'rb').read())
 
-    field = S3FileField(upload_to='uploads', storage=FakeS3Storage())
+    field = S3FileField(storage=FakeS3Storage())
 
-    s3_file = S3File('/media/uploads/text.txt', field)
+    s3_file = S3File('/media/s3/uploads/text.txt', field)
     s3_file.file = dest_file
     s3_file.delete()
 

tests/test_storage.py

@@ -1,10 +1,12 @@
-from django.conf import settings
-
+import pytest
 import boto3
 from botocore.client import Config
+from botocore.exceptions import ClientError
+from django.conf import settings
 
 from buckets.storage import S3Storage
 from buckets.test.mocks import create_file, make_dirs  # noqa
+from buckets.exceptions import S3ResourceNotFound
 
 
 def get_boto_resource(storage):
@@ -59,3 +61,21 @@ def test_upload_file(make_dirs):  # noqa
     s3 = get_boto_resource(storage)
     o = s3.Object(storage.bucket_name, 'test/' + name).get()
     assert("Some content" in o['Body'].read(o['ContentLength']).decode())
+
+
+def test_delete_file(make_dirs):  # noqa
+    storage = S3Storage()
+    s3 = get_boto_resource(storage)
+    s3.Object(storage.bucket_name, 'test/delete.txt').put(Body=b'content')
+
+    storage.delete('test/delete.txt')
+
+    with pytest.raises(ClientError) as e:
+        s3.Object(storage.bucket_name, 'test/delete.txt').load()
+        assert e.response['Error']['Code'] == "404"
+
+
+def test_delete_non_exsisting_file():
+    storage = S3Storage()
+    with pytest.raises(S3ResourceNotFound):
+        storage.delete('test/awkward.txt')

tests/test_test.py

@@ -1,3 +1,4 @@
+import pytest
 import os
 from django.conf import settings
 from django.core.urlresolvers import reverse, resolve
@@ -7,6 +8,7 @@
 from buckets.test.mocks import create_file, make_dirs  # noqa
 from buckets.test.storage import FakeS3Storage
 from buckets.test import views
+from buckets.exceptions import S3ResourceNotFound
 
 
 #############################################################################
@@ -52,15 +54,21 @@ def test_save_with_subdir(make_dirs):  # noqa
 def test_delete(make_dirs):  # noqa
     file = create_file()
     with open(os.path.join(settings.MEDIA_ROOT,
-              's3', 'uploads', 'text.txt'), 'wb') as dest_file:
+              's3', 'uploads', 'delete.txt'), 'wb') as dest_file:
         dest_file.write(open(file.name, 'rb').read())
 
     store = FakeS3Storage()
-    store.delete('/media/s3/uploads/text.txt')
+    store.delete('delete.txt')
     assert not os.path.isfile(
         os.path.join(settings.MEDIA_ROOT, 's3,' 'uploads', 'text.txt'))
 
 
+def test_delete_non_exising_file(make_dirs):  # noqa
+    store = FakeS3Storage()
+    with pytest.raises(S3ResourceNotFound):
+        store.delete('/media/s3/uploads/delete.txt')
+
+
 def test_get_signed_url():
     store = FakeS3Storage()
 

tests/test_urls.py

@@ -1,7 +1,12 @@
 from django.core.urlresolvers import reverse, resolve
-from buckets.views import signed_url
+from buckets.views import signed_url, delete_resource
 
 
 def test_signed_url():
     assert reverse('s3_signed_url') == '/s3/signed-url/'
     assert resolve('/s3/signed-url/').func == signed_url
+
+
+def test_delete_resource():
+    assert reverse('s3_delete_resource') == '/s3/delete-resource/'
+    assert resolve('/s3/delete-resource/').func == delete_resource

tests/test_views.py

@@ -1,10 +1,14 @@
+import os
 import pytest
 import json
 
+from django.conf import settings
 from django.http import HttpRequest
 from django.core.files.storage import FileSystemStorage
 
 from buckets import views, exceptions
+from buckets.test.storage import FakeS3Storage
+from buckets.test.mocks import create_file, make_dirs  # noqa
 
 
 def test_validate_valid_payload():
@@ -73,3 +77,28 @@ def test_post_signed_url_with_invalid_payload():
 
     assert response.status_code == 400
     assert 'key' in json.loads(content)
+
+
+def test_delete_resource(make_dirs, monkeypatch):  # noqa
+    monkeypatch.setattr(views, 'default_storage', FakeS3Storage())
+    create_file(subdir='uploads', name='delete.txt')
+
+    request = HttpRequest()
+    setattr(request, 'method', 'POST')
+    setattr(request, 'POST', {'key': 'delete.txt'})
+    response = views.delete_resource(request)
+    assert response.status_code == 204
+    assert not os.path.isfile(
+        os.path.join(settings.MEDIA_ROOT, 's3,' 'uploads', 'delete.txt'))
+
+
+def test_delete_non_existing_resource(make_dirs, monkeypatch):  # noqa
+    monkeypatch.setattr(views, 'default_storage', FakeS3Storage())
+
+    request = HttpRequest()
+    setattr(request, 'method', 'POST')
+    setattr(request, 'POST', {'key': 'delete.txt'})
+    response = views.delete_resource(request)
+    content = json.loads(response.content.decode('utf-8'))
+    assert response.status_code == 400
+    assert content['error'] == 'S3 resource does not exist.'