diff --git a/backend/alembic/versions/0009_inventory_asset_id_history.py b/backend/alembic/versions/0009_inventory_asset_id_history.py new file mode 100644 index 0000000..df956c2 --- /dev/null +++ b/backend/alembic/versions/0009_inventory_asset_id_history.py @@ -0,0 +1,81 @@ +"""add inventory Asset ID history + +Revision ID: 0009_inventory_asset_id_history +Revises: 0008_manual_miner_inventory +Create Date: 2026-07-28 +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects import postgresql + + +revision: str = "0009_inventory_asset_id_history" +down_revision: Union[str, None] = "0008_manual_miner_inventory" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.create_table( + "miner_asset_id_history", + sa.Column( + "id", + postgresql.UUID(as_uuid=True), + primary_key=True, + server_default=sa.text("gen_random_uuid()"), + ), + sa.Column( + "miner_id", + postgresql.UUID(as_uuid=True), + sa.ForeignKey("miners.id", ondelete="RESTRICT"), + nullable=False, + ), + sa.Column("old_asset_id", sa.Text(), nullable=True), + sa.Column("new_asset_id", sa.Text(), nullable=False), + sa.Column( + "changed_at", + sa.DateTime(timezone=True), + nullable=False, + server_default=sa.text("now()"), + ), + sa.Column( + "changed_by", + postgresql.UUID(as_uuid=True), + sa.ForeignKey("users.id", ondelete="RESTRICT"), + nullable=False, + ), + sa.CheckConstraint( + "old_asset_id IS NULL OR " + "(old_asset_id = trim(old_asset_id) AND old_asset_id <> '')", + name="ck_miner_asset_id_history_old_normalized", + ), + sa.CheckConstraint( + "new_asset_id = trim(new_asset_id) AND new_asset_id <> ''", + name="ck_miner_asset_id_history_new_normalized", + ), + ) + op.create_index( + "idx_miner_asset_id_history_miner_changed_at", + "miner_asset_id_history", + ["miner_id", "changed_at", "id"], + ) + op.create_index( + "idx_miner_asset_id_history_changed_by", + "miner_asset_id_history", + ["changed_by"], + ) + + +def downgrade() -> None: + op.drop_index( + "idx_miner_asset_id_history_changed_by", + table_name="miner_asset_id_history", + ) + op.drop_index( + "idx_miner_asset_id_history_miner_changed_at", + table_name="miner_asset_id_history", + ) + op.drop_table("miner_asset_id_history") diff --git a/backend/app/inventory/__init__.py b/backend/app/inventory/__init__.py index 95a5c01..a22ddc7 100644 --- a/backend/app/inventory/__init__.py +++ b/backend/app/inventory/__init__.py @@ -8,6 +8,7 @@ from app.inventory.exceptions import ( ) from app.inventory.models import ( InventoryCreateRequest, + InventoryMetadataUpdateRequest, InventoryMinerDetail, InventoryMinerSummary, InventorySearchFilters, @@ -21,6 +22,7 @@ __all__ = [ "DuplicateAssetID", "InventoryCreateRequest", "InventoryError", + "InventoryMetadataUpdateRequest", "InventoryMinerDetail", "InventoryMinerSummary", "InventoryRepository", diff --git a/backend/app/inventory/models.py b/backend/app/inventory/models.py index af49176..3a15b4c 100644 --- a/backend/app/inventory/models.py +++ b/backend/app/inventory/models.py @@ -4,7 +4,7 @@ from datetime import datetime from typing import Literal from uuid import UUID -from pydantic import BaseModel, Field, IPvAnyAddress +from pydantic import BaseModel, ConfigDict, Field, IPvAnyAddress LifecycleStatus = Literal["active", "spare", "retired"] @@ -60,6 +60,16 @@ class InventoryCreateRequest(BaseModel): tags: list[str] = Field(default_factory=list) +class InventoryMetadataUpdateRequest(BaseModel): + model_config = ConfigDict(extra="forbid") + + asset_id: str | None = Field(default=None, max_length=120) + miner_sn: str | None = Field(default=None, max_length=255) + psu_serial: str | None = Field(default=None, max_length=255) + model: str | None = Field(default=None, max_length=255) + notes: str | None = None + + class InventoryUpdateRequest(BaseModel): asset_id: str | None = Field(default=None, max_length=120) current_mac: str | None = Field(default=None, max_length=64) diff --git a/backend/app/inventory/repository.py b/backend/app/inventory/repository.py index f20b256..7b7035e 100644 --- a/backend/app/inventory/repository.py +++ b/backend/app/inventory/repository.py @@ -145,8 +145,18 @@ class InventoryRepository: connection.close() @staticmethod - def _fetch_miner(cur, where_sql: str, params: Sequence[Any]) -> dict | None: - cur.execute(f"{MINER_SELECT} WHERE {where_sql}", params) + def _fetch_miner( + cur, + where_sql: str, + params: Sequence[Any], + *, + for_update: bool = False, + ) -> dict | None: + lock_sql = " FOR UPDATE OF miners" if for_update else "" + cur.execute( + f"{MINER_SELECT} WHERE {where_sql}{lock_sql}", + params, + ) row = cur.fetchone() return dict(row) if row else None @@ -155,6 +165,16 @@ class InventoryRepository: with connection.cursor(row_factory=dict_row) as cur: return self._fetch_miner(cur, "miners.id = %s", (miner_id,)) + def get_miner_by_id_for_update(self, miner_id: UUID) -> dict | None: + with self._connection_scope() as connection: + with connection.cursor(row_factory=dict_row) as cur: + return self._fetch_miner( + cur, + "miners.id = %s", + (miner_id,), + for_update=True, + ) + def get_miner_by_asset_id(self, asset_id: str) -> dict | None: with self._connection_scope() as connection: with connection.cursor(row_factory=dict_row) as cur: @@ -291,6 +311,56 @@ class InventoryRepository: return None return self._fetch_miner(cur, "miners.id = %s", (miner_id,)) + def add_asset_id_history( + self, + miner_id: UUID, + old_asset_id: str | None, + new_asset_id: str, + changed_by: UUID, + ) -> dict: + with self._connection_scope(write=True) as connection: + with connection.cursor(row_factory=dict_row) as cur: + cur.execute( + """ + INSERT INTO miner_asset_id_history ( + miner_id, + old_asset_id, + new_asset_id, + changed_by + ) + VALUES (%s, %s, %s, %s) + RETURNING + id, + miner_id, + old_asset_id, + new_asset_id, + changed_at, + changed_by + """, + (miner_id, old_asset_id, new_asset_id, changed_by), + ) + return dict(cur.fetchone()) + + def list_asset_id_history(self, miner_id: UUID) -> list[dict]: + with self._connection_scope() as connection: + with connection.cursor(row_factory=dict_row) as cur: + cur.execute( + """ + SELECT + id, + miner_id, + old_asset_id, + new_asset_id, + changed_at, + changed_by + FROM miner_asset_id_history + WHERE miner_id = %s + ORDER BY changed_at, id + """, + (miner_id,), + ) + return [dict(row) for row in cur.fetchall()] + def list_tags(self) -> list[dict]: with self._connection_scope() as connection: with connection.cursor(row_factory=dict_row) as cur: diff --git a/backend/app/inventory/router.py b/backend/app/inventory/router.py index 7f51810..262b933 100644 --- a/backend/app/inventory/router.py +++ b/backend/app/inventory/router.py @@ -7,9 +7,15 @@ from fastapi import APIRouter, Depends, HTTPException, Query, status from app.identity import require_permission from app.inventory.dependencies import get_inventory_service -from app.inventory.exceptions import InventoryValidationError, MinerNotFound +from app.inventory.exceptions import ( + DuplicateAssetID, + InventoryValidationError, + MinerNotFound, +) from app.inventory.models import ( + InventoryCreateRequest, InventoryListMeta, + InventoryMetadataUpdateRequest, InventoryMinerDetailResponse, InventoryMinerListResponse, InventorySearchFilters, @@ -19,15 +25,76 @@ from app.inventory.service import InventoryService router = APIRouter(prefix="/api/inventory/miners", tags=["inventory"]) require_inventory_read = require_permission("miners.view") +require_inventory_write = require_permission("miners.update") InventoryServiceDependency = Annotated[ InventoryService, Depends(get_inventory_service), ] InventoryReadUser = Annotated[dict, Depends(require_inventory_read)] +InventoryWriteUser = Annotated[dict, Depends(require_inventory_write)] InventoryQuery = Annotated[InventorySearchFilters, Query()] +@router.post( + "", + response_model=InventoryMinerDetailResponse, + status_code=status.HTTP_201_CREATED, +) +def create_inventory_miner( + request: InventoryCreateRequest, + service: InventoryServiceDependency, + current_user: InventoryWriteUser, +) -> InventoryMinerDetailResponse: + del current_user + try: + miner = service.create_miner(request) + except DuplicateAssetID as exc: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=str(exc), + ) from exc + except InventoryValidationError as exc: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=str(exc), + ) from exc + + return InventoryMinerDetailResponse(data=miner) + + +@router.patch("/{miner_id}", response_model=InventoryMinerDetailResponse) +def update_inventory_miner_metadata( + miner_id: UUID, + request: InventoryMetadataUpdateRequest, + service: InventoryServiceDependency, + current_user: InventoryWriteUser, +) -> InventoryMinerDetailResponse: + try: + miner = service.update_metadata( + miner_id, + request, + actor_user_id=UUID(str(current_user["id"])), + ) + except MinerNotFound as exc: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="Miner not found", + ) from exc + except DuplicateAssetID as exc: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=str(exc), + ) from exc + except InventoryValidationError as exc: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=str(exc), + ) from exc + + return InventoryMinerDetailResponse(data=miner) + + @router.get("", response_model=InventoryMinerListResponse) def list_inventory_miners( filters: InventoryQuery, diff --git a/backend/app/inventory/service.py b/backend/app/inventory/service.py index 13fa37f..18bd20a 100644 --- a/backend/app/inventory/service.py +++ b/backend/app/inventory/service.py @@ -13,6 +13,7 @@ from app.inventory.exceptions import ( ) from app.inventory.models import ( InventoryCreateRequest, + InventoryMetadataUpdateRequest, InventoryMinerDetail, InventoryMinerSummary, InventorySearchFilters, @@ -197,6 +198,85 @@ class InventoryService: return self._detail_model(row) + def update_metadata( + self, + miner_id: UUID, + request: InventoryMetadataUpdateRequest, + *, + actor_user_id: UUID, + ) -> InventoryMinerDetail: + fields_set = request.model_fields_set + if not fields_set: + raise InventoryValidationError( + "At least one metadata field must be provided" + ) + + requested_asset_id = None + if "asset_id" in fields_set: + if request.asset_id is None: + raise InventoryValidationError("Asset ID cannot be cleared") + requested_asset_id = self.normalize_asset_id(request.asset_id) + + requested_values: dict[str, Any] = {} + field_map = { + "miner_sn": "miner_sn", + "psu_serial": "power_sn", + "model": "model", + "notes": "notes", + } + for model_field, database_column in field_map.items(): + if model_field in fields_set: + requested_values[database_column] = self._clean_optional_text( + getattr(request, model_field) + ) + + try: + with self.repository.transaction() as repository: + existing = repository.get_miner_by_id_for_update(miner_id) + if not existing: + raise MinerNotFound(miner_id) + + previous_asset_id = ( + str(existing.get("asset_tag") or "").strip() or None + ) + asset_id_changed = ( + requested_asset_id is not None + and requested_asset_id != previous_asset_id + ) + + values = { + key: value + for key, value in requested_values.items() + if existing.get(key) != value + } + if asset_id_changed: + duplicate = repository.get_miner_by_asset_id( + requested_asset_id + ) + if duplicate and duplicate["id"] != miner_id: + raise DuplicateAssetID(requested_asset_id) + values["asset_tag"] = requested_asset_id + + if not values: + return self._detail_model(existing) + + self._before_update(existing, values) + row = repository.update_miner(miner_id, values) + if asset_id_changed: + repository.add_asset_id_history( + miner_id, + previous_asset_id, + requested_asset_id, + actor_user_id, + ) + row = repository.get_miner_by_id(miner_id) + self._after_update(row) + except UniqueViolation as exc: + asset_id = requested_asset_id or "unknown" + raise DuplicateAssetID(asset_id) from exc + + return self._detail_model(row) + def update_miner( self, miner_id: UUID, @@ -213,10 +293,13 @@ class InventoryService: if request.asset_id is None: raise InventoryValidationError("Asset ID cannot be cleared") asset_id = self.normalize_asset_id(request.asset_id) - duplicate = self.repository.get_miner_by_asset_id(asset_id) - if duplicate and duplicate["id"] != miner_id: - raise DuplicateAssetID(asset_id) - values["asset_tag"] = asset_id + existing_asset_id = ( + str(existing.get("asset_tag") or "").strip() or None + ) + if asset_id != existing_asset_id: + raise InventoryValidationError( + "Asset ID changes require actor-attributed metadata update" + ) else: asset_id = existing["asset_tag"] diff --git a/backend/tests/test_inventory_domain.py b/backend/tests/test_inventory_domain.py index eb8db17..6840ac4 100644 --- a/backend/tests/test_inventory_domain.py +++ b/backend/tests/test_inventory_domain.py @@ -1,13 +1,17 @@ import ast import unittest +from copy import deepcopy from contextlib import contextmanager from datetime import datetime, timezone from pathlib import Path from uuid import UUID, uuid4 +from psycopg.errors import ForeignKeyViolation, UniqueViolation + from app.inventory import ( DuplicateAssetID, InventoryCreateRequest, + InventoryMetadataUpdateRequest, InventoryRepository, InventorySearchFilters, InventoryService, @@ -48,14 +52,26 @@ class FakeInventoryRepository: def __init__(self): self.rows = {} self.created_values = None + self.asset_id_history = [] + self.update_error = None @contextmanager def transaction(self): - yield self + rows_snapshot = deepcopy(self.rows) + history_snapshot = deepcopy(self.asset_id_history) + try: + yield self + except Exception: + self.rows = rows_snapshot + self.asset_id_history = history_snapshot + raise def get_miner_by_id(self, miner_id): return self.rows.get(miner_id) + def get_miner_by_id_for_update(self, miner_id): + return self.get_miner_by_id(miner_id) + def get_miner_by_asset_id(self, asset_id): key = asset_id.strip().casefold() return next( @@ -86,6 +102,8 @@ class FakeInventoryRepository: return row def update_miner(self, miner_id, values): + if self.update_error is not None: + raise self.update_error row = self.rows.get(miner_id) if not row: return None @@ -93,6 +111,27 @@ class FakeInventoryRepository: row["updated_at"] = datetime.now(timezone.utc) return row + def add_asset_id_history( + self, miner_id, old_asset_id, new_asset_id, changed_by + ): + history = { + "id": uuid4(), + "miner_id": miner_id, + "old_asset_id": old_asset_id, + "new_asset_id": new_asset_id, + "changed_at": datetime.now(timezone.utc), + "changed_by": changed_by, + } + self.asset_id_history.append(history) + return history + + def list_asset_id_history(self, miner_id): + return [ + row + for row in self.asset_id_history + if row["miner_id"] == miner_id + ] + def add_tag(self, miner_id, tag_name): now = datetime.now(timezone.utc) tag = { @@ -121,6 +160,7 @@ class InventoryServiceTests(unittest.TestCase): def setUp(self): self.repository = FakeInventoryRepository() self.service = InventoryService(self.repository) + self.actor_user_id = uuid4() def test_create_normalizes_asset_id_tags_and_never_seen(self): created = self.service.create_miner( @@ -167,6 +207,123 @@ class InventoryServiceTests(unittest.TestCase): with self.assertRaises(InventoryValidationError): self.service.normalize_tags(["valid", "\t\n"]) + def test_metadata_update_preserves_unspecified_fields(self): + row = inventory_row( + asset_tag="ASSET-001", + miner_sn="MINER-OLD", + power_sn="PSU-OLD", + model="M50", + notes="Old notes", + ) + self.repository.rows[row["id"]] = row + + updated = self.service.update_metadata( + row["id"], + InventoryMetadataUpdateRequest( + model="M60", + notes=None, + ), + actor_user_id=self.actor_user_id, + ) + + self.assertEqual(updated.id, row["id"]) + self.assertEqual(updated.asset_id, "ASSET-001") + self.assertEqual(updated.miner_sn, "MINER-OLD") + self.assertEqual(updated.psu_serial, "PSU-OLD") + self.assertEqual(updated.model, "M60") + self.assertIsNone(updated.notes) + self.assertEqual(self.repository.asset_id_history, []) + + def test_asset_id_change_preserves_uuid_and_creates_history(self): + row = inventory_row(asset_tag="ASSET-OLD") + self.repository.rows[row["id"]] = row + + updated = self.service.update_metadata( + row["id"], + InventoryMetadataUpdateRequest(asset_id=" ASSET-NEW "), + actor_user_id=self.actor_user_id, + ) + + self.assertEqual(updated.id, row["id"]) + self.assertEqual(updated.asset_id, "ASSET-NEW") + self.assertEqual(len(self.repository.asset_id_history), 1) + history = self.repository.asset_id_history[0] + self.assertEqual(history["miner_id"], row["id"]) + self.assertEqual(history["old_asset_id"], "ASSET-OLD") + self.assertEqual(history["new_asset_id"], "ASSET-NEW") + self.assertEqual(history["changed_by"], self.actor_user_id) + + def test_unchanged_normalized_asset_id_creates_no_history(self): + row = inventory_row(asset_tag="ASSET-001") + original_updated_at = row["updated_at"] + self.repository.rows[row["id"]] = row + + updated = self.service.update_metadata( + row["id"], + InventoryMetadataUpdateRequest(asset_id=" ASSET-001 "), + actor_user_id=self.actor_user_id, + ) + + self.assertEqual(updated.asset_id, "ASSET-001") + self.assertEqual(updated.updated_at, original_updated_at) + self.assertEqual(self.repository.asset_id_history, []) + + def test_duplicate_asset_id_leaves_miner_and_history_unchanged(self): + row = inventory_row(asset_tag="ASSET-OLD") + duplicate = inventory_row(asset_tag="ASSET-TAKEN") + self.repository.rows[row["id"]] = row + self.repository.rows[duplicate["id"]] = duplicate + + with self.assertRaises(DuplicateAssetID): + self.service.update_metadata( + row["id"], + InventoryMetadataUpdateRequest(asset_id=" asset-taken "), + actor_user_id=self.actor_user_id, + ) + + self.assertEqual( + self.repository.rows[row["id"]]["asset_tag"], + "ASSET-OLD", + ) + self.assertEqual(self.repository.asset_id_history, []) + + def test_empty_or_invalid_metadata_update_does_not_write(self): + row = inventory_row(asset_tag="ASSET-OLD", model="M50") + self.repository.rows[row["id"]] = row + + for request in ( + InventoryMetadataUpdateRequest(), + InventoryMetadataUpdateRequest(asset_id=" ", model="M60"), + ): + with self.subTest(request=request): + with self.assertRaises(InventoryValidationError): + self.service.update_metadata( + row["id"], + request, + actor_user_id=self.actor_user_id, + ) + + self.assertEqual(self.repository.rows[row["id"]]["model"], "M50") + self.assertEqual(self.repository.asset_id_history, []) + + def test_uniqueness_race_becomes_duplicate_asset_id(self): + row = inventory_row(asset_tag="ASSET-OLD") + self.repository.rows[row["id"]] = row + self.repository.update_error = UniqueViolation("duplicate Asset ID") + + with self.assertRaises(DuplicateAssetID): + self.service.update_metadata( + row["id"], + InventoryMetadataUpdateRequest(asset_id="ASSET-NEW"), + actor_user_id=self.actor_user_id, + ) + + self.assertEqual( + self.repository.rows[row["id"]]["asset_tag"], + "ASSET-OLD", + ) + self.assertEqual(self.repository.asset_id_history, []) + def test_update_maps_product_names_without_repository_rules(self): row = inventory_row(asset_tag="ASSET-OLD", power_sn="PSU-OLD") self.repository.rows[row["id"]] = row @@ -174,16 +331,34 @@ class InventoryServiceTests(unittest.TestCase): updated = self.service.update_miner( row["id"], InventoryUpdateRequest( - asset_id=" ASSET-NEW ", psu_serial=" PSU-NEW ", lifecycle_status="retired", ), ) - self.assertEqual(updated.asset_id, "ASSET-NEW") + self.assertEqual(updated.asset_id, "ASSET-OLD") self.assertEqual(updated.psu_serial, "PSU-NEW") self.assertEqual(updated.lifecycle_status, "retired") + def test_legacy_update_rejects_asset_id_change_without_actor(self): + row = inventory_row(asset_tag="ASSET-OLD") + self.repository.rows[row["id"]] = row + + with self.assertRaisesRegex( + InventoryValidationError, + "actor-attributed metadata update", + ): + self.service.update_miner( + row["id"], + InventoryUpdateRequest(asset_id="ASSET-NEW"), + ) + + self.assertEqual( + self.repository.rows[row["id"]]["asset_tag"], + "ASSET-OLD", + ) + self.assertEqual(self.repository.asset_id_history, []) + def test_search_validation_precedes_repository(self): with self.assertRaises(InventoryValidationError): self.service.search_miners( @@ -197,11 +372,30 @@ class InventoryRepositoryIntegrationTests(unittest.TestCase): cls.repository = InventoryRepository() def setUp(self): + self.actor_user_id = UUID("00000000-0000-0000-0000-000000000951") with self.repository.transaction() as repository: with repository._connection.cursor() as cur: + cur.execute("DELETE FROM miner_asset_id_history") cur.execute("DELETE FROM miner_tags") cur.execute("DELETE FROM tags") cur.execute("DELETE FROM miners") + cur.execute( + """ + INSERT INTO users ( + id, username, display_name, password_hash, role_id + ) + SELECT %s, %s, %s, %s, id + FROM roles + WHERE name = 'Admin' + ON CONFLICT (id) DO NOTHING + """, + ( + self.actor_user_id, + "inventory-history-test", + "Inventory History Test", + "not-used", + ), + ) def test_repository_crud_search_and_tags(self): created = self.repository.create_miner( @@ -262,6 +456,64 @@ class InventoryRepositoryIntegrationTests(unittest.TestCase): [], ) + def test_repeated_asset_id_changes_preserve_chronological_history(self): + service = InventoryService(self.repository) + created = service.create_miner( + InventoryCreateRequest(asset_id="ASSET-FIRST") + ) + + second = service.update_metadata( + created.id, + InventoryMetadataUpdateRequest(asset_id="ASSET-SECOND"), + actor_user_id=self.actor_user_id, + ) + third = service.update_metadata( + created.id, + InventoryMetadataUpdateRequest(asset_id="ASSET-THIRD"), + actor_user_id=self.actor_user_id, + ) + + self.assertEqual(created.id, second.id) + self.assertEqual(second.id, third.id) + history = self.repository.list_asset_id_history(created.id) + self.assertEqual(len(history), 2) + self.assertEqual( + [ + (row["old_asset_id"], row["new_asset_id"]) + for row in history + ], + [ + ("ASSET-FIRST", "ASSET-SECOND"), + ("ASSET-SECOND", "ASSET-THIRD"), + ], + ) + self.assertLessEqual( + history[0]["changed_at"], + history[1]["changed_at"], + ) + + def test_history_failure_rolls_back_asset_id_update(self): + service = InventoryService(self.repository) + created = service.create_miner( + InventoryCreateRequest(asset_id="ASSET-ROLLBACK-OLD") + ) + + with self.assertRaises(ForeignKeyViolation): + service.update_metadata( + created.id, + InventoryMetadataUpdateRequest( + asset_id="ASSET-ROLLBACK-NEW" + ), + actor_user_id=uuid4(), + ) + + persisted = self.repository.get_miner_by_id(created.id) + self.assertEqual(persisted["asset_tag"], "ASSET-ROLLBACK-OLD") + self.assertEqual( + self.repository.list_asset_id_history(created.id), + [], + ) + def test_transaction_rolls_back_repository_writes(self): with self.assertRaises(RuntimeError): with self.repository.transaction() as repository: diff --git a/backend/tests/test_inventory_router.py b/backend/tests/test_inventory_router.py index ede1fc9..d6a9bf6 100644 --- a/backend/tests/test_inventory_router.py +++ b/backend/tests/test_inventory_router.py @@ -10,10 +10,13 @@ from fastapi.testclient import TestClient from app.identity import get_current_user from app.inventory.dependencies import get_inventory_service from app.inventory.exceptions import ( + DuplicateAssetID, InventoryValidationError, MinerNotFound, ) from app.inventory.models import ( + InventoryCreateRequest, + InventoryMetadataUpdateRequest, InventoryMinerDetail, InventoryMinerSummary, InventorySearchFilters, @@ -28,6 +31,14 @@ class StubInventoryService: self.search_filters: InventorySearchFilters | None = None self.detail_miner_id: UUID | None = None self.search_error: InventoryValidationError | None = None + self.create_result: InventoryMinerDetail | None = None + self.create_error: Exception | None = None + self.create_request: InventoryCreateRequest | None = None + self.update_result: InventoryMinerDetail | None = None + self.update_error: Exception | None = None + self.update_miner_id: UUID | None = None + self.update_request: InventoryMetadataUpdateRequest | None = None + self.update_actor_user_id: UUID | None = None def search_miners( self, @@ -38,6 +49,33 @@ class StubInventoryService: raise self.search_error return self.search_result + def create_miner( + self, + request: InventoryCreateRequest, + ) -> InventoryMinerDetail: + self.create_request = request + if self.create_error is not None: + raise self.create_error + if self.create_result is None: + raise AssertionError("create_result was not configured") + return self.create_result + + def update_metadata( + self, + miner_id: UUID, + request: InventoryMetadataUpdateRequest, + *, + actor_user_id: UUID, + ) -> InventoryMinerDetail: + self.update_miner_id = miner_id + self.update_request = request + self.update_actor_user_id = actor_user_id + if self.update_error is not None: + raise self.update_error + if self.update_result is None: + raise AssertionError("update_result was not configured") + return self.update_result + def get_miner(self, miner_id: UUID) -> InventoryMinerDetail: self.detail_miner_id = miner_id if self.detail_result is None or self.detail_result.id != miner_id: @@ -89,6 +127,219 @@ class InventoryRouterTests(unittest.TestCase): "permissions": list(permissions), } + def test_create_requires_authentication(self): + response = self.client.post( + "/api/inventory/miners", + json={"asset_id": "ASSET-NEW"}, + ) + self.assertEqual(response.status_code, 401) + self.assertEqual(response.json()["detail"], "Login required") + + def test_create_requires_miners_update_permission(self): + self.authenticate(permissions=("miners.view",)) + response = self.client.post( + "/api/inventory/miners", + json={"asset_id": "ASSET-NEW"}, + ) + self.assertEqual(response.status_code, 403) + self.assertIsNone(self.service.create_request) + + def test_create_returns_new_miner_in_typed_envelope(self): + self.authenticate(permissions=("miners.update",)) + created = miner_detail() + self.service.create_result = created + + response = self.client.post( + "/api/inventory/miners", + json={ + "asset_id": " ASSET-NEW ", + "lifecycle_status": "spare", + "site_id": str(uuid4()), + "tags": ["Priority", "Repair"], + }, + ) + + self.assertEqual(response.status_code, 201) + self.assertEqual(response.json()["data"]["id"], str(created.id)) + self.assertEqual(response.json()["meta"], {}) + self.assertIsNone(response.json()["error"]) + request = self.service.create_request + self.assertIsNotNone(request) + self.assertEqual(request.asset_id, " ASSET-NEW ") + self.assertEqual(request.lifecycle_status, "spare") + self.assertEqual(request.tags, ["Priority", "Repair"]) + + def test_duplicate_asset_id_returns_conflict(self): + self.authenticate(permissions=("miners.update",)) + self.service.create_error = DuplicateAssetID("ASSET-001") + + response = self.client.post( + "/api/inventory/miners", + json={"asset_id": "ASSET-001"}, + ) + + self.assertEqual(response.status_code, 409) + self.assertEqual( + response.json()["detail"], + "Asset ID already exists: ASSET-001", + ) + + def test_invalid_create_body_returns_validation_error(self): + self.authenticate(permissions=("miners.update",)) + for body in ( + {}, + {"asset_id": "A" * 121}, + {"asset_id": "ASSET-001", "site_id": "not-a-uuid"}, + ): + with self.subTest(body=body): + response = self.client.post( + "/api/inventory/miners", + json=body, + ) + self.assertEqual(response.status_code, 422) + self.assertIsNone(self.service.create_request) + + def test_create_maps_domain_validation_error(self): + self.authenticate(permissions=("miners.update",)) + self.service.create_error = InventoryValidationError( + "Tag names cannot be blank" + ) + + response = self.client.post( + "/api/inventory/miners", + json={"asset_id": "ASSET-001", "tags": [" "]}, + ) + + self.assertEqual(response.status_code, 422) + self.assertEqual( + response.json()["detail"], + "Tag names cannot be blank", + ) + + def test_patch_requires_authentication(self): + response = self.client.patch( + f"/api/inventory/miners/{uuid4()}", + json={"model": "M60"}, + ) + self.assertEqual(response.status_code, 401) + self.assertEqual(response.json()["detail"], "Login required") + + def test_patch_requires_miners_update_permission(self): + self.authenticate(permissions=("miners.view",)) + response = self.client.patch( + f"/api/inventory/miners/{uuid4()}", + json={"model": "M60"}, + ) + self.assertEqual(response.status_code, 403) + self.assertIsNone(self.service.update_request) + + def test_patch_unknown_miner_returns_not_found(self): + self.authenticate(permissions=("miners.update",)) + miner_id = uuid4() + self.service.update_error = MinerNotFound(miner_id) + + response = self.client.patch( + f"/api/inventory/miners/{miner_id}", + json={"model": "M60"}, + ) + + self.assertEqual(response.status_code, 404) + self.assertEqual(response.json()["detail"], "Miner not found") + + def test_patch_malformed_uuid_returns_validation_error(self): + self.authenticate(permissions=("miners.update",)) + response = self.client.patch( + "/api/inventory/miners/not-a-uuid", + json={"model": "M60"}, + ) + self.assertEqual(response.status_code, 422) + self.assertIsNone(self.service.update_request) + + def test_empty_patch_maps_domain_validation_error(self): + self.authenticate(permissions=("miners.update",)) + self.service.update_error = InventoryValidationError( + "At least one metadata field must be provided" + ) + + response = self.client.patch( + f"/api/inventory/miners/{uuid4()}", + json={}, + ) + + self.assertEqual(response.status_code, 422) + self.assertEqual( + response.json()["detail"], + "At least one metadata field must be provided", + ) + + def test_patch_rejects_unknown_and_operational_fields(self): + self.authenticate(permissions=("miners.update",)) + for body in ( + {"unknown_field": "value"}, + {"lifecycle_status": "retired"}, + {"current_ip": "192.0.2.10"}, + {"current_mac": "AA:BB:CC:DD:EE:FF"}, + {"control_board_sn": "CB-NEW"}, + {"site_id": str(uuid4())}, + {"room_id": str(uuid4())}, + {"tags": ["Priority"]}, + {"firmware_version": "1.2.3"}, + {"hostname": "miner-host"}, + ): + with self.subTest(body=body): + response = self.client.patch( + f"/api/inventory/miners/{uuid4()}", + json=body, + ) + self.assertEqual(response.status_code, 422) + self.assertIsNone(self.service.update_request) + + def test_valid_metadata_patch_returns_typed_envelope(self): + self.authenticate(permissions=("miners.update",)) + updated = miner_detail() + self.service.update_result = updated + + response = self.client.patch( + f"/api/inventory/miners/{updated.id}", + json={ + "asset_id": " ASSET-NEW ", + "miner_sn": "MINER-NEW", + "psu_serial": None, + "model": "M60", + "notes": "Bench verified", + }, + ) + + self.assertEqual(response.status_code, 200) + body = response.json() + self.assertEqual(body["data"]["id"], str(updated.id)) + self.assertEqual(body["meta"], {}) + self.assertIsNone(body["error"]) + self.assertEqual(self.service.update_miner_id, updated.id) + self.assertIsInstance(self.service.update_actor_user_id, UUID) + request = self.service.update_request + self.assertIsNotNone(request) + self.assertEqual( + request.model_fields_set, + {"asset_id", "miner_sn", "psu_serial", "model", "notes"}, + ) + self.assertIsNone(request.psu_serial) + + def test_patch_duplicate_asset_id_returns_conflict(self): + self.authenticate(permissions=("miners.update",)) + self.service.update_error = DuplicateAssetID("ASSET-TAKEN") + + response = self.client.patch( + f"/api/inventory/miners/{uuid4()}", + json={"asset_id": "ASSET-TAKEN"}, + ) + + self.assertEqual(response.status_code, 409) + self.assertEqual( + response.json()["detail"], + "Asset ID already exists: ASSET-TAKEN", + ) + def test_list_requires_authentication(self): response = self.client.get("/api/inventory/miners") self.assertEqual(response.status_code, 401) diff --git a/scripts/test-asset-id-history-migration.sh b/scripts/test-asset-id-history-migration.sh new file mode 100755 index 0000000..7c23e3a --- /dev/null +++ b/scripts/test-asset-id-history-migration.sh @@ -0,0 +1,219 @@ +#!/usr/bin/env bash +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "${repo_root}" + +project_name="${MINEROPS_ASSET_HISTORY_TEST_PROJECT:-minerops-asset-history-$$}" +empty_database="minerops_asset_history_empty" +compose=( + docker compose + --project-name "${project_name}" + --env-file .env.dev + -f compose.dev.yaml +) + +cleanup() { + status=$? + if (( status != 0 )); then + echo + echo "Asset ID history migration test failed. Recent PostgreSQL logs:" + "${compose[@]}" logs --no-color --tail 120 postgres || true + fi + "${compose[@]}" down --volumes --remove-orphans >/dev/null 2>&1 || true + exit "${status}" +} + +trap cleanup EXIT + +if [[ ! -f .env.dev ]]; then + echo "Missing .env.dev. Run ./scripts/dev-env-init.sh first." >&2 + exit 1 +fi + +run_alembic() { + "${compose[@]}" run --rm --no-deps backend alembic "$@" +} + +echo "Starting disposable PostgreSQL..." +"${compose[@]}" up --detach postgres +for attempt in $(seq 1 60); do + if "${compose[@]}" exec -T postgres sh -ec \ + 'pg_isready -U "$POSTGRES_USER" -d "$POSTGRES_DB"' \ + >/dev/null 2>&1; then + break + fi + if (( attempt == 60 )); then + echo "PostgreSQL did not become ready." >&2 + exit 1 + fi + sleep 1 +done + +echo "Building migration image..." +"${compose[@]}" build backend + +echo "Upgrading the current head to Asset ID history..." +run_alembic upgrade 0008_manual_miner_inventory +run_alembic upgrade head + +current_revision="$(run_alembic current 2>/dev/null)" +[[ "${current_revision}" == *"0009_inventory_asset_id_history"* ]] + +echo "Checking history schema, constraints, foreign keys, and indexes..." +"${compose[@]}" exec -T postgres sh -ec \ + 'psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d "$POSTGRES_DB"' \ + <<'SQL' +DO $$ +DECLARE + v_role_id UUID; +BEGIN + IF to_regclass('public.miner_asset_id_history') IS NULL THEN + RAISE EXCEPTION 'Asset ID history table was not created'; + END IF; + + IF NOT EXISTS ( + SELECT 1 + FROM information_schema.columns + WHERE table_schema = 'public' + AND table_name = 'miner_asset_id_history' + AND column_name = 'old_asset_id' + AND is_nullable = 'YES' + ) OR EXISTS ( + SELECT 1 + FROM information_schema.columns + WHERE table_schema = 'public' + AND table_name = 'miner_asset_id_history' + AND column_name IN ('miner_id', 'new_asset_id', 'changed_at', 'changed_by') + AND is_nullable <> 'NO' + ) THEN + RAISE EXCEPTION 'Asset ID history nullability is incorrect'; + END IF; + + IF NOT EXISTS ( + SELECT 1 FROM pg_indexes + WHERE schemaname = 'public' + AND tablename = 'miner_asset_id_history' + AND indexname = 'idx_miner_asset_id_history_miner_changed_at' + AND indexdef LIKE '%(miner_id, changed_at, id)%' + ) THEN + RAISE EXCEPTION 'Chronological miner history index is missing'; + END IF; + + SELECT id INTO v_role_id FROM roles WHERE name = 'Admin'; + INSERT INTO users ( + id, username, display_name, password_hash, role_id + ) VALUES ( + '00000000-0000-0000-0000-000000000901', + 'asset-history-test', + 'Asset History Test', + 'not-used', + v_role_id + ); + INSERT INTO miners (id, asset_tag, status) + VALUES ( + '00000000-0000-0000-0000-000000000902', + 'ASSET-OLD', + 'manual' + ); + INSERT INTO miner_asset_id_history ( + miner_id, old_asset_id, new_asset_id, changed_at, changed_by + ) VALUES + ( + '00000000-0000-0000-0000-000000000902', + 'ASSET-OLD', + 'ASSET-NEW', + '2026-01-01 00:00:00+00', + '00000000-0000-0000-0000-000000000901' + ), + ( + '00000000-0000-0000-0000-000000000902', + 'ASSET-NEW', + 'ASSET-LATEST', + '2026-01-02 00:00:00+00', + '00000000-0000-0000-0000-000000000901' + ); + + BEGIN + INSERT INTO miner_asset_id_history ( + miner_id, old_asset_id, new_asset_id, changed_by + ) VALUES ( + '00000000-0000-0000-0000-000000000902', + ' ASSET-OLD ', + 'ASSET-BAD', + '00000000-0000-0000-0000-000000000901' + ); + RAISE EXCEPTION 'Unnormalized old Asset ID was accepted'; + EXCEPTION WHEN check_violation THEN + NULL; + END; + + BEGIN + INSERT INTO miner_asset_id_history ( + miner_id, old_asset_id, new_asset_id, changed_by + ) VALUES ( + '00000000-0000-0000-0000-000000000999', + NULL, + 'ASSET-NEW', + '00000000-0000-0000-0000-000000000901' + ); + RAISE EXCEPTION 'Unknown miner history was accepted'; + EXCEPTION WHEN foreign_key_violation THEN + NULL; + END; + + BEGIN + INSERT INTO miner_asset_id_history ( + miner_id, old_asset_id, new_asset_id, changed_by + ) VALUES ( + '00000000-0000-0000-0000-000000000902', + 'ASSET-OLD', + 'ASSET-NEW', + NULL + ); + RAISE EXCEPTION 'History without an actor was accepted'; + EXCEPTION WHEN not_null_violation THEN + NULL; + END; + + BEGIN + DELETE FROM miners + WHERE id = '00000000-0000-0000-0000-000000000902'; + RAISE EXCEPTION 'Miner with permanent Asset ID history was deleted'; + EXCEPTION WHEN foreign_key_violation THEN + NULL; + END; +END +$$; +SQL + +echo "Downgrading and re-upgrading Asset ID history..." +run_alembic downgrade 0008_manual_miner_inventory +table_after_downgrade="$( + "${compose[@]}" exec -T postgres sh -ec \ + 'psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d "$POSTGRES_DB" -At' <<'SQL' +SELECT to_regclass('public.miner_asset_id_history'); +SQL +)" +if [[ -n "${table_after_downgrade}" ]]; then + echo "Asset ID history table remained after downgrade." >&2 + exit 1 +fi +run_alembic upgrade head + +echo "Migrating an empty database from base through head..." +"${compose[@]}" exec -T postgres sh -ec \ + 'psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d postgres \ + -c "CREATE DATABASE minerops_asset_history_empty"' >/dev/null +"${compose[@]}" run --rm --no-deps \ + -e "POSTGRES_DB=${empty_database}" \ + backend alembic upgrade head + +empty_revision="$( + "${compose[@]}" run --rm --no-deps \ + -e "POSTGRES_DB=${empty_database}" \ + backend alembic current 2>/dev/null +)" +[[ "${empty_revision}" == *"0009_inventory_asset_id_history"* ]] + +echo "Asset ID history migration test passed." diff --git a/scripts/test-dev-baseline.sh b/scripts/test-dev-baseline.sh index afaee47..383f1cb 100755 --- a/scripts/test-dev-baseline.sh +++ b/scripts/test-dev-baseline.sh @@ -88,7 +88,7 @@ health_json="$(cat "${health_file}")" echo "Checking migrations and required schema..." current_revision="$("${compose[@]}" exec -T backend alembic current 2>/dev/null)" -[[ "${current_revision}" == *"0008_manual_miner_inventory"* ]] +[[ "${current_revision}" == *"0009_inventory_asset_id_history"* ]] missing_tables="$( "${compose[@]}" exec -T postgres sh -c \ diff --git a/sql/verify-development-schema.sql b/sql/verify-development-schema.sql index 79f5010..9ee8e2c 100644 --- a/sql/verify-development-schema.sql +++ b/sql/verify-development-schema.sql @@ -6,6 +6,7 @@ WITH expected(table_name) AS ( ('miners'), ('tags'), ('miner_tags'), + ('miner_asset_id_history'), ('miner_status_snapshots'), ('miner_status_hourly_rollups'), ('miner_status_daily_rollups'),