diff --git a/api/placer.py b/api/placer.py
index dc16c4482756d4392541ab2d6440e5e24a5c7f25..9b454828e9cf1d2f76a08ca51544f4c708267530 100644
--- a/api/placer.py
+++ b/api/placer.py
@@ -18,6 +18,7 @@ from .jobs import rules
from .jobs.jobs import Job
from .types import Origin
from .web import encoder
+from .web.errors import FileFormException
class Placer(object):
"""
@@ -74,7 +75,7 @@ class Placer(object):
Helper function that throws unless metadata was provided.
"""
if self.metadata == None:
- raise Exception('Metadata required')
+ raise FileFormException('Metadata required')
def save_file(self, field=None, file_attrs=None):
"""
@@ -140,7 +141,7 @@ class UIDPlacer(Placer):
super(UIDPlacer, self).__init__(container_type, container, id_, metadata, timestamp, origin, context)
self.metadata_for_file = {}
self.session_id = None
-
+ self.count = 0
def check(self):
self.requireMetadata()
@@ -149,22 +150,25 @@ class UIDPlacer(Placer):
metadata_validator = validators.from_schema_path(payload_schema_uri)
metadata_validator(self.metadata, 'POST')
- # If not a superuser request, pass uid of user making the upload request
- targets = self.create_hierarchy(self.metadata, type_=self.match_type, user=self.context.get('uid'))
-
- self.metadata_for_file = {}
-
- for target in targets:
- if target[0].level is 'session':
- self.session_id = target[0].id_
- for name in target[1]:
- self.metadata_for_file[name] = {
- 'container': target[0],
- 'metadata': target[1][name]
- }
def process_file_field(self, field, file_attrs):
+ # Only create the hierarchy once
+ if self.count == 0:
+ # If not a superuser request, pass uid of user making the upload request
+ targets = self.create_hierarchy(self.metadata, type_=self.match_type, user=self.context.get('uid'))
+
+ self.metadata_for_file = {}
+
+ for target in targets:
+ if target[0].level is 'session':
+ self.session_id = target[0].id_
+ for name in target[1]:
+ self.metadata_for_file[name] = {
+ 'container': target[0],
+ 'metadata': target[1][name]
+ }
+ self.count += 1
# For the file, given self.targets, choose a target
name = field.filename
@@ -194,6 +198,9 @@ class UIDPlacer(Placer):
self.saved.append(file_attrs)
def finalize(self):
+ # Check that there is at least one file being uploaded
+ if self.count < 1:
+ raise FileFormException("No files selected for upload")
if self.session_id:
self.container_type = 'session'
self.id_ = self.session_id
diff --git a/api/upload.py b/api/upload.py
index 35db758b50b1f8d7031561b93dd43574faebb8ab..12e051da4970aaf7050cb8824896863bdfa16425 100644
--- a/api/upload.py
+++ b/api/upload.py
@@ -5,7 +5,7 @@ import os.path
import shutil
from .web import base
-from .web.errors import FileStoreException
+from .web.errors import FileStoreException, FileFormException
from . import config
from . import files
from . import placer as pl
@@ -95,7 +95,8 @@ def process_upload(request, strategy, container_type=None, id_=None, origin=None
# TODO: Change schemas to enabled targeted uploads of more than one file.
# Ref docs from placer.TargetedPlacer for details.
if strategy == Strategy.targeted and len(file_fields) > 1:
- raise Exception("Targeted uploads can only send one file")
+ raise FileFormException("Targeted uploads can only send one file")
+
for field in file_fields:
diff --git a/api/web/base.py b/api/web/base.py
index c9deb3886b22fb6c96f3c76782fdbbf7c418fbfc..068b90ebddb97b0f3553ec063e7f0d42acc2f835 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, errors.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 e6a5729d785c90d4996c212f6bbcaa322eb36c4b..2073426fa380a9ca7b439a0ff783a1adc3bb88f6 100644
--- a/tests/integration_tests/python/test_uploads.py
+++ b/tests/integration_tests/python/test_uploads.py
@@ -55,6 +55,20 @@ 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 without any files
+ file_form = upload_file_form(
+ group={'_id': group_1},
+ project={'label': project_label_1, "files":[]},
+ session={'uid': session_uid+"1", "files":[], 'subject': {
+ 'code': prefix + '-subject-code',
+ 'files': []
+ }}
+ )
+ print file_form
+ r = as_admin.post('/upload/reaper', files={"metadata": file_form.get("metadata")})
+ print r.json()
+ assert r.status_code == 400
+
# get session created by the upload
project_1 = as_admin.get('/groups/' + group_1 + '/projects').json()[0]['_id']
session = as_admin.get('/projects/' + project_1 + '/sessions').json()[0]['_id']
@@ -173,6 +187,25 @@ def test_reaper_upload_unknown_group_project(data_builder, file_form, as_root, a
)
assert r.ok
+ # Try uploading 0 files
+ r = as_root.post('/upload/label', files={"metadata":file_form(
+ 'acquisition.csv',
+ meta={
+ 'group': {'_id': 'not_a_real_group'},
+ 'project': {
+ 'label': 'new_project',
+ },
+ 'session': {
+ 'label': 'test_session_label',
+ },
+ 'acquisition': {
+ 'label': 'test_acquisition_label',
+ 'files': [{'name': 'acquisition.csv'}]
+ }
+ }).get("metadata")}
+ )
+ assert r.status_code == 400
+
# get session created by the upload
r = as_root.get('/groups/unknown/projects')
@@ -362,7 +395,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')
@@ -390,6 +423,10 @@ def test_uid_upload(data_builder, file_form, as_admin, as_user, as_public):
r = as_user.post('/upload/uid', files=file_form(*uid_files, meta=uid_meta))
assert r.status_code == 403
+ # 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 == 400
+
# uid-upload files
r = as_admin.post('/upload/uid', files=file_form(*uid_files, meta=uid_meta))
assert r.ok