From 6561eff5a5928d73d6f39c0d4db97e041106f70f Mon Sep 17 00:00:00 2001 From: Ajay Mudgal Date: Fri, 19 Dec 2025 13:50:27 +0000 Subject: [PATCH 1/3] Sonarcloud fixes --- application/common/dos.py | 2 +- application/common/errors.py | 4 +++ application/common/nhs.py | 2 +- application/common/secretsmanager.py | 6 +++-- application/event_replay/event_replay.py | 14 +++++++--- .../ingest_change_event.py | 27 ++++++++++++++----- application/service_matcher/requirements.txt | 1 - .../service_matcher/service_matcher.py | 7 ++++- .../data_processing/service_histories.py | 4 +-- .../service_histories_change.py | 9 +++---- .../reject_pending_changes/pending_changes.py | 4 +-- .../service_sync/reject_pending_changes/s3.py | 9 ++++++- .../tests/test_pending_changes.py | 6 ++--- application/service_sync/requirements.txt | 1 - application/service_sync/service_sync.py | 6 +++-- infrastructure/stacks/application/lambda.tf | 1 + .../integration/steps/functions/assertions.py | 25 ++++++++++------- .../steps/functions/aws/cloudwatch.py | 6 ++--- test/integration/steps/functions/aws/s3.py | 7 +++-- .../steps/functions/dos/get_data.py | 6 ++--- test/integration/steps/functions/generator.py | 4 +-- test/integration/steps/test_steps.py | 14 +++++----- test/smoke/functions/service.py | 4 +-- test/smoke/test_steps.py | 4 +-- 24 files changed, 111 insertions(+), 62 deletions(-) diff --git a/application/common/dos.py b/application/common/dos.py index ed23e99d8..1b92f4559 100644 --- a/application/common/dos.py +++ b/application/common/dos.py @@ -254,7 +254,7 @@ def db_rows_to_spec_open_times(db_rows: Iterable[dict]) -> list[SpecifiedOpening for date, rows in groupby(date_sorted_rows, lambda row: row["date"]): is_open = True open_periods = [] - for row in list(rows): + for row in rows: if row["isclosed"] is True: is_open = False else: diff --git a/application/common/errors.py b/application/common/errors.py index 3aab94552..070b94aea 100644 --- a/application/common/errors.py +++ b/application/common/errors.py @@ -4,3 +4,7 @@ class ValidationError(Exception): class DynamoDBError(Exception): """Exception raised for all DynamoDB errors.""" + + +class SecretsManagerError(Exception): + """Exception raised for AWS Secrets Manager errors.""" diff --git a/application/common/nhs.py b/application/common/nhs.py index 461b4e097..d0b77b6d4 100644 --- a/application/common/nhs.py +++ b/application/common/nhs.py @@ -165,7 +165,7 @@ def _get_specified_opening_times(self: Self) -> list[SpecifiedOpeningTime]: date = datetime.strptime(date_str, "%b %d %Y").date() is_open = True - for item in list(op_dict_list): + for item in op_dict_list: if item["IsOpen"]: open_periods.append(OpenPeriod.from_string_times(item["OpeningTime"], item["ClosingTime"])) else: diff --git a/application/common/secretsmanager.py b/application/common/secretsmanager.py index eaf9711d9..234f0f46c 100644 --- a/application/common/secretsmanager.py +++ b/application/common/secretsmanager.py @@ -4,6 +4,8 @@ from boto3 import client from botocore.exceptions import ClientError +from .errors import SecretsManagerError + logger = Logger() secrets_manager = client(service_name="secretsmanager") @@ -16,7 +18,7 @@ def get_secret(secret_name: str) -> dict[str, str]: secret_name (str): Secret name to get Raises: - e: ClientError caused by secrets manager + SecretsManagerError: When unable to retrieve secret from AWS Secrets Manager Returns: Dict[str, str]: Secrets as a dictionary @@ -25,6 +27,6 @@ def get_secret(secret_name: str) -> dict[str, str]: secret_value_response = secrets_manager.get_secret_value(SecretId=secret_name) except ClientError as err: msg = f"Failed getting secret '{secret_name}' from secrets manager" - raise Exception(msg) from err # noqa: TRY002 + raise SecretsManagerError(msg) from err secrets_json_str = secret_value_response["SecretString"] return loads(secrets_json_str) diff --git a/application/event_replay/event_replay.py b/application/event_replay/event_replay.py index 62d635358..341cc2eaf 100644 --- a/application/event_replay/event_replay.py +++ b/application/event_replay/event_replay.py @@ -8,10 +8,19 @@ from aws_lambda_powertools.utilities.typing import LambdaContext from boto3 import client from boto3.dynamodb.types import TypeDeserializer +from botocore.config import Config from simplejson import dumps from common.middlewares import unhandled_exception_logging +# Configure boto3 client with explicit timeout to prevent hanging in Lambda +# Timeouts tuned for typical DynamoDB/SQS operations while preventing indefinite hangs +boto_config = Config(connect_timeout=10, read_timeout=15) + +# Create clients at module level for connection reuse across Lambda invocations +dynamodb_client = client("dynamodb", config=boto_config) +sqs_client = client("sqs", config=boto_config) + tracer = Tracer() logger = Logger() @@ -75,7 +84,7 @@ def get_change_event(odscode: str, sequence_number: Decimal) -> dict[str, Any]: Returns: dict[str, Any]: The change event """ - response = client("dynamodb").query( + response = dynamodb_client.query( TableName=getenv("CHANGE_EVENTS_TABLE_NAME"), IndexName="gsi_ods_sequence", ProjectionExpression="Event", @@ -112,11 +121,10 @@ def send_change_event(change_event: dict[str, Any], odscode: str, sequence_numbe sequence_number (int): The sequence number of the change event correlation_id (str): The correlation id of the event replay """ - sqs = client("sqs") queue_url = getenv("CHANGE_EVENT_SQS_URL") logger.info("Sending change event to SQS", queue_url=queue_url) change_event_str = dumps(change_event) - response = sqs.send_message( + response = sqs_client.send_message( QueueUrl=queue_url, MessageBody=change_event_str, MessageGroupId=odscode, diff --git a/application/ingest_change_event/ingest_change_event.py b/application/ingest_change_event/ingest_change_event.py index 029b6edb8..6192d8846 100644 --- a/application/ingest_change_event/ingest_change_event.py +++ b/application/ingest_change_event/ingest_change_event.py @@ -7,6 +7,8 @@ from aws_lambda_powertools.utilities.data_classes import SQSEvent, event_source from aws_lambda_powertools.utilities.typing.lambda_context import LambdaContext from boto3 import client +from botocore.config import Config +from botocore.exceptions import ClientError from .change_event_validation import validate_change_event from common.dynamodb import add_change_event_to_dynamodb, get_latest_sequence_id_for_a_given_odscode_from_dynamodb @@ -14,9 +16,13 @@ from common.types import HoldingQueueChangeEventItem from common.utilities import extract_body, get_sequence_number +# Configure boto3 client with explicit timeout to prevent hanging in Lambda +boto_config = Config(connect_timeout=10, read_timeout=15) + logger = Logger() tracer = Tracer() -sqs = client("sqs") +# Create SQS client at module level for connection reuse across Lambda invocations +sqs_client = client("sqs", config=boto_config) @redact_staff_key_from_event() @@ -82,8 +88,17 @@ def lambda_handler(event: SQSEvent, context: LambdaContext) -> None: # noqa: AR correlation_id=logger.get_correlation_id(), ) logger.debug("Change event validated", holding_queue_change_event_item=holding_queue_change_event_item) - sqs.send_message( - QueueUrl=getenv("HOLDING_QUEUE_URL"), - MessageBody=dumps(holding_queue_change_event_item), - MessageGroupId=ods_code, - ) + try: + sqs_client.send_message( + QueueUrl=getenv("HOLDING_QUEUE_URL"), + MessageBody=dumps(holding_queue_change_event_item), + MessageGroupId=ods_code, + ) + except ClientError as err: + logger.error( + "Failed to send message to holding queue", + error=str(err), + error_code=err.response["Error"]["Code"], + queue_url=getenv("HOLDING_QUEUE_URL"), + ) + raise diff --git a/application/service_matcher/requirements.txt b/application/service_matcher/requirements.txt index 0a79a142c..69d278fcb 100644 --- a/application/service_matcher/requirements.txt +++ b/application/service_matcher/requirements.txt @@ -1,3 +1,2 @@ aws-lambda-powertools[tracer] ~= 3.20.0 psycopg[binary] -pytz diff --git a/application/service_matcher/service_matcher.py b/application/service_matcher/service_matcher.py index 03127f505..71c6b5522 100644 --- a/application/service_matcher/service_matcher.py +++ b/application/service_matcher/service_matcher.py @@ -8,6 +8,7 @@ from aws_lambda_powertools.utilities.data_classes import SQSEvent, event_source from aws_lambda_powertools.utilities.typing.lambda_context import LambdaContext from boto3 import client +from botocore.config import Config from .matching import get_matching_services from .review_matches import review_matches @@ -16,9 +17,13 @@ from common.types import HoldingQueueChangeEventItem, UpdateRequest from common.utilities import extract_body +# Configure boto3 client with explicit timeout to prevent hanging in Lambda +boto_config = Config(connect_timeout=10, read_timeout=15) + logger = Logger() tracer = Tracer() -sqs = client("sqs") +# Create SQS client at module level for connection reuse across Lambda invocations +sqs_client = client("sqs", config=boto_config) @unhandled_exception_logging() diff --git a/application/service_sync/data_processing/service_histories.py b/application/service_sync/data_processing/service_histories.py index a951fdca3..0a82e7d40 100644 --- a/application/service_sync/data_processing/service_histories.py +++ b/application/service_sync/data_processing/service_histories.py @@ -7,7 +7,7 @@ from aws_lambda_powertools.logging import Logger from psycopg import Connection from psycopg.rows import dict_row -from pytz import timezone +from zoneinfo import ZoneInfo from .service_histories_change import ServiceHistoriesChange from common.constants import ( @@ -209,7 +209,7 @@ def save_service_histories(self: Self, connection: Connection) -> None: # Generate the epoch time in seconds rounded down to the nearest second at the time of saving current_epoch_time = str(int(time())) # Get local datetime and format it to DoS date/time format - current_date_time = datetime.now(timezone("Europe/London")).strftime("%Y-%m-%d %H:%M:%S") + current_date_time = datetime.now(ZoneInfo("Europe/London")).strftime("%Y-%m-%d %H:%M:%S") # Rename the new_change key to the current epoch time self.service_history[current_epoch_time] = self.service_history.pop("new_change") # Add the current time to the service_histories json diff --git a/application/service_sync/data_processing/service_histories_change.py b/application/service_sync/data_processing/service_histories_change.py index 12aa79df3..98f64df5b 100644 --- a/application/service_sync/data_processing/service_histories_change.py +++ b/application/service_sync/data_processing/service_histories_change.py @@ -19,19 +19,19 @@ class ServiceHistoriesChange: """A change to be added to the servicehistories table.""" - data: str + data: str | dict previous_value: str change_key: str change_action: str area: str def __init__( - self: Self, data: str, previous_value: str, change_key: str, area: str = DOS_DEMOGRAPHICS_AREA_TYPE + self: Self, data: str | dict, previous_value: str, change_key: str, area: str = DOS_DEMOGRAPHICS_AREA_TYPE ) -> None: """Initialises the ServiceHistoriesChange object. Args: - data (str): The data to be added to the servicehistories table. + data (str | dict): The data to be added to the servicehistories table. previous_value (str): The previous value of the data to be added to the servicehistories table. change_key (str): The change key for the data to be added to the servicehistories table. area (str): The area of the data to be added to the servicehistories table. @@ -76,8 +76,7 @@ def get_sgsd_change_action(self: Self) -> str: Returns: str: Change action - add, delete """ - new_value: dict[list[str]] = self.data - value = next(iter(new_value.keys())) + value = next(iter(self.data.keys())) return "add" if value == "add" else "delete" def get_opening_times_change_action(self: Self) -> str: diff --git a/application/service_sync/reject_pending_changes/pending_changes.py b/application/service_sync/reject_pending_changes/pending_changes.py index cedc84b15..3590054e8 100644 --- a/application/service_sync/reject_pending_changes/pending_changes.py +++ b/application/service_sync/reject_pending_changes/pending_changes.py @@ -9,7 +9,7 @@ from boto3 import client from psycopg import Connection from psycopg.rows import DictRow -from pytz import timezone +from zoneinfo import ZoneInfo from ..service_update_logger import ServiceUpdateLogger from .s3 import put_content_to_s3 @@ -153,7 +153,7 @@ def reject_pending_changes(connection: Connection, pending_changes: list[Pending ) query_vars = { "USER_NAME": DOS_INTEGRATION_USER_NAME, - "TIMESTAMP": datetime.now(timezone("Europe/London")), + "TIMESTAMP": datetime.now(ZoneInfo("Europe/London")), } cursor = query_dos_db(connection=connection, query=sql_query, query_vars=query_vars) cursor.close() diff --git a/application/service_sync/reject_pending_changes/s3.py b/application/service_sync/reject_pending_changes/s3.py index ce314ede5..86de8095b 100644 --- a/application/service_sync/reject_pending_changes/s3.py +++ b/application/service_sync/reject_pending_changes/s3.py @@ -14,5 +14,12 @@ def put_content_to_s3(content: bytes, s3_filename: str) -> None: s3_filename (str): The filename when the file is stored in S3 """ bucket = getenv("SEND_EMAIL_BUCKET_NAME") - client("s3").put_object(Body=content, Bucket=bucket, Key=s3_filename, ServerSideEncryption="AES256") + aws_account_id = getenv("AWS_ACCOUNT_ID") + client("s3").put_object( + Body=content, + Bucket=bucket, + Key=s3_filename, + ServerSideEncryption="AES256", + ExpectedBucketOwner=aws_account_id, + ) logger.info(f"Uploaded to S3 as {s3_filename}", bucket=bucket, s3_filename=s3_filename) diff --git a/application/service_sync/reject_pending_changes/tests/test_pending_changes.py b/application/service_sync/reject_pending_changes/tests/test_pending_changes.py index 71039d4e4..058e02841 100644 --- a/application/service_sync/reject_pending_changes/tests/test_pending_changes.py +++ b/application/service_sync/reject_pending_changes/tests/test_pending_changes.py @@ -4,7 +4,7 @@ from unittest.mock import MagicMock, call, patch import pytest -from pytz import timezone +from zoneinfo import ZoneInfo from application.service_sync.reject_pending_changes.pending_changes import ( PendingChange, @@ -272,7 +272,7 @@ def test_reject_pending_changes_single_rejection(mock_query_dos_db: MagicMock, m response = reject_pending_changes(connection, pending_changes) # Assert assert None is response - mock_datetime.now.assert_called_once_with(timezone("Europe/London")) + mock_datetime.now.assert_called_once_with(ZoneInfo("Europe/London")) mock_query_dos_db.assert_called_once_with( connection=connection, query=( @@ -299,7 +299,7 @@ def test_reject_pending_changes_multiple_rejections(mock_query_dos_db: MagicMock response = reject_pending_changes(connection, pending_changes) # Assert assert None is response - mock_datetime.now.assert_called_once_with(timezone("Europe/London")) + mock_datetime.now.assert_called_once_with(ZoneInfo("Europe/London")) mock_query_dos_db.assert_called_once_with( connection=connection, query=( diff --git a/application/service_sync/requirements.txt b/application/service_sync/requirements.txt index 0a79a142c..69d278fcb 100644 --- a/application/service_sync/requirements.txt +++ b/application/service_sync/requirements.txt @@ -1,3 +1,2 @@ aws-lambda-powertools[tracer] ~= 3.20.0 psycopg[binary] -pytz diff --git a/application/service_sync/service_sync.py b/application/service_sync/service_sync.py index 880603ad0..00ed6097d 100644 --- a/application/service_sync/service_sync.py +++ b/application/service_sync/service_sync.py @@ -8,6 +8,7 @@ from aws_lambda_powertools.utilities.data_classes.sqs_event import SQSRecord from aws_lambda_powertools.utilities.typing import LambdaContext from boto3 import client +from botocore.config import Config from .data_processing.check_for_change import compare_nhs_uk_and_dos_data from .data_processing.get_data import get_dos_service_and_history @@ -20,6 +21,8 @@ tracer = Tracer() logger = Logger() +boto_config = Config(connect_timeout=10, read_timeout=15) +sqs_client = client("sqs", config=boto_config) @tracer.capture_lambda_handler() @@ -82,6 +85,5 @@ def remove_sqs_message_from_queue(receipt_handle: str) -> None: Args: receipt_handle (str): The SQS message receipt handle """ - sqs = client("sqs") - sqs.delete_message(QueueUrl=getenv("UPDATE_REQUEST_QUEUE_URL"), ReceiptHandle=receipt_handle) + sqs_client.delete_message(QueueUrl=getenv("UPDATE_REQUEST_QUEUE_URL"), ReceiptHandle=receipt_handle) logger.info("Removed SQS message from queue", receipt_handle=receipt_handle) diff --git a/infrastructure/stacks/application/lambda.tf b/infrastructure/stacks/application/lambda.tf index d5b3fdbb7..82e64d6e8 100644 --- a/infrastructure/stacks/application/lambda.tf +++ b/infrastructure/stacks/application/lambda.tf @@ -366,6 +366,7 @@ module "service_sync_lambda" { "TEAM_EMAIL_ADDRESS" = local.project_team_email_address "SYSTEM_EMAIL_ADDRESS" = local.project_system_email_address "SEND_EMAIL_LAMBDA_NAME" = var.send_email_lambda + "AWS_ACCOUNT_ID" = tostring(var.aws_account_id) } } diff --git a/test/integration/steps/functions/assertions.py b/test/integration/steps/functions/assertions.py index 4d38b2e68..08d84413a 100644 --- a/test/integration/steps/functions/assertions.py +++ b/test/integration/steps/functions/assertions.py @@ -22,6 +22,21 @@ def assert_standard_closing(dos_times: list, ce_times: list[dict]) -> int: return counter +def _assert_day_opening_times( + entry: dict, currentday: str, dates: dict, change_type: str, strict: bool | None, valid_change_types: list +) -> None: + """Helper function to assert opening times for a single day.""" + assert entry[currentday]["data"]["add"][0] == dates["times"], "ERROR: Dates do not match" + if strict: + assert entry[currentday]["changetype"] == change_type, "ERROR: Incorrect changetype" + else: + assert entry[currentday]["changetype"] in valid_change_types, "ERROR: Incorrect changetype" + if entry[currentday]["changetype"] == "add": + assert "remove" not in entry[currentday]["data"], "ERROR: Remove is present in service history" + elif entry[currentday]["changetype"] == "modify": + assert "remove" in entry[currentday]["data"], f"ERROR: Remove is not present for {currentday}" + + def assert_standard_openings( change_type: str, dos_times: list[dict], @@ -45,14 +60,6 @@ def assert_standard_openings( currentday = next(iter(entry.keys())) for dates in ce_times: if dates["name"] == currentday: - assert entry[currentday]["data"]["add"][0] == dates["times"], "ERROR: Dates do not match" - if strict: - assert entry[currentday]["changetype"] == change_type, "ERROR: Incorrect changetype" - else: - assert entry[currentday]["changetype"] in valid_change_types, "ERROR: Incorrect changetype" - if entry[currentday]["changetype"] == "add": - assert "remove" not in entry[currentday]["data"], "ERROR: Remove is present in service history" - elif entry[currentday]["changetype"] == "modify": - assert "remove" in entry[currentday]["data"], f"ERROR: Remove is not present for {currentday}" + _assert_day_opening_times(entry, currentday, dates, change_type, strict, valid_change_types) counter += 1 return counter diff --git a/test/integration/steps/functions/aws/cloudwatch.py b/test/integration/steps/functions/aws/cloudwatch.py index a18ac31c1..befdf8266 100644 --- a/test/integration/steps/functions/aws/cloudwatch.py +++ b/test/integration/steps/functions/aws/cloudwatch.py @@ -7,7 +7,7 @@ from boto3 import client from botocore.exceptions import ClientError -from pytz import timezone +from zoneinfo import ZoneInfo LAMBDA_CLIENT_LOGS = client("logs") @@ -40,7 +40,7 @@ def get_logs( start_query_response = LAMBDA_CLIENT_LOGS.start_query( logGroupName=log_group_name, startTime=int(start_time), - endTime=int(datetime.now(timezone("Europe/London")).timestamp()), + endTime=int(datetime.now(ZoneInfo("Europe/London")).timestamp()), queryString=query, ) except ClientError as error: @@ -87,7 +87,7 @@ def negative_log_check(query: str, event_lambda: str, start_time: Timestamp) -> start_query_response = LAMBDA_CLIENT_LOGS.start_query( logGroupName=log_group_name, startTime=int(start_time), - endTime=int(datetime.now(timezone("Europe/London")).timestamp()), + endTime=int(datetime.now(ZoneInfo("Europe/London")).timestamp()), queryString=query, ) except ClientError as error: diff --git a/test/integration/steps/functions/aws/s3.py b/test/integration/steps/functions/aws/s3.py index 1a8bad50d..1bf2bc0a6 100644 --- a/test/integration/steps/functions/aws/s3.py +++ b/test/integration/steps/functions/aws/s3.py @@ -21,11 +21,14 @@ def get_s3_email_file(context: Context) -> Context: sleep(45) email_file_name = "email_file.json" shared_environment = getenv("SHARED_ENVIRONMENT") + aws_account_id = getenv("AWS_ACCOUNT_ID") bucket_name = f"uec-dos-int-{shared_environment}-send-email-bucket" - response = S3_CLIENT.list_objects(Bucket=bucket_name) + response = S3_CLIENT.list_objects(Bucket=bucket_name, ExpectedBucketOwner=aws_account_id) object_key = response["Contents"][-1]["Key"] s3_resource = resource("s3") - s3_resource.meta.client.download_file(bucket_name, object_key, email_file_name) + s3_resource.meta.client.download_file( + bucket_name, object_key, email_file_name, ExtraArgs={"ExpectedBucketOwner": aws_account_id} + ) with open(email_file_name) as email_file: context.other = load(email_file) remove("./email_file.json") diff --git a/test/integration/steps/functions/dos/get_data.py b/test/integration/steps/functions/dos/get_data.py index 55e0764c9..067ba1874 100644 --- a/test/integration/steps/functions/dos/get_data.py +++ b/test/integration/steps/functions/dos/get_data.py @@ -1,10 +1,10 @@ from ast import literal_eval -from datetime import datetime, timedelta +from datetime import UTC, datetime, timedelta from json import loads from time import sleep from typing import Any -from pytz import UTC, timezone +from zoneinfo import ZoneInfo from integration.steps.functions.aws.aws_lambda import invoke_dos_db_handler_lambda @@ -16,7 +16,7 @@ def wait_for_service_update(service_id: str) -> Any: updated_date_time_str: str = get_service_table_field(service_id, "modifiedtime") updated_date_time = datetime.strptime(updated_date_time_str, "%Y-%m-%d %H:%M:%S%z") updated_date_time = updated_date_time.replace(tzinfo=UTC) - two_mins_ago = datetime.now(tz=timezone("Europe/London")) - timedelta(minutes=2) + two_mins_ago = datetime.now(tz=ZoneInfo("Europe/London")) - timedelta(minutes=2) two_mins_ago = two_mins_ago.replace(tzinfo=UTC) if updated_date_time > two_mins_ago: break diff --git a/test/integration/steps/functions/generator.py b/test/integration/steps/functions/generator.py index 9e1e31608..b0125f674 100644 --- a/test/integration/steps/functions/generator.py +++ b/test/integration/steps/functions/generator.py @@ -5,7 +5,7 @@ from re import fullmatch from typing import Any -from pytz import timezone +from zoneinfo import ZoneInfo from .context import Context from .utils import invoke_dos_db_handler_lambda @@ -358,7 +358,7 @@ def build_change_event_opening_times(context: Context) -> list: for days in context.generator_data["standard_openings"] ) if "specified_openings" in context.generator_data: - present = datetime.now(timezone("Europe/London")) + present = datetime.now(ZoneInfo("Europe/London")) opening_times.extend( { "AdditionalOpeningDate": days["date"], diff --git a/test/integration/steps/test_steps.py b/test/integration/steps/test_steps.py index 5fe220266..d1ca62e33 100644 --- a/test/integration/steps/test_steps.py +++ b/test/integration/steps/test_steps.py @@ -10,7 +10,7 @@ from faker import Faker from pytest_bdd import given, scenarios, then, when from pytest_bdd.parsers import parse -from pytz import timezone +from zoneinfo import ZoneInfo from .functions.api import process_payload, process_payload_with_sequence from .functions.assertions import assert_standard_closing, assert_standard_openings @@ -600,7 +600,7 @@ def future_set_specified_opening_date(future_past: str, context: Context) -> Con """ year = 0 if future_past.lower() == "future": - year = dt.now(tz=timezone("Europe/London")).year + 1 + year = dt.now(tz=ZoneInfo("Europe/London")).year + 1 context.change_event["OpeningTimes"].append( { "Weekday": "", @@ -883,7 +883,7 @@ def the_change_event_is_sent_for_processing(context: Context, valid_or_invalid: """ if context.phone is not None or context.website is not None: context.change_event["Contacts"] = build_change_event_contacts(context) - context.start_time = dt.now(tz=timezone("Europe/London")).timestamp() + context.start_time = dt.now(tz=ZoneInfo("Europe/London")).timestamp() context.correlation_id = generate_correlation_id() context.response = process_payload(context, valid_or_invalid == "valid", context.correlation_id) context.sequence_number = context.response.request.headers["sequence-number"] @@ -905,7 +905,7 @@ def the_change_event_is_sent_with_custom_sequence(context: Context, seqid: str) Returns: Context: The context object. """ - context.start_time = dt.now(tz=timezone("Europe/London")).timestamp() + context.start_time = dt.now(tz=ZoneInfo("Europe/London")).timestamp() context.correlation_id = generate_correlation_id() context.response = process_payload_with_sequence(context, context.correlation_id, seqid) context.sequence_number = seqid @@ -927,7 +927,7 @@ def the_change_event_is_sent_with_no_sequence(context: Context) -> Context: Returns: Context: The context object. """ - context.start_time = dt.now(tz=timezone("Europe/London")).timestamp() + context.start_time = dt.now(tz=ZoneInfo("Europe/London")).timestamp() context.correlation_id = generate_correlation_id() context.response = process_payload_with_sequence(context, context.correlation_id, None) return context @@ -947,7 +947,7 @@ def the_change_event_is_sent_with_duplicate_sequence(context: Context) -> Contex Returns: Context: The context object. """ - context.start_time = dt.now(tz=timezone("Europe/London")).timestamp() + context.start_time = dt.now(tz=ZoneInfo("Europe/London")).timestamp() context.correlation_id = generate_correlation_id() context.change_event["Address1"] = "New Test Address Value" seqid = 0 @@ -1836,7 +1836,7 @@ def _(context: Context) -> Context: Returns: Context: The context object. """ - context.start_time = dt.now(tz=timezone("Europe/London")).timestamp() + context.start_time = dt.now(tz=ZoneInfo("Europe/London")).timestamp() context.response = invoke_quality_checker_lambda() return context diff --git a/test/smoke/functions/service.py b/test/smoke/functions/service.py index 07d05f0fd..79ee12a89 100644 --- a/test/smoke/functions/service.py +++ b/test/smoke/functions/service.py @@ -1,9 +1,7 @@ -from datetime import datetime +from datetime import UTC, datetime from json import loads from time import sleep -from pytz import UTC - from .aws import invoke_dos_db_handler_lambda from .change_event import ChangeEvent from .constants import ( diff --git a/test/smoke/test_steps.py b/test/smoke/test_steps.py index 22d92b358..a602ba00a 100644 --- a/test/smoke/test_steps.py +++ b/test/smoke/test_steps.py @@ -4,7 +4,7 @@ import pytest from faker import Faker from pytest_bdd import given, scenarios, then, when -from pytz import timezone +from zoneinfo import ZoneInfo from .functions.change_event import ChangeEvent from .functions.change_event_request import send_change_event @@ -143,7 +143,7 @@ def _(smoke_test_context: SmokeTestContext) -> SmokeTestContext: Returns: SmokeTestContext: The smoke test context """ - smoke_test_context.request_start_time = datetime.now(tz=timezone("Europe/London")) + smoke_test_context.request_start_time = datetime.now(tz=ZoneInfo("Europe/London")) change_event_json = smoke_test_context.updated_service.create_change_event() send_change_event(change_event_json) return smoke_test_context From 7d2f0babcf6c02ae381a644fb762e4b96c228453 Mon Sep 17 00:00:00 2001 From: Ajay Mudgal Date: Mon, 22 Dec 2025 14:21:11 +0000 Subject: [PATCH 2/3] Fixing sonar issues --- .../common/tests/test_secretsmanager.py | 4 +++- .../event_replay/tests/test_event_replay.py | 19 ++++++++----------- .../tests/test_ingest_change_event.py | 10 +++++----- .../service_matcher/service_matcher.py | 2 +- .../tests/test_service_matcher.py | 2 +- .../reject_pending_changes/tests/test_s3.py | 3 +++ .../service_sync/tests/test_service_sync.py | 5 ++--- 7 files changed, 23 insertions(+), 22 deletions(-) diff --git a/application/common/tests/test_secretsmanager.py b/application/common/tests/test_secretsmanager.py index e8be6ef76..757d17488 100644 --- a/application/common/tests/test_secretsmanager.py +++ b/application/common/tests/test_secretsmanager.py @@ -4,6 +4,8 @@ import pytest from moto import mock_aws +from application.common.errors import SecretsManagerError + FILE_PATH = "application.common.secretsmanager" @@ -26,5 +28,5 @@ def test_get_secret() -> None: def test_get_secret_resource_not_found() -> None: from application.common.secretsmanager import get_secret - with pytest.raises(Exception, match="Failed getting secret 'fake_secret_name' from secrets manager"): + with pytest.raises(SecretsManagerError, match="Failed getting secret 'fake_secret_name' from secrets manager"): get_secret("fake_secret_name") diff --git a/application/event_replay/tests/test_event_replay.py b/application/event_replay/tests/test_event_replay.py index 9b045b7d8..b99e15512 100644 --- a/application/event_replay/tests/test_event_replay.py +++ b/application/event_replay/tests/test_event_replay.py @@ -102,7 +102,7 @@ def test_build_correlation_id(mock_time_ns: MagicMock) -> None: assert response == f"{time}-local-replayed-event" -@patch(f"{FILE_PATH}.client") +@patch(f"{FILE_PATH}.dynamodb_client") def test_get_change_event(mock_client: MagicMock, change_event: dict[str, str], event: dict[str, str]) -> None: # Arrange table_name = "my-table" @@ -110,13 +110,12 @@ def test_get_change_event(mock_client: MagicMock, change_event: dict[str, str], environ["AWS_REGION"] = "eu-west-1" serializer = TypeSerializer() - mock_client.return_value.query.return_value = {"Items": [{"Event": serializer.serialize(change_event)}]} + mock_client.query.return_value = {"Items": [{"Event": serializer.serialize(change_event)}]} # Act response = get_change_event(event["odscode"], Decimal(event["sequence_number"])) # Assert assert response == change_event - mock_client.assert_called_with("dynamodb") - mock_client().query.assert_called_with( + mock_client.query.assert_called_with( TableName=table_name, IndexName="gsi_ods_sequence", ProjectionExpression="Event", @@ -130,7 +129,7 @@ def test_get_change_event(mock_client: MagicMock, change_event: dict[str, str], del environ["AWS_REGION"] -@patch(f"{FILE_PATH}.client") +@patch(f"{FILE_PATH}.dynamodb_client") def test_get_change_event_no_change_event_in_dynamodb( mock_client: MagicMock, change_event: dict[str, str], event: dict[str, str] ) -> None: @@ -138,13 +137,12 @@ def test_get_change_event_no_change_event_in_dynamodb( table_name = "my-table" environ["CHANGE_EVENTS_TABLE_NAME"] = table_name environ["AWS_REGION"] = "eu-west-1" - mock_client.return_value.query.return_value = {"Items": []} + mock_client.query.return_value = {"Items": []} # Act with pytest.raises(ValueError, match="No change event found for ods code FXXX1 and sequence number 1"): get_change_event(event["odscode"], Decimal(event["sequence_number"])) # Assert - mock_client.assert_called_with("dynamodb") - mock_client().query.assert_called_with( + mock_client.query.assert_called_with( TableName=table_name, IndexName="gsi_ods_sequence", ProjectionExpression="Event", @@ -158,7 +156,7 @@ def test_get_change_event_no_change_event_in_dynamodb( del environ["AWS_REGION"] -@patch(f"{FILE_PATH}.client") +@patch(f"{FILE_PATH}.sqs_client") def test_send_change_event(mock_client: MagicMock, change_event: dict[str, str], event: dict[str, str]) -> None: # Arrange correlation_id = "CORRELATION_ID" @@ -166,8 +164,7 @@ def test_send_change_event(mock_client: MagicMock, change_event: dict[str, str], # Act send_change_event(change_event, event["odscode"], int(event["sequence_number"]), correlation_id) # Assert - mock_client.assert_called_with("sqs") - mock_client().send_message.assert_called_with( + mock_client.send_message.assert_called_with( QueueUrl=queue_url, MessageBody=dumps(change_event), MessageGroupId=event["odscode"], diff --git a/application/ingest_change_event/tests/test_ingest_change_event.py b/application/ingest_change_event/tests/test_ingest_change_event.py index 98400438a..892b27c24 100644 --- a/application/ingest_change_event/tests/test_ingest_change_event.py +++ b/application/ingest_change_event/tests/test_ingest_change_event.py @@ -12,7 +12,7 @@ FILE_PATH = "application.ingest_change_event.ingest_change_event" -@patch(f"{FILE_PATH}.sqs") +@patch(f"{FILE_PATH}.sqs_client") @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem") @patch(f"{FILE_PATH}.add_change_event_to_dynamodb") @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb") @@ -72,7 +72,7 @@ def test_lambda_handler( del environ["HOLDING_QUEUE_URL"] -@patch(f"{FILE_PATH}.sqs") +@patch(f"{FILE_PATH}.sqs_client") @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem") @patch(f"{FILE_PATH}.add_change_event_to_dynamodb") @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb") @@ -130,7 +130,7 @@ def test_lambda_handler_with_sensitive_staff_key( @patch.object(Logger, "error") -@patch(f"{FILE_PATH}.sqs") +@patch(f"{FILE_PATH}.sqs_client") @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem") @patch(f"{FILE_PATH}.add_change_event_to_dynamodb") @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb") @@ -183,7 +183,7 @@ def test_lambda_handler_no_sequence_number( @patch.object(Logger, "error") -@patch(f"{FILE_PATH}.sqs") +@patch(f"{FILE_PATH}.sqs_client") @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem") @patch(f"{FILE_PATH}.add_change_event_to_dynamodb") @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb") @@ -239,7 +239,7 @@ def test_lambda_handler_less_than_latest_sequence_number( del environ["HOLDING_QUEUE_URL"] -@patch(f"{FILE_PATH}.sqs") +@patch(f"{FILE_PATH}.sqs_client") @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem") @patch(f"{FILE_PATH}.add_change_event_to_dynamodb") @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb") diff --git a/application/service_matcher/service_matcher.py b/application/service_matcher/service_matcher.py index 71c6b5522..97924872e 100644 --- a/application/service_matcher/service_matcher.py +++ b/application/service_matcher/service_matcher.py @@ -120,7 +120,7 @@ def send_update_requests( for i, chunk in enumerate(chunks): # TODO: Handle errors? logger.debug(f"Sending off message chunk {i+1}/{len(chunks)}") - response = sqs.send_message_batch(QueueUrl=environ["UPDATE_REQUEST_QUEUE_URL"], Entries=chunk) + response = sqs_client.send_message_batch(QueueUrl=environ["UPDATE_REQUEST_QUEUE_URL"], Entries=chunk) logger.debug("Sent off message chunk", response=response) logger.warning( "Sent Off Update Request", diff --git a/application/service_matcher/tests/test_service_matcher.py b/application/service_matcher/tests/test_service_matcher.py index 8c88b3175..701421652 100644 --- a/application/service_matcher/tests/test_service_matcher.py +++ b/application/service_matcher/tests/test_service_matcher.py @@ -119,7 +119,7 @@ def test_lambda_handler_should_throw_exception_if_event_records_len_not_eq_one(l del environ["ENV"] -@patch(f"{FILE_PATH}.sqs") +@patch(f"{FILE_PATH}.sqs_client") @patch.object(Logger, "get_correlation_id", return_value="1") @patch.object(Logger, "warning") def test_send_update_requests( diff --git a/application/service_sync/reject_pending_changes/tests/test_s3.py b/application/service_sync/reject_pending_changes/tests/test_s3.py index 8c2624e6a..17c1bccd1 100644 --- a/application/service_sync/reject_pending_changes/tests/test_s3.py +++ b/application/service_sync/reject_pending_changes/tests/test_s3.py @@ -10,6 +10,7 @@ def test_put_content_to_s3(mock_client: MagicMock) -> None: # Arrange environ["SEND_EMAIL_BUCKET_NAME"] = bucket_name = "bucket_name" + environ["AWS_ACCOUNT_ID"] = aws_account_id = "123456789012" s3_filename = "s3_filename" content = b"content" # Act @@ -21,6 +22,8 @@ def test_put_content_to_s3(mock_client: MagicMock) -> None: Bucket=bucket_name, Key=s3_filename, ServerSideEncryption="AES256", + ExpectedBucketOwner=aws_account_id, ) # Cleanup del environ["SEND_EMAIL_BUCKET_NAME"] + del environ["AWS_ACCOUNT_ID"] diff --git a/application/service_sync/tests/test_service_sync.py b/application/service_sync/tests/test_service_sync.py index adfeb786c..f073ff07d 100644 --- a/application/service_sync/tests/test_service_sync.py +++ b/application/service_sync/tests/test_service_sync.py @@ -122,15 +122,14 @@ def test_lambda_handler_exception( @patch.object(Logger, "info") -@patch(f"{FILE_PATH}.client") +@patch(f"{FILE_PATH}.sqs_client") def test_remove_sqs_message_from_queue(mock_client: MagicMock, mock_logger_info: MagicMock) -> None: # Arrange environ["UPDATE_REQUEST_QUEUE_URL"] = update_request_queue_url = "update_request_queue_url" # Act remove_sqs_message_from_queue(receipt_handle=RECEIPT_HANDLE) # Assert - mock_client.assert_called_once_with("sqs") - mock_client.return_value.delete_message.assert_called_once_with( + mock_client.delete_message.assert_called_once_with( QueueUrl=update_request_queue_url, ReceiptHandle=RECEIPT_HANDLE, ) From 7837eb8d54578b218b0c29717acdce6be8a5aefc Mon Sep 17 00:00:00 2001 From: Ajay Mudgal Date: Mon, 22 Dec 2025 15:11:27 +0000 Subject: [PATCH 3/3] Ruff fixes for import ordering issues --- application/ingest_change_event/ingest_change_event.py | 3 +-- application/service_sync/data_processing/service_histories.py | 2 +- .../service_sync/reject_pending_changes/pending_changes.py | 2 +- .../reject_pending_changes/tests/test_pending_changes.py | 2 +- test/integration/steps/functions/aws/cloudwatch.py | 2 +- test/integration/steps/functions/dos/get_data.py | 1 - test/integration/steps/functions/generator.py | 1 - test/integration/steps/test_steps.py | 2 +- test/smoke/test_steps.py | 2 +- 9 files changed, 7 insertions(+), 10 deletions(-) diff --git a/application/ingest_change_event/ingest_change_event.py b/application/ingest_change_event/ingest_change_event.py index 6192d8846..26c0132ce 100644 --- a/application/ingest_change_event/ingest_change_event.py +++ b/application/ingest_change_event/ingest_change_event.py @@ -95,9 +95,8 @@ def lambda_handler(event: SQSEvent, context: LambdaContext) -> None: # noqa: AR MessageGroupId=ods_code, ) except ClientError as err: - logger.error( + logger.exception( "Failed to send message to holding queue", - error=str(err), error_code=err.response["Error"]["Code"], queue_url=getenv("HOLDING_QUEUE_URL"), ) diff --git a/application/service_sync/data_processing/service_histories.py b/application/service_sync/data_processing/service_histories.py index 0a82e7d40..dc3405004 100644 --- a/application/service_sync/data_processing/service_histories.py +++ b/application/service_sync/data_processing/service_histories.py @@ -3,11 +3,11 @@ from json import dumps, loads from time import time from typing import Any, Self +from zoneinfo import ZoneInfo from aws_lambda_powertools.logging import Logger from psycopg import Connection from psycopg.rows import dict_row -from zoneinfo import ZoneInfo from .service_histories_change import ServiceHistoriesChange from common.constants import ( diff --git a/application/service_sync/reject_pending_changes/pending_changes.py b/application/service_sync/reject_pending_changes/pending_changes.py index 3590054e8..fa813f06b 100644 --- a/application/service_sync/reject_pending_changes/pending_changes.py +++ b/application/service_sync/reject_pending_changes/pending_changes.py @@ -4,12 +4,12 @@ from os import environ from time import time_ns from typing import Self +from zoneinfo import ZoneInfo from aws_lambda_powertools.logging import Logger from boto3 import client from psycopg import Connection from psycopg.rows import DictRow -from zoneinfo import ZoneInfo from ..service_update_logger import ServiceUpdateLogger from .s3 import put_content_to_s3 diff --git a/application/service_sync/reject_pending_changes/tests/test_pending_changes.py b/application/service_sync/reject_pending_changes/tests/test_pending_changes.py index 058e02841..8530696c4 100644 --- a/application/service_sync/reject_pending_changes/tests/test_pending_changes.py +++ b/application/service_sync/reject_pending_changes/tests/test_pending_changes.py @@ -2,9 +2,9 @@ from os import environ from random import choices from unittest.mock import MagicMock, call, patch +from zoneinfo import ZoneInfo import pytest -from zoneinfo import ZoneInfo from application.service_sync.reject_pending_changes.pending_changes import ( PendingChange, diff --git a/test/integration/steps/functions/aws/cloudwatch.py b/test/integration/steps/functions/aws/cloudwatch.py index befdf8266..384c6769d 100644 --- a/test/integration/steps/functions/aws/cloudwatch.py +++ b/test/integration/steps/functions/aws/cloudwatch.py @@ -4,10 +4,10 @@ from os import getenv from sqlite3 import Timestamp from time import sleep +from zoneinfo import ZoneInfo from boto3 import client from botocore.exceptions import ClientError -from zoneinfo import ZoneInfo LAMBDA_CLIENT_LOGS = client("logs") diff --git a/test/integration/steps/functions/dos/get_data.py b/test/integration/steps/functions/dos/get_data.py index 067ba1874..fbc7712a8 100644 --- a/test/integration/steps/functions/dos/get_data.py +++ b/test/integration/steps/functions/dos/get_data.py @@ -3,7 +3,6 @@ from json import loads from time import sleep from typing import Any - from zoneinfo import ZoneInfo from integration.steps.functions.aws.aws_lambda import invoke_dos_db_handler_lambda diff --git a/test/integration/steps/functions/generator.py b/test/integration/steps/functions/generator.py index b0125f674..0007d9e3d 100644 --- a/test/integration/steps/functions/generator.py +++ b/test/integration/steps/functions/generator.py @@ -4,7 +4,6 @@ from random import randrange from re import fullmatch from typing import Any - from zoneinfo import ZoneInfo from .context import Context diff --git a/test/integration/steps/test_steps.py b/test/integration/steps/test_steps.py index d1ca62e33..5130cc378 100644 --- a/test/integration/steps/test_steps.py +++ b/test/integration/steps/test_steps.py @@ -6,11 +6,11 @@ from os import environ, getenv from random import randint from time import sleep +from zoneinfo import ZoneInfo from faker import Faker from pytest_bdd import given, scenarios, then, when from pytest_bdd.parsers import parse -from zoneinfo import ZoneInfo from .functions.api import process_payload, process_payload_with_sequence from .functions.assertions import assert_standard_closing, assert_standard_openings diff --git a/test/smoke/test_steps.py b/test/smoke/test_steps.py index a602ba00a..f5f981dbd 100644 --- a/test/smoke/test_steps.py +++ b/test/smoke/test_steps.py @@ -1,10 +1,10 @@ from datetime import datetime from re import sub +from zoneinfo import ZoneInfo import pytest from faker import Faker from pytest_bdd import given, scenarios, then, when -from zoneinfo import ZoneInfo from .functions.change_event import ChangeEvent from .functions.change_event_request import send_change_event