Merge pull request #15 from Cadasta/storage

Remove file only from storage when the model is saved

buckets/__init__.py

@@ -1 +1 @@
-__version__ = '0.1.18'
+__version__ = '0.1.19a4'

buckets/fields.py

@@ -6,15 +6,24 @@
 from .widgets import S3FileUploadWidget
 
 
+def key_from_url(url, upload_to):
+    key = url.split('/')[-1]
+
+    if upload_to:
+        key = upload_to + '/' + key
+    return key
+
+
 class S3File(object):
     """ This is the internal value an `S3FileField`. It provides access to the
         actual file on S3, e.g. for post-processing. It usually uses an
         instance of S3Storage to download, upload or delete the file on S3."""
-    def __init__(self, url, field):
+    def __init__(self, url, field, original_url=None):
         self.field = field
         self.storage = field.storage
         self.url = url
         self.committed = True
+        self.original_url = original_url if original_url is not None else url
 
     def _get_file(self):
         if not hasattr(self, '_file') or not self._file:
@@ -30,11 +39,7 @@ def _set_file(self, file):
         self.committed = False
 
     def _del_file(self):
-        name = self.url.split('/')[-1]
-
-        if self.field.upload_to:
-            name = self.field.upload_to + '/' + name
-
+        name = key_from_url(self.url, self.field.upload_to)
         self.storage.delete(name)
         if hasattr(self, '_file'):
             del self._file
@@ -71,7 +76,13 @@ def __set__(self, instance, value):
         if isinstance(value, S3File):
             instance.__dict__[self.field.name] = value
         else:
-            instance.__dict__[self.field.name] = S3File(value, self.field)
+            o = None
+            f = instance.__dict__.get(self.field.name)
+            if f:
+                o = f.url
+            instance.__dict__[self.field.name] = S3File(value,
+                                                        self.field,
+                                                        original_url=o)
 
 
 class S3FileField(models.Field):
@@ -115,12 +126,11 @@ def deconstruct(self):
         return name, path, args, kwargs
 
     def from_db_value(self, value, expression, connection, context):
-        return S3File(value, self)
+        return S3File(value, self, original_url=value)
 
     def to_python(self, value):
         if value is None or isinstance(value, S3File):
             return value
-
         return S3File(value, self)
 
     def get_prep_value(self, value):
@@ -132,6 +142,10 @@ def get_prep_value(self, value):
     def pre_save(self, model_instance, add):
         file = getattr(model_instance, self.name)
         file.save()
+        if not add and file.original_url and not file.url:
+            key = key_from_url(file.original_url, self.upload_to)
+            self.storage.delete(key)
+            return ''
 
         return file.url
 

buckets/static/buckets/js/script.js

@@ -156,21 +156,9 @@
         e.preventDefault();
 
         var el = e.target.parentElement.parentElement;
-        var urlArray = el.querySelector('.file-url').value.split('/'),
-            headers  = { 'X-CSRFToken': getCookie('csrftoken')},
-            form = new FormData();
-
-        var url = urlArray[urlArray.length - 1];
-        if (el.getAttribute('data-upload-to').length) {
-            url = el.getAttribute('data-upload-to') + '/' + url;
-        }
-        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');
-        });
+        el.querySelector('.file-url').value = '';
+        el.querySelector('.file-input').value = '';
+        el.classList.remove('uploaded');
     }
 
     function addEventHandlers(el) {

buckets/storage.py

@@ -6,7 +6,6 @@
 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
 
@@ -60,10 +59,7 @@ def _save(self, name, content):
 
     def delete(self, name):
         s3 = self.get_boto_ressource()
-        if self.exists(name):
-            s3.Object(self.bucket_name, name).delete()
-        else:
-            raise S3ResourceNotFound()
+        s3.Object(self.bucket_name, name).delete()
 
     def exists(self, name):
         s3 = self.get_boto_ressource()

buckets/test/storage.py

@@ -3,7 +3,6 @@
 
 from django.conf import settings
 from buckets.utils import ensure_dirs, random_id
-from buckets.exceptions import S3ResourceNotFound
 
 
 class FakeS3Storage(object):
@@ -40,8 +39,8 @@ def delete(self, name):
         uploaded = os.path.join(self.dir, 'uploads', name)
         try:
             os.remove(uploaded)
-        except:
-            raise S3ResourceNotFound()
+        except FileNotFoundError:
+            pass
 
     def exists(self, key):
         path = os.path.join(self.dir, 'uploads', key)

buckets/urls.py

@@ -3,6 +3,4 @@
 
 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, HttpResponse
+from django.http import JsonResponse
 from django.utils.translation import ugettext as _
 from django.core.files.storage import default_storage
 
-from buckets.exceptions import InvalidPayload, S3ResourceNotFound
+from buckets.exceptions import InvalidPayload
 
 
 def validate_payload(payload):
@@ -32,13 +32,3 @@ 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)

buckets/widgets.py

@@ -51,7 +51,7 @@ def render(self, name, value, attrs=None):
             file_url=file_url,
             element_id=self.build_attrs(attrs).get('id'),
             file_name=basename(file_url) if file_url else '',
-            uploaded_class=('uploaded' if value else ''),
+            uploaded_class=('uploaded' if file_url else ''),
             upload_to=self.upload_to,
             accepted_types=accepted_types
         )

example/exampleapp/migrations/0002_auto_20161215_0326.py

@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.3 on 2016-12-15 03:26
+from __future__ import unicode_literals
+
+import buckets.fields
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('exampleapp', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='filemodel',
+            name='file',
+            field=buckets.fields.S3FileField(blank=True, null=True, upload_to='test'),
+        ),
+    ]

example/exampleapp/models.py

@@ -6,4 +6,4 @@
 
 class FileModel(models.Model):
     name = models.CharField(max_length=200)
-    file = S3FileField(upload_to='test', accepted_types=TYPES)
+    file = S3FileField(upload_to='test', accepted_types=TYPES, null=True, blank=True)

example/settings.py

@@ -85,11 +85,13 @@
     }
 }
 
-DEFAULT_FILE_STORAGE = 'buckets.test.storage.FakeS3Storage'
+# DEFAULT_FILE_STORAGE = 'buckets.test.storage.FakeS3Storage'
+DEFAULT_FILE_STORAGE = 'buckets.storage.S3Storage'
 AWS = {
     'BUCKET': os.environ.get('AWS_BUCKET'),
     'ACCESS_KEY': os.environ.get('AWS_ACCESS_KEY'),
     'SECRET_KEY': os.environ.get('AWS_SECRET_KEY'),
+    'REGION': os.environ.get('AWS_REGION'),
 }
 
 

tests/test_fields.py

@@ -4,7 +4,7 @@
 from django.conf import settings
 from django.core.files.uploadedfile import SimpleUploadedFile
 
-from buckets.fields import S3File, S3FileField
+from buckets.fields import S3File, S3FileField, key_from_url
 from buckets.widgets import S3FileUploadWidget
 from buckets.test.mocks import create_file, make_dirs  # noqa
 from buckets.test.storage import FakeS3Storage
@@ -181,6 +181,34 @@ def test_pre_save():
         s3_file='http://example.com'
     )
     field = S3FileField(name='s3_file')
+    field._original_url = 'http://example.com'
     url = field.pre_save(model_instance, False)
 
     assert url == 'http://example.com'
+
+
+@pytest.mark.django_db
+def test_pre_save_delete_file():
+    file = create_file()
+    with open(os.path.join(settings.MEDIA_ROOT,
+              's3/uploads/text.txt'), 'wb') as dest_file:
+        dest_file.write(open(file.name, 'rb').read())
+
+    model_instance = FileModel(s3_file='/media/s3/uploads/text.txt')
+    model_instance.save()
+    model_instance.refresh_from_db()
+
+    field = model_instance.s3_file.field
+    field.storage = FakeS3Storage()
+    model_instance.s3_file = ''
+    url = field.pre_save(model_instance, False)
+    assert url == ''
+    assert not os.path.isfile(os.path.join(settings.MEDIA_ROOT,
+                              's3/uploads/text.txt'))
+
+
+def test_key_from_url():
+    assert (key_from_url('http://example.com/some/dir/file.txt', None) ==
+            'file.txt')
+    assert (key_from_url('http://example.com/some/dir/file.txt', 'some/dir') ==
+            'some/dir/file.txt')

tests/test_storage.py

@@ -6,7 +6,6 @@
 
 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):
@@ -77,5 +76,4 @@ def test_delete_file(make_dirs):  # noqa
 
 def test_delete_non_exsisting_file():
     storage = S3Storage()
-    with pytest.raises(S3ResourceNotFound):
-        storage.delete('test/awkward.txt')
+    storage.delete('test/awkward.txt')

tests/test_test.py

@@ -1,4 +1,3 @@
-import pytest
 import os
 from django.conf import settings
 from django.core.urlresolvers import reverse, resolve
@@ -8,7 +7,6 @@
 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
 
 
 #############################################################################
@@ -65,8 +63,7 @@ def test_delete(make_dirs):  # noqa
 
 def test_delete_non_exising_file(make_dirs):  # noqa
     store = FakeS3Storage()
-    with pytest.raises(S3ResourceNotFound):
-        store.delete('/media/s3/uploads/delete.txt')
+    store.delete('/media/s3/uploads/delete.txt')
 
 
 def test_get_signed_url():

tests/test_urls.py

@@ -1,12 +1,7 @@
 from django.core.urlresolvers import reverse, resolve
-from buckets.views import signed_url, delete_resource
+from buckets.views import signed_url
 
 
 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,13 +1,10 @@
-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
 
 
@@ -77,28 +74,3 @@ 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.'