From b6e48be5cc656bc30849ce57e2422b1af2d654c6 Mon Sep 17 00:00:00 2001
From: Harsha Kethineni <harshakethineni@flywheel.io>
Date: Wed, 22 Nov 2017 16:49:02 -0600
Subject: [PATCH] Checks file form in process_upload

---
 api/files.py    | 10 ----------
 api/placer.py   | 38 +++++++++-----------------------------
 api/upload.py   |  6 +++---
 api/web/base.py |  2 +-
 4 files changed, 13 insertions(+), 43 deletions(-)

diff --git a/api/files.py b/api/files.py
index 436b3a5e..0585b599 100644
--- a/api/files.py
+++ b/api/files.py
@@ -48,16 +48,6 @@ 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 38391756..9e67722f 100644
--- a/api/placer.py
+++ b/api/placer.py
@@ -18,14 +18,14 @@ 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):
     """
     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, file_fields):
+    def __init__(self, container_type, container, id_, metadata, timestamp, origin, context):
         self.container_type = container_type
         self.container      = container
         self.id_            = id_
@@ -43,7 +43,6 @@ 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):
@@ -76,15 +75,7 @@ class Placer(object):
         Helper function that throws unless metadata was provided.
         """
         if self.metadata == None:
-            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')
-
+            raise FileFormException('Metadata required')
 
     def save_file(self, field=None, file_attrs=None):
         """
@@ -123,16 +114,6 @@ 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)
@@ -157,14 +138,13 @@ 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, file_fields):
-        super(UIDPlacer, self).__init__(container_type, container, id_, metadata, timestamp, origin, context, file_fields)
+    def __init__(self, container_type, container, id_, metadata, timestamp, origin, context):
+        super(UIDPlacer, self).__init__(container_type, container, id_, metadata, timestamp, origin, context)
         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)
@@ -335,8 +315,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, file_fields):
-        super(TokenPlacer, self).__init__(container_type, container, id_, metadata, timestamp, origin, context, file_fields)
+    def __init__(self, container_type, container, id_, metadata, timestamp, origin, context):
+        super(TokenPlacer, self).__init__(container_type, container, id_, metadata, timestamp, origin, context)
 
         self.paths  =   []
         self.folder =   None
@@ -374,8 +354,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, file_fields):
-        super(PackfilePlacer, self).__init__(container_type, container, id_, metadata, timestamp, origin, context, file_fields)
+    def __init__(self, container_type, container, id_, metadata, timestamp, origin, context):
+        super(PackfilePlacer, self).__init__(container_type, container, id_, metadata, timestamp, origin, context)
 
         # This endpoint is an SSE endpoint
         self.sse            = True
diff --git a/api/upload.py b/api/upload.py
index 75569e16..cfc779d7 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
@@ -93,9 +93,9 @@ 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")
     elif strategy in [Strategy.reaper, Strategy.uidupload, Strategy.labelupload, Strategy.uidmatch] and len(file_fields) < 1:
-        raise Exception("No files selected for uploading")
+        raise FileFormException("No files selected for uploading")
     placer.check()
 
 
diff --git a/api/web/base.py b/api/web/base.py
index 7d46a397..068b90eb 100644
--- a/api/web/base.py
+++ b/api/web/base.py
@@ -358,7 +358,7 @@ class RequestHandler(webapp2.RequestHandler):
             code = 400
         elif isinstance(exception, errors.FileFormException):
             code = 400
-        elif isinstance(exception, files.FileFormException):
+        elif isinstance(exception, errors.FileFormException):
             code = 400
         elif isinstance(exception, ElasticsearchException):
             code = 503
-- 
GitLab