define MagicPlanConfig class to get environment variables

applications/magic_plan/handler.py

@@ -1,29 +1,37 @@
+import os
 from typing import Any
 
-from backend.app.config import get_settings
+import boto3
+
+from infrastructure.magic_plan.config import MagicPlanConfig
 from infrastructure.magic_plan.magic_plan_client import MagicPlanClient
-from orchestration.magic_plan_orchestrator import MagicPlanService
+from infrastructure.s3.s3_client import S3Client
+from orchestration.magic_plan_orchestrator import MagicPlanOrchestrator
 from applications.magic_plan.magic_plan_trigger_request import MagicPlanTriggerRequest
 from domain.magicplan.models import Plan
-from backend.app.db.models.tasks import SourceEnum
+from utilities.aws_lambda.subtask_handler import subtask_handler
-from backend.utils.subtasks import task_handler
+from utilities.logger import setup_logger
-from utils.logger import setup_logger
 
 logger = setup_logger()
 
 
-@task_handler(task_source="magic_plan", source=SourceEnum.HUBSPOT_DEAL)
+@subtask_handler()
 def handler(body: dict[str, Any], context: Any) -> str:
-    settings = get_settings()
+    config = MagicPlanConfig.from_env(os.environ)
     payload = MagicPlanTriggerRequest.model_validate(body)
     client = MagicPlanClient(
-        customer_id=settings.MAGICPLAN_CUSTOMER_ID,
+        customer_id=config.customer_id,
-        api_key=settings.MAGICPLAN_API_KEY,
+        api_key=config.api_key,
     )
+
+    boto3_client: Any = boto3.client  # type: ignore
+    boto_s3: Any = boto3_client("s3")
+    s3_client = S3Client(
+        boto_s3_client=boto_s3, bucket="retrofit-energy-assessments-dev"
+    )
+
     # TODO: read s3_bucket from env var so staging/prod use the correct bucket
-    plan: Plan = MagicPlanService(
+    plan: Plan = MagicPlanOrchestrator(client, s3_client).run(payload)
-        client, s3_bucket="retrofit-energy-assessments-dev"
-    ).run(payload)
     logger.info("Saved MagicPlan plan uid=%s", plan.uid)
     return plan.uid
 

applications/magic_plan/handler/Dockerfile

@@ -5,7 +5,8 @@ WORKDIR /var/task
 COPY applications/magic_plan/handler/requirements.txt .
 RUN pip install --no-cache-dir -r requirements.txt
 
-COPY utils/ utils/
+# COPY utils/ utils/
+COPY utilities/ utilities/
 COPY backend/ backend/
 COPY applications/ applications/
 COPY domain/ domain/

infrastructure/magic_plan/config.py

@@ -0,0 +1,15 @@
+from dataclasses import dataclass
+from typing import Mapping
+
+
+@dataclass(frozen=True)
+class MagicPlanConfig:
+    customer_id: str
+    api_key: str
+
+    @classmethod
+    def from_env(cls, env: Mapping[str, str]) -> "MagicPlanConfig":
+        return cls(
+            customer_id=env["MAGICPLAN_CUSTOMER_ID"],
+            api_key=env["MAGICPLAN_API_KEY"],
+        )

orchestration/magic_plan_orchestrator.py

@@ -1,34 +1,39 @@
 import gzip
 import json
 from datetime import datetime, timezone
+import os
 from typing import Optional, cast
 
 from domain.magicplan.api.response import MagicPlanPlan, PlanSummary
 from domain.magicplan.mapper import map_plan
 from domain.magicplan.models import Plan
 
-from backend.app.db.connection import db_session
 from backend.app.db.models.uploaded_file import (
     FileSourceEnum,
     FileTypeEnum,
     UploadedFile,
 )
 from applications.magic_plan.address_matcher import find_matching_plan
+from infrastructure.postgres.config import PostgresConfig
+from infrastructure.postgres.engine import make_engine, make_session
+from infrastructure.s3.s3_client import S3Client
 from repositories.magic_plan.magic_plan_postgres_repository import (
     MagicPlanPostgresRepository,
 )
 from infrastructure.magic_plan.magic_plan_client import MagicPlanClient
 from applications.magic_plan.magic_plan_trigger_request import MagicPlanTriggerRequest
-from utils.logger import setup_logger
+from utilities.logger import setup_logger
-from utils.s3 import save_data_to_s3
 
 logger = setup_logger()
 
 
-class MagicPlanService:
+class MagicPlanOrchestrator:
-    def __init__(self, client: MagicPlanClient, s3_bucket: str) -> None:
+    def __init__(
-        self._client = client
+        self, magic_plan_api_client: MagicPlanClient, s3_client: S3Client
-        self._s3_bucket = s3_bucket
+    ) -> None:
+        self._api_client = magic_plan_api_client
+        # self._s3_bucket = s3_bucket
+        self._s3_client = s3_client
 
     def run(self, request: MagicPlanTriggerRequest) -> Plan:
         address = request.address
@@ -37,13 +42,13 @@ class MagicPlanService:
         if uprn is not None:
             logger.info("MagicPlanService.run uprn=%s", uprn)
 
-        plans: list[PlanSummary] = self._client.get_plans()
+        plans: list[PlanSummary] = self._api_client.get_plans()
         matched: Optional[PlanSummary] = find_matching_plan(plans, address)
 
         if matched is None:
             raise ValueError(f"No MagicPlan found for address: {address!r}")
 
-        raw_bytes: bytes = self._client.get_plan_raw(matched.id)
+        raw_bytes: bytes = self._api_client.get_plan_raw(matched.id)
         magic_plan: MagicPlanPlan = MagicPlanPlan.model_validate(
             json.loads(raw_bytes)["data"]
         )
@@ -56,12 +61,14 @@ class MagicPlanService:
             hubspot_deal_id=request.hubspot_deal_id,
         )
 
-        with db_session() as session:
+        engine = make_engine(PostgresConfig.from_env(os.environ))
-            session.add(uploaded_file)
+        session = make_session(engine)
-            session.flush()
+
-            MagicPlanPostgresRepository(session).save(
+        session.add(uploaded_file)
-                plan, cast(int, uploaded_file.id)
+        session.flush()
-            )  # TODO: refactor to use postgres Unit of Work
+        MagicPlanPostgresRepository(session).save(
+            plan, cast(int, uploaded_file.id)
+        )  # TODO: refactor to use postgres Unit of Work
 
         return plan
 
@@ -77,9 +84,11 @@ class MagicPlanService:
             s3_key = f"documents/uprn/{uprn}/magic_plan_{plan_id}.json.gz"
         else:
             s3_key = f"documents/hubspot_deal_id/{hubspot_deal_id}/magic_plan_{plan_id}.json.gz"
-        save_data_to_s3(compressed, self._s3_bucket, s3_key)
+
+        self._s3_client.put_object(s3_key, compressed)
+
         return UploadedFile(
-            s3_file_bucket=self._s3_bucket,
+            s3_file_bucket=self._s3_client.bucket,
             s3_file_key=s3_key,
             s3_upload_timestamp=datetime.now(timezone.utc),
             uprn=int(uprn) if uprn is not None else None,

repositories/magic_plan/magic_plan_postgres_repository.py

@@ -44,7 +44,9 @@ class MagicPlanPostgresRepository(MagicPlanRepository):
             )
             .returning(col(MagicPlanPlanModel.id))
         )
-        return cast(int, self._session.execute(stmt).scalar_one())  # pyright: ignore[reportDeprecated]
+        return cast(
+            int, self._session.execute(stmt).scalar_one()
+        )  # pyright: ignore[reportDeprecated]
 
     def _delete_children(self, plan_id: int) -> None:
         floor_subq = (
@@ -69,7 +71,9 @@ class MagicPlanPostgresRepository(MagicPlanRepository):
         )
         self._session.execute(  # pyright: ignore[reportDeprecated]
             delete(MagicPlanWindowVentilationModel).where(
-                col(MagicPlanWindowVentilationModel.magic_plan_window_id).in_(window_subq)
+                col(MagicPlanWindowVentilationModel.magic_plan_window_id).in_(
+                    window_subq
+                )
             )
         )
         self._session.execute(  # pyright: ignore[reportDeprecated]
@@ -128,9 +132,7 @@ class MagicPlanPostgresRepository(MagicPlanRepository):
     ) -> tuple[list[int], list[int]]:
         all_rooms = [room for floor in floors for room in floor.rooms]
         window_rows: list[dict[str, Any]] = [
-            MagicPlanWindowModel.from_domain(window, room_id).model_dump(
+            MagicPlanWindowModel.from_domain(window, room_id).model_dump(exclude={"id"})
-                exclude={"id"}
-            )
             for room, room_id in zip(all_rooms, room_ids)
             for window in room.windows
         ]

tests/applications/magic_plan/test_magic_plan_handler.py

@@ -9,12 +9,7 @@ from applications.magic_plan.handler import handler
 ADDRESS = "2 Laburnum Way Bromley BR2 8BZ"
 PLAN_UID = "a7285ed1-878d-47eb-8aa6-85ef9e187516"
 
-
+_ENV = {"MAGICPLAN_CUSTOMER_ID": "cust-123", "MAGICPLAN_API_KEY": "key-abc"}
-def _make_settings(**overrides: str) -> MagicMock:
-    settings = MagicMock()
-    settings.MAGICPLAN_CUSTOMER_ID = overrides.get("customer_id", "cust-123")
-    settings.MAGICPLAN_API_KEY = overrides.get("api_key", "key-abc")
-    return settings
 
 
 def _call_handler(body: dict[str, Any]) -> Any:
@@ -29,22 +24,20 @@ def mock_plan() -> MagicMock:
 
 
 @pytest.fixture()
-def mock_service(mock_plan: MagicMock) -> MagicMock:
+def mock_orchestrator(mock_plan: MagicMock) -> MagicMock:
-    service = MagicMock()
+    orchestrator = MagicMock()
-    service.run.return_value = mock_plan
+    orchestrator.run.return_value = mock_plan
-    return service
+    return orchestrator
 
 
 # --- request validation ---
 
 
 def test_handler_raises_on_missing_address(mock_plan: MagicMock) -> None:
-    # Arrange
     body: dict[str, Any] = {}
-    with patch("applications.magic_plan.handler.get_settings", return_value=_make_settings()), \
+    with patch("applications.magic_plan.handler.os.environ", _ENV), \
          patch("applications.magic_plan.handler.MagicPlanClient"), \
-         patch("applications.magic_plan.handler.MagicPlanService"):
+         patch("applications.magic_plan.handler.MagicPlanOrchestrator"):
-        # Act / Assert
         with pytest.raises(ValidationError):
             _call_handler(body)
 
@@ -52,58 +45,47 @@ def test_handler_raises_on_missing_address(mock_plan: MagicMock) -> None:
 # --- client construction ---
 
 
-def test_handler_constructs_client_from_settings(mock_service: MagicMock) -> None:
+def test_handler_constructs_client_from_env(mock_orchestrator: MagicMock) -> None:
-    # Arrange
     body = {"address": ADDRESS, "hubspot_deal_id": "deal-123"}
-    with patch("applications.magic_plan.handler.get_settings", return_value=_make_settings(customer_id="cust-xyz", api_key="key-xyz")), \
+    env = {"MAGICPLAN_CUSTOMER_ID": "cust-xyz", "MAGICPLAN_API_KEY": "key-xyz"}
+    with patch("applications.magic_plan.handler.os.environ", env), \
          patch("applications.magic_plan.handler.MagicPlanClient") as MockClient, \
-         patch("applications.magic_plan.handler.MagicPlanService", return_value=mock_service):
+         patch("applications.magic_plan.handler.MagicPlanOrchestrator", return_value=mock_orchestrator):
-        # Act
         _call_handler(body)
-    # Assert
     MockClient.assert_called_once_with(customer_id="cust-xyz", api_key="key-xyz")
 
 
-# --- service orchestration ---
+# --- orchestrator orchestration ---
 
 
-def test_handler_calls_service_run_with_address(mock_service: MagicMock) -> None:
+def test_handler_calls_orchestrator_run_with_address(mock_orchestrator: MagicMock) -> None:
-    # Arrange
     body = {"address": ADDRESS, "hubspot_deal_id": "deal-123"}
-    with patch("applications.magic_plan.handler.get_settings", return_value=_make_settings()), \
+    with patch("applications.magic_plan.handler.os.environ", _ENV), \
          patch("applications.magic_plan.handler.MagicPlanClient"), \
-         patch("applications.magic_plan.handler.MagicPlanService", return_value=mock_service):
+         patch("applications.magic_plan.handler.MagicPlanOrchestrator", return_value=mock_orchestrator):
-        # Act
         _call_handler(body)
-    # Assert
+    mock_orchestrator.run.assert_called_once()
-    mock_service.run.assert_called_once()
+    request = mock_orchestrator.run.call_args.args[0]
-    request = mock_service.run.call_args.args[0]
     assert request.address == ADDRESS
     assert request.uprn is None
 
 
-def test_handler_passes_uprn_to_service(mock_service: MagicMock) -> None:
+def test_handler_passes_uprn_to_orchestrator(mock_orchestrator: MagicMock) -> None:
-    # Arrange
     body = {"address": ADDRESS, "uprn": "100023336956", "hubspot_deal_id": "deal-123"}
-    with patch("applications.magic_plan.handler.get_settings", return_value=_make_settings()), \
+    with patch("applications.magic_plan.handler.os.environ", _ENV), \
          patch("applications.magic_plan.handler.MagicPlanClient"), \
-         patch("applications.magic_plan.handler.MagicPlanService", return_value=mock_service):
+         patch("applications.magic_plan.handler.MagicPlanOrchestrator", return_value=mock_orchestrator):
-        # Act
         _call_handler(body)
-    # Assert
+    mock_orchestrator.run.assert_called_once()
-    mock_service.run.assert_called_once()
+    request = mock_orchestrator.run.call_args.args[0]
-    request = mock_service.run.call_args.args[0]
     assert request.address == ADDRESS
     assert request.uprn == "100023336956"
 
 
-def test_handler_returns_plan_uid(mock_service: MagicMock) -> None:
+def test_handler_returns_plan_uid(mock_orchestrator: MagicMock) -> None:
-    # Arrange
     body = {"address": ADDRESS, "hubspot_deal_id": "deal-123"}
-    with patch("applications.magic_plan.handler.get_settings", return_value=_make_settings()), \
+    with patch("applications.magic_plan.handler.os.environ", _ENV), \
          patch("applications.magic_plan.handler.MagicPlanClient"), \
-         patch("applications.magic_plan.handler.MagicPlanService", return_value=mock_service):
+         patch("applications.magic_plan.handler.MagicPlanOrchestrator", return_value=mock_orchestrator):
-        # Act
         result = _call_handler(body)
-    # Assert
     assert result == PLAN_UID

tests/infrastructure/magic_plan/test_magic_plan_config.py

@@ -0,0 +1,21 @@
+import pytest
+
+from infrastructure.magic_plan.config import MagicPlanConfig
+
+_ENV = {"MAGICPLAN_CUSTOMER_ID": "cust-123", "MAGICPLAN_API_KEY": "key-abc"}
+
+
+def test_from_env_constructs_config() -> None:
+    config = MagicPlanConfig.from_env(_ENV)
+    assert config.customer_id == "cust-123"
+    assert config.api_key == "key-abc"
+
+
+def test_from_env_raises_on_missing_customer_id() -> None:
+    with pytest.raises(KeyError):
+        MagicPlanConfig.from_env({"MAGICPLAN_API_KEY": "key-abc"})
+
+
+def test_from_env_raises_on_missing_api_key() -> None:
+    with pytest.raises(KeyError):
+        MagicPlanConfig.from_env({"MAGICPLAN_CUSTOMER_ID": "cust-123"})

tests/orchestration/magic_plan/test_magic_plan_orchestrator.py

@@ -14,7 +14,7 @@ from backend.app.db.models.uploaded_file import (
     UploadedFile,
 )
 from infrastructure.magic_plan.magic_plan_client import MagicPlanClient
-from orchestration.magic_plan_orchestrator import MagicPlanService
+from orchestration.magic_plan_orchestrator import MagicPlanOrchestrator
 from applications.magic_plan.magic_plan_trigger_request import MagicPlanTriggerRequest
 
 FIXTURE_DIR = Path(__file__).parents[2] / "magic_plan"
@@ -24,25 +24,19 @@ S3_BUCKET = "test-bucket"
 
 @pytest.fixture(scope="module")
 def domain_plan() -> Plan:
-    data = json.loads(
+    data = json.loads((FIXTURE_DIR / "magicplan_api_plan_response.json").read_text())
-        (FIXTURE_DIR / "magicplan_api_plan_response.json").read_text()
-    )
     return map_plan(MagicPlanPlan.model_validate(data["data"]))
 
 
 @pytest.fixture(scope="module")
 def api_magic_plan() -> MagicPlanPlan:
-    data = json.loads(
+    data = json.loads((FIXTURE_DIR / "magicplan_api_plan_response.json").read_text())
-        (FIXTURE_DIR / "magicplan_api_plan_response.json").read_text()
-    )
     return MagicPlanPlan.model_validate(data["data"])
 
 
 @pytest.fixture(scope="module")
 def plan_summary() -> PlanSummary:
-    data = json.loads(
+    data = json.loads((FIXTURE_DIR / "magicplan_api_plan_response.json").read_text())
-        (FIXTURE_DIR / "magicplan_api_plan_response.json").read_text()
-    )
     return MagicPlanPlan.model_validate(data["data"]).plan
 
 
@@ -55,8 +49,8 @@ def mock_client() -> MagicMock:
     return client
 
 
-def _make_service(mock_client: MagicMock) -> MagicPlanService:
+def _make_service(mock_client: MagicMock) -> MagicPlanOrchestrator:
-    return MagicPlanService(client=mock_client, s3_bucket=S3_BUCKET)
+    return MagicPlanOrchestrator(magic_plan_api_client=mock_client, s3_bucket=S3_BUCKET)
 
 
 def _make_request(
@@ -195,7 +189,9 @@ def test_run_uploads_to_s3_with_uprn_key(
     # Arrange
     mock_client.get_plans.return_value = [plan_summary]
     request = _make_request(uprn="100023336956")
-    service = MagicPlanService(client=mock_client, s3_bucket=S3_BUCKET)
+    service = MagicPlanOrchestrator(
+        magic_plan_api_client=mock_client, s3_bucket=S3_BUCKET
+    )
     with patch(
         "orchestration.magic_plan_orchestrator.find_matching_plan",
         return_value=plan_summary,
@@ -225,7 +221,9 @@ def test_run_uploads_to_s3_with_deal_id_key_when_uprn_absent(
     mock_client.get_plans.return_value = [plan_summary]
     mock_client.get_plan.return_value = api_magic_plan
     request = _make_request(hubspot_deal_id="deal-456", uprn=None)
-    service = MagicPlanService(client=mock_client, s3_bucket=S3_BUCKET)
+    service = MagicPlanOrchestrator(
+        magic_plan_api_client=mock_client, s3_bucket=S3_BUCKET
+    )
     with patch(
         "orchestration.magic_plan_orchestrator.find_matching_plan",
         return_value=plan_summary,
@@ -258,7 +256,9 @@ def test_run_creates_uploaded_file_record(
     mock_client.get_plans.return_value = [plan_summary]
     mock_client.get_plan.return_value = api_magic_plan
     request = _make_request(hubspot_deal_id="deal-789", uprn="100023336956")
-    service = MagicPlanService(client=mock_client, s3_bucket=S3_BUCKET)
+    service = MagicPlanOrchestrator(
+        magic_plan_api_client=mock_client, s3_bucket=S3_BUCKET
+    )
     mock_session = MagicMock()
     with patch(
         "orchestration.magic_plan_orchestrator.find_matching_plan",

utilities/logger.py

@@ -0,0 +1,41 @@
+import logging
+from os import PathLike
+from typing import Optional, Union
+
+
+def setup_logger(
+    log_file: Optional[Union[str, PathLike[str]]] = None,
+    level: int = logging.INFO,
+    overwrite_handler: bool = False,
+) -> logging.Logger:
+    # Create a logger and set the logging level
+    logger = logging.getLogger()
+    logger.setLevel(level)
+
+    # if logger already has handlers, just return it
+    if logger.hasHandlers() and not overwrite_handler:
+        return logger
+
+    # Define the log message format
+    log_format = "%(asctime)s [%(levelname)s] %(message)s"
+    date_format = "%Y-%m-%d %H:%M:%S"
+    formatter = logging.Formatter(log_format, datefmt=date_format)
+
+    # Create a file handler and set the file path and format
+    if log_file:
+        file_handler = logging.FileHandler(log_file)
+        file_handler.setLevel(level)
+        file_handler.setFormatter(formatter)
+        logger.addHandler(file_handler)
+
+    # Create a console handler and set the format
+    console_handler = logging.StreamHandler()
+    console_handler.setLevel(level)
+
+    # Set the formatter for the handlers
+    console_handler.setFormatter(formatter)
+
+    # Add the handlers to the logger
+    logger.addHandler(console_handler)
+
+    return logger