Skip to content
This repository has been archived by the owner on Jan 29, 2022. It is now read-only.

Make file creation and validation happen prior to blob upload #140

Merged
merged 6 commits into from
Feb 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dkc/core/admin/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions dkc/core/migrations/0009_file_pre_creation.py
Original file line number Diff line number Diff line change
@@ -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
),
zachmullen marked this conversation as resolved.
Show resolved Hide resolved
),
]
10 changes: 3 additions & 7 deletions dkc/core/models/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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
brianhelba marked this conversation as resolved.
Show resolved Hide resolved

instance.folder.increment_size(instance.size)


Expand Down
9 changes: 6 additions & 3 deletions dkc/core/rest/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -22,7 +23,6 @@ class Meta:
'description',
'size',
'content_type',
'blob',
'sha512',
'folder',
'creator',
Expand Down Expand Up @@ -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']
brianhelba marked this conversation as resolved.
Show resolved Hide resolved


class FileViewSet(ModelViewSet):
Expand All @@ -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)
5 changes: 5 additions & 0 deletions dkc/core/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions dkc/core/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class Meta:
name = factory.Faker('file_name')
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)
Expand Down
45 changes: 45 additions & 0 deletions dkc/core/tests/test_file_rest.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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
zachmullen marked this conversation as resolved.
Show resolved Hide resolved


@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
8 changes: 4 additions & 4 deletions dkc/core/tests/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down