From 5351b4b4af8593631e5fc88b2b522b8c6861d26a Mon Sep 17 00:00:00 2001
From: Renzo Frigato <rfrigato@stanford.edu>
Date: Thu, 19 Nov 2015 18:10:52 -0800
Subject: [PATCH] fix bugs on file uploads and debug views

---
 api/api.py                  | 16 +++++++++-------
 api/debuginfo.py            | 21 +++++++++++++++------
 api/files.py                | 11 ++++++-----
 api/handlers/listhandler.py | 26 +++++++++++++-------------
 api/schemas/mongo/file.json |  3 ++-
 bin/bootstrap.py            |  9 +++++++--
 6 files changed, 52 insertions(+), 34 deletions(-)

diff --git a/api/api.py b/api/api.py
index 387abe33..2bd8cdf0 100644
--- a/api/api.py
+++ b/api/api.py
@@ -71,10 +71,14 @@ routes = [
     ]),
     webapp2.Route(r'/api/groups',                                   grouphandler.GroupHandler, handler_method='get_all', methods=['GET']),
     webapp2.Route(r'/api/groups',                                   grouphandler.GroupHandler, methods=['POST']),
-    webapp2.Route(_format(r'/api/groups/<_id:{group_id_re}>'),      grouphandler.GroupHandler, name='group'),
+    webapp2.Route(_format(r'/api/groups/<_id:{group_id_re}>'),      grouphandler.GroupHandler, name='group_details'),
 
-    webapp2.Route(_format(r'/api/<cont_name:groups>/<cid:{group_id_re}>/<list_name:roles>'),                                        listhandler.ListHandler, name='group'),
-    webapp2.Route(_format(r'/api/<cont_name:groups>/<cid:{group_id_re}>/<list_name:roles>/<site:{site_id_re}>/<_id:{user_id_re}>'),    listhandler.ListHandler, name='group', methods=['GET', 'PUT', 'DELETE']),
+    webapp2.Route(_format(r'/api/<cont_name:{cont_name_re}>'),                          containerhandler.ContainerHandler, name='cont_list', handler_method='get_all', methods=['GET']),
+    webapp2.Route(_format(r'/api/<cont_name:{cont_name_re}>'),                          containerhandler.ContainerHandler, methods=['POST']),
+    webapp2.Route(_format(r'/api/<cont_name:{cont_name_re}>/<cid:{cid_re}>'),           containerhandler.ContainerHandler, name='cont_details', methods=['GET','PUT','DELETE']),
+
+    webapp2.Route(_format(r'/api/<cont_name:groups>/<cid:{group_id_re}>/<list_name:roles>'),                                        listhandler.ListHandler, name='group_roles_post'),
+    webapp2.Route(_format(r'/api/<cont_name:groups>/<cid:{group_id_re}>/<list_name:roles>/<site:{site_id_re}>/<_id:{user_id_re}>'),    listhandler.ListHandler, name='group_roles', methods=['GET', 'PUT', 'DELETE']),
 
     webapp2.Route(_format(r'/api/<cont_name:{cont_name_re}>/<cid:{cid_re}>/<list_name:tags>'),                                      listhandler.ListHandler, methods=['POST'], name='tags_post'),
     webapp2.Route(_format(r'/api/<cont_name:{cont_name_re}>/<cid:{cid_re}>/<list_name:tags>/<value:{tag_re}>'),                     listhandler.ListHandler, name='tags'),
@@ -100,12 +104,10 @@ routes = [
 
     webapp2.Route(r'/api/projects/groups',                                              containerhandler.ContainerHandler, handler_method='get_groups_with_project', methods=['GET']),
 
-    webapp2.Route(_format(r'/api/<par_cont_name:groups>/<par_id:{group_id_re}>/<cont_name:projects>'),          containerhandler.ContainerHandler, name='cont_sublist', handler_method='get_all', methods=['GET']),
+    webapp2.Route(_format(r'/api/<par_cont_name:groups>/<par_id:{group_id_re}>/<cont_name:projects>'),          containerhandler.ContainerHandler, name='cont_sublist_groups', handler_method='get_all', methods=['GET']),
     webapp2.Route(_format(r'/api/<par_cont_name:{cont_name_re}>/<par_id:{cid_re}>/<cont_name:{cont_name_re}>'), containerhandler.ContainerHandler, name='cont_sublist', handler_method='get_all', methods=['GET']),
 
-    webapp2.Route(_format(r'/api/<cont_name:{cont_name_re}>'),                          containerhandler.ContainerHandler, name='cont_list', handler_method='get_all', methods=['GET']),
-    webapp2.Route(_format(r'/api/<cont_name:{cont_name_re}>'),                          containerhandler.ContainerHandler, methods=['POST']),
-    webapp2.Route(_format(r'/api/<cont_name:{cont_name_re}>/<cid:{cid_re}>'),           containerhandler.ContainerHandler, name='cont_details', methods=['GET','PUT','DELETE']),
+
 ]
 
 
diff --git a/api/debuginfo.py b/api/debuginfo.py
index 88908443..9d9b6ec2 100644
--- a/api/debuginfo.py
+++ b/api/debuginfo.py
@@ -17,18 +17,27 @@ def add_debuginfo(handler, cont_name, response):
 def _add_di_list(handler, cont_name, response):
     for elem in response:
         _add_di(handler, cont_name, elem)
-        elem['debug']['details'] = handler.uri_for(
-            'cont_details',
-            cont_name=cont_name,
-            cid=elem['_id'],
-            _full=True) + '?' + handler.request.query_string
+        if cont_name == 'groups':
+            elem['debug']['details'] = handler.uri_for(
+                'group_details' if cont_name == 'groups' else 'cont_details',
+                cont_name=cont_name,
+                _id=elem['_id'],
+                _full=True) + '?' + handler.request.query_string
+        else:
+            elem['debug']['details'] = handler.uri_for(
+                'cont_details',
+                cont_name=cont_name,
+                cid=elem['_id'],
+                _full=True) + '?' + handler.request.query_string
+
 
 def _add_di(handler, cont_name, response):
     response['debug'] = {}
     if child_containers.get(cont_name):
         child_cont_name = child_containers[cont_name]
+
         response['debug'][child_cont_name] = handler.uri_for(
-            'cont_sublist',
+            'cont_sublist_groups' if cont_name == 'groups' else 'cont_sublist',
             par_cont_name=cont_name,
             par_id=response['_id'],
             cont_name=child_cont_name,
diff --git a/api/files.py b/api/files.py
index 0fc43d13..72fc281e 100644
--- a/api/files.py
+++ b/api/files.py
@@ -16,7 +16,7 @@ class FileStoreException(Exception):
 
 class HashingFile(file):
     def __init__(self, file_path, hash_alg):
-        super(HashingFile, self).__init__(file_path, "w+b")
+        super(HashingFile, self).__init__(file_path, "wb")
         self.hash_alg = hashlib.new(hash_alg)
 
     def write(self, data):
@@ -83,8 +83,9 @@ class FileStore(object):
     def _save_body_file(self, dest_path, filename, hash_alg):
         if not filename:
             raise FileStoreException('filename is required for body uploads')
-        self.received_file = HashingFile(os.path.join(dest_path, filename))
-        for chunk in iter(lambda: self.request.body_file.read(2**20), ''):
+        self.filename = filename
+        self.received_file = HashingFile(os.path.join(dest_path, filename), hash_alg)
+        for chunk in iter(lambda: self.body.read(2**20), ''):
             self.received_file.write(chunk)
         self.tags = None
         self.metadata = None
@@ -97,7 +98,7 @@ class FileStore(object):
         shutil.move(filepath, target_filepath)
         self.path = target_path
 
-    def identical(self, filepath, sha384):
+    def identical(self, filepath, hash_):
         filepath1 = os.path.join(self.path, self.filename)
         if zipfile.is_zipfile(filepath) and zipfile.is_zipfile(filepath1):
             with zipfile.ZipFile(filepath) as zf1, zipfile.ZipFile(filepath1) as zf2:
@@ -113,4 +114,4 @@ class FileStore(object):
                 else:
                     return True
         else:
-            return sha384 == self.sha38
+            return hash_ == self.hash
diff --git a/api/handlers/listhandler.py b/api/handlers/listhandler.py
index 6c57af1a..d636de09 100644
--- a/api/handlers/listhandler.py
+++ b/api/handlers/listhandler.py
@@ -344,7 +344,7 @@ class FileListHandler(ListHandler):
                     self.response.headers['Content-Disposition'] = 'attachment; filename="' + filename + '"'
 
     def delete(self, cont_name, list_name, **kwargs):
-        filename = kwargs.get('filename')
+        filename = kwargs.get('name')
         _id = kwargs.get('cid')
         result = super(FileListHandler, self).delete(cont_name, list_name, **kwargs)
         filepath = os.path.join(self.app.config['data_path'], str(_id)[-3:] + '/' + str(_id), filename)
@@ -362,46 +362,46 @@ class FileListHandler(ListHandler):
         _id = kwargs.pop('cid')
         container, permchecker, storage, mongo_validator, payload_validator, keycheck = self._initialize_request(cont_name, list_name, _id)
         payload = self.request.POST.mixed()
-        filename = payload.get('filename') or kwargs.get('filename')
+        filename = payload.get('filename') or kwargs.get('name')
 
         result = None
         with tempfile.TemporaryDirectory(prefix='.tmp', dir=self.app.config['upload_path']) as tempdir_path:
             file_store = files.FileStore(self.request, tempdir_path, filename=filename)
-            success = file_store.save_temp_file(tempdir_path)
-            if not success:
-                self.abort(400, 'Content-MD5 mismatch.')
             file_datetime = datetime.datetime.utcnow()
             file_properties = {
                 'name': file_store.filename,
-                'size': file_store.filesize,
-                'hash': file_store.sha384,
-                'tags': file_store.tags,
-                'metadata': file_store.metadata,
+                'size': file_store.size,
+                'hash': file_store.hash,
                 'created': file_datetime,
                 'modified': file_datetime,
                 'unprocessed': True
             }
-            file_properties['tags'] = file_properties.get('tags', [])
+            if file_store.metadata:
+                file_properties['metadata'] = file_store.metadata
+            if file_store.tags:
+                file_properties['tags'] = file_store.tags
             dest_path = os.path.join(self.app.config['data_path'], str(_id)[-3:] + '/' + str(_id))
+            query_params = None
             if not force:
                 method = 'POST'
             else:
-                filepath = os.path.join(file_store.tempdir_path, filename)
+                filepath = os.path.join(file_store.path, filename)
                 for f in container['files']:
                     if f['name'] == filename:
-                        if file_store.identical(os.path.join(data_path, filename), f['hash']):
+                        if file_store.identical(os.path.join(tempdir_path, filename), f['hash']):
                             log.debug('Dropping    %s (identical)' % filename)
                             os.remove(filepath)
                             self.abort(409, 'identical file exists')
                         else:
                             log.debug('Replacing   %s' % filename)
                             method = 'PUT'
+                            query_params = {'name':filename}
                         break
                 else:
                     method = 'POST'
             payload_validator(payload, method)
             payload.update(file_properties)
-            result = keycheck(mongo_validator(permchecker(storage.exec_op)))(method, _id=_id, payload=payload)
+            result = keycheck(mongo_validator(permchecker(storage.exec_op)))(method, _id=_id, query_params=query_params, payload=payload)
             if not result or result.modified_count != 1:
                 self.abort(404, 'Element not added in list {} of container {} {}'.format(storage.list_name, storage.cont_name, _id))
             try:
diff --git a/api/schemas/mongo/file.json b/api/schemas/mongo/file.json
index 2645ba7c..4cffb48c 100644
--- a/api/schemas/mongo/file.json
+++ b/api/schemas/mongo/file.json
@@ -24,6 +24,7 @@
       "type": "object"
     }
   },
-  "required": ["name", "created", "modified", "type", "size", "hash", "unprocessed"],
+  "required": ["name", "created", "modified", "size", "hash", "unprocessed"],
+  "key_fields": ["name"],
   "additionalProperties": false
 }
diff --git a/bin/bootstrap.py b/bin/bootstrap.py
index d33cfdad..477d86a6 100755
--- a/bin/bootstrap.py
+++ b/bin/bootstrap.py
@@ -126,17 +126,22 @@ def data(_, args):
             for chunk in iter(lambda: fd.read(2**20), ''):
                 hash_.update(chunk)
         destpath = os.path.join(args.storage_path, container.path)
+        filename = os.path.basename(filepath)
         if not os.path.exists(destpath):
             os.makedirs(destpath)
         if args.copy:
-            destpath = os.path.join(destpath, os.path.basename(filepath))
+            destpath = os.path.join(destpath, filename)
             shutil.copyfile(filepath, destpath)
         else:
             shutil.move(filepath, destpath)
+        created = modified = datetime.datetime.utcnow()
         fileinfo = {
+            'name': filename,
             'size': size,
             'hash': hash_.hexdigest(),
-            'unprocessed': True
+            'unprocessed': True,
+            'created': created,
+            'modified': modified
         }
         container.add_file(fileinfo)
 
-- 
GitLab