Merge pull request #99 from opensafely-core/thaw

Unfreeze core objects.

Author: bloodearnest

airlock/api.py

@@ -44,7 +44,7 @@ def get_url_for_path(self):
         raise NotImplementedError()
 
 
-@dataclass(frozen=True)
+@dataclass
 class Workspace(AirlockContainer):
     """Simple wrapper around a workspace directory on disk.
 
@@ -89,6 +89,9 @@ def abspath(self, relpath):
 
         return path
 
+    def __hash__(self):
+        return hash(self.name)
+
 
 @dataclass(frozen=True)
 class RequestFile:
@@ -109,7 +112,7 @@ class FileGroup:
     files: list[RequestFile]
 
 
-@dataclass(frozen=True)
+@dataclass
 class ReleaseRequest(AirlockContainer):
     """Represents a release request made by a user.
 
@@ -316,30 +319,22 @@ def set_status(
 
         # validate first
         self.check_status(release_request, to_status, user)
-
-        # Ensure the state of this object is updated to reflect the change in status
-        # This is deliberately bypassing the frozen aspect of the class to keep
-        # things consistent. It does change the hash.
-        release_request.__dict__["status"] = to_status
+        release_request.status = to_status
 
     def add_file_to_request(
         self,
         release_request: ReleaseRequest,
         relpath: Path,
         user: User,
         group_name: Optional[str] = "default",
-    ) -> ReleaseRequest:
+    ):
         """Add a file to a request.
 
         Subclasses should do what they need to create the filegroup and
         record the file metadata as needed and THEN
         call super().add_file_to_request(...) to do the
         copying. If the copying fails (e.g. due to permission errors raised
         below), the subclasses should roll back any changes.
-
-        After calling add_file_to_request, the current release_request's
-        filegroups will be out of date. Subclasses should return a new
-        ReleaseRequest to ensure filegroups are current.
         """
         if user.username != release_request.author:
             raise self.RequestPermissionDenied(

local_db/api.py

@@ -135,7 +135,7 @@ def add_file_to_request(
         relpath: Path,
         user: User,
         group_name: Optional[str] = "default",
-    ) -> ReleaseRequest:
+    ):
         with transaction.atomic():
             # Get/create the FileGroupMetadata if it doesn't already exist
             filegroupmetadata = self._get_or_create_filegroupmetadata(
@@ -163,5 +163,6 @@ def add_file_to_request(
             # state that allows adding files.
             super().add_file_to_request(release_request, relpath, user, group_name)
 
-        # return a new request object with the updated groups
-        return self._request(self._find_metadata(release_request.id))
+        # update instance
+        metadata = self._find_metadata(release_request.id)
+        release_request.filegroups = self._get_filegroups(metadata)

tests/unit/test_api.py

@@ -294,7 +294,7 @@ def test_release_request_filegroups_with_no_files(api):
 def test_release_request_filegroups_default_filegroup(api):
     release_request, path, author = setup_empty_release_request()
     assert release_request.filegroups == {}
-    release_request = api.add_file_to_request(release_request, path, author)
+    api.add_file_to_request(release_request, path, author)
     assert len(release_request.filegroups) == 1
     filegroup = release_request.filegroups["default"]
     assert filegroup.name == "default"
@@ -305,9 +305,7 @@ def test_release_request_filegroups_default_filegroup(api):
 def test_release_request_filegroups_named_filegroup(api):
     release_request, path, author = setup_empty_release_request()
     assert release_request.filegroups == {}
-    release_request = api.add_file_to_request(
-        release_request, path, author, "test_group"
-    )
+    api.add_file_to_request(release_request, path, author, "test_group")
     assert len(release_request.filegroups) == 1
     filegroup = release_request.filegroups["test_group"]
     assert filegroup.name == "test_group"
@@ -317,9 +315,7 @@ def test_release_request_filegroups_named_filegroup(api):
 
 def test_release_request_filegroups_multiple_filegroups(api):
     release_request, path, author = setup_empty_release_request()
-    release_request = api.add_file_to_request(
-        release_request, path, author, "test_group"
-    )
+    api.add_file_to_request(release_request, path, author, "test_group")
     assert len(release_request.filegroups) == 1
 
     workspace = api.get_workspace("workspace")
@@ -347,7 +343,7 @@ def test_release_request_filegroups_multiple_filegroups(api):
 def test_release_request_add_same_file(api):
     release_request, path, author = setup_empty_release_request()
     assert release_request.filegroups == {}
-    release_request = api.add_file_to_request(release_request, path, author)
+    api.add_file_to_request(release_request, path, author)
     assert len(release_request.filegroups) == 1
     assert len(release_request.filegroups["default"].files) == 1