Commit bbb5dcbf authored by George Papathanail's avatar George Papathanail
Browse files

fix(qod): don't let a session that never started block the device 

A create that fails ends up UNAVAILABLE with status_info NETWORK_TERMINATED, the same state as a network-terminated session. find_active_by_device treated it as still active for the 360 s cleanup window, so every new create for that device returned 409 for 6 minutes. That check now also requires started_at to be set, so only sessions that actually became AVAILABLE keep blocking the device.
parent 78037e73

src/open_exposure_gateway/adapters/database/repos/qod_sessions.py

+1 −0
Changes for src/open_exposure_gateway/adapters/database/repos/qod_sessions.py: 1 added line, 0 removed lines.
Original line number Diff line number Diff line
@@ -49,6 +49,7 @@ class SqlQodSessionRepository(QodSessionRepository): 
                    and_(

                        QodSessionRow.state == QodSessionState.UNAVAILABLE,

                        QodSessionRow.status_info == "NETWORK_TERMINATED",

                        QodSessionRow.started_at.is_not(None),

                    ),

                ),

                or_(*identifier_matches),

tests/integration/test_postgres.py

+4 −0
Changes for tests/integration/test_postgres.py: 4 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -639,6 +639,10 @@ async def test_qod_session_repo_find_active_by_device_scopes_by_identifier_state 
    saved.state = QodSessionState.UNAVAILABLE

    saved.status_info = "NETWORK_TERMINATED"

    await repo.save(saved)

    assert await repo.find_active_by_device({"phoneNumber": "+123456789"}, "tenant-a") is None



    saved.started_at = datetime.now(timezone.utc)

    await repo.save(saved)

    assert await repo.find_active_by_device({"phoneNumber": "+123456789"}, "tenant-a") is not None



    saved.status_info = "DURATION_EXPIRED"

tests/unit/fakes.py

+3 −1
Changes for tests/unit/fakes.py: 3 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -719,7 +719,9 @@ class FakeQodSessionRepository(QodSessionRepository): 
        active_states = (QodSessionState.REQUESTED, QodSessionState.AVAILABLE)

        for row in self.rows.values():

            holds_resources = row.state in active_states or (

                row.state == QodSessionState.UNAVAILABLE and row.status_info == "NETWORK_TERMINATED"

                row.state == QodSessionState.UNAVAILABLE

                and row.status_info == "NETWORK_TERMINATED"

                and row.started_at is not None

            )

            if not holds_resources or row.device is None:

                continue

tests/unit/test_qod_flows.py

+61 −0
Changes for tests/unit/test_qod_flows.py: 61 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -503,6 +503,67 @@ class TestQodSessionConflictFlow: 


        assert response.status_code == 201



    async def test_same_device_is_accepted_right_after_a_failed_create(

        self,

        api_client: TestClient,

        live_qod: None,

        fake_bus: FakeDataBus,

        qod_session_repo: FakeQodSessionRepository,

    ) -> None:

        first = api_client.post(f"{QOD_BASE}/sessions", json=PHONE_SESSION_BODY)

        session_id = UUID(first.json()["sessionId"])

        operation_id = qod_session_repo.rows[session_id].operation_id

        await fake_bus.publish(

            str(Subject.OPERATION_COMPLETED),

            completion_payload(

                str(operation_id),

                status="failed",

                instances=[

                    {

                        "service_instance_id": str(uuid4()),

                        "zone_id": str(uuid4()),

                        "status": "failed",

                        "error": {"status": 502, "code": "NEF", "message": "instance error"},

                    }

                ],

            ),

        )

        failed = qod_session_repo.rows[session_id]

        assert failed.state == QodSessionState.UNAVAILABLE

        assert failed.status_info == "NETWORK_TERMINATED"

        assert failed.started_at is None



        response = api_client.post(f"{QOD_BASE}/sessions", json=PHONE_SESSION_BODY)



        assert response.status_code == 201



    async def test_network_terminated_session_that_started_still_blocks_the_device(

        self,

        api_client: TestClient,

        live_qod: None,

        fake_bus: FakeDataBus,

        qod_session_repo: FakeQodSessionRepository,

    ) -> None:

        first = api_client.post(f"{QOD_BASE}/sessions", json=PHONE_SESSION_BODY)

        session_id = UUID(first.json()["sessionId"])

        operation_id = qod_session_repo.rows[session_id].operation_id

        await fake_bus.publish(

            str(Subject.OPERATION_COMPLETED),

            completion_payload(str(operation_id)),

        )

        await fake_bus.publish(

            str(Subject.OPERATION_STATUS),

            operation_status_payload(str(operation_id)),

        )

        terminated = qod_session_repo.rows[session_id]

        assert terminated.state == QodSessionState.UNAVAILABLE

        assert terminated.status_info == "NETWORK_TERMINATED"

        assert terminated.started_at is not None



        response = api_client.post(f"{QOD_BASE}/sessions", json=PHONE_SESSION_BODY)



        assert response.status_code == 409



    def test_different_device_is_accepted(self, api_client: TestClient) -> None:

        api_client.post(f"{QOD_BASE}/sessions", json=PHONE_SESSION_BODY)

        other = {**SESSION_BODY, "device": {"phoneNumber": "+987654321"}}

tests/unit/test_qod_service.py

+22 −2
Changes for tests/unit/test_qod_service.py: 22 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -571,11 +571,14 @@ class TestCreateSessionConflict: 
        session_id: UUID,

        state: QodSessionState,

        status_info: Literal["DURATION_EXPIRED", "NETWORK_TERMINATED"] | None = None,

        started_at: datetime | None = None,

    ) -> None:

        stored = await qod_session_repo.get_by_id(session_id)

        assert stored is not None

        await qod_session_repo.save(

            stored.model_copy(update={"state": state, "status_info": status_info})

            stored.model_copy(

                update={"state": state, "status_info": status_info, "started_at": started_at}

            )

        )



    @pytest.mark.parametrize(
@@ -598,7 +601,8 @@ class TestCreateSessionConflict: 
        first = await service.create_session(

            request=_make_request(), tenant_id="tenant-1", app_provider_id="provider-1"

        )

        await self._set_state(qod_session_repo, first.sessionId, state, status_info)

        started_at = None if state == QodSessionState.REQUESTED else datetime.now(timezone.utc)

        await self._set_state(qod_session_repo, first.sessionId, state, status_info, started_at)



        with pytest.raises(ConflictException) as exc_info:

            await service.create_session(
@@ -639,6 +643,22 @@ class TestCreateSessionConflict: 


        assert second.sessionId != first.sessionId



    async def test_allows_new_session_after_one_that_failed_before_starting(

        self, service: QualityOnDemandService, qod_session_repo: FakeQodSessionRepository

    ) -> None:

        first = await service.create_session(

            request=_make_request(), tenant_id="tenant-1", app_provider_id="provider-1"

        )

        await self._set_state(

            qod_session_repo, first.sessionId, QodSessionState.UNAVAILABLE, "NETWORK_TERMINATED"

        )



        second = await service.create_session(

            request=_make_request(), tenant_id="tenant-1", app_provider_id="provider-1"

        )



        assert second.sessionId != first.sessionId



    async def test_allows_a_different_device(

        self, service: QualityOnDemandService, qod_session_repo: FakeQodSessionRepository

    ) -> None: