Handle request abspath in the api layer.

A path locating a file in a request has the form `group/path/file`, and
the file on disk is just `root/path/file`. Previously, this discrepency
was handled in file_browser_api layer, at tree construction time.

Building on the previous commit, we move this to be a responsibility of
the ReleaseRequest container. Its abspath() now expected the full path
including group, validates the group part, and returns the abspath on
disk.

This is much cleaner, and removes some logic and attributes from
PathItem.

airlock/api.py

@@ -160,8 +160,16 @@ def get_url(self, relpath=ROOT_PATH):
         )
 
     def abspath(self, relpath):
+        relpath = UrlPath(relpath)
         root = self.root()
-        path = root / relpath
+
+        group = relpath.parts[0]
+
+        if group not in self.filegroups:
+            raise ProviderAPI.FileNotFound(f"bad group {group} in url {relpath}")
+
+        filepath = relpath.relative_to(group)
+        path = root / filepath
 
         # protect against traversal
         path.resolve().relative_to(root)
@@ -359,7 +367,7 @@ def add_file_to_request(
 
         workspace = self.get_workspace(release_request.workspace)
         src = workspace.abspath(relpath)
-        dst = release_request.abspath(relpath)
+        dst = release_request.root() / relpath
         dst.parent.mkdir(exist_ok=True, parents=True)
         shutil.copy(src, dst)
 

airlock/file_browser_api.py

@@ -43,20 +43,16 @@ class PathNotFound(Exception):
     # should this node be expanded in the tree?
     expanded: bool = False
 
-    # relative filepath on disk. defaults to path, but allows us to customise it.
-    filepath: UrlPath = None
     # what to display for this node when rendering the tree. Defaults to name,
     # but this allow it to be overridden.
     display_text: str = None
 
     def __post_init__(self):
         # ensure is UrlPath
         self.relpath = UrlPath(self.relpath)
-        if self.filepath is None:
-            self.filepath = self.relpath
 
     def _absolute_path(self):
-        return self.container.abspath(self.filepath)
+        return self.container.abspath(self.relpath)
 
     def is_directory(self):
         """Does this contain other things?"""
@@ -298,8 +294,6 @@ def build_filegroup_tree(file_parts, path, parent):
                 container=container,
                 relpath=child_path,
                 parent=parent,
-                # actual path on disk, striping the group part
-                filepath=child_path.relative_to(child_path.parts[0]),
                 selected=selected,
             )
 

tests/integration/views/test_workspace.py

@@ -159,7 +159,7 @@ def test_workspace_request_file_creates(client_with_user, api):
     filegroup = release_request.filegroups["default"]
     assert filegroup.name == "default"
     assert str(filegroup.files[0].relpath) == "test/path.txt"
-    assert release_request.abspath("test/path.txt").exists()
+    assert release_request.abspath("default/test/path.txt").exists()
 
 
 def test_workspace_request_file_request_already_exists(client_with_user, api):
@@ -178,7 +178,7 @@ def test_workspace_request_file_request_already_exists(client_with_user, api):
     assert response.status_code == 302
     current_release_request = api.get_current_request(workspace.name, user)
     assert current_release_request.id == release_request.id
-    assert release_request.abspath("test/path.txt").exists()
+    assert current_release_request.abspath("default/test/path.txt").exists()
     filegroup = current_release_request.filegroups["default"]
     assert filegroup.name == "default"
     assert str(filegroup.files[0].relpath) == "test/path.txt"

tests/unit/test_api.py

@@ -5,7 +5,7 @@
 import pytest
 
 import old_api
-from airlock.api import ProviderAPI, Status, Workspace
+from airlock.api import ProviderAPI, Status, UrlPath, Workspace
 from airlock.users import User
 from tests import factories
 
@@ -69,7 +69,7 @@ def test_provider_request_release_files(mock_old_api):
     factories.write_request_file(release_request, "group", relpath, "test")
     factories.api.set_status(release_request, Status.APPROVED, checker)
 
-    abspath = release_request.abspath(relpath)
+    abspath = release_request.abspath("group" / relpath)
 
     api = ProviderAPI()
     api.release_files(release_request, checker)
@@ -310,11 +310,10 @@ def test_add_file_to_request_states(status, success, api):
 
     if success:
         api.add_file_to_request(release_request, path, author)
-        assert release_request.abspath(path).exists()
+        assert release_request.abspath("default" / path).exists()
     else:
         with pytest.raises(api.RequestPermissionDenied):
             api.add_file_to_request(release_request, path, author)
-        assert not release_request.abspath(path).exists()
 
 
 def test_request_release_invalid_state():
@@ -326,6 +325,17 @@ def test_request_release_invalid_state():
         )
 
 
+def test_request_release_abspath(api):
+    path = UrlPath("foo/bar.txt")
+    release_request = factories.create_release_request("id")
+    factories.write_request_file(release_request, "default", path)
+
+    with pytest.raises(api.FileNotFound):
+        release_request.abspath("badgroup" / path)
+
+    assert release_request.abspath("default" / path).exists()
+
+
 def setup_empty_release_request():
     author = User(1, "author", ["workspace"], False)
     path = Path("path/file.txt")