VED-902 Fix timestamp validation for EA and batch (#1072)

Author: dlzhry2nhs

lambdas/filenameprocessor/src/constants.py

@@ -38,6 +38,10 @@
     Exception: 500,
 }
 
+# Filename timestamp constants
+VALID_TIMESTAMP_LENGTH = 17
+VALID_TIMEZONE_OFFSETS = {"00", "01"}
+
 
 class FileStatus(StrEnum):
     """File status constants"""

lambdas/filenameprocessor/src/file_name_processor.py

@@ -6,7 +6,6 @@
 (ODS code has multiple lengths)
 """
 
-import argparse
 from uuid import uuid4
 
 from audit_table import upsert_audit_table
@@ -264,7 +263,6 @@ def handle_extended_attributes_file(
             FileStatus.PROCESSING,
         )
 
-        # TODO: agree the prefix with DPS
         dest_file_key = f"{DPS_DESTINATION_PREFIX}/{file_key}"
         copy_file_to_external_bucket(
             bucket_name,
@@ -339,18 +337,3 @@ def lambda_handler(event: dict, context) -> None:  # pylint: disable=unused-argu
         handle_record(record)
 
     logger.info("Filename processor lambda task completed")
-
-
-def run_local():
-    parser = argparse.ArgumentParser("file_name_processor")
-    parser.add_argument("--bucket", required=True, help="Bucket name.", type=str)
-    parser.add_argument("--key", required=True, help="Object key.", type=str)
-    args = parser.parse_args()
-
-    event = {"Records": [{"s3": {"bucket": {"name": args.bucket}, "object": {"key": args.key}}}]}
-    print(event)
-    print(lambda_handler(event=event, context={}))
-
-
-if __name__ == "__main__":
-    run_local()

lambdas/filenameprocessor/src/file_validation.py

@@ -3,7 +3,14 @@
 from datetime import datetime
 from re import match
 
-from constants import EXTENDED_ATTRIBUTES_FILE_PREFIX, EXTENDED_ATTRIBUTES_VACC_TYPE, VALID_EA_VERSIONS, VALID_VERSIONS
+from constants import (
+    EXTENDED_ATTRIBUTES_FILE_PREFIX,
+    EXTENDED_ATTRIBUTES_VACC_TYPE,
+    VALID_EA_VERSIONS,
+    VALID_TIMESTAMP_LENGTH,
+    VALID_TIMEZONE_OFFSETS,
+    VALID_VERSIONS,
+)
 from elasticache import (
     get_supplier_system_from_cache,
     get_valid_vaccine_types_from_cache,
@@ -20,20 +27,32 @@ def is_file_in_directory_root(file_key: str) -> bool:
 
 def is_valid_datetime(timestamp: str) -> bool:
     """
-    Returns a bool to indicate whether the timestamp is a valid datetime in the format 'YYYYmmddTHHMMSSzz'
-    where 'zz' is a two digit number indicating the timezone
+    Returns a bool to indicate whether the file timestamp matches the expected NHS file submission format:
+    - YYYY = Year
+    - MM = Month (01-12)
+    - DD = Date (01-31)
+    - T = fixed value of “T”
+    - hh = Hours (00-23)
+    - mm = Minutes (00-59)
+    - ss = Seconds (00-59)
+    - time zone offset = 00 for GMT, 01 for BST
+
+    e.g. 20220514T10081501
     """
     # Check that datetime (excluding timezone) is a valid datetime in the expected format.
-    if len(timestamp) < 15:
+    if len(timestamp) != VALID_TIMESTAMP_LENGTH:
         return False
 
-    # Note that any digits after the seconds (i.e. from the 16th character onwards, usually expected to represent
-    # timezone), do not need to be validated
     try:
         datetime.strptime(timestamp[:15], "%Y%m%dT%H%M%S")
     except ValueError:
         return False
 
+    time_zone_offset = timestamp[-2:]
+
+    if time_zone_offset not in VALID_TIMEZONE_OFFSETS:
+        return False
+
     return True
 
 

lambdas/filenameprocessor/tests/test_file_key_validation.py

@@ -45,18 +45,20 @@ def test_is_valid_datetime(self, _):
         """Tests that is_valid_datetime returns True for valid datetimes, and false otherwise"""
         # Test case tuples are structured as (date_time_string, expected_result)
         test_cases = [
-            ("20200101T12345600", True),  # Valid datetime string with timezone
-            ("20200101T123456", True),  # Valid datetime string without timezone
+            ("20200101T12345600", True),  # Valid datetime string with timezone offset "00" GMT
+            ("20200101T12345601", True),  # Valid datetime string with timezone offset "01" BST
+            ("20200101T123456", False),  # Invalid datetime string without timezone
             (
                 "20200101T123456extracharacters",
-                True,
-            ),  # Valid datetime string with additional characters
+                False,
+            ),  # Invalid datetime string with additional characters
             ("20201301T12345600", False),  # Invalid month
             ("20200100T12345600", False),  # Invalid day
             ("20200230T12345600", False),  # Invalid combination of month and day
             ("20200101T24345600", False),  # Invalid hours
             ("20200101T12605600", False),  # Invalid minutes
             ("20200101T12346000", False),  # Invalid seconds
+            ("20200101T12345609", False),  # Invalid timezone offset
             ("2020010112345600", False),  # Invalid missing the 'T'
             ("20200101T12345", False),  # Invalid string too short
         ]
@@ -124,7 +126,7 @@ def test_validate_extended_attributes_file_key(self, mock_get_redis_client):
             ),
             # Valid extended attributes file key with different organization code
             (
-                "Vaccination_Extended_Attributes_v1_5_YGM41_20221231T23595999.csv",
+                "Vaccination_Extended_Attributes_v1_5_YGM41_20221231T23595900.csv",
                 "YGM41",
             ),
         ]

lambdas/filenameprocessor/tests/test_lambda_handler.py

@@ -1,7 +1,6 @@
 """Tests for lambda_handler"""
 
 import json
-import sys
 from contextlib import ExitStack
 from copy import deepcopy
 from json import loads as json_loads
@@ -778,37 +777,3 @@ def test_unexpected_bucket_name_and_filename_validation_fails(self):
             self.assertIn("Unable to process file", args[0])
             self.assertIn(invalid_file_key, args)
             self.assertIn("unknown-bucket", args)
-
-
-class TestMainEntryPoint(TestCase):
-    def test_run_local_constructs_event_and_calls_lambda_handler(self):
-        test_args = [
-            "file_name_processor.py",
-            "--bucket",
-            "test-bucket",
-            "--key",
-            "some/path/file.csv",
-        ]
-
-        expected_event = {
-            "Records": [
-                {
-                    "s3": {
-                        "bucket": {"name": "test-bucket"},
-                        "object": {"key": "some/path/file.csv"},
-                    }
-                }
-            ]
-        }
-
-        with (
-            patch.object(sys, "argv", test_args),
-            patch("file_name_processor.lambda_handler") as mock_lambda_handler,
-            patch("file_name_processor.print") as mock_print,
-        ):
-            import file_name_processor
-
-            file_name_processor.run_local()
-
-            mock_lambda_handler.assert_called_once_with(event=expected_event, context={})
-            mock_print.assert_called()