[rel-db] Fix base tests and deletion

openslides_backend/action/action_worker.py

@@ -10,6 +10,7 @@
 from gunicorn.http.message import Request
 from gunicorn.http.wsgi import Response
 from gunicorn.workers.gthread import ThreadWorker
+from psycopg.types.json import Jsonb
 
 from openslides_backend.services.database.extended_database import ExtendedDatabase
 from openslides_backend.services.postgresql.db_connection_handling import (
@@ -210,7 +211,7 @@ def final_action_worker_write(
                         fields={
                             "state": state,
                             "timestamp": current_time,
-                            "result": response,
+                            "result": Jsonb(response),
                         },
                     )
                 ],

openslides_backend/action/actions/agenda_item/delete.py

@@ -7,7 +7,6 @@
     fqid_from_collection_and_id,
     id_from_fqid,
 )
-from ....shared.typing import DeletedModel
 from ...generics.delete import DeleteAction
 from ...util.default_schema import DefaultSchema
 from ...util.register import register_action
@@ -31,15 +30,12 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]:
             fqid,
             ["content_object_id"],
         )
-        if agenda_item.get("content_object_id"):
-            content_object_fqid = agenda_item["content_object_id"]
+        if content_object_fqid := agenda_item.get("content_object_id"):
             if collection_from_fqid(
                 content_object_fqid
-            ) == "topic" and not self.datastore.is_deleted(content_object_fqid):
-                self.apply_instance(DeletedModel(), fqid)
+            ) == "topic" and not self.datastore.is_to_be_deleted(content_object_fqid):
                 self.execute_other_action(
                     TopicDelete,
                     [{"id": id_from_fqid(content_object_fqid)}],
                 )
-                self.apply_instance(DeletedModel(), content_object_fqid)
         return instance

openslides_backend/action/actions/assignment_candidate/delete.py

@@ -34,7 +34,9 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]:
                 mapped_fields=["phase", "meeting_id"],
                 lock_result=False,
             )
-            if assignment.get("phase") == "finished" and not self.is_meeting_deleted(
+            if assignment.get(
+                "phase"
+            ) == "finished" and not self.is_meeting_to_be_deleted(
                 assignment.get("meeting_id", 0)
             ):
                 raise ActionException(

openslides_backend/action/actions/group/delete.py

@@ -43,7 +43,7 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]:
                 "meeting_id",
             ],
         )
-        if len(group.get("meeting_user_ids", [])) and not self.is_meeting_deleted(
+        if len(group.get("meeting_user_ids", [])) and not self.is_meeting_to_be_deleted(
             group["meeting_id"]
         ):
             raise ActionException("You cannot delete a group with users.")

openslides_backend/action/actions/mediafile/delete.py

@@ -30,7 +30,7 @@ def get_tree_ids(self, id_: int) -> list[int]:
         )
         if node.get("child_ids"):
             for child_id in node["child_ids"]:
-                if not self.is_deleted(
+                if not self.is_to_be_deleted(
                     fqid_from_collection_and_id("mediafile", child_id)
                 ):
                     tree_ids.extend(self.get_tree_ids(child_id))

openslides_backend/action/actions/meeting/delete.py

@@ -1,5 +1,7 @@
 from typing import Any
 
+from openslides_backend.action.actions.topic.delete import TopicDelete
+
 from ....models.models import Meeting
 from ....shared.patterns import fqid_from_collection_and_id
 from ...generics.delete import DeleteAction
@@ -25,3 +27,12 @@ def get_committee_id(self, instance: dict[str, Any]) -> int:
             ["committee_id"],
         )
         return meeting["committee_id"]
+
+    def update_instance(self, instance: dict[str, Any]) -> dict:
+        meeting = self.datastore.get(
+            fqid_from_collection_and_id("meeting", instance["id"]), []
+        )
+        if topic_ids := meeting.get("topic_ids"):
+            for topic_fqid in topic_ids:
+                self.execute_other_action(TopicDelete, [{"id": topic_fqid}])
+        return instance

openslides_backend/action/actions/meeting/replace_projector_id.py

@@ -27,10 +27,10 @@ def get_updated_instances(self, payload: ActionData) -> ActionData:
         for instance in payload:
             projector_id = instance.pop("projector_id")
             fields = Meeting.all_default_projectors()
-            meeting = self.datastore.get(
-                fqid_from_collection_and_id(self.model.collection, instance["id"]),
-                fields + ["reference_projector_id"],
-            )
+            fqid = fqid_from_collection_and_id(self.model.collection, instance["id"])
+            if self.datastore.is_to_be_deleted(fqid):
+                continue
+            meeting = self.datastore.get(fqid, fields + ["reference_projector_id"])
             changed = False
             for field in fields:
                 change_list = meeting.get(field)

openslides_backend/action/actions/motion_workflow/delete.py

@@ -27,7 +27,7 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]:
             fqid_from_collection_and_id("motion_workflow", instance["id"]),
             ["meeting_id"],
         )
-        if not self.is_meeting_deleted(workflow["meeting_id"]):
+        if not self.is_meeting_to_be_deleted(workflow["meeting_id"]):
             meeting = self.datastore.get(
                 fqid_from_collection_and_id("meeting", workflow["meeting_id"]),
                 [

openslides_backend/action/actions/projection/delete.py

@@ -34,11 +34,11 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]:
         if not (
             projection.get("current_projector_id")
             or projection.get("preview_projector_id")
-            or self.is_meeting_deleted(projection["meeting_id"])
-            or self.is_deleted(projection["content_object_id"])
+            or self.is_meeting_to_be_deleted(projection["meeting_id"])
+            or self.is_to_be_deleted(projection["content_object_id"])
             or (
                 "history_projector_id" in projection
-                and self.is_deleted(
+                and self.is_to_be_deleted(
                     fqid_from_collection_and_id(
                         "projector", projection["history_projector_id"]
                     )

openslides_backend/action/actions/projector/delete.py

@@ -27,7 +27,7 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]:
         )
         if (
             meeting_id := projector.get("used_as_reference_projector_meeting_id")
-        ) and not self.is_meeting_deleted(meeting_id):
+        ) and not self.is_meeting_to_be_deleted(meeting_id):
             raise ActionException(
                 "A used as reference projector is not allowed to delete."
             )

openslides_backend/action/actions/projector_countdown/delete.py

@@ -31,7 +31,7 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]:
         meeting_id = projector_countdown.get(
             "used_as_list_of_speakers_countdown_meeting_id"
         ) or projector_countdown.get("used_as_poll_countdown_meeting_id")
-        if meeting_id and not self.is_meeting_deleted(meeting_id):
+        if meeting_id and not self.is_meeting_to_be_deleted(meeting_id):
             raise ActionException(
                 "List of speakers or poll countdown is not allowed to delete."
             )

openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py

@@ -14,7 +14,7 @@ def conditionally_delete_speakers(self, speaker_ids: list[int]) -> None:
         speaker_to_read_ids = [
             speaker_id
             for speaker_id in speaker_ids
-            if not self.datastore.is_deleted(
+            if not self.datastore.is_to_be_deleted(
                 fqid_from_collection_and_id("speaker", speaker_id)
             )
         ]

openslides_backend/action/generics/delete.py

@@ -1,7 +1,7 @@
 from collections.abc import Iterable
-from typing import Any
+from typing import Any, cast
 
-from ...models.fields import OnDelete
+from ...models.fields import BaseRelationField, OnDelete
 from ...shared.exceptions import ActionException, ProtectedModelsException
 from ...shared.interfaces.event import Event, EventType
 from ...shared.patterns import (
@@ -26,13 +26,19 @@ def base_update_instance(self, instance: dict[str, Any]) -> dict[str, Any]:
         """
         Takes care of on_delete handling.
         """
-        # Fetch db instance with all relevant fields
-        # Executed before update_instance so that actions can manually set a
-        # DeletedModel or other changed_models without changing the result of this.
         this_fqid = fqid_from_collection_and_id(self.model.collection, instance["id"])
+
+        # Have been here. Seen it all.
+        if self.datastore.is_to_be_deleted(this_fqid):
+            return instance
+        self.datastore.apply_to_be_deleted(this_fqid)
+
         relevant_fields = [
             field.get_own_field_name() for field in self.model.get_relation_fields()
         ]
+        # Fetch db instance with all relevant fields
+        # Executed before update_instance so that actions can manually set a
+        # DeletedModel or other changed_models without changing the result of this.
         db_instance = self.datastore.get(
             fqid=this_fqid,
             mapped_fields=relevant_fields,
@@ -43,28 +49,31 @@ def base_update_instance(self, instance: dict[str, Any]) -> dict[str, Any]:
 
         # Update instance and set relation fields to None.
         # Gather all delete actions with action data and also all models to be deleted
-        delete_actions: list[tuple[FullQualifiedId, type[Action], ActionData]] = []
-        self.datastore.apply_changed_model(this_fqid, DeletedModel())
-        for field in self.model.get_relation_fields():
+        delete_actions: list[tuple[type[Action], ActionData]] = []
+        for field_name in sorted(db_instance):
+            if field_name == "id":
+                continue
+            field = cast(BaseRelationField, self.model.get_field(field_name))
             # Check on_delete.
             if field.on_delete != OnDelete.SET_NULL:
                 # Extract all foreign keys as fqids from the model
-                foreign_fqids: list[FullQualifiedId] = []
-                value = db_instance.get(field.get_own_field_name(), [])
+                value = db_instance.get(field_name, [])
                 foreign_fqids = transform_to_fqids(value, field.get_target_collection())
 
                 if field.on_delete == OnDelete.PROTECT:
                     protected_fqids = [
-                        fqid for fqid in foreign_fqids if not self.is_deleted(fqid)
+                        fqid
+                        for fqid in foreign_fqids
+                        if not self.datastore.is_to_be_deleted_for_protected(fqid)
                     ]
                     if protected_fqids:
                         raise ProtectedModelsException(this_fqid, protected_fqids)
                 else:
                     # field.on_delete == OnDelete.CASCADE
                     # Execute the delete action for all fqids
                     for fqid in foreign_fqids:
-                        if self.is_deleted(fqid):
-                            # skip models that are already deleted
+                        if self.datastore.is_to_be_deleted(fqid):
+                            # skip models that are already tracked for deletion
                             continue
                         delete_action_class = actions_map.get(
                             f"{collection_from_fqid(fqid)}.delete"
@@ -76,37 +85,38 @@ def base_update_instance(self, instance: dict[str, Any]) -> dict[str, Any]:
                             )
                         # Assume that the delete action uses the standard action data
                         action_data = [{"id": id_from_fqid(fqid)}]
-                        delete_actions.append((fqid, delete_action_class, action_data))
+                        delete_actions.append((delete_action_class, action_data))
+                        self.datastore.apply_to_be_deleted_for_protected(fqid)
             else:
                 # field.on_delete == OnDelete.SET_NULL
-                instance[field.get_own_field_name()] = None
+                instance[field_name] = None
 
         # Add additional relation models and execute all previously gathered delete actions
         # catch all protected models exception to gather all protected fqids
         all_protected_fqids: list[FullQualifiedId] = []
-        for fqid, delete_action_class, delete_action_data in delete_actions:
+        for delete_action_class, delete_action_data in delete_actions:
             try:
                 self.execute_other_action(delete_action_class, delete_action_data)
-                self.datastore.apply_changed_model(fqid, DeletedModel())
             except ProtectedModelsException as e:
                 all_protected_fqids.extend(e.fqids)
 
         if all_protected_fqids:
             raise ProtectedModelsException(this_fqid, all_protected_fqids)
 
+        self.datastore.apply_changed_model(this_fqid, DeletedModel())
         return instance
 
     def create_events(self, instance: dict[str, Any]) -> Iterable[Event]:
         fqid = fqid_from_collection_and_id(self.model.collection, instance["id"])
         yield self.build_event(EventType.Delete, fqid)
 
-    def is_meeting_deleted(self, meeting_id: int) -> bool:
+    def is_meeting_to_be_deleted(self, meeting_id: int) -> bool:
         """
-        Returns whether the given meeting was deleted during this request or not.
+        Returns whether the given meeting was/will be deleted during this request or not.
         """
-        return self.datastore.is_deleted(
+        return self.datastore.is_to_be_deleted(
             fqid_from_collection_and_id("meeting", meeting_id)
         )
 
-    def is_deleted(self, fqid: FullQualifiedId) -> bool:
-        return self.datastore.is_deleted(fqid)
+    def is_to_be_deleted(self, fqid: FullQualifiedId) -> bool:
+        return self.datastore.is_to_be_deleted(fqid)

openslides_backend/action/mixins/extend_history_mixin.py

@@ -19,9 +19,11 @@ class ExtendHistoryMixin(Action):
     def create_events(self, instance: dict[str, Any]) -> Iterable[Event]:
         yield from super().create_events(instance)
         field = self.model.get_field(self.extend_history_to)
+        fqid = fqid_from_collection_and_id(self.model.collection, instance["id"])
         model = self.datastore.get(
-            fqid_from_collection_and_id(self.model.collection, instance["id"]),
+            fqid,
             [self.extend_history_to],
+            use_changed_models=not self.datastore.is_deleted(fqid),
         )
         value = model[self.extend_history_to]
         if isinstance(field, GenericRelationField):

openslides_backend/action/relations/relation_manager.py

@@ -40,6 +40,7 @@ def get_relation_updates(
             # only relations are handled here
             if not isinstance(field, BaseRelationField):
                 continue
+
             handler = SingleRelationHandler(
                 self.datastore,
                 field,

openslides_backend/action/relations/single_relation_handler.py

@@ -232,11 +232,11 @@ def relation_diffs(
         # Calculate add set and remove set
         new_fqids = set(rel_fqids)
         add = new_fqids - current_fqids
-        # filter out deleted models, so that in case of cascade deletion no data is lost
+        # filter out deleted models for improved performance
         remove = {
             fqid
             for fqid in current_fqids - new_fqids
-            if not self.datastore.is_deleted(fqid)
+            if not self.datastore.is_to_be_deleted(fqid)
         }
 
         return add, remove

openslides_backend/services/database/database_writer.py

@@ -6,6 +6,7 @@
     CheckViolation,
     DatatypeMismatch,
     GeneratedAlways,
+    InFailedSqlTransaction,
     NotNullViolation,
     ProgrammingError,
     SyntaxError,
@@ -530,6 +531,10 @@ def execute_sql(
                         raise ModelDoesNotExist(error_fqid)
                     id_ = result.get("id", 0)
                     return id_
+        except InFailedSqlTransaction as e:
+            raise BadCodingException(
+                f"Tried to set {error_fqid} in an already broken transaction: {e}"
+            )
         except UniqueViolation as e:
             if "duplicate key value violates unique constraint" in e.args[0]:
                 if "Key (id)" in e.args[0]:

openslides_backend/services/database/extended_database.py

@@ -99,6 +99,8 @@ def __init__(
         self.env = env
         self.logger = logging.getLogger(__name__)
         self._changed_models = defaultdict(lambda: defaultdict(dict))
+        self._to_be_deleted: set[FullQualifiedId] = set()
+        self._to_be_deleted_for_protected: set[FullQualifiedId] = set()
         self.connection = connection
         self.database_reader = DatabaseReader(self.connection, logging, env)
         self.database_writer = DatabaseWriter(self.connection, logging, env)
@@ -118,6 +120,14 @@ def apply_changed_model(
         if "id" not in self._changed_models[collection][id_]:
             self._changed_models[collection][id_]["id"] = id_
 
+    def apply_to_be_deleted(self, fqid: FullQualifiedId) -> None:
+        """Meaning both: to be deleted in the future and the past."""
+        self._to_be_deleted.add(fqid)
+
+    def apply_to_be_deleted_for_protected(self, fqid: FullQualifiedId) -> None:
+        """Meaning both: to be deleted in the future and the past. Only used for protected models deletion."""
+        self._to_be_deleted_for_protected.add(fqid)
+
     def get_changed_model(
         self, collection_or_fqid: str, id_: int | None = None
     ) -> PartialModel:
@@ -128,6 +138,14 @@ def get_changed_model(
     def get_changed_models(self, collection: str) -> dict[Id, PartialModel]:
         return self._changed_models.get(collection, dict())
 
+    def is_to_be_deleted(self, fqid: FullQualifiedId) -> bool:
+        """Meaning both: to be deleted in the future and the past."""
+        return fqid in self._to_be_deleted
+
+    def is_to_be_deleted_for_protected(self, fqid: FullQualifiedId) -> bool:
+        """Meaning both: to be deleted in the future and the past. Only used for protected models deletion."""
+        return fqid in self._to_be_deleted_for_protected or fqid in self._to_be_deleted
+
     def get(
         self,
         fqid: FullQualifiedId,

openslides_backend/services/database/interface.py

@@ -36,6 +36,12 @@ def apply_changed_model(
         self, fqid: FullQualifiedId, instance: PartialModel, replace: bool = False
     ) -> None: ...
 
+    @abstractmethod
+    def apply_to_be_deleted(self, fqid: FullQualifiedId) -> None: ...
+
+    @abstractmethod
+    def apply_to_be_deleted_for_protected(self, fqid: FullQualifiedId) -> None: ...
+
     @abstractmethod
     def get_changed_model(
         self, collection_or_fqid: str, id_: Id | None = None
@@ -44,6 +50,12 @@ def get_changed_model(
     @abstractmethod
     def get_changed_models(self, collection: str) -> dict[Id, PartialModel]: ...
 
+    @abstractmethod
+    def is_to_be_deleted(self, fqid: FullQualifiedId) -> bool: ...
+
+    @abstractmethod
+    def is_to_be_deleted_for_protected(self, fqid: FullQualifiedId) -> bool: ...
+
     @abstractmethod
     def get(
         self,