M8-R08: add atomic meter close and binding transfer

tests/test_api_meters.py

@@ -31,7 +31,8 @@ Retroactive recompute integration
 
 from __future__ import annotations
 
-from datetime import UTC, datetime, timedelta
+from datetime import UTC, datetime, timedelta, timezone
+from decimal import Decimal
 from unittest.mock import patch
 
 import pytest
@@ -39,7 +40,7 @@ from fastapi.testclient import TestClient
 from sqlalchemy import create_engine, event, select
 from sqlalchemy.orm import Session
 
-from app.models.energy import EnergyCostPeriod, Meter
+from app.models.energy import EnergyCostPeriod, Meter, MeterCostPeriod
 from app.models.meter_source import MeterSource, MeterSourceBinding, MeterSourceChannel
 
 # ---------------------------------------------------------------------------
@@ -68,8 +69,15 @@ def _declare_payload(**overrides) -> dict:
     return base
 
 
-def _add_bound_channel(engine, *, meter_id: int, started_at: datetime) -> str:
-    """Persist one test-only DSMR channel binding and return its public UUID."""
+def _add_bound_channel(
+    engine,
+    *,
+    meter_id: int,
+    started_at: datetime,
+    ended_at: datetime | None = None,
+    unit: str = "kWh",
+) -> str:
+    """Persist one test-only channel binding and return its public UUID."""
     with Session(engine) as session:
         source = MeterSource(
             name="Test DSMR",
@@ -84,9 +92,9 @@ def _add_bound_channel(engine, *, meter_id: int, started_at: datetime) -> str:
         session.flush()
         channel = MeterSourceChannel(
             source_id=source.id,
-            channel_key="electricity-total",
-            label="Electricity total",
-            unit="kWh",
+            channel_key=f"test-total-{meter_id}",
+            label="Test total",
+            unit=unit,
             created_at=started_at,
             updated_at=started_at,
         )
@@ -97,6 +105,7 @@ def _add_bound_channel(engine, *, meter_id: int, started_at: datetime) -> str:
                 meter_id=meter_id,
                 channel_id=channel.id,
                 started_at=started_at,
+                ended_at=ended_at,
                 created_at=started_at,
                 updated_at=started_at,
             )
@@ -595,6 +604,7 @@ def test_declare_meter_retroactive_triggers_recompute(meters_client):
         # recompute_range should have been called with start == t_past
         assert mock_recompute.called
         assert mock_recompute.call_args.kwargs["commit"] is False
+        assert mock_recompute.call_args.kwargs["strict"] is True
         call_args = mock_recompute.call_args
         recompute_start = call_args[0][1]  # positional arg index 1 (session is 0)
         # Normalise for comparison
@@ -735,6 +745,7 @@ def test_patch_meter_started_at_retroactive_triggers_recompute(meters_client):
         # recompute should be triggered
         assert mock_recompute.called
         assert mock_recompute.call_args.kwargs["commit"] is False
+        assert mock_recompute.call_args.kwargs["strict"] is True
         call_args = mock_recompute.call_args
         recompute_start = call_args[0][1]
         if recompute_start.tzinfo is None:
@@ -776,6 +787,59 @@ def test_patch_meter_started_at_interval_violation_returns_422(meters_client):
     assert resp.status_code == 422
 
 
+@pytest.mark.parametrize("shift", ["later", "earlier"])
+def test_patch_meter_started_at_rejects_boundary_shift_that_strands_binding(
+    meters_client, mock_publish_discovery, shift
+):
+    """Rejected boundary shifts leave adjacent meters/bindings untouched and emit no side effects."""
+    client, engine = meters_client
+    _login(client)
+    t0 = datetime(2024, 1, 1, tzinfo=UTC)
+    boundary = datetime(2025, 1, 1, tzinfo=UTC)
+    with patch("app.api.routes.api.meters.recompute_range", return_value=0):
+        meter_a = client.post(
+            "/api/energy/meters", json=_declare_payload(label="A", started_at=t0.isoformat()),
+            headers={"X-CSRF-Token": _CSRF},
+        ).json()
+        meter_b = client.post(
+            "/api/energy/meters", json=_declare_payload(
+                label="B", started_at=boundary.isoformat(), reason="meter_swap"
+            ), headers={"X-CSRF-Token": _CSRF},
+        ).json()
+    if shift == "later":
+        _add_bound_channel(engine, meter_id=meter_b["id"], started_at=boundary)
+        proposed = boundary + timedelta(days=1)
+    else:
+        _add_bound_channel(engine, meter_id=meter_a["id"], started_at=t0)
+        with Session(engine) as session:
+            binding = session.scalar(select(MeterSourceBinding))
+            assert binding is not None
+            binding.ended_at = boundary
+            session.commit()
+        proposed = boundary - timedelta(days=1)
+
+    mock_publish_discovery.reset_mock()
+    with patch("app.api.routes.api.meters.recompute_range", return_value=0) as recompute:
+        response = client.patch(
+            f"/api/energy/meters/{meter_b['id']}", json={"started_at": proposed.isoformat()},
+            headers={"X-CSRF-Token": _CSRF},
+        )
+    assert response.status_code == 422
+    recompute.assert_not_called()
+    mock_publish_discovery.assert_not_called()
+
+    with Session(engine) as observer:
+        assert observer.get(Meter, meter_a["id"]).ended_at.replace(tzinfo=UTC) == boundary
+        assert observer.get(Meter, meter_b["id"]).started_at.replace(tzinfo=UTC) == boundary
+        binding = observer.scalar(select(MeterSourceBinding))
+        assert binding is not None
+        if shift == "later":
+            assert binding.meter_id == meter_b["id"] and binding.ended_at is None
+        else:
+            assert binding.meter_id == meter_a["id"]
+            assert binding.ended_at.replace(tzinfo=UTC) == boundary
+
+
 def test_patch_meter_no_recompute_when_started_at_not_changed(meters_client):
     """PATCH that only changes label does NOT trigger recompute."""
     client, _ = meters_client
@@ -801,6 +865,201 @@ def test_patch_meter_no_recompute_when_started_at_not_changed(meters_client):
         assert not mock_recompute.called
 
 
+@pytest.mark.parametrize("representation", ["aware_utc", "naive_local"])
+def test_patch_meter_rejects_future_started_at_before_any_side_effect(
+    meters_client, mock_publish_discovery, monkeypatch, representation
+):
+    """Future aware and local-naive starts leave all persisted state untouched."""
+    client, engine = meters_client
+    _login(client)
+    started = datetime.now(UTC) - timedelta(hours=2)
+    with patch("app.api.routes.api.meters.recompute_range", return_value=0):
+        declared = client.post(
+            "/api/energy/meters",
+            json=_declare_payload(
+                label="Original", note="Original note", started_at=started.isoformat()
+            ),
+            headers={"X-CSRF-Token": _CSRF},
+        )
+    meter_id = declared.json()["id"]
+    mock_publish_discovery.reset_mock()
+    future = datetime.now(UTC) + timedelta(hours=2)
+    if representation == "aware_utc":
+        proposed = future.isoformat()
+    else:
+        monkeypatch.setattr(
+            "app.services.timezone.local_tz", lambda: timezone(timedelta(hours=2))
+        )
+        proposed = (future + timedelta(hours=2)).replace(tzinfo=None).isoformat()
+
+    with patch("app.api.routes.api.meters.recompute_range", return_value=0) as recompute:
+        response = client.patch(
+            f"/api/energy/meters/{meter_id}",
+            json={"label": "Changed", "note": "Changed note", "started_at": proposed},
+            headers={"X-CSRF-Token": _CSRF},
+        )
+
+    assert response.status_code == 422
+    recompute.assert_not_called()
+    mock_publish_discovery.assert_not_called()
+    with Session(engine) as observer:
+        meter = observer.get(Meter, meter_id)
+        assert meter is not None
+        assert meter.label == "Original"
+        assert meter.note == "Original note"
+        assert meter.started_at.replace(tzinfo=UTC) == started
+
+
+@pytest.mark.parametrize("commodity", ["heating", "hot_water"])
+def test_patch_thermal_started_at_uses_thermal_recompute(meters_client, monkeypatch, commodity):
+    """A successful thermal correction never routes through electricity recompute."""
+    from app.services import meter_cost
+
+    client, _ = meters_client
+    _login(client)
+    started = datetime.now(UTC) - timedelta(hours=3)
+    calls = []
+    monkeypatch.setattr(
+        meter_cost,
+        "recompute_range",
+        lambda db, start, end, *, commit: calls.append((start, end, commit)) or 0,
+    )
+    with patch("app.api.routes.api.meters.recompute_range", return_value=0) as electricity:
+        declared = client.post(
+            "/api/energy/meters",
+            json=_declare_payload(
+                commodity=commodity,
+                label=f"{commodity} meter",
+                started_at=started.isoformat(),
+            ),
+            headers={"X-CSRF-Token": _CSRF},
+        )
+        assert declared.status_code == 201
+        calls.clear()
+        electricity.reset_mock()
+        response = client.patch(
+            f"/api/energy/meters/{declared.json()['id']}",
+            json={"started_at": (started + timedelta(minutes=30)).isoformat()},
+            headers={"X-CSRF-Token": _CSRF},
+        )
+
+    assert response.status_code == 200
+    assert len(calls) == 1
+    assert calls[0][2] is False
+    electricity.assert_not_called()
+
+
+def _create_adjacent_thermal_patch_state(engine, commodity: str):
+    """Create an editable thermal boundary with two untouched bindings."""
+    old_start = datetime.now(UTC) - timedelta(hours=5)
+    boundary = old_start + timedelta(hours=1)
+    shifted = boundary + timedelta(minutes=30)
+    current_binding_start = shifted + timedelta(minutes=30)
+    with Session(engine) as session:
+        previous = Meter(
+            label="Previous thermal meter",
+            commodity=commodity,
+            started_at=old_start,
+            ended_at=boundary,
+            reason="initial",
+            created_at=old_start,
+        )
+        current = Meter(
+            label="Current thermal meter",
+            commodity=commodity,
+            started_at=boundary,
+            reason="meter_swap",
+            created_at=boundary,
+        )
+        session.add_all([previous, current])
+        session.commit()
+        previous_id, current_id = previous.id, current.id
+    unit = {"heating": "GJ", "hot_water": "m³"}[commodity]
+    _add_bound_channel(
+        engine, meter_id=previous_id, started_at=old_start, ended_at=boundary, unit=unit
+    )
+    _add_bound_channel(engine, meter_id=current_id, started_at=current_binding_start, unit=unit)
+    return previous_id, current_id, boundary, shifted, current_binding_start
+
+
+@pytest.mark.parametrize("commodity", ["heating", "hot_water"])
+@pytest.mark.parametrize("failure", ["recompute", "flush", "commit"])
+def test_patch_thermal_failure_rolls_back_lifecycle_and_cost_state(
+    meters_client, mock_publish_discovery, monkeypatch, commodity, failure
+):
+    """Thermal PATCH failures roll back meters, bindings, cost writes, and HA."""
+    from app.services import meter_cost
+
+    client, engine = meters_client
+    _login(client)
+    previous_id, current_id, boundary, shifted, binding_start = _create_adjacent_thermal_patch_state(
+        engine, commodity
+    )
+
+    def recompute_with_uncommitted_cost(db, start, end, *, commit):
+        assert commit is False
+        db.add(
+            MeterCostPeriod(
+                commodity=commodity,
+                period_start=shifted,
+                period_end=shifted + timedelta(minutes=15),
+                quantity=Decimal("0"),
+                cost=Decimal("0"),
+                currency="EUR",
+                cost_breakdown={},
+                pricing_snapshot={},
+                quality="invalid",
+                degraded=True,
+                degraded_reason="test rollback",
+                created_at=shifted,
+                updated_at=shifted,
+            )
+        )
+        if failure == "recompute":
+            raise RuntimeError("thermal recompute failed")
+        return 0
+
+    monkeypatch.setattr(meter_cost, "recompute_range", recompute_with_uncommitted_cost)
+    if failure == "flush":
+        monkeypatch.setattr(
+            Session,
+            "flush",
+            lambda *_args, **_kwargs: (_ for _ in ()).throw(RuntimeError("thermal flush failed")),
+        )
+    if failure == "commit":
+        def fail_commit(_session: Session) -> None:
+            raise RuntimeError("thermal commit failed")
+
+        event.listen(Session, "before_commit", fail_commit)
+
+    try:
+        expected = f"thermal {failure} failed"
+        with pytest.raises(RuntimeError, match=expected):
+            client.patch(
+                f"/api/energy/meters/{current_id}",
+                json={"started_at": shifted.isoformat()},
+                headers={"X-CSRF-Token": _CSRF},
+            )
+    finally:
+        if failure == "commit":
+            event.remove(Session, "before_commit", fail_commit)
+        if failure == "flush":
+            monkeypatch.undo()
+
+    mock_publish_discovery.assert_not_called()
+    with Session(engine) as observer:
+        previous = observer.get(Meter, previous_id)
+        current = observer.get(Meter, current_id)
+        assert previous is not None and previous.ended_at.replace(tzinfo=UTC) == boundary
+        assert current is not None and current.started_at.replace(tzinfo=UTC) == boundary
+        bindings = observer.execute(
+            select(MeterSourceBinding).order_by(MeterSourceBinding.meter_id)
+        ).scalars().all()
+        assert bindings[0].ended_at.replace(tzinfo=UTC) == boundary
+        assert bindings[1].started_at.replace(tzinfo=UTC) == binding_start
+        assert observer.execute(select(MeterCostPeriod)).scalars().all() == []
+
+
 # ---------------------------------------------------------------------------
 # Timeline continuity (recompute mocked to avoid slow computation over empty quarters)
 # ---------------------------------------------------------------------------
@@ -994,3 +1253,173 @@ def test_declare_meter_succeeds_when_publish_discovery_raises(meters_client):
     # The meter must be created successfully despite the discovery failure.
     assert resp.status_code == 201
     assert resp.json()["label"] == "Best Effort Meter"
+
+
+@pytest.mark.parametrize("commodity", ["heating", "hot_water"])
+def test_thermal_declare_and_close_use_meter_cost_recompute(meters_client, monkeypatch, commodity):
+    """Thermal lifecycle routes use the meter-cost helper's actual signature."""
+    from app.services import meter_cost
+
+    client, _ = meters_client
+    _login(client)
+    calls = []
+    monkeypatch.setattr(meter_cost, "recompute_range", lambda db, start, end, *, commit: calls.append((start, end, commit)) or 0)
+    started = datetime.now(UTC) - timedelta(hours=2)
+    declared = client.post("/api/energy/meters", headers={"X-CSRF-Token": _CSRF}, json={
+        "label": f"{commodity} meter", "started_at": started.isoformat(), "reason": "initial", "commodity": commodity,
+    })
+    assert declared.status_code == 201
+    closed = client.post(f"/api/energy/meters/{declared.json()['id']}/close", headers={"X-CSRF-Token": _CSRF}, json={
+        "ended_at": (started + timedelta(hours=1)).isoformat(),
+    })
+    assert closed.status_code == 200
+    assert len(calls) == 2 and all(call[2] is False for call in calls)
+
+
+def test_close_meter_closes_open_bindings_and_enforces_auth_csrf(meters_client):
+    client, engine = meters_client
+    started = datetime.now(UTC) - timedelta(hours=2)
+    _login(client)
+    with patch("app.api.routes.api.meters.recompute_range", return_value=0):
+        declared = client.post("/api/energy/meters", headers={"X-CSRF-Token": _CSRF}, json=_declare_payload(
+            label="closable", started_at=started.isoformat(), reason="initial",
+        ))
+    assert declared.status_code == 201
+    _add_bound_channel(engine, meter_id=declared.json()["id"], started_at=started)
+    boundary = started + timedelta(hours=1)
+    assert client.post(f"/api/energy/meters/{declared.json()['id']}/close", json={"ended_at": boundary.isoformat()}).status_code == 403
+    with patch("app.api.routes.api.meters.recompute_range", return_value=0):
+        closed = client.post(f"/api/energy/meters/{declared.json()['id']}/close", headers={"X-CSRF-Token": _CSRF}, json={
+            "ended_at": boundary.isoformat(),
+        })
+    assert closed.status_code == 200
+    with Session(engine) as session:
+        binding = session.scalar(select(MeterSourceBinding))
+        assert binding is not None and binding.ended_at.replace(tzinfo=UTC) == boundary
+
+
+@pytest.mark.parametrize("commodity,unit", [("electricity", "kWh"), ("heating", "GJ"), ("hot_water", "m³")])
+@pytest.mark.parametrize("operation", ["close", "declare"])
+def test_lifecycle_rejects_retained_closed_binding_beyond_proposed_end(
+    meters_client, mock_publish_discovery, commodity, unit, operation,
+):
+    """Close and declare fail closed before recompute or HA for every commodity."""
+    client, engine = meters_client
+    _login(client)
+    start = datetime.now(UTC) - timedelta(hours=3)
+    boundary = start + timedelta(hours=1)
+    retained_end = start + timedelta(hours=2)
+    declared = client.post(
+        "/api/energy/meters", headers={"X-CSRF-Token": _CSRF},
+        json=_declare_payload(label="retained-history", started_at=start.isoformat(), reason="initial", commodity=commodity),
+    )
+    assert declared.status_code == 201
+    meter_id = declared.json()["id"]
+    _add_bound_channel(engine, meter_id=meter_id, started_at=start, ended_at=retained_end, unit=unit)
+    mock_publish_discovery.reset_mock()
+
+    with patch("app.api.routes.api.meters._recompute_commodity", side_effect=AssertionError("must not recompute")):
+        if operation == "close":
+            response = client.post(
+                f"/api/energy/meters/{meter_id}/close", headers={"X-CSRF-Token": _CSRF},
+                json={"ended_at": boundary.isoformat()},
+            )
+        else:
+            response = client.post(
+                "/api/energy/meters", headers={"X-CSRF-Token": _CSRF},
+                json=_declare_payload(
+                    label="replacement", started_at=boundary.isoformat(), reason="meter_swap", commodity=commodity,
+                ),
+            )
+    assert response.status_code == 422
+    assert mock_publish_discovery.call_count == 0
+    with Session(engine) as observer:
+        meter = observer.get(Meter, meter_id)
+        binding = observer.scalar(select(MeterSourceBinding).where(MeterSourceBinding.meter_id == meter_id))
+        assert meter is not None and meter.ended_at is None
+        assert binding is not None and binding.ended_at.replace(tzinfo=UTC) == retained_end
+        assert observer.scalars(select(Meter).where(Meter.commodity == commodity)).all() == [meter]
+
+
+def test_close_flushes_lifecycle_boundary_before_strict_recompute(meters_client):
+    """The strict recompute query observes the just-closed meter and binding."""
+    client, engine = meters_client
+    _login(client)
+    started = datetime.now(UTC) - timedelta(hours=2)
+    boundary = started + timedelta(hours=1)
+    with patch("app.api.routes.api.meters.recompute_range", return_value=0):
+        declared = client.post(
+            "/api/energy/meters", headers={"X-CSRF-Token": _CSRF},
+            json=_declare_payload(label="flush-visible", started_at=started.isoformat(), reason="initial"),
+        )
+    meter_id = declared.json()["id"]
+    _add_bound_channel(engine, meter_id=meter_id, started_at=started)
+
+    def observe(session, *_args, **kwargs):
+        assert kwargs == {"commit": False, "strict": True}
+        observed_meter = session.get(Meter, meter_id)
+        observed_binding = session.scalar(select(MeterSourceBinding))
+        assert observed_meter is not None and observed_meter.ended_at is not None
+        assert observed_binding is not None and observed_binding.ended_at is not None
+        return 0
+
+    with patch("app.api.routes.api.meters.recompute_range", side_effect=observe):
+        response = client.post(
+            f"/api/energy/meters/{meter_id}/close", headers={"X-CSRF-Token": _CSRF},
+            json={"ended_at": boundary.isoformat()},
+        )
+    assert response.status_code == 200
+
+
+def test_close_strict_compute_failure_rolls_back_persisted_lifecycle_state(meters_client, monkeypatch):
+    """A real per-period strict failure rolls back the close in a fresh Session."""
+    from app.services import energy_cost
+
+    client, engine = meters_client
+    _login(client)
+    started = datetime.now(UTC) - timedelta(hours=2)
+    boundary = started + timedelta(hours=1)
+    with patch("app.api.routes.api.meters.recompute_range", return_value=0):
+        declared = client.post(
+            "/api/energy/meters", headers={"X-CSRF-Token": _CSRF},
+            json=_declare_payload(label="strict-rollback", started_at=started.isoformat(), reason="initial"),
+        )
+    meter_id = declared.json()["id"]
+    _add_bound_channel(engine, meter_id=meter_id, started_at=started)
+    monkeypatch.setattr(
+        energy_cost, "compute_period", lambda *_args, **_kwargs: (_ for _ in ()).throw(RuntimeError("strict boom"))
+    )
+
+    with pytest.raises(RuntimeError, match="strict boom"):
+        client.post(
+            f"/api/energy/meters/{meter_id}/close", headers={"X-CSRF-Token": _CSRF},
+            json={"ended_at": boundary.isoformat()},
+        )
+    with Session(engine) as observer:
+        assert observer.get(Meter, meter_id).ended_at is None
+        assert observer.scalar(select(MeterSourceBinding)).ended_at is None
+
+
+def test_close_flush_failure_rolls_back_persisted_lifecycle_state(meters_client, monkeypatch):
+    """The mandatory pre-recompute flush shares the route rollback boundary."""
+    client, engine = meters_client
+    _login(client)
+    started = datetime.now(UTC) - timedelta(hours=2)
+    with patch("app.api.routes.api.meters.recompute_range", return_value=0):
+        declared = client.post(
+            "/api/energy/meters", headers={"X-CSRF-Token": _CSRF},
+            json=_declare_payload(label="flush-rollback", started_at=started.isoformat(), reason="initial"),
+        )
+    meter_id = declared.json()["id"]
+    _add_bound_channel(engine, meter_id=meter_id, started_at=started)
+
+    monkeypatch.setattr(Session, "flush", lambda *_args, **_kwargs: (_ for _ in ()).throw(RuntimeError("flush boom")))
+    with pytest.raises(RuntimeError, match="flush boom"):
+        client.post(
+            f"/api/energy/meters/{meter_id}/close", headers={"X-CSRF-Token": _CSRF},
+            json={"ended_at": (started + timedelta(hours=1)).isoformat()},
+        )
+    monkeypatch.undo()
+    with Session(engine) as observer:
+        assert observer.get(Meter, meter_id).ended_at is None
+        assert observer.scalar(select(MeterSourceBinding)).ended_at is None