From e84de954fb800c4f0eca7fa8a0b2eaab69a9b4fa Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Fri, 5 Jun 2026 15:46:32 +0000 Subject: [PATCH] define MagicPlanConfig class to get environment variables --- applications/magic_plan/handler.py | 32 +++++---- applications/magic_plan/handler/Dockerfile | 3 +- infrastructure/magic_plan/__init__.py | 0 infrastructure/magic_plan/config.py | 15 ++++ orchestration/magic_plan_orchestrator.py | 43 +++++++----- .../magic_plan_postgres_repository.py | 12 ++-- .../magic_plan/test_magic_plan_handler.py | 68 +++++++------------ tests/infrastructure/magic_plan/__init__.py | 0 .../magic_plan/test_magic_plan_config.py | 21 ++++++ .../test_magic_plan_orchestrator.py | 30 ++++---- utilities/logger.py | 41 +++++++++++ 11 files changed, 172 insertions(+), 93 deletions(-) create mode 100644 infrastructure/magic_plan/__init__.py create mode 100644 infrastructure/magic_plan/config.py create mode 100644 tests/infrastructure/magic_plan/__init__.py create mode 100644 tests/infrastructure/magic_plan/test_magic_plan_config.py create mode 100644 utilities/logger.py diff --git a/applications/magic_plan/handler.py b/applications/magic_plan/handler.py index cc155d87..4dc88c53 100644 --- a/applications/magic_plan/handler.py +++ b/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 backend.utils.subtasks import task_handler -from utils.logger import setup_logger +from utilities.aws_lambda.subtask_handler import subtask_handler +from utilities.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, - api_key=settings.MAGICPLAN_API_KEY, + customer_id=config.customer_id, + 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( - client, s3_bucket="retrofit-energy-assessments-dev" - ).run(payload) + plan: Plan = MagicPlanOrchestrator(client, s3_client).run(payload) logger.info("Saved MagicPlan plan uid=%s", plan.uid) return plan.uid diff --git a/applications/magic_plan/handler/Dockerfile b/applications/magic_plan/handler/Dockerfile index 9baa91af..b26be208 100644 --- a/applications/magic_plan/handler/Dockerfile +++ b/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/ diff --git a/infrastructure/magic_plan/__init__.py b/infrastructure/magic_plan/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/infrastructure/magic_plan/config.py b/infrastructure/magic_plan/config.py new file mode 100644 index 00000000..c9d4b8b8 --- /dev/null +++ b/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"], + ) diff --git a/orchestration/magic_plan_orchestrator.py b/orchestration/magic_plan_orchestrator.py index 2d8e8c6b..a586c3ba 100644 --- a/orchestration/magic_plan_orchestrator.py +++ b/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 utils.s3 import save_data_to_s3 +from utilities.logger import setup_logger logger = setup_logger() -class MagicPlanService: - def __init__(self, client: MagicPlanClient, s3_bucket: str) -> None: - self._client = client - self._s3_bucket = s3_bucket +class MagicPlanOrchestrator: + def __init__( + self, magic_plan_api_client: MagicPlanClient, s3_client: S3Client + ) -> 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: - session.add(uploaded_file) - session.flush() - MagicPlanPostgresRepository(session).save( - plan, cast(int, uploaded_file.id) - ) # TODO: refactor to use postgres Unit of Work + engine = make_engine(PostgresConfig.from_env(os.environ)) + session = make_session(engine) + + session.add(uploaded_file) + session.flush() + 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, diff --git a/repositories/magic_plan/magic_plan_postgres_repository.py b/repositories/magic_plan/magic_plan_postgres_repository.py index ee8d1bc8..baf15e56 100644 --- a/repositories/magic_plan/magic_plan_postgres_repository.py +++ b/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( - exclude={"id"} - ) + MagicPlanWindowModel.from_domain(window, room_id).model_dump(exclude={"id"}) for room, room_id in zip(all_rooms, room_ids) for window in room.windows ] diff --git a/tests/applications/magic_plan/test_magic_plan_handler.py b/tests/applications/magic_plan/test_magic_plan_handler.py index 3f2d8257..e0e6c878 100644 --- a/tests/applications/magic_plan/test_magic_plan_handler.py +++ b/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" - -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 +_ENV = {"MAGICPLAN_CUSTOMER_ID": "cust-123", "MAGICPLAN_API_KEY": "key-abc"} 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: - service = MagicMock() - service.run.return_value = mock_plan - return service +def mock_orchestrator(mock_plan: MagicMock) -> MagicMock: + orchestrator = MagicMock() + orchestrator.run.return_value = mock_plan + 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"): - # Act / Assert + patch("applications.magic_plan.handler.MagicPlanOrchestrator"): 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: - # Arrange +def test_handler_constructs_client_from_env(mock_orchestrator: MagicMock) -> None: 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): - # Act + patch("applications.magic_plan.handler.MagicPlanOrchestrator", return_value=mock_orchestrator): _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: - # Arrange +def test_handler_calls_orchestrator_run_with_address(mock_orchestrator: MagicMock) -> None: 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): - # Act + patch("applications.magic_plan.handler.MagicPlanOrchestrator", return_value=mock_orchestrator): _call_handler(body) - # Assert - mock_service.run.assert_called_once() - request = mock_service.run.call_args.args[0] + mock_orchestrator.run.assert_called_once() + request = mock_orchestrator.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: - # Arrange +def test_handler_passes_uprn_to_orchestrator(mock_orchestrator: MagicMock) -> None: 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): - # Act + patch("applications.magic_plan.handler.MagicPlanOrchestrator", return_value=mock_orchestrator): _call_handler(body) - # Assert - mock_service.run.assert_called_once() - request = mock_service.run.call_args.args[0] + mock_orchestrator.run.assert_called_once() + request = mock_orchestrator.run.call_args.args[0] assert request.address == ADDRESS assert request.uprn == "100023336956" -def test_handler_returns_plan_uid(mock_service: MagicMock) -> None: - # Arrange +def test_handler_returns_plan_uid(mock_orchestrator: MagicMock) -> None: 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): - # Act + patch("applications.magic_plan.handler.MagicPlanOrchestrator", return_value=mock_orchestrator): result = _call_handler(body) - # Assert assert result == PLAN_UID diff --git a/tests/infrastructure/magic_plan/__init__.py b/tests/infrastructure/magic_plan/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/infrastructure/magic_plan/test_magic_plan_config.py b/tests/infrastructure/magic_plan/test_magic_plan_config.py new file mode 100644 index 00000000..cf9848f6 --- /dev/null +++ b/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"}) diff --git a/tests/orchestration/magic_plan/test_magic_plan_orchestrator.py b/tests/orchestration/magic_plan/test_magic_plan_orchestrator.py index 508e1152..15d1a723 100644 --- a/tests/orchestration/magic_plan/test_magic_plan_orchestrator.py +++ b/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( - (FIXTURE_DIR / "magicplan_api_plan_response.json").read_text() - ) + data = json.loads((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( - (FIXTURE_DIR / "magicplan_api_plan_response.json").read_text() - ) + data = json.loads((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( - (FIXTURE_DIR / "magicplan_api_plan_response.json").read_text() - ) + data = json.loads((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: - return MagicPlanService(client=mock_client, s3_bucket=S3_BUCKET) +def _make_service(mock_client: MagicMock) -> MagicPlanOrchestrator: + 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", diff --git a/utilities/logger.py b/utilities/logger.py new file mode 100644 index 00000000..45370d3d --- /dev/null +++ b/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