diff --git a/api/files.py b/api/files.py
index 0585b5993410695dccf95cd4e997ad3088fd8bf2..436b3a5e385dd1e09b6915b3ec0eb03b82380620 100644
--- a/api/files.py
+++ b/api/files.py
@@ -48,6 +48,16 @@ def hash_file_formatted(path, hash_alg=None, buffer_size=65536):
return util.format_hash(hash_alg, hasher.hexdigest())
+<<<<<<< HEAD
+=======
+
+class FileStoreException(Exception):
+ pass
+
+class FileFormException(Exception):
+ pass
+
+>>>>>>> Empty uploads 400 instead of 500
class HashingFile(file):
def __init__(self, file_path, hash_alg):
super(HashingFile, self).__init__(file_path, "wb")
diff --git a/api/placer.py b/api/placer.py
index 49ee2b746a8e32d7d4e2ffaaed9819b94c9ba836..3839175607894c9d5a800905f1742f30738b57cb 100644
--- a/api/placer.py
+++ b/api/placer.py
@@ -19,12 +19,13 @@ from .jobs.jobs import Job
from .types import Origin
from .web import encoder
+
class Placer(object):
"""
Interface for a placer, which knows how to process files and place them where they belong - on disk and database.
"""
- def __init__(self, container_type, container, id_, metadata, timestamp, origin, context):
+ def __init__(self, container_type, container, id_, metadata, timestamp, origin, context, file_fields):
self.container_type = container_type
self.container = container
self.id_ = id_
@@ -42,6 +43,7 @@ class Placer(object):
# A list of files that have been saved via save_file() usually returned by finalize()
self.saved = []
+ self.file_fields = file_fields
def check(self):
@@ -74,7 +76,15 @@ class Placer(object):
Helper function that throws unless metadata was provided.
"""
if self.metadata == None:
- raise Exception('Metadata required')
+ raise files.FileFormException('Metadata required')
+
+ def requireFiles(self):
+ """
+ Helper function that throws unless metadata was provided.
+ """
+ if not self.file_fields or len(self.file_fields) < 1:
+ raise files.FileFormException('No files selected to upload')
+
def save_file(self, field=None, file_attrs=None):
"""
@@ -113,6 +123,16 @@ class TargetedPlacer(Placer):
An exception is thrown in upload.process_upload() if you try. This could be fixed by making a better schema.
"""
+ # TODO: Change schemas to enabled targeted uploads of more than one file.
+ # Ref docs from placer.TargetedPlacer for details.
+ def requireFiles(self):
+ """
+ Helper function that throws unless metadata was provided.
+ """
+ if not self.file_fields or len(self.file_fields) != 1:
+ raise files.FileFormException("Targeted uploads can only send one file")
+
+
def check(self):
self.requireTarget()
validators.validate_data(self.metadata, 'file.json', 'input', 'POST', optional=True)
@@ -137,14 +157,14 @@ class UIDPlacer(Placer):
create_hierarchy = staticmethod(hierarchy.upsert_top_down_hierarchy)
match_type = 'uid'
- def __init__(self, container_type, container, id_, metadata, timestamp, origin, context):
- super(UIDPlacer, self).__init__(container_type, container, id_, metadata, timestamp, origin, context)
+ def __init__(self, container_type, container, id_, metadata, timestamp, origin, context, file_fields):
+ super(UIDPlacer, self).__init__(container_type, container, id_, metadata, timestamp, origin, context, file_fields)
self.metadata_for_file = {}
self.session_id = None
-
def check(self):
self.requireMetadata()
+ self.requireFiles()
payload_schema_uri = validators.schema_uri('input', self.metadata_schema)
metadata_validator = validators.from_schema_path(payload_schema_uri)
@@ -315,8 +335,8 @@ class TokenPlacer(Placer):
Intended for use with a token that tracks where the files will be stored.
"""
- def __init__(self, container_type, container, id_, metadata, timestamp, origin, context):
- super(TokenPlacer, self).__init__(container_type, container, id_, metadata, timestamp, origin, context)
+ def __init__(self, container_type, container, id_, metadata, timestamp, origin, context, file_fields):
+ super(TokenPlacer, self).__init__(container_type, container, id_, metadata, timestamp, origin, context, file_fields)
self.paths = []
self.folder = None
@@ -354,8 +374,8 @@ class PackfilePlacer(Placer):
A placer that can accept N files, save them into a zip archive, and place the result on an acquisition.
"""
- def __init__(self, container_type, container, id_, metadata, timestamp, origin, context):
- super(PackfilePlacer, self).__init__(container_type, container, id_, metadata, timestamp, origin, context)
+ def __init__(self, container_type, container, id_, metadata, timestamp, origin, context, file_fields):
+ super(PackfilePlacer, self).__init__(container_type, container, id_, metadata, timestamp, origin, context, file_fields)
# This endpoint is an SSE endpoint
self.sse = True
diff --git a/api/web/base.py b/api/web/base.py
index c9deb3886b22fb6c96f3c76782fdbbf7c418fbfc..7d46a39792810b9419a9049358ba664cfe737ee2 100644
--- a/api/web/base.py
+++ b/api/web/base.py
@@ -358,6 +358,8 @@ class RequestHandler(webapp2.RequestHandler):
code = 400
elif isinstance(exception, errors.FileFormException):
code = 400
+ elif isinstance(exception, files.FileFormException):
+ code = 400
elif isinstance(exception, ElasticsearchException):
code = 503
message = "Search is currently down. Try again later."
diff --git a/tests/integration_tests/python/test_uploads.py b/tests/integration_tests/python/test_uploads.py
index 5b5e366d68d43b0770391c49c3ad2d447988f42c..202a2a9a660e09ca2b19a20db28af15618f40bbd 100644
--- a/tests/integration_tests/python/test_uploads.py
+++ b/tests/integration_tests/python/test_uploads.py
@@ -55,7 +55,7 @@ def test_reaper_upload(data_builder, randstr, upload_file_form, as_admin):
))
assert r.ok
- # reaper-upload files to group_1/project_label_1 using session_uid
+ # reaper-upload files to group_1/project_label_1 using session_uid without any files
file_form = upload_file_form(
group={'_id': group_1},
project={'label': project_label_1, "files":[]},
@@ -67,7 +67,7 @@ def test_reaper_upload(data_builder, randstr, upload_file_form, as_admin):
print file_form
r = as_admin.post('/upload/reaper', files={"metadata": file_form.get("metadata")})
print r.json()
- assert r.status_code == 500
+ assert r.status_code == 400
# get session created by the upload
project_1 = as_admin.get('/groups/' + group_1 + '/projects').json()[0]['_id']
@@ -204,7 +204,7 @@ def test_reaper_upload_unknown_group_project(data_builder, file_form, as_root, a
}
}).get("metadata")}
)
- assert r.status_code == 500
+ assert r.status_code == 400
# get session created by the upload
@@ -271,7 +271,7 @@ def test_uid_upload(data_builder, file_form, as_admin, as_user, as_public):
# try to uid-upload w/o metadata
r = as_admin.post('/upload/uid', files=file_form('test.csv'))
- assert r.status_code == 500
+ assert r.status_code == 400
# NOTE unused.csv is testing code that discards files not referenced from meta
uid_files = ('project.csv', 'subject.csv', 'session.csv', 'acquisition.csv', 'unused.csv')
@@ -301,7 +301,7 @@ def test_uid_upload(data_builder, file_form, as_admin, as_user, as_public):
# try to uid-upload no files
r = as_admin.post('/upload/uid', files={"metadata": file_form(*uid_files, meta=uid_meta).get("metadata")})
- assert r.status_code == 500
+ assert r.status_code == 400
# uid-upload files
r = as_admin.post('/upload/uid', files=file_form(*uid_files, meta=uid_meta))