Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 50 additions & 4 deletions samcli/commands/sync/sync_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,48 @@ def update_infra_sync_time(self) -> None:
self.latest_infra_sync_time = datetime.now(timezone.utc)


def _parse_datetime_from_toml(datetime_str: str) -> datetime:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I learned from my past experience is never write a cutsom function around date time. There will always be a conner case you didn't consider before. If this is a valid usecase let's try to find a public lib that does it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking of using epoch time instead of this timezone.

"""
Parse datetime string from TOML file and ensure it's timezone-aware UTC.

Handles three formats:
1. With explicit timezone: "2024-05-08T15:16:43+00:00"
2. Without timezone: "2024-05-08T15:16:43"
3. With Z suffix (Zulu time): "2024-05-08T15:16:43Z"

Python 3.9/3.10 don't support 'Z' suffix in fromisoformat(), so we convert it.

Parameters
----------
datetime_str: str
ISO format datetime string from TOML file

Returns
-------
datetime
Timezone-aware datetime in UTC

Raises
------
ValueError
If datetime_str is not a valid ISO format datetime string
"""
try:
# Handle 'Z' suffix for UTC timezone (Python 3.9/3.10 compatibility)
if datetime_str.endswith("Z"):
datetime_str = datetime_str[:-1] + "+00:00"

parsed_datetime = datetime.fromisoformat(datetime_str)

# Ensure timezone-aware (handles old sync.toml files without timezone)
if parsed_datetime.tzinfo is None:
parsed_datetime = parsed_datetime.replace(tzinfo=timezone.utc)

return parsed_datetime
except (ValueError, AttributeError) as e:
raise ValueError(f"Invalid datetime format in sync.toml: '{datetime_str}'") from e


def _sync_state_to_toml_document(sync_state: SyncState) -> TOMLDocument:
"""
Writes the sync state information to the TOML file.
Expand Down Expand Up @@ -129,9 +171,12 @@ def _toml_document_to_sync_state(toml_document: Dict) -> Optional[SyncState]:
if resource_sync_states_toml_table:
for resource_id in resource_sync_states_toml_table:
resource_sync_state_toml_table = resource_sync_states_toml_table.get(resource_id)
sync_time_str = resource_sync_state_toml_table.get(SYNC_TIME)
# Parse datetime and ensure it's timezone-aware UTC (consistent with how we write)
sync_time = _parse_datetime_from_toml(sync_time_str)
resource_sync_state = ResourceSyncState(
resource_sync_state_toml_table.get(HASH),
datetime.fromisoformat(resource_sync_state_toml_table.get(SYNC_TIME)),
sync_time,
)

# For Nested stack resources, replace "-" with "/"
Expand All @@ -142,9 +187,10 @@ def _toml_document_to_sync_state(toml_document: Dict) -> Optional[SyncState]:
latest_infra_sync_time = None
if sync_state_toml_table:
dependency_layer = sync_state_toml_table.get(DEPENDENCY_LAYER)
latest_infra_sync_time = sync_state_toml_table.get(LATEST_INFRA_SYNC_TIME)
if latest_infra_sync_time:
latest_infra_sync_time = datetime.fromisoformat(str(latest_infra_sync_time))
latest_infra_sync_time_str = sync_state_toml_table.get(LATEST_INFRA_SYNC_TIME)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we sure LATEST_INFRA_SYNC_TIME will always exist?

if latest_infra_sync_time_str:
# Parse datetime and ensure it's timezone-aware UTC (consistent with how we write)
latest_infra_sync_time = _parse_datetime_from_toml(str(latest_infra_sync_time_str))
sync_state = SyncState(dependency_layer, resource_sync_states, latest_infra_sync_time)

return sync_state
Expand Down
175 changes: 174 additions & 1 deletion tests/unit/commands/sync/test_sync_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
)
from samcli.lib.build.build_graph import DEFAULT_DEPENDENCIES_DIR

MOCK_RESOURCE_SYNC_TIME = datetime(2023, 2, 8, 12, 12, 12)
MOCK_RESOURCE_SYNC_TIME = datetime(2023, 2, 8, 12, 12, 12, tzinfo=timezone.utc)
MOCK_INFRA_SYNC_TIME = datetime.now(timezone.utc)


Expand Down Expand Up @@ -288,3 +288,176 @@ def test_sync_context_has_no_previous_state_if_file_doesnt_exist(self, patched_r
self.assertIsNone(self.sync_context._previous_state)
self.assertIsNotNone(self.sync_context._current_state)
patched_rmtree_if_exists.assert_not_called()


class TestTimezoneNaiveDatetimeHandling(TestCase):
"""Tests for timezone-naive datetime handling in sync.toml"""

def test_toml_to_sync_state_with_timezone_naive_timestamps(self):
"""Test that timezone-naive timestamps are converted to UTC timezone-aware"""
# Create TOML with timezone-naive timestamps (missing +00:00)
toml_str = """
[sync_state]
dependency_layer = true
latest_infra_sync_time = "2025-12-03T22:10:11.916279"

[resource_sync_states]

[resource_sync_states.MockResourceId]
hash = "mock-hash"
sync_time = "2025-12-03T22:10:35.345701"
"""
toml_doc = tomlkit.loads(toml_str)
sync_state = _toml_document_to_sync_state(toml_doc)

# Verify latest_infra_sync_time is timezone-aware UTC
self.assertIsNotNone(sync_state.latest_infra_sync_time)
self.assertIsNotNone(sync_state.latest_infra_sync_time.tzinfo)
self.assertEqual(sync_state.latest_infra_sync_time.tzinfo, timezone.utc)

# Verify resource sync_time is timezone-aware UTC
resource_sync_state = sync_state.resource_sync_states.get("MockResourceId")
self.assertIsNotNone(resource_sync_state)
self.assertIsNotNone(resource_sync_state.sync_time.tzinfo)
self.assertEqual(resource_sync_state.sync_time.tzinfo, timezone.utc)

def test_toml_to_sync_state_with_timezone_aware_timestamps(self):
"""Test that timezone-aware timestamps are preserved correctly"""
# Create TOML with timezone-aware timestamps (with +00:00)
toml_str = """
[sync_state]
dependency_layer = true
latest_infra_sync_time = "2025-12-03T22:10:11.916279+00:00"

[resource_sync_states]

[resource_sync_states.MockResourceId]
hash = "mock-hash"
sync_time = "2025-12-03T22:10:35.345701+00:00"
"""
toml_doc = tomlkit.loads(toml_str)
sync_state = _toml_document_to_sync_state(toml_doc)

# Verify latest_infra_sync_time is timezone-aware UTC
self.assertIsNotNone(sync_state.latest_infra_sync_time)
self.assertIsNotNone(sync_state.latest_infra_sync_time.tzinfo)
self.assertEqual(sync_state.latest_infra_sync_time.tzinfo, timezone.utc)

# Verify resource sync_time is timezone-aware UTC
resource_sync_state = sync_state.resource_sync_states.get("MockResourceId")
self.assertIsNotNone(resource_sync_state)
self.assertIsNotNone(resource_sync_state.sync_time.tzinfo)
self.assertEqual(resource_sync_state.sync_time.tzinfo, timezone.utc)

def test_toml_to_sync_state_mixed_timezone_formats(self):
"""Test handling of mixed timezone-aware and timezone-naive timestamps"""
# Create TOML with mixed formats
toml_str = """
[sync_state]
dependency_layer = true
latest_infra_sync_time = "2025-12-03T22:10:11.916279"

[resource_sync_states]

[resource_sync_states.Resource1]
hash = "hash1"
sync_time = "2025-12-03T22:10:35.345701+00:00"

[resource_sync_states.Resource2]
hash = "hash2"
sync_time = "2025-12-03T22:10:40.123456"
"""
toml_doc = tomlkit.loads(toml_str)
sync_state = _toml_document_to_sync_state(toml_doc)

# All timestamps should be timezone-aware UTC
self.assertIsNotNone(sync_state.latest_infra_sync_time.tzinfo)
self.assertEqual(sync_state.latest_infra_sync_time.tzinfo, timezone.utc)

resource1 = sync_state.resource_sync_states.get("Resource1")
self.assertIsNotNone(resource1.sync_time.tzinfo)
self.assertEqual(resource1.sync_time.tzinfo, timezone.utc)

resource2 = sync_state.resource_sync_states.get("Resource2")
self.assertIsNotNone(resource2.sync_time.tzinfo)
self.assertEqual(resource2.sync_time.tzinfo, timezone.utc)

def test_datetime_comparison_after_fix(self):
"""Test that datetime comparison works after loading timezone-naive timestamps"""
# Simulate the bug scenario: load timezone-naive timestamp and compare with current time
toml_str = """
[sync_state]
dependency_layer = true
latest_infra_sync_time = "2025-12-03T22:10:11.916279"

[resource_sync_states]
"""
toml_doc = tomlkit.loads(toml_str)
sync_state = _toml_document_to_sync_state(toml_doc)

# This should not raise TypeError: can't subtract offset-naive and offset-aware datetimes
current_time = datetime.now(timezone.utc)
try:
time_diff = current_time - sync_state.latest_infra_sync_time
# If we get here, the fix worked
self.assertIsNotNone(time_diff)
except TypeError as e:
self.fail(f"DateTime comparison failed with TypeError: {e}")


class TestZSuffixDatetimeHandling(TestCase):
"""Tests for handling 'Z' suffix in datetime strings (Python 3.9/3.10 compatibility)"""

@parameterized.expand(
[
("z_suffix_infra", "2024-05-08T15:16:43Z", {}),
(
"z_suffix_resource",
"2025-12-03T22:10:11+00:00",
{"MockResourceId": ("mock-hash", "2024-05-08T15:16:43Z")},
),
("z_suffix_both", "2024-05-08T15:16:43Z", {"Resource1": ("hash1", "2024-05-08T15:16:43Z")}),
(
"mixed_formats",
"2024-05-08T15:16:43Z",
{"Resource1": ("hash1", "2025-12-03T22:10:35+00:00"), "Resource2": ("hash2", "2024-05-08T15:17:00Z")},
),
(
"nested_resource",
"2024-05-08T15:16:43Z",
{"Parent/Child/MockResourceId": ("nested-hash", "2024-05-08T15:16:43Z")},
),
]
)
def test_z_suffix_datetime_parsing(self, test_name, infra_sync_time, resources):
"""Test that 'Z' suffix timestamps are correctly parsed across all Python versions"""
toml_str = TOML_TEMPLATE.format(dependency_layer="true", latest_infra_sync_time=f'"{infra_sync_time}"')

for resource_id, (resource_hash, resource_sync_time) in resources.items():
toml_str += RESOURCE_SYNC_STATE_TEMPLATE.format(
resource_id_toml=resource_id.replace("/", "-"),
resource_hash=resource_hash,
resource_sync_time=resource_sync_time,
)

toml_doc = tomlkit.loads(toml_str)
sync_state = _toml_document_to_sync_state(toml_doc)

# Verify all timestamps are timezone-aware UTC
self.assertEqual(sync_state.latest_infra_sync_time.tzinfo, timezone.utc)
for resource_id in resources.keys():
self.assertEqual(sync_state.resource_sync_states[resource_id].sync_time.tzinfo, timezone.utc)

# Verify datetime comparison works (the original bug fix)
time_diff = datetime.now(timezone.utc) - sync_state.latest_infra_sync_time
self.assertIsNotNone(time_diff)

def test_invalid_datetime_format_raises_error(self):
"""Test that invalid datetime strings raise ValueError with helpful message"""
toml_str = TOML_TEMPLATE.format(dependency_layer="true", latest_infra_sync_time='"invalid-datetime"')
toml_doc = tomlkit.loads(toml_str)

with self.assertRaises(ValueError) as context:
_toml_document_to_sync_state(toml_doc)

self.assertIn("Invalid datetime format in sync.toml", str(context.exception))
Loading