[BREAKING] - remove is_deleted for Connection model

Author: maxim-lixakov

docs/changelog/next_release/170.breaking.rst

@@ -0,0 +1 @@
+Add deletion of **connections** records instead of marking them as deleted

syncmaster/db/migrations/versions/2023-11-23_0004_create_connection_table.py

@@ -33,7 +33,6 @@ def upgrade():
         sa.Column("name", sa.String(length=128), nullable=False),
         sa.Column("description", sa.String(length=512), nullable=False),
         sa.Column("data", sa.JSON(), nullable=False),
-        sa.Column("is_deleted", sa.Boolean(), nullable=False),
         sa.Column("created_at", sa.DateTime(), server_default=sa.text("now()"), nullable=False),
         sa.Column("updated_at", sa.DateTime(), server_default=sa.text("now()"), nullable=False),
         sa.Column("search_vector", postgresql.TSVECTOR(), sa.Computed(sql_expression, persisted=True), nullable=False),

syncmaster/db/models/connection.py

@@ -8,12 +8,12 @@
 from sqlalchemy.dialects.postgresql import TSVECTOR
 from sqlalchemy.orm import Mapped, declared_attr, mapped_column, relationship
 
-from syncmaster.db.mixins import DeletableMixin, ResourceMixin, TimestampMixin
+from syncmaster.db.mixins import ResourceMixin, TimestampMixin
 from syncmaster.db.models.base import Base
 from syncmaster.db.models.group import Group
 
 
-class Connection(Base, ResourceMixin, DeletableMixin, TimestampMixin):
+class Connection(Base, ResourceMixin, TimestampMixin):
     data: Mapped[dict[str, Any]] = mapped_column(JSON, nullable=False, default={})
 
     group: Mapped[Group] = relationship("Group")

syncmaster/db/repositories/connection.py

@@ -32,7 +32,6 @@ async def paginate(
         connection_type: list[str] | None = None,
     ) -> Pagination:
         stmt = select(Connection).where(
-            Connection.is_deleted.is_(False),
             Connection.group_id == group_id,
         )
         if search_query:
@@ -53,7 +52,7 @@ async def read_by_id(
         self,
         connection_id: int,
     ) -> Connection:
-        stmt = select(Connection).where(Connection.id == connection_id, Connection.is_deleted.is_(False))
+        stmt = select(Connection).where(Connection.id == connection_id)
         result: ScalarResult[Connection] = await self._session.scalars(stmt)
         try:
             return result.one()
@@ -99,7 +98,6 @@ async def update(
 
             return await self._update(
                 Connection.id == connection_id,
-                Connection.is_deleted.is_(False),
                 name=name or connection.name,
                 description=description or connection.description,
                 data=data,

tests/test_unit/test_connections/test_copy_connection.py

@@ -44,7 +44,7 @@ async def test_maintainer_plus_can_copy_connection_with_delete_source(
         "user": "user",
         "password": "password",
     }
-    assert not current.is_deleted
+    assert current is not None
 
     query_copy_not_exist = select(Connection).filter(
         Connection.group_id == empty_group.id,
@@ -73,14 +73,13 @@ async def test_maintainer_plus_can_copy_connection_with_delete_source(
     }
     assert result.status_code == 200
 
-    await session.refresh(group_connection.connection)
     query_prev_row = select(Connection).where(Connection.id == curr_id)
     result_prev_row = await session.scalars(query_prev_row)
-    origin = result_prev_row.one()
+    origin = result_prev_row.first()
 
     q_creds_origin = select(AuthData).where(AuthData.connection_id == curr_id)
     creds_origin = await session.scalars(q_creds_origin)
-    creds_origin = creds_origin.one()
+    creds_origin = creds_origin.first()
 
     query_new_row = select(Connection).filter(
         Connection.group_id == empty_group.id,
@@ -94,14 +93,25 @@ async def test_maintainer_plus_can_copy_connection_with_delete_source(
     creds_new = creds_new.one_or_none()
 
     # Assert
-    assert origin.id == curr_id
-    assert decrypt_auth_data(creds_origin.value, settings) == {
-        "type": "postgres",
-        "user": "user",
-        "password": "password",
-    }
-    assert origin.is_deleted == is_delete_source
-    assert not new.is_deleted
+
+    if not is_delete_source:
+        assert origin.id == curr_id
+        assert decrypt_auth_data(creds_origin.value, settings) == {
+            "type": "postgres",
+            "user": "user",
+            "password": "password",
+        }
+
+    session.expunge(current)
+    origin = await session.get(Connection, current.id)
+    if is_delete_source:
+        # Assert that origin connection was deleted
+        assert origin is None
+    else:
+        # Assert that origin connection was not deleted
+        assert origin is not None
+
+    assert new is not None
     assert not creds_new
 
 
@@ -132,7 +142,7 @@ async def test_superuser_can_copy_connection(
         "user": "user",
         "password": "password",
     }
-    assert not current.is_deleted
+    assert current is not None
 
     query_copy_not_exist = select(Connection).filter(
         Connection.group_id == empty_group.id,
@@ -161,37 +171,36 @@ async def test_superuser_can_copy_connection(
     }
     assert result.status_code == 200
 
-    query_prev_row = select(Connection).where(Connection.id == curr_id)
-    result_prev_row = await session.scalars(query_prev_row)
-    origin = result_prev_row.one()
     query_new_row = select(Connection).filter(
         Connection.group_id == empty_group.id,
         Connection.name == group_connection.name,
     )
     result_new_row = await session.scalars(query_new_row)
     new = result_new_row.one()  # copied
 
-    # Assert
-    await session.refresh(group_connection.connection)
-
-    q_creds_origin = select(AuthData).where(AuthData.connection_id == origin.id)
+    q_creds_origin = select(AuthData).where(AuthData.connection_id == current.id)
     creds_origin = await session.scalars(q_creds_origin)
-    creds_origin = creds_origin.one()
+    creds_origin = creds_origin.first()
 
     q_creds_new = select(AuthData).where(AuthData.connection_id == new.id)
     creds_new = await session.scalars(q_creds_new)
     creds_new = creds_new.one_or_none()
 
-    assert origin.id == curr_id
-    assert origin.group_id == curr_group_id
-    assert decrypt_auth_data(creds_origin.value, settings) == {
-        "type": "postgres",
-        "user": "user",
-        "password": "password",
-    }
-    assert origin.is_deleted == is_delete_source
-    assert not new.is_deleted
-    assert not creds_new
+    # Assert
+    session.expunge(current)
+    origin = await session.get(Connection, current.id)
+    if is_delete_source:
+        # Assert that origin connection was deleted
+        assert origin is None
+    else:
+        assert origin.id == curr_id
+        assert origin.group_id == curr_group_id
+        assert decrypt_auth_data(creds_origin.value, settings) == {
+            "type": "postgres",
+            "user": "user",
+            "password": "password",
+        }
+        assert not creds_new
 
 
 async def test_unauthorized_user_cannot_copy_connection(
@@ -291,7 +300,7 @@ async def test_not_in_both_groups_user_can_not_copy_connection(
         "user": "user",
         "password": "password",
     }
-    assert not current.is_deleted
+    assert current is not None
 
     query_copy_not_exist = select(Connection).filter(
         Connection.group_id == empty_group.id,
@@ -349,7 +358,7 @@ async def test_groupless_user_can_not_copy_connection(
         "user": "user",
         "password": "password",
     }
-    assert not current.is_deleted
+    assert current is not None
 
     query_copy_not_exist = select(Connection).filter(
         Connection.group_id == empty_group.id,

tests/test_unit/test_connections/test_delete_connection.py

@@ -14,11 +14,11 @@ async def test_maintainer_plus_can_delete_connection(
     role_maintainer_plus: UserTestRoles,
     session: AsyncSession,
 ):
-    # Arraange
+    # Arrange
     user = group_connection.owner_group.get_member_of_role(role_maintainer_plus)
     connection_id = group_connection.connection.id
     connection = await session.get(Connection, connection_id)
-    assert not connection.is_deleted
+    assert connection is not None
 
     # Act
     result = await client.delete(
@@ -33,9 +33,10 @@ async def test_maintainer_plus_can_delete_connection(
         "message": "Connection was deleted",
     }
     assert result.status_code == 200
-    deleted_connection = await session.get(Connection, connection_id)
-    await session.refresh(deleted_connection)
-    assert deleted_connection.is_deleted
+
+    session.expunge(connection)
+    connection = await session.get(Connection, connection_id)
+    assert connection is None
 
 
 async def test_groupless_user_cannot_delete_connection(
@@ -59,8 +60,10 @@ async def test_groupless_user_cannot_delete_connection(
             "details": None,
         },
     }
-    await session.refresh(group_connection.connection)
-    assert not group_connection.connection.is_deleted
+
+    session.expunge(group_connection)
+    connection = await session.get(Connection, group_connection.id)
+    assert connection is not None
 
 
 async def test_maintainer_plus_cannot_delete_connection_with_linked_transfer(
@@ -87,8 +90,10 @@ async def test_maintainer_plus_cannot_delete_connection_with_linked_transfer(
         },
     }
     assert result.status_code == 409
-    await session.refresh(group_transfer.source_connection)
-    assert not group_transfer.source_connection.is_deleted
+
+    session.expunge(group_transfer.source_connection)
+    connection = await session.get(Connection, group_transfer.source_connection.id)
+    assert connection is not None
 
 
 async def test_other_group_member_cannot_delete_group_connection(