Fix artefact build uniqueness (#30)

* Use latest sqlalchemy that supports NULLS NOT DISTINCT

* Add NULLS NOT DISTINCT to unique index

* Fix bug as artefact builds shouldn't be generated randomly

* Fix linting issues

* Small change

* Small change

backend/migrations/versions/2023_06_20_1000-b33ee8dd41b1_use_nulls_not_distinct.py

@@ -0,0 +1,33 @@
+"""Use nulls not distinct
+
+Revision ID: b33ee8dd41b1
+Revises: 6a80dad01d24
+Create Date: 2023-06-20 10:00:07.676129+00:00
+
+"""
+from alembic import op
+
+
+# revision identifiers, used by Alembic.
+revision = "b33ee8dd41b1"
+down_revision = "6a80dad01d24"
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    op.execute(
+        "ALTER TABLE artefact_build "
+        "DROP CONSTRAINT IF EXISTS artefact_build_artefact_id_architecture_revision_key"
+        ", ADD CONSTRAINT unique_artefact_build "
+        "UNIQUE NULLS NOT DISTINCT (artefact_id, architecture, revision);"
+    )
+
+
+def downgrade() -> None:
+    op.execute(
+        "ALTER TABLE artefact_build "
+        "DROP CONSTRAINT IF EXISTS unique_artefact_build"
+        ", ADD CONSTRAINT artefact_build_artefact_id_architecture_revision_key "
+        "UNIQUE (artefact_id, architecture, revision);"
+    )

backend/pyproject.toml

@@ -8,7 +8,7 @@ authors = []
 python = "^3.10"
 fastapi = "^0.95.0"
 uvicorn = {extras = ["standard"], version = "^0.21.1"}
-sqlalchemy = "^2.0.9"
+sqlalchemy = "^2.0.16"
 pg8000 = "^1.29.4"
 alembic = "^1.10.3"
 poetry-dynamic-versioning = {extras = ["plugin"], version = "^0.22.0"}

backend/test_observer/data_access/models.py

@@ -109,7 +109,15 @@ class ArtefactBuild(Base):
         back_populates="artefact_build", cascade="all, delete-orphan"
     )
 
-    __table_args__ = (UniqueConstraint("artefact_id", "architecture", "revision"),)
+    __table_args__ = (
+        UniqueConstraint(
+            "artefact_id",
+            "architecture",
+            "revision",
+            name="unique_artefact_build",
+            postgresql_nulls_not_distinct=True,
+        ),
+    )
 
 
 class Environment(Base):

backend/tests/controllers/artefacts/test_artefacts.py

@@ -21,19 +21,19 @@
 from sqlalchemy.orm import Session
 from test_observer.data_access.models import ArtefactBuild, Environment, TestExecution
 
-from tests.helpers import create_artefact, create_artefact_builds
+from tests.helpers import create_artefact
 
 
 def test_get_artefact_builds(db_session: Session, test_client: TestClient):
     artefact = create_artefact(db_session, "beta")
-    artefact_build = create_artefact_builds(db_session, artefact, 1)[0]
+    artefact_build = ArtefactBuild(architecture="amd64", artefact=artefact, revision=1)
     environment = Environment(
         name="some-environment", architecture=artefact_build.architecture
     )
     test_execution = TestExecution(
         artefact_build=artefact_build, environment=environment
     )
-    db_session.add_all([environment, test_execution])
+    db_session.add_all([environment, test_execution, artefact_build])
     db_session.commit()
 
     response = test_client.get(f"/v1/artefacts/{artefact.id}/builds")

backend/tests/controllers/promoter/test_promoter.py

@@ -25,8 +25,9 @@
 from fastapi.testclient import TestClient
 from requests_mock import Mocker
 from sqlalchemy.orm import Session
+from test_observer.data_access.models import ArtefactBuild
 
-from tests.helpers import create_artefact, create_artefact_builds
+from tests.helpers import create_artefact
 
 
 def test_run_to_move_artefact_snap(
@@ -45,7 +46,6 @@ def test_run_to_move_artefact_snap(
         source={"store": "ubuntu"},
         created_at=datetime.utcnow(),
     )
-    create_artefact_builds(db_session, artefact)
     create_artefact(
         db_session,
         "edge",
@@ -54,6 +54,15 @@ def test_run_to_move_artefact_snap(
         source={"store": "ubuntu"},
         created_at=datetime.utcnow() - timedelta(days=1),
     )
+    db_session.add_all(
+        [
+            ArtefactBuild(architecture="amd64", artefact=artefact, revision=1),
+            ArtefactBuild(architecture="amd64", artefact=artefact, revision=2),
+            ArtefactBuild(architecture="arm64", artefact=artefact, revision=1),
+        ]
+    )
+    db_session.commit()
+
     requests_mock.get(
         "https://api.snapcraft.io/v2/snaps/info/core20",
         json={
@@ -106,7 +115,6 @@ def test_run_to_move_artefact_deb(
         source={"series": "kinetic", "repo": "main"},
         created_at=datetime.utcnow(),
     )
-    create_artefact_builds(db_session, artefact)
     create_artefact(
         db_session,
         "proposed",
@@ -115,6 +123,8 @@ def test_run_to_move_artefact_deb(
         source={"series": "kinetic", "repo": "main"},
         created_at=datetime.utcnow() - timedelta(days=1),
     )
+    db_session.add(ArtefactBuild(architecture="amd64", artefact=artefact))
+    db_session.commit()
 
     with open("tests/test_data/Packages-proposed.gz", "rb") as f:
         proposed_content = f.read()

backend/tests/helpers.py

@@ -1,9 +1,7 @@
-from random import choice
 from datetime import datetime
 
 from sqlalchemy.orm import Session
-from test_observer.data_access.models import Artefact, Stage, ArtefactBuild
-from test_observer.data_access.models_enums import FamilyName
+from test_observer.data_access.models import Artefact, Stage
 
 
 def create_artefact(db_session: Session, stage_name: str, **kwargs) -> Artefact:
@@ -19,31 +17,3 @@ def create_artefact(db_session: Session, stage_name: str, **kwargs) -> Artefact:
     db_session.add(artefact)
     db_session.commit()
     return artefact
-
-
-def create_artefact_builds(
-    db_session: Session, artefact: Artefact, num_builds: int = 5
-) -> list[ArtefactBuild]:
-    """
-    Create a number of ArtefactBuild instances for an Artefact and add them
-    to the database
-    """
-    architectures = ["arm64", "x86", "amd64"]
-
-    for i in range(num_builds):
-        architecture = choice(architectures)
-
-        build = ArtefactBuild(
-            architecture=architecture,
-            revision=i + 1
-            if artefact.stage.family.name == FamilyName.SNAP.value
-            else None,
-            artefact_id=artefact.id,
-        )
-
-        # Add the build to the artefact's builds
-        artefact.builds.append(build)
-
-    # Commit the changes to the database
-    db_session.commit()
-    return artefact.builds