starred/deltaglider
1
0
Fork 0
mirror of https://github.com/beshu-tech/deltaglider.git synced 2026-03-24 18:11:20 +01:00
Code Issues 1 Packages Projects Releases Wiki Activity

refactor: typed result dataclasses, centralized metadata aliases, config extraction

Browse Source 
- Replace dict[str,Any] returns in delete/delete_recursive with DeleteResult
  and RecursiveDeleteResult dataclasses for type safety
- Extract _delete_reference/_delete_delta/_classify_objects_for_deletion
  helper methods from oversized delete methods in service.py
- Centralize metadata key aliases in METADATA_KEY_ALIASES dict with
  resolve_metadata() replacing duplicated _meta_value() lookups
- Add DeltaGliderConfig dataclass with from_env() for centralized config
- Add ObjectKey.full_key property, remove dead _multipart_uploads dict
- Update all consumers (client, CLI, tests) for dataclass access patterns

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Simone Scarduzio
2026-02-06 23:16:57 +01:00
parent 012662c377
commit 87f425734f
10 changed files with 647 additions and 579 deletions
Download Patch File Download Diff File Expand all files Collapse all files

220
tests/integration/test_delete_objects_recursive.py
View File

@@ -6,6 +6,7 @@ from unittest.mock import Mock, patch
import pytest
from deltaglider import create_client
from deltaglider.core.models import DeleteResult, RecursiveDeleteResult
class MockStorage:
@@ -177,14 +178,16 @@ class TestDeleteObjectsRecursiveStatisticsAggregation:
def test_aggregates_deleted_count_from_service_and_single_deletes(self, client):
"""Test that deleted counts are aggregated correctly."""
# Setup: Mock service.delete_recursive to return specific counts
mock_result = {
"deleted_count": 5,
"failed_count": 0,
"deltas_deleted": 2,
"references_deleted": 1,
"direct_deleted": 2,
"other_deleted": 0,
}
mock_result = RecursiveDeleteResult(
bucket="test-bucket",
prefix="test/",
deleted_count=5,
failed_count=0,
deltas_deleted=2,
references_deleted=1,
direct_deleted=2,
other_deleted=0,
)
client.service.delete_recursive = Mock(return_value=mock_result)
# Execute
@@ -204,14 +207,16 @@ class TestDeleteObjectsRecursiveStatisticsAggregation:
client.service.storage.objects["test-bucket/file.txt"] = {"size": 100}
# Mock service.delete_recursive to return additional counts
mock_result = {
"deleted_count": 3,
"failed_count": 0,
"deltas_deleted": 1,
"references_deleted": 0,
"direct_deleted": 2,
"other_deleted": 0,
}
mock_result = RecursiveDeleteResult(
bucket="test-bucket",
prefix="file.txt",
deleted_count=3,
failed_count=0,
deltas_deleted=1,
references_deleted=0,
direct_deleted=2,
other_deleted=0,
)
client.service.delete_recursive = Mock(return_value=mock_result)
# Execute
@@ -245,15 +250,17 @@ class TestDeleteObjectsRecursiveErrorHandling:
def test_service_errors_propagated_in_response(self, client):
"""Test that errors from service.delete_recursive are propagated."""
# Mock service to return errors
mock_result = {
"deleted_count": 2,
"failed_count": 1,
"deltas_deleted": 2,
"references_deleted": 0,
"direct_deleted": 0,
"other_deleted": 0,
"errors": ["Error deleting object1", "Error deleting object2"],
}
mock_result = RecursiveDeleteResult(
bucket="test-bucket",
prefix="test/",
deleted_count=2,
failed_count=1,
deltas_deleted=2,
references_deleted=0,
direct_deleted=0,
other_deleted=0,
errors=["Error deleting object1", "Error deleting object2"],
)
client.service.delete_recursive = Mock(return_value=mock_result)
# Execute
@@ -271,15 +278,17 @@ class TestDeleteObjectsRecursiveErrorHandling:
client.service.storage.objects["test-bucket/file.txt"] = {"size": 100}
# Mock service to also return errors
mock_result = {
"deleted_count": 1,
"failed_count": 1,
"deltas_deleted": 0,
"references_deleted": 0,
"direct_deleted": 0,
"other_deleted": 0,
"errors": ["Service delete error"],
}
mock_result = RecursiveDeleteResult(
bucket="test-bucket",
prefix="file.txt",
deleted_count=1,
failed_count=1,
deltas_deleted=0,
references_deleted=0,
direct_deleted=0,
other_deleted=0,
errors=["Service delete error"],
)
client.service.delete_recursive = Mock(return_value=mock_result)
# Mock delete_with_delta_suffix to raise exception
@@ -302,15 +311,17 @@ class TestDeleteObjectsRecursiveWarningsHandling:
def test_service_warnings_propagated_in_response(self, client):
"""Test that warnings from service.delete_recursive are propagated."""
# Mock service to return warnings
mock_result = {
"deleted_count": 3,
"failed_count": 0,
"deltas_deleted": 2,
"references_deleted": 1,
"direct_deleted": 0,
"other_deleted": 0,
"warnings": ["Reference deleted, 2 dependent deltas invalidated"],
}
mock_result = RecursiveDeleteResult(
bucket="test-bucket",
prefix="test/",
deleted_count=3,
failed_count=0,
deltas_deleted=2,
references_deleted=1,
direct_deleted=0,
other_deleted=0,
warnings=["Reference deleted, 2 dependent deltas invalidated"],
)
client.service.delete_recursive = Mock(return_value=mock_result)
# Execute
@@ -326,25 +337,29 @@ class TestDeleteObjectsRecursiveWarningsHandling:
client.service.storage.objects["test-bucket/ref.bin"] = {"size": 100}
# Mock service
mock_result = {
"deleted_count": 0,
"failed_count": 0,
"deltas_deleted": 0,
"references_deleted": 0,
"direct_deleted": 0,
"other_deleted": 0,
}
mock_result = RecursiveDeleteResult(
bucket="test-bucket",
prefix="ref.bin",
deleted_count=0,
failed_count=0,
deltas_deleted=0,
references_deleted=0,
direct_deleted=0,
other_deleted=0,
)
client.service.delete_recursive = Mock(return_value=mock_result)
# Mock delete_with_delta_suffix to return warnings
with patch("deltaglider.client.delete_with_delta_suffix") as mock_delete:
mock_delete.return_value = (
"ref.bin",
{
"deleted": True,
"type": "reference",
"warnings": ["Warning from single delete"],
},
DeleteResult(
key="ref.bin",
bucket="test-bucket",
deleted=True,
type="reference",
warnings=["Warning from single delete"],
),
)
# Execute
@@ -364,26 +379,29 @@ class TestDeleteObjectsRecursiveSingleDeleteDetails:
client.service.storage.objects["test-bucket/file.txt"] = {"size": 100}
# Mock service
mock_result = {
"deleted_count": 0,
"failed_count": 0,
"deltas_deleted": 0,
"references_deleted": 0,
"direct_deleted": 0,
"other_deleted": 0,
}
mock_result = RecursiveDeleteResult(
bucket="test-bucket",
prefix="file.txt",
deleted_count=0,
failed_count=0,
deltas_deleted=0,
references_deleted=0,
direct_deleted=0,
other_deleted=0,
)
client.service.delete_recursive = Mock(return_value=mock_result)
# Mock delete_with_delta_suffix
with patch("deltaglider.client.delete_with_delta_suffix") as mock_delete:
mock_delete.return_value = (
"file.txt",
{
"deleted": True,
"type": "direct",
"dependent_deltas": 0,
"warnings": [],
},
DeleteResult(
key="file.txt",
bucket="test-bucket",
deleted=True,
type="direct",
dependent_deltas=0,
),
)
# Execute
@@ -412,25 +430,28 @@ class TestDeleteObjectsRecursiveSingleDeleteDetails:
actual_key = "file.zip.delta" if key == "file.zip" else key
return (
actual_key,
{
"deleted": True,
"type": "delta",
"dependent_deltas": 0,
"warnings": [],
},
DeleteResult(
key=actual_key,
bucket=bucket,
deleted=True,
type="delta",
dependent_deltas=0,
),
)
client_delete_helpers.delete_with_delta_suffix = mock_delete
# Mock service
mock_result = {
"deleted_count": 0,
"failed_count": 0,
"deltas_deleted": 0,
"references_deleted": 0,
"direct_deleted": 0,
"other_deleted": 0,
}
mock_result = RecursiveDeleteResult(
bucket="test-bucket",
prefix="file.zip",
deleted_count=0,
failed_count=0,
deltas_deleted=0,
references_deleted=0,
direct_deleted=0,
other_deleted=0,
)
client.service.delete_recursive = Mock(return_value=mock_result)
try:
@@ -479,26 +500,29 @@ class TestDeleteObjectsRecursiveEdgeCases:
client.service.storage.objects["test-bucket/file.txt"] = {"size": 100}
# Mock service
mock_result = {
"deleted_count": 0,
"failed_count": 0,
"deltas_deleted": 0,
"references_deleted": 0,
"direct_deleted": 0,
"other_deleted": 0,
}
mock_result = RecursiveDeleteResult(
bucket="test-bucket",
prefix="file.txt",
deleted_count=0,
failed_count=0,
deltas_deleted=0,
references_deleted=0,
direct_deleted=0,
other_deleted=0,
)
client.service.delete_recursive = Mock(return_value=mock_result)
# Mock delete_with_delta_suffix to return unknown type
with patch("deltaglider.client.delete_with_delta_suffix") as mock_delete:
mock_delete.return_value = (
"file.txt",
{
"deleted": True,
"type": "unknown_type", # Not in single_counts keys
"dependent_deltas": 0,
"warnings": [],
},
DeleteResult(
key="file.txt",
bucket="test-bucket",
deleted=True,
type="unknown_type", # Not in single_counts keys
dependent_deltas=0,
),
)
# Execute

29
tests/integration/test_filtering_and_cleanup.py
View File

@@ -8,6 +8,7 @@ import pytest
from deltaglider.app.cli.main import create_service
from deltaglider.client import DeltaGliderClient
from deltaglider.core import ObjectKey
from deltaglider.core.models import DeleteResult, RecursiveDeleteResult
from deltaglider.ports.storage import ObjectHead
@@ -243,12 +244,12 @@ class TestSingleDeleteCleanup:
result = service.delete(ObjectKey(bucket="test-bucket", key="releases/app.zip.delta"))
# Verify delta was deleted
assert result["deleted"] is True
assert result["type"] == "delta"
assert result.deleted is True
assert result.type == "delta"
# Verify reference.bin cleanup was triggered
assert "cleaned_reference" in result
assert result["cleaned_reference"] == "releases/reference.bin"
assert result.cleaned_reference is not None
assert result.cleaned_reference == "releases/reference.bin"
# Verify both files were deleted
assert mock_storage.delete.call_count == 2
@@ -295,11 +296,11 @@ class TestSingleDeleteCleanup:
result = service.delete(ObjectKey(bucket="test-bucket", key="releases/app-v1.zip.delta"))
# Verify delta was deleted
assert result["deleted"] is True
assert result["type"] == "delta"
assert result.deleted is True
assert result.type == "delta"
# Verify reference.bin was NOT cleaned up
assert "cleaned_reference" not in result
assert result.cleaned_reference is None
# Verify only the delta was deleted, not reference.bin
assert mock_storage.delete.call_count == 1
@@ -342,11 +343,11 @@ class TestSingleDeleteCleanup:
result = service.delete(ObjectKey(bucket="test-bucket", key="releases/app.zip.delta"))
# Verify delta was deleted
assert result["deleted"] is True
assert result["type"] == "delta"
assert result.deleted is True
assert result.type == "delta"
# Verify no reference cleanup (since it didn't exist)
assert "cleaned_reference" not in result
assert result.cleaned_reference is None
# Only delta should be deleted
assert mock_storage.delete.call_count == 1
@@ -395,7 +396,7 @@ class TestSingleDeleteCleanup:
result = service.delete(ObjectKey(bucket="test-bucket", key="releases/1.0/app.zip.delta"))
# Should clean up only 1.0/reference.bin
assert result["cleaned_reference"] == "releases/1.0/reference.bin"
assert result.cleaned_reference == "releases/1.0/reference.bin"
# Verify correct files deleted
delete_calls = [call[0][0] for call in mock_storage.delete.call_args_list]
@@ -436,9 +437,9 @@ class TestRecursiveDeleteCleanup:
result = service.delete_recursive("test-bucket", "data/")
# Should delete both delta and reference
assert result["deleted_count"] == 2
assert result["deltas_deleted"] == 1
assert result["references_deleted"] == 1
assert result.deleted_count == 2
assert result.deltas_deleted == 1
assert result.references_deleted == 1
if __name__ == "__main__":

127
tests/integration/test_recursive_delete_reference_cleanup.py
View File

@@ -5,6 +5,7 @@ from unittest.mock import Mock, patch
import pytest
from deltaglider.app.cli.main import create_service
from deltaglider.core.models import DeleteResult, RecursiveDeleteResult
from deltaglider.ports.storage import ObjectHead
@@ -28,10 +29,10 @@ class TestRecursiveDeleteReferenceCleanup:
result = service.delete_recursive("test-bucket", "nonexistent/")
assert result["deleted_count"] == 0
assert result["failed_count"] == 0
assert isinstance(result["errors"], list)
assert isinstance(result["warnings"], list)
assert result.deleted_count == 0
assert result.failed_count == 0
assert isinstance(result.errors, list)
assert isinstance(result.warnings, list)
def test_delete_recursive_returns_structured_result(self):
"""Test that delete_recursive returns a properly structured result."""
@@ -57,26 +58,22 @@ class TestRecursiveDeleteReferenceCleanup:
result = service.delete_recursive("test-bucket", "test/")
# Verify structure
required_keys = [
"bucket",
"prefix",
"deleted_count",
"failed_count",
"deltas_deleted",
"references_deleted",
"direct_deleted",
"other_deleted",
"errors",
"warnings",
]
for key in required_keys:
assert key in result, f"Missing key: {key}"
# Verify structure - result is a RecursiveDeleteResult dataclass
assert hasattr(result, "bucket")
assert hasattr(result, "prefix")
assert hasattr(result, "deleted_count")
assert hasattr(result, "failed_count")
assert hasattr(result, "deltas_deleted")
assert hasattr(result, "references_deleted")
assert hasattr(result, "direct_deleted")
assert hasattr(result, "other_deleted")
assert hasattr(result, "errors")
assert hasattr(result, "warnings")
assert isinstance(result["deleted_count"], int)
assert isinstance(result["failed_count"], int)
assert isinstance(result["errors"], list)
assert isinstance(result["warnings"], list)
assert isinstance(result.deleted_count, int)
assert isinstance(result.failed_count, int)
assert isinstance(result.errors, list)
assert isinstance(result.warnings, list)
def test_delete_recursive_categorizes_objects_correctly(self):
"""Test that delete_recursive correctly categorizes different object types."""
@@ -117,12 +114,12 @@ class TestRecursiveDeleteReferenceCleanup:
result = service.delete_recursive("test-bucket", "test/")
# Should categorize correctly - the exact categorization depends on implementation
assert result["deltas_deleted"] == 1 # app.zip.delta
assert result["references_deleted"] == 1 # reference.bin
assert result.deltas_deleted == 1 # app.zip.delta
assert result.references_deleted == 1 # reference.bin
# Direct and other files may be categorized differently based on metadata detection
assert result["direct_deleted"] + result["other_deleted"] == 2 # readme.txt + config.json
assert result["deleted_count"] == 4 # total
assert result["failed_count"] == 0
assert result.direct_deleted + result.other_deleted == 2 # readme.txt + config.json
assert result.deleted_count == 4 # total
assert result.failed_count == 0
def test_delete_recursive_handles_storage_errors_gracefully(self):
"""Test that delete_recursive handles individual storage errors gracefully."""
@@ -151,10 +148,10 @@ class TestRecursiveDeleteReferenceCleanup:
result = service.delete_recursive("test-bucket", "test/")
# Should handle partial failure
assert result["deleted_count"] == 1 # good.zip.delta succeeded
assert result["failed_count"] == 1 # bad.zip.delta failed
assert len(result["errors"]) == 1
assert "bad" in result["errors"][0]
assert result.deleted_count == 1 # good.zip.delta succeeded
assert result.failed_count == 1 # bad.zip.delta failed
assert len(result.errors) == 1
assert "bad" in result.errors[0]
def test_affected_deltaspaces_discovery(self):
"""Test that the system discovers affected deltaspaces when deleting deltas."""
@@ -206,8 +203,8 @@ class TestRecursiveDeleteReferenceCleanup:
result = service.delete_recursive("test-bucket", "project/team-a/v1/")
# Should have discovered and evaluated the parent reference
assert result["deleted_count"] >= 1 # At least the delta file
assert result["failed_count"] == 0
assert result.deleted_count >= 1 # At least the delta file
assert result.failed_count == 0
def test_cli_uses_core_service_method(self):
"""Test that CLI rm -r command uses the core service delete_recursive method."""
@@ -222,14 +219,12 @@ class TestRecursiveDeleteReferenceCleanup:
mock_create_service.return_value = mock_service
# Mock successful deletion
mock_service.delete_recursive.return_value = {
"bucket": "test-bucket",
"prefix": "test/",
"deleted_count": 2,
"failed_count": 0,
"warnings": [],
"errors": [],
}
mock_service.delete_recursive.return_value = RecursiveDeleteResult(
bucket="test-bucket",
prefix="test/",
deleted_count=2,
failed_count=0,
)
result = runner.invoke(cli, ["rm", "-r", "s3://test-bucket/test/"])
@@ -294,8 +289,8 @@ class TestRecursiveDeleteReferenceCleanup:
result = service.delete(ObjectKey(bucket="test-bucket", key="test/file.zip.delta"))
assert result["deleted"]
assert result["type"] == "delta"
assert result.deleted
assert result.type == "delta"
def test_reference_cleanup_intelligence_basic(self):
"""Basic test to verify reference cleanup intelligence is working."""
@@ -328,10 +323,10 @@ class TestRecursiveDeleteReferenceCleanup:
result = service.delete_recursive("test-bucket", "simple/")
# Should delete both delta and reference since there are no other dependencies
assert result["deleted_count"] == 2
assert result["deltas_deleted"] == 1
assert result["references_deleted"] == 1
assert result["failed_count"] == 0
assert result.deleted_count == 2
assert result.deltas_deleted == 1
assert result.references_deleted == 1
assert result.failed_count == 0
def test_comprehensive_result_validation(self):
"""Test that all result fields are properly populated."""
@@ -366,31 +361,31 @@ class TestRecursiveDeleteReferenceCleanup:
result = service.delete_recursive("test-bucket", "mixed/")
# Validate all expected fields are present and have correct types
assert isinstance(result["bucket"], str)
assert isinstance(result["prefix"], str)
assert isinstance(result["deleted_count"], int)
assert isinstance(result["failed_count"], int)
assert isinstance(result["deltas_deleted"], int)
assert isinstance(result["references_deleted"], int)
assert isinstance(result["direct_deleted"], int)
assert isinstance(result["other_deleted"], int)
assert isinstance(result["errors"], list)
assert isinstance(result["warnings"], list)
assert isinstance(result.bucket, str)
assert isinstance(result.prefix, str)
assert isinstance(result.deleted_count, int)
assert isinstance(result.failed_count, int)
assert isinstance(result.deltas_deleted, int)
assert isinstance(result.references_deleted, int)
assert isinstance(result.direct_deleted, int)
assert isinstance(result.other_deleted, int)
assert isinstance(result.errors, list)
assert isinstance(result.warnings, list)
# Validate counts add up
total_by_type = (
result["deltas_deleted"]
+ result["references_deleted"]
+ result["direct_deleted"]
+ result["other_deleted"]
result.deltas_deleted
+ result.references_deleted
+ result.direct_deleted
+ result.other_deleted
)
assert result["deleted_count"] == total_by_type
assert result.deleted_count == total_by_type
# Validate specific counts for this scenario
assert result["deltas_deleted"] == 1
assert result["references_deleted"] == 1
assert result.deltas_deleted == 1
assert result.references_deleted == 1
# Direct and other files may be categorized differently
assert result["direct_deleted"] + result["other_deleted"] == 2
assert result.direct_deleted + result.other_deleted == 2
if __name__ == "__main__":