More cleanup and testing

distributed-system-analysis · Oct 6, 2023 · ae97fd8 · ae97fd8
1 parent d0c2efe
commit ae97fd8
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 20 deletions.
diff --git a/lib/pbench/server/api/resources/datasets_contents.py b/lib/pbench/server/api/resources/datasets_contents.py
@@ -83,20 +83,25 @@ def _get(
         )
 
         details: CacheObject = info["details"]
+        current_app.logger.info(
+            "{!r} -> {{name: {!r}, loc: {!r}}}",
+            str(path),
+            details.name,
+            str(details.location),
+        )
         if details.type is CacheType.DIRECTORY:
             children = info["children"] if "children" in info else {}
             dir_list = []
             file_list = []
 
             for c, value in children.items():
-                p = path / c
                 d: CacheObject = value["details"]
                 if d.type == CacheType.DIRECTORY:
                     dir_list.append(
                         {
                             "name": c,
                             "type": d.type.name,
-                            "uri": f"{origin}/contents/{p}",
+                            "uri": f"{origin}/contents/{d.location}",
                         }
                     )
                 elif d.type == CacheType.FILE:
@@ -105,23 +110,29 @@ def _get(
                             "name": c,
                             "size": d.size,
                             "type": d.type.name,
-                            "uri": f"{origin}/inventory/{p}",
+                            "uri": f"{origin}/inventory/{d.location}",
                         }
                     )
 
             dir_list.sort(key=lambda d: d["name"])
             file_list.sort(key=lambda d: d["name"])
+
+            # Normalize because we want the "root" directory to be reported as
+            # "" rather than as Path's favored "."
+            loc = "" if str(details.location) == "." else str(details.location)
             val = {
                 "name": details.name,
                 "type": details.type.name,
                 "directories": dir_list,
                 "files": file_list,
+                "uri": f"{origin}/contents/{loc}",
             }
         else:
             val = {
                 "name": details.name,
                 "size": details.size,
                 "type": details.type.name,
+                "uri": f"{origin}/inventory/{details.location}",
             }
 
         try:

diff --git a/lib/pbench/test/functional/server/test_datasets.py b/lib/pbench/test/functional/server/test_datasets.py
@@ -689,6 +689,9 @@ def test_contents(self, server_client: PbenchServerClient, login_user):
             # Even if they're empty, both values must be lists
             assert isinstance(json["directories"], list)
             assert isinstance(json["files"], list)
+            assert json["name"] == ""
+            assert json["type"] == "DIRECTORY"
+            assert json["uri"] == response.url
 
             # We expect to find at least a metadata.log at the top level of the
             # tarball unless the dataset is marked archive-only.

diff --git a/lib/pbench/test/unit/server/test_datasets_contents.py b/lib/pbench/test/unit/server/test_datasets_contents.py
@@ -62,14 +62,17 @@ def test_unauthorized_access(self, query_get_as):
 
     @pytest.mark.parametrize("key", (None, "", "subdir1"))
     def test_path_is_directory(self, query_get_as, monkeypatch, key):
-        base = Path("/mock/cache/ABC")
-
         def mock_find_entry(_s, _d: str, path: Optional[Path]):
-            file = base / (path if path else "")
+            file = path if path else Path("")
             return {
                 "children": {},
                 "details": CacheObject(
-                    file.name, file, None, None, None, CacheType.DIRECTORY
+                    file.name if key else "",
+                    file,
+                    None,
+                    None,
+                    None,
+                    CacheType.DIRECTORY,
                 ),
             }
 
@@ -81,8 +84,9 @@ def mock_find_entry(_s, _d: str, path: Optional[Path]):
         assert response.json == {
             "directories": [],
             "files": [],
-            "name": key if key else base.name,
+            "name": key if key else "",
             "type": "DIRECTORY",
+            "uri": f"https://localhost/api/v1/datasets/random_md5_string4/contents/{key if key else ''}",
         }
 
     def test_not_a_file(self, query_get_as, monkeypatch):
@@ -98,40 +102,50 @@ def mock_find_entry(_s, _d: str, path: Optional[Path]):
 
     def test_file_info(self, query_get_as, monkeypatch):
         name = "f1.json"
-        base = Path("/mock/cache/ABC")
 
         def mock_find_entry(_s, _d: str, path: Optional[Path]):
-            file = base / (path if path else "")
             return {
-                "details": CacheObject(file.name, file, None, None, 16, CacheType.FILE)
+                "details": CacheObject(path.name, path, None, None, 16, CacheType.FILE)
             }
 
         monkeypatch.setattr(CacheManager, "find_entry", mock_find_entry)
         response = query_get_as("fio_2", name, HTTPStatus.OK)
         assert response.status_code == HTTPStatus.OK
-        assert response.json == {"name": name, "size": 16, "type": "FILE"}
+        assert response.json == {
+            "name": name,
+            "size": 16,
+            "type": "FILE",
+            "uri": f"https://localhost/api/v1/datasets/random_md5_string4/inventory/{name}",
+        }
 
     def test_dir_info(self, query_get_as, monkeypatch):
-        name = "sample1"
-        base = Path("/mock/cache/ABC")
-
         def mock_find_entry(_s, _d: str, path: Optional[Path]):
-            file = base / (path if path else "")
+            base = path if path else Path("")
             return {
                 "children": {
                     "default": {
                         "details": CacheObject(
                             "default",
-                            base / name / "default",
+                            base / "default",
                             None,
                             None,
                             None,
                             CacheType.DIRECTORY,
                         )
-                    }
+                    },
+                    "file.txt": {
+                        "details": CacheObject(
+                            "file.txt",
+                            base / "file.txt",
+                            None,
+                            None,
+                            42,
+                            CacheType.FILE,
+                        )
+                    },
                 },
                 "details": CacheObject(
-                    file.name, file, None, None, 16, CacheType.DIRECTORY
+                    base.name, base, None, None, None, CacheType.DIRECTORY
                 ),
             }
 
@@ -146,7 +160,15 @@ def mock_find_entry(_s, _d: str, path: Optional[Path]):
                     "uri": "https://localhost/api/v1/datasets/random_md5_string4/contents/sample1/default",
                 }
             ],
-            "files": [],
+            "files": [
+                {
+                    "name": "file.txt",
+                    "size": 42,
+                    "type": "FILE",
+                    "uri": "https://localhost/api/v1/datasets/random_md5_string4/inventory/sample1/file.txt",
+                }
+            ],
             "name": "sample1",
             "type": "DIRECTORY",
+            "uri": "https://localhost/api/v1/datasets/random_md5_string4/contents/sample1",
         }