feat(fm): one agreement per partner, edited in place

Sergio Gimenezrequested to merge
feat/one-agreement-per-partner into develop 
sequenceDiagram
  participant P as portal / OEG
  participant FM as our FM
  participant DB as federation_agreements
  participant X as Partner FM
  P->>FM: POST /internal/agreements (partner already has one)
  FM-->>P: 409 agreement-exists, "Edit agreement {id} instead."
  P->>FM: PATCH /internal/agreements/{id} {permitted_zone_ids, zone_resources}
  Note over FM: create checks run on the merged agreement<br/>(e.g. zone_resources must name permitted zones → 400)
  FM->>DB: update in place (unique: one non-expired row per partner)
  FM->>X: ZONES REMOVE / ADD for the zones that changed
  P->>FM: PATCH {status: suspended}
  FM->>X: ZONES REMOVE (all offered zones)
  P->>FM: PATCH {status: active}
  FM->>X: ZONES ADD

A partner could hold several agreements, and the newest valid one won. Changing zones or terms meant creating another, which in the portal showed up as a list where the last entry quietly replaced the others. GSMA OPG.04 has no agreement object (zones are offered "based on the prior agreement"), so the shape is ours to pick, and one editable agreement per partner is what the portal needs.

POST /internal/agreements now answers 409 agreement-exists when the partner already has an agreement that isn't expired, and names it. PATCH takes every field except the partner: zones, zone terms, API types and app mappings as well as status and dates. It runs the create checks on the merged result, so a PATCH that leaves zone_resources pointing at a removed zone gets a 400. A PATCH that would bring an expired agreement back next to the live one also gets the 409. Zone edits reach a federated partner as ZONES ADD/REMOVE through the notifier that already exists.

A partial unique index enforces the rule: one row per partner where status <> 'expired'. Migration 0008 resolves partners that already have several. It keeps the agreement FM applies today (active and valid now, else active, else the newest) and marks the rest expired, so the history stays readable.

flowchart LR
  A["operator-3 (Athens)<br/>active, valid until 2027-12-31"] -->|kept| K[active]
  B["operator-3 (Athens)<br/>suspended"] -->|0008| E[expired]

Merge order: OEG !47 relays the new 409 (without it OEG turns it into a 502); it's safe to merge before this. Portal !15 (merged) uses the wider PATCH and should go last, because develop rejects the new fields.

Result
full suite, Postgres + NATS + Keycloak as in CI 442 passed, 1 skipped; same on a fresh database
mypy, ruff clean
migration on the testbed (ns oop, three operators, DBs backed up first) OP-1 had two agreements for operator-3: the active one stays, the suspended one becomes expired; OP-2 and OP-3 unchanged
second agreement for the same partner 409 agreement-exists, "Edit agreement 1a4d2b51… instead."
PATCH removing Barcelona / adding it back 200; OP-1 FM zone_change_notified REMOVE / ADD delivered: true; OP-2 FM applies the callback; OP-2's zone list loses and regains Barcelona
PATCH leaving zone_resources for a zone not permitted 400, zone_resources lists zones missing from permitted_zone_ids
PATCH reviving an expired agreement next to the live one 409 agreement-exists
portal (!15 (merged) branch): edit end date, add and remove a flavour, suspend, activate every PATCH 200; suspend drops Barcelona from OP-2's zones, activate brings it back

op1-agreement-card

op2-zones-without-barcelona

🤖 Generated with Claude Code

Merge request reports

src/federation_manager/adapters/database/agreement_repo.py

+21 −5
Changes for src/federation_manager/adapters/database/agreement_repo.py: 21 added lines, 5 removed lines.
Original line number Diff line number Diff line
@@ -2,11 +2,15 @@ from typing import Any 
from uuid import UUID



from sqlalchemy import Row, and_, func, insert, or_, select, update

from sqlalchemy.exc import IntegrityError

from sqlalchemy.ext.asyncio import AsyncSession



from federation_manager.adapters.database.tables import federation_agreements as agreements

from federation_manager.domain.errors import AgreementExists

from federation_manager.domain.models import Agreement, AppMapping



_ONE_PER_PARTNER = "uq_federation_agreements_one_per_partner"





class PostgresAgreementRepo:

    def __init__(self, session: AsyncSession) -> None:
@@ -43,18 +47,30 @@ class PostgresAgreementRepo: 
        return [_to_agreement(row) for row in (await self._session.execute(stmt)).all()]



    async def add(self, agreement: Agreement) -> None:

        await self._session.execute(

        await self._write(

            agreement,

            insert(agreements).values(

                id=agreement.id, partner_op_id=agreement.partner_op_id, **_values(agreement)

            ),

        )

        )

        await self._session.commit()



    async def update(self, agreement: Agreement) -> None:

        await self._session.execute(

            update(agreements).where(agreements.c.id == agreement.id).values(**_values(agreement))

        await self._write(

            agreement,

            update(agreements).where(agreements.c.id == agreement.id).values(**_values(agreement)),

        )



    async def _write(self, agreement: Agreement, stmt: Any) -> None:

        try:

            await self._session.execute(stmt)

            await self._session.commit()

        except IntegrityError as error:

            await self._session.rollback()

            # a concurrent create won, or an expired old agreement is being brought back

            # while the partner has a live one

            if _ONE_PER_PARTNER not in str(error.orig):

                raise

            raise AgreementExists(agreement.partner_op_id, None) from None





def _to_agreement(row: Row[Any]) -> Agreement:

src/federation_manager/adapters/database/tables.py

+7 −0
Changes for src/federation_manager/adapters/database/tables.py: 7 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -106,6 +106,13 @@ federation_agreements = Table( 
    ),

    Index("idx_federation_agreements_partner", "partner_op_id"),

    Index("idx_federation_agreements_status_validity", "status", "valid_from", "valid_until"),

    # one agreement per partner; rows from before that rule stay, as expired history

    Index(

        "uq_federation_agreements_one_per_partner",

        "partner_op_id",

        unique=True,

        postgresql_where=text("status <> 'expired'"),

    ),

    CheckConstraint(

        _one_of("status", "draft", "active", "suspended", "expired"),

        name="ck_federation_agreements_status",

src/federation_manager/api/internal/agreements.py

+53 −19
Changes for src/federation_manager/api/internal/agreements.py: 53 added lines, 19 removed lines.
Original line number Diff line number Diff line
@@ -74,22 +74,14 @@ class AgreementCreate(BaseModel): 
    def fields(self) -> dict[str, Any]:

        return {

            "partner_op_id": self.partner_op_id,

            "permitted_api_types": set(self.permitted_api_types),

            "permitted_zone_ids": set(self.permitted_zone_ids),

            "zone_resources": {

                zone: terms.model_dump(mode="json", by_alias=True, exclude_none=True)

                for zone, terms in self.zone_resources.items()

            },

            "app_mappings": tuple(

                AppMapping(

                    app_id=m.appId,

                    app_version=m.appVersion,

                    flavour_id=m.flavourId,

                    service_specification_id=m.service_specification_id,

                )

                for m in self.service_spec_mappings.apps

            **_terms(

                {

                    "permitted_api_types": self.permitted_api_types,

                    "permitted_zone_ids": self.permitted_zone_ids,

                    "zone_resources": self.zone_resources,

                    "service_spec_mappings": self.service_spec_mappings,

                }

            ),

            "api_family_mappings": dict(self.service_spec_mappings.api_families),

            "usage_limits": self.usage_limits,

            "valid_from": self.valid_from,

            "valid_until": self.valid_until,
@@ -98,24 +90,66 @@ class AgreementCreate(BaseModel): 




class AgreementUpdate(BaseModel):

    """Partial update of status, limits and validity; other terms need a new agreement."""

    """Partial update: a partner has one agreement, so any of its terms can change in place."""



    model_config = ConfigDict(extra="forbid")



    status: Status | None = None

    permitted_api_types: list[ApiType] | None = None

    permitted_zone_ids: list[UUID] | None = None

    zone_resources: dict[UUID, ZoneTerms] | None = None

    service_spec_mappings: ServiceSpecMappings | None = None

    usage_limits: dict[str, Any] | None = None

    valid_from: AwareDatetime | None = None

    valid_until: AwareDatetime | None = None



    @model_validator(mode="after")

    def _required_not_cleared(self) -> Self:

        for field in ("status", "usage_limits", "valid_from"):

            if field in self.model_fields_set and getattr(self, field) is None:

        # only valid_until can be cleared; every other field always has a value

        for field in self.model_fields_set - {"valid_until"}:

            if getattr(self, field) is None:

                raise ValueError(f"{field} cannot be null")

        return self



    def changes(self) -> dict[str, Any]:

        return self.model_dump(exclude_unset=True)

        sent = {field: getattr(self, field) for field in self.model_fields_set}

        plain = {k: v for k, v in sent.items() if k not in _TERM_FIELDS}

        return {**plain, **_terms(sent)}





_TERM_FIELDS = {

    "permitted_api_types",

    "permitted_zone_ids",

    "zone_resources",

    "service_spec_mappings",

}





def _terms(sent: dict[str, Any]) -> dict[str, Any]:

    """The agreement's terms as the domain holds them, for whichever of them were sent."""

    terms: dict[str, Any] = {}

    if "permitted_api_types" in sent:

        terms["permitted_api_types"] = set(sent["permitted_api_types"])

    if "permitted_zone_ids" in sent:

        terms["permitted_zone_ids"] = set(sent["permitted_zone_ids"])

    if "zone_resources" in sent:

        terms["zone_resources"] = {

            zone: zone_terms.model_dump(mode="json", by_alias=True, exclude_none=True)

            for zone, zone_terms in sent["zone_resources"].items()

        }

    if "service_spec_mappings" in sent:

        mappings: ServiceSpecMappings = sent["service_spec_mappings"]

        terms["app_mappings"] = tuple(

            AppMapping(

                app_id=m.appId,

                app_version=m.appVersion,

                flavour_id=m.flavourId,

                service_specification_id=m.service_specification_id,

            )

            for m in mappings.apps

        )

        terms["api_family_mappings"] = dict(mappings.api_families)

    return terms





class AgreementView(BaseModel):

src/federation_manager/api/errors.py

+26 −0
Changes for src/federation_manager/api/errors.py: 26 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -9,8 +9,10 @@ from pydantic.json_schema import models_json_schema 


from federation_manager.contracts.ewbi import InvalidParam, ProblemDetails

from federation_manager.domain.errors import (

    AgreementExists,

    AgreementExpired,

    AgreementNotFound,

    AgreementTermsInvalid,

    AgreementValidityInvalid,

    AgreementViolation,

    AppInstanceUnknown,
@@ -414,6 +416,30 @@ def register_exception_handlers(app: FastAPI) -> None: 
            422, "agreement-validity-invalid", "Invalid Validity Period", str(exc), request.url.path

        )



    @app.exception_handler(AgreementExists)

    async def _agreement_exists(request: Request, exc: AgreementExists) -> JSONResponse:

        existing = f" Edit agreement {exc.agreement_id} instead." if exc.agreement_id else ""

        return problem(

            409,

            "agreement-exists",

            "Agreement Exists",

            f"This partner already has an agreement.{existing}",

            request.url.path,

        )



    @app.exception_handler(AgreementTermsInvalid)

    async def _agreement_terms_invalid(

        request: Request, exc: AgreementTermsInvalid

    ) -> JSONResponse:

        return problem(

            400,

            "invalid-request",

            "Invalid Request",

            "One or more request parameters or body fields are invalid; see invalidParams.",

            request.url.path,

            invalid_params=[InvalidParam(param="zone_resources", reason=str(exc))],

        )



    @app.exception_handler(PartnerCredentialsIncomplete)

    async def _credentials_incomplete(

        request: Request, exc: PartnerCredentialsIncomplete

src/federation_manager/application/agreements.py

+24 −1
Changes for src/federation_manager/application/agreements.py: 24 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -5,7 +5,9 @@ from uuid import UUID, uuid4 


from federation_manager.application.zone_notifications import ZoneOfferNotifier

from federation_manager.domain.errors import (

    AgreementExists,

    AgreementNotFound,

    AgreementTermsInvalid,

    AgreementValidityInvalid,

    PartnerNotRegistered,

)
@@ -14,7 +16,11 @@ from federation_manager.domain.ports import AgreementRepositoryPort, PartnerRepo 




class AgreementService:

    """Records the terms two operators agreed on; FM enforces them on every partner request."""

    """Records the terms two operators agreed on; FM enforces them on every partner request.



    A partner has one agreement, edited in place, so what applies is never a matter of which

    of several rows is newest.

    """



    def __init__(

        self,
@@ -31,8 +37,12 @@ class AgreementService: 


    async def create(self, fields: dict[str, Any]) -> Agreement:

        partner = await self._require_partner(fields["partner_op_id"])

        existing = await self._agreements.list_for_partner(partner.id)

        if existing:

            raise AgreementExists(partner.id, _current(existing).id)

        agreement = Agreement(id=self._new_id(), **fields)

        _require_valid_period(agreement)

        _require_terms_for_permitted_zones(agreement)

        before = await self._offer(partner)

        await self._agreements.add(agreement)

        await self._tell(partner, before)
@@ -47,6 +57,7 @@ class AgreementService: 
    async def update(self, agreement_id: UUID, changes: dict[str, Any]) -> Agreement:

        updated = replace(await self.get(agreement_id), **changes)

        _require_valid_period(updated)

        _require_terms_for_permitted_zones(updated)

        partner = await self._require_partner(updated.partner_op_id)

        before = await self._offer(partner)

        await self._agreements.update(updated)
@@ -76,3 +87,15 @@ class AgreementService: 
def _require_valid_period(agreement: Agreement) -> None:

    if agreement.valid_until is not None and agreement.valid_until <= agreement.valid_from:

        raise AgreementValidityInvalid





def _require_terms_for_permitted_zones(agreement: Agreement) -> None:

    # checked on the merged agreement, since a PATCH may change either side alone

    if set(agreement.zone_resources) - agreement.permitted_zone_ids:

        raise AgreementTermsInvalid("zone_resources lists zones missing from permitted_zone_ids")





def _current(agreements: list[Agreement]) -> Agreement:

    # partners agreed before agreements became one per partner keep the older ones as expired

    live = [a for a in agreements if a.status != "expired"]

    return (live or agreements)[-1]