[DOP-19793] Make queue name unique within a group

docs/changelog/next_release/119.bugfix.rst

@@ -0,0 +1 @@
+Now `Queue.name` is made unique within `group_id`

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

@@ -21,8 +21,9 @@
 def upgrade():
     op.create_table(
         "queue",
-        sa.Column("name", sa.String(length=128), nullable=False),
         sa.Column("id", sa.BigInteger(), nullable=False),
+        sa.Column("name", sa.String(length=128), nullable=False),
+        sa.Column("slug", sa.String(length=256), nullable=False),
         sa.Column("group_id", sa.BigInteger(), nullable=False),
         sa.Column("description", sa.String(length=512), nullable=False),
         sa.Column("created_at", sa.DateTime(), server_default=sa.text("now()"), nullable=False),
@@ -41,7 +42,7 @@ def upgrade():
             ondelete="CASCADE",
         ),
         sa.PrimaryKeyConstraint("id", name=op.f("pk__queue")),
-        sa.UniqueConstraint("name", name=op.f("uq__queue__name")),
+        sa.UniqueConstraint("slug", name=op.f("uq__queue__slug")),
     )
     op.create_index(op.f("ix__queue__group_id"), "queue", ["group_id"], unique=False)
 

syncmaster/db/models/group.py

@@ -77,7 +77,7 @@ class Group(Base, TimestampMixin, DeletableMixin):
 
     owner: Mapped[User] = relationship(User)
     members: Mapped[list[User]] = relationship(User, secondary="user_group")
-    queue: Mapped[Queue] = relationship(back_populates="group")
+    queue: Mapped[Queue] = relationship(back_populates="group", cascade="all, delete-orphan")
 
     search_vector: Mapped[str] = mapped_column(
         TSVECTOR,

syncmaster/db/models/queue.py

@@ -17,7 +17,8 @@
 
 
 class Queue(Base, ResourceMixin, TimestampMixin, DeletableMixin):
-    name: Mapped[str] = mapped_column(String(128), nullable=False, unique=True)
+    name: Mapped[str] = mapped_column(String(128), nullable=False)
+    slug: Mapped[str] = mapped_column(String(256), nullable=False, unique=True)
 
     transfers: Mapped[list[Transfer]] = relationship(back_populates="queue")
     group: Mapped[Group] = relationship(back_populates="queue")
@@ -31,4 +32,4 @@ class Queue(Base, ResourceMixin, TimestampMixin, DeletableMixin):
     )
 
     def __repr__(self):
-        return f"<Queue name={self.name} description={self.description}>"
+        return f"<Queue name={self.name} slug={self.slug} description={self.description}>"

syncmaster/db/repositories/queue.py

@@ -197,7 +197,7 @@ async def get_resource_permission(self, user: User, resource_id: int) -> Permiss
     @staticmethod
     def _raise_error(err: DBAPIError) -> NoReturn:
         constraint = err.__cause__.__cause__.constraint_name
-        if constraint == "uq__queue__name":
+        if constraint == "uq__queue__slug":
             raise DuplicatedQueueNameError
 
         raise SyncmasterError from err

syncmaster/schemas/v1/queue.py

@@ -1,21 +1,27 @@
 # SPDX-FileCopyrightText: 2023-2024 MTS PJSC
 # SPDX-License-Identifier: Apache-2.0
-from pydantic import BaseModel, Field, constr
+from pydantic import BaseModel, Field, computed_field, constr
 
 from syncmaster.schemas.v1.page import PageSchema
 
 
 class CreateQueueSchema(BaseModel):
     name: constr(max_length=128, pattern=r"^[-_a-zA-Z0-9]+$") = Field(  # noqa: F722
         ...,
-        description="Queue name",
+        description="Queue name that allows letters, numbers, dashes, and underscores",
     )
     group_id: int = Field(..., description="Queue owner group id")
     description: str = Field(default="", description="Additional description")
 
+    @computed_field
+    @property
+    def slug(self) -> str:
+        return f"{self.group_id}-{self.name}"
+
 
 class ReadQueueSchema(BaseModel):
     name: str
+    slug: str
     description: str | None = None
     group_id: int
     id: int

tests/test_unit/conftest.py

@@ -1,4 +1,5 @@
 import secrets
+from collections.abc import AsyncGenerator
 
 import pytest_asyncio
 from sqlalchemy.ext.asyncio import AsyncSession
@@ -169,7 +170,7 @@ async def user_with_many_roles(session: AsyncSession, settings: Settings, simple
 
 
 @pytest_asyncio.fixture
-async def empty_group(session: AsyncSession, settings) -> MockGroup:
+async def empty_group(session: AsyncSession, settings) -> AsyncGenerator[MockGroup, None]:
     owner = await create_user(
         session=session,
         username="empty_group_owner",
@@ -195,7 +196,7 @@ async def empty_group(session: AsyncSession, settings) -> MockGroup:
 
 
 @pytest_asyncio.fixture
-async def group(session: AsyncSession, settings: Settings) -> MockGroup:
+async def group(session: AsyncSession, settings: Settings) -> AsyncGenerator[MockGroup, None]:
     owner = await create_user(
         session=session,
         username="notempty_group_owner",
@@ -239,7 +240,7 @@ async def group(session: AsyncSession, settings: Settings) -> MockGroup:
 async def mock_group(
     session: AsyncSession,
     settings: Settings,
-):
+) -> AsyncGenerator[MockGroup, None]:
     group_owner = await create_user(
         session=session,
         username=f"{secrets.token_hex(5)}_group_connection_owner",
@@ -289,7 +290,7 @@ async def group_queue(
     session: AsyncSession,
     settings: Settings,
     mock_group: MockGroup,
-) -> Queue:
+) -> AsyncGenerator[Queue, None]:
     queue = await create_queue(
         session=session,
         name=f"{secrets.token_hex(5)}_test_queue",

tests/test_unit/test_queue/test_create_queue.py

@@ -35,6 +35,7 @@ async def test_maintainer_plus_can_create_queue(
         "name": "New_queue",
         "description": "Some interesting description",
         "group_id": mock_group.group.id,
+        "slug": f"{mock_group.group.id}-New_queue",
     }
     assert result.status_code == 200
     queue = (await session.scalars(select(Queue).filter_by(id=result.json()["id"]))).one()
@@ -43,6 +44,7 @@ async def test_maintainer_plus_can_create_queue(
     assert queue.group_id == mock_group.group.id
     assert queue.name == "New_queue"
     assert queue.description == "Some interesting description"
+    assert queue.slug == f"{mock_group.group.id}-New_queue"
 
     await session.delete(queue)
     await session.commit()
@@ -71,6 +73,7 @@ async def test_superuser_can_create_queue(
         "name": "New_queue",
         "description": "Some interesting description",
         "group_id": mock_group.group.id,
+        "slug": f"{mock_group.group.id}-New_queue",
     }
     assert result.status_code == 200
     queue = (await session.scalars(select(Queue).filter_by(id=result.json()["id"]))).one()
@@ -79,6 +82,7 @@ async def test_superuser_can_create_queue(
     assert queue.group_id == mock_group.group.id
     assert queue.name == "New_queue"
     assert queue.description == "Some interesting description"
+    assert queue.slug == f"{mock_group.group.id}-New_queue"
 
     await session.delete(queue)
     await session.commit()
@@ -395,3 +399,45 @@ async def test_maintainer_plus_can_not_create_queue_with_duplicate_name_error(
             "details": None,
         },
     }
+
+
+async def test_maintainer_plus_can_create_queues_with_the_same_name_but_diff_groups(
+    client: AsyncClient,
+    session: AsyncSession,
+    role_maintainer_plus: UserTestRoles,
+    mock_group: MockGroup,
+    group: MockGroup,
+):
+    # Arrange
+    mock_group_user = mock_group.get_member_of_role(role_maintainer_plus)
+    group_user = group.get_member_of_role(role_maintainer_plus)
+
+    # Act
+    await client.post(
+        "v1/queues",
+        headers={"Authorization": f"Bearer {mock_group_user.token}"},
+        json={
+            "name": "New_queue",
+            "description": "Some interesting description",
+            "group_id": mock_group.group.id,
+        },
+    )
+    result = await client.post(
+        "v1/queues",
+        headers={"Authorization": f"Bearer {group_user.token}"},
+        json={
+            "name": "New_queue",
+            "description": "Some interesting description",
+            "group_id": group.group.id,
+        },
+    )
+
+    # Assert
+    assert result.status_code == 200
+    assert result.json() == {
+        "id": result.json()["id"],
+        "name": "New_queue",
+        "description": "Some interesting description",
+        "group_id": group.group.id,
+        "slug": f"{group.group.id}-New_queue",
+    }

tests/test_unit/test_queue/test_read_queue.py

@@ -28,6 +28,7 @@ async def test_group_member_can_read_queue(
         "name": group_queue.name,
         "description": group_queue.description,
         "group_id": group_queue.group_id,
+        "slug": f"{group_queue.group.id}-{group_queue.name}",
     }
     assert result.status_code == 200
 
@@ -49,6 +50,7 @@ async def test_superuser_can_read_queue(
         "name": group_queue.name,
         "description": group_queue.description,
         "group_id": group_queue.group_id,
+        "slug": f"{group_queue.group.id}-{group_queue.name}",
     }
     assert result.status_code == 200
 

tests/test_unit/test_queue/test_read_queues.py

@@ -35,6 +35,7 @@ async def test_group_member_can_read_queues(
                 "name": group_queue.name,
                 "description": group_queue.description,
                 "group_id": mock_group.id,
+                "slug": f"{mock_group.id}-{group_queue.name}",
             },
         ],
         "meta": {
@@ -73,6 +74,7 @@ async def test_superuser_can_read_queues(
                 "name": group_queue.name,
                 "description": group_queue.description,
                 "group_id": mock_group.id,
+                "slug": f"{mock_group.id}-{group_queue.name}",
             },
         ],
         "meta": {
@@ -216,6 +218,7 @@ async def test_search_queues_with_query(
                 "name": group_queue.name,
                 "description": group_queue.description,
                 "group_id": mock_group.id,
+                "slug": f"{mock_group.id}-{group_queue.name}",
             },
         ],
     }

tests/test_unit/test_queue/test_update_queue.py

@@ -33,6 +33,7 @@ async def test_maintainer_plus_can_update_queue(
         "name": group_queue.name,
         "description": "New description",
         "group_id": group_queue.group_id,
+        "slug": f"{group_queue.group_id}-{group_queue.name}",
     }
     assert result.status_code == 200
 
@@ -44,6 +45,7 @@ async def test_maintainer_plus_can_update_queue(
         "name": queue.name,
         "description": queue.description,
         "group_id": queue.group_id,
+        "slug": f"{queue.group_id}-{queue.name}",
     }
 
 
@@ -70,6 +72,7 @@ async def test_superuser_can_update_queue(
         "name": group_queue.name,
         "description": "New description",
         "group_id": group_queue.group_id,
+        "slug": f"{group_queue.group_id}-{group_queue.name}",
     }
 
     queue = await session.get(Queue, group_queue.id)
@@ -80,6 +83,7 @@ async def test_superuser_can_update_queue(
         "name": queue.name,
         "description": queue.description,
         "group_id": queue.group_id,
+        "slug": f"{queue.group_id}-{queue.name}",
     }
 
 

tests/test_unit/utils.py

@@ -77,6 +77,7 @@ async def create_queue(
         name=name,
         description=description,
         group_id=group_id,
+        slug=f"{group_id}-{name}",
     )
     session.add(queue)
     await session.commit()