fix: adds sorting to file listing (#960)

renku/service/serializers/cache.py

@@ -71,6 +71,7 @@ class FileUploadContext(Schema):
 
     relative_path = fields.String(required=True)
     is_archive = fields.Boolean(missing=False)
+    is_dir = fields.Boolean(required=True)
     unpack_archive = fields.Boolean(missing=False)
 
 

renku/service/utils/__init__.py

@@ -44,6 +44,15 @@ def make_file_path(user, cached_file):
         )
 
 
+def valid_file(user, cached_file):
+    """Ensure file system and cache state matches."""
+    file_path = make_file_path(user, cached_file)
+
+    if file_path.exists():
+        cached_file['is_dir'] = file_path.is_dir()
+        return cached_file
+
+
 def repo_sync(repo_path, remote_names=('origin', )):
     """Sync the repo with the remotes."""
     repo = Repo(repo_path)

renku/service/views/cache.py

@@ -34,7 +34,7 @@
     FileUploadResponse, FileUploadResponseRPC, ProjectCloneContext, \
     ProjectCloneRequest, ProjectCloneResponse, ProjectCloneResponseRPC, \
     ProjectListResponse, ProjectListResponseRPC, extract_file
-from renku.service.utils import make_file_path, make_project_path
+from renku.service.utils import make_project_path, valid_file
 from renku.service.views.decorators import accepts_json, handle_base_except, \
     handle_git_except, handle_renku_except, handle_validation_except, \
     header_doc, requires_cache, requires_identity
@@ -58,12 +58,17 @@
 @requires_identity
 def list_uploaded_files_view(user, cache):
     """List uploaded files ready to be added to projects."""
-    files = [
-        f for f in cache.get_files(user) if make_file_path(user, f).exists()
-    ]
+    files = filter(None, [valid_file(user, f) for f in cache.get_files(user)])
+
+    response_payload = {
+        'files':
+            sorted(
+                files, key=lambda rec: (rec['is_dir'], rec['relative_path'])
+            )
+    }
 
     response = FileListResponseRPC().load({
-        'result': FileListResponse().load({'files': files})
+        'result': FileListResponse().load(response_payload)
     })
     return jsonify(response)
 
@@ -132,24 +137,26 @@ def upload_file_view(user, cache):
             )
 
         for file_ in temp_dir.glob('**/*'):
+            relative_path = file_.relative_to(
+                CACHE_UPLOADS_PATH / user['user_id']
+            )
+
             file_obj = {
                 'file_name': file_.name,
                 'file_size': os.stat(str(file_path)).st_size,
-                'relative_path':
-                    str(
-                        file_.relative_to(
-                            CACHE_UPLOADS_PATH / user['user_id']
-                        )
-                    )
+                'relative_path': str(relative_path),
+                'is_dir': relative_path.is_dir(),
             }
 
             files.append(FileUploadContext().load(file_obj, unknown=EXCLUDE))
 
     else:
-        response_builder['file_size'] = os.stat(str(file_path)).st_size
-        response_builder['relative_path'] = str(
-            file_path.relative_to(CACHE_UPLOADS_PATH / user['user_id'])
+        relative_path = file_path.relative_to(
+            CACHE_UPLOADS_PATH / user['user_id']
         )
+        response_builder['file_size'] = os.stat(str(file_path)).st_size
+        response_builder['relative_path'] = str(relative_path)
+        response_builder['is_dir'] = relative_path.is_dir()
 
         files.append(
             FileUploadContext().load(response_builder, unknown=EXCLUDE)

tests/service/test_cache_views.py

@@ -534,6 +534,24 @@ def test_upload_tar_unpack_archive(datapack_tar, svc_client_with_repo):
         assert not file_['is_archive']
         assert not file_['unpack_archive']
 
+    response = svc_client.get(
+        '/cache.files_list',
+        headers=headers,
+    )
+
+    assert response
+    assert 200 == response.status_code
+    assert {'result'} == set(response.json.keys())
+    assert 3 == len(response.json['result']['files'])
+
+    uploaded_dir = response.json['result']['files'].pop()
+    assert uploaded_dir['is_dir']
+
+    paths = [
+        _file['relative_path'] for _file in response.json['result']['files']
+    ]
+    assert sorted(paths) == paths
+
 
 @pytest.mark.service
 def test_upload_tar_archive(datapack_tar, svc_client_with_repo):
@@ -596,6 +614,7 @@ def test_field_upload_resp_fields(datapack_tar, svc_client_with_repo):
         'is_archive',
         'timestamp',
         'is_archive',
+        'is_dir',
         'unpack_archive',
         'relative_path',
     } == set(response.json['result']['files'][0].keys())