Merge pull request #140 from girder/file-pre-creation

Make file creation and validation happen prior to blob upload

dkc/core/admin/file.py

@@ -37,7 +37,7 @@ class FileAdmin(admin.ModelAdmin):
     autocomplete_fields = ['folder']
 
     def get_readonly_fields(self, request, obj=None):
-        fields = ['sha512', 'size', 'created', 'modified', 'creator']
+        fields = ['sha512', 'created', 'modified']
         # Allow setting of folder only on initial creation
         if obj is None:
             return fields

dkc/core/migrations/0009_file_pre_creation.py

@@ -0,0 +1,34 @@
+# Generated by Django 3.1.6 on 2021-02-05 00:07
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+import s3_file_field.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('core', '0008_quota'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='file',
+            name='blob',
+            field=s3_file_field.fields.S3FileField(blank=True),
+        ),
+        migrations.AlterField(
+            model_name='file',
+            name='size',
+            field=models.PositiveBigIntegerField(),
+        ),
+        migrations.AlterField(
+            model_name='quota',
+            name='user',
+            field=models.OneToOneField(
+                on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL
+            ),
+        ),
+    ]

dkc/core/models/file.py

@@ -34,9 +34,9 @@ class Meta:
         ],
     )
     description = models.TextField(max_length=3000, blank=True)
-    size = models.PositiveBigIntegerField(editable=False)
+    size = models.PositiveBigIntegerField()
     content_type = models.CharField(max_length=255, default='application/octet-stream')
-    blob = S3FileField()
+    blob = S3FileField(blank=True)
     sha512 = models.CharField(max_length=128, blank=True, default='', db_index=True, editable=False)
     user_metadata = JSONObjectField()
     folder = models.ForeignKey(Folder, on_delete=models.CASCADE, related_name='files')
@@ -84,12 +84,8 @@ def has_permission(self, user: User, permission: Permission) -> bool:
 
 @receiver(models.signals.pre_save, sender=File)
 def _file_pre_save(sender: Type[File], instance: File, **kwargs):
-    # TODO if we allow changing a file's blob, we also need to update the size
-
+    # TODO if we allow changing a file's blob & size, we'll need more logic here
     if not instance.pk:
-        # TODO: Test how this works in minio
-        instance.size = instance.blob.size
-
         instance.folder.increment_size(instance.size)
 
 

dkc/core/rest/file.py

@@ -4,6 +4,7 @@
 from django.shortcuts import get_object_or_404
 from rest_framework import serializers
 from rest_framework.decorators import action
+from rest_framework.response import Response
 from rest_framework.viewsets import ModelViewSet
 
 from dkc.core.models import File, Folder
@@ -22,7 +23,6 @@ class Meta:
             'description',
             'size',
             'content_type',
-            'blob',
             'sha512',
             'folder',
             'creator',
@@ -64,7 +64,8 @@ def _validate_unique_folder_siblings(self, attrs):
 
 class FileUpdateSerializer(FileSerializer):
     class Meta(FileSerializer.Meta):
-        read_only_fields = FileSerializer.Meta.read_only_fields + ['folder']
+        fields = FileSerializer.Meta.fields + ['blob']
+        read_only_fields = FileSerializer.Meta.read_only_fields + ['folder', 'size']
 
 
 class FileViewSet(ModelViewSet):
@@ -91,4 +92,6 @@ def perform_create(self, serializer: FileSerializer):
     @action(detail=True)
     def download(self, request, pk=None):
         file = get_object_or_404(File, pk=pk)
-        return HttpResponseRedirect(file.blob.url)
+        if file.blob:  # FieldFiles are falsy when not populated with a file
+            return HttpResponseRedirect(file.blob.url)
+        return Response(status=204)

dkc/core/tests/conftest.py

@@ -24,6 +24,11 @@ def child_folder(folder, folder_factory):
     return folder_factory(parent=folder)
 
 
+@pytest.fixture
+def pending_file(file_factory):
+    return file_factory(size=42, blob=None, content_type='text/plain')
+
+
 @pytest.fixture
 def public_folder(folder_factory):
     return folder_factory(tree__public=True)

dkc/core/tests/factories.py

@@ -61,6 +61,7 @@ class Params:
     name = factory.LazyAttributeSequence(lambda o, n: f'{o.name_base}-{n}.{o.name_ext}')
     description = factory.Faker('paragraph')
     blob = factory.django.FileField(data=b'fakefilebytes', filename='fake.txt')
+    size = factory.SelfAttribute('blob.size')
     user_metadata = _metadata_faker
     folder = factory.SubFactory(FolderFactory)
     creator = factory.SubFactory(UserFactory)

dkc/core/tests/test_file_rest.py

@@ -1,5 +1,7 @@
 import pytest
 
+from dkc.core.models import File
+
 
 @pytest.mark.django_db
 def test_file_rest_retrieve(admin_api_client, file):
@@ -16,3 +18,46 @@ def test_file_list_default_ordering(admin_api_client, folder, file_factory):
     resp = admin_api_client.get('/api/v2/files', data={'folder': folder.id})
     assert resp.status_code == 200
     assert [f['name'] for f in resp.data['results']] == ['A', 'B', 'C']
+
+
+@pytest.mark.django_db
+def test_file_rest_create_process(admin_api_client, folder):
+    """Test initialization of a file, without its blob."""
+    resp = admin_api_client.post(
+        '/api/v2/files',
+        data={
+            'folder': folder.id,
+            'name': 'test.txt',
+            'size': 42,
+        },
+    )
+    assert resp.status_code == 201
+    assert resp.data['size'] == 42
+    folder.refresh_from_db()
+    assert folder.size == 42
+    saved_file = File.objects.get(id=resp.data['id'])
+    assert bool(saved_file.blob) is False
+
+
+@pytest.mark.django_db
+def test_file_rest_download_pending_file(admin_api_client, pending_file):
+    """Test downloading a file prior to its blob being set does something sane."""
+    resp = admin_api_client.get(f'/api/v2/files/{pending_file.id}/download')
+    assert resp.status_code == 204
+
+
+@pytest.mark.django_db
+def test_file_rest_cannot_update_size(admin_api_client, file):
+    resp = admin_api_client.patch(f'/api/v2/files/{file.id}', data={'size': file.size + 1})
+    assert resp.data['size'] == file.size
+
+
+@pytest.mark.django_db
+def test_file_rest_set_blob(admin_api_client, pending_file, s3ff_field_value):
+    resp = admin_api_client.patch(
+        f'/api/v2/files/{pending_file.id}', data={'blob': s3ff_field_value}
+    )
+    assert resp.status_code == 200
+    assert resp.data['size'] == pending_file.size
+    pending_file.refresh_from_db()
+    assert pending_file.blob

dkc/core/tests/test_permissions.py

@@ -293,13 +293,13 @@ def test_anonymous_user_cannot_create_root_folder(api_client):
 
 
 @pytest.mark.django_db
-def test_anonymous_user_cannot_create_files(api_client, folder, s3ff_field_value):
+def test_anonymous_user_cannot_create_files(api_client, folder):
     resp = api_client.post(
         '/api/v2/files',
         data={
             'name': 'test.txt',
             'folder': folder.id,
-            'blob': s3ff_field_value,
+            'size': 42,
         },
     )
     assert resp.status_code == 401
@@ -313,14 +313,14 @@ def test_folder_create_permission_enforcement(api_client, folder, user):
 
 
 @pytest.mark.django_db
-def test_file_create_permission_enforcement(api_client, folder, user, s3ff_field_value):
+def test_file_create_permission_enforcement(api_client, folder, user):
     api_client.force_authenticate(user=user)
     resp = api_client.post(
         '/api/v2/files',
         data={
             'name': 'test.txt',
             'folder': folder.id,
-            'blob': s3ff_field_value,
+            'size': 42,
         },
     )
     assert resp.status_code == 403