Unfreeze core objects.

Update the two places we mutate state to mutate the object.

Manually made Workspace hashable, for convenience in tests, but also it
is just one bit of immutable data.

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