From 4c2e21295f62c62859146f804df73242a4b9309b Mon Sep 17 00:00:00 2001 From: Mondo Diaz Date: Mon, 5 Jan 2026 15:19:05 -0600 Subject: [PATCH] Add comprehensive ref_count tests and fix resumable upload double-counting bug - Add tests for cascade deletion ref_count (package/project delete) - Add tests for tag update ref_count adjustments - Fix resumable upload bug where ref_count was incremented manually AND by SQL trigger - ref_count is now exclusively managed by SQL triggers on tag INSERT/DELETE/UPDATE --- CHANGELOG.md | 3 + backend/app/routes.py | 8 +- backend/tests/test_ref_count.py | 282 ++++++++++++++++++++++++++++++++ 3 files changed, 291 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8eb74e8..531822f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,10 +32,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added integration tests for upload scenarios and ref_count management (#35) - Added integration tests for S3 verification and failure cleanup (#35) - Added integration tests for all stats endpoints (#35) +- Added integration tests for cascade deletion ref_count behavior (package/project delete) (#35) +- Added integration tests for tag update ref_count adjustments (#35) - Added test dependencies to requirements.txt (pytest, pytest-asyncio, pytest-cov, httpx, moto) (#35) ### Fixed - Fixed Helm chart `minio.ingress` conflicting with Bitnami MinIO subchart by renaming to `minioIngress` (#48) - Fixed JSON report serialization error for Decimal types in `GET /api/v1/stats/report` (#34) +- Fixed resumable upload double-counting ref_count when tag provided (removed manual increment, SQL triggers handle it) (#35) ## [0.3.0] - 2025-12-15 ### Changed diff --git a/backend/app/routes.py b/backend/app/routes.py index a962ff6..dfb6b3d 100644 --- a/backend/app/routes.py +++ b/backend/app/routes.py @@ -1167,8 +1167,12 @@ def init_resumable_upload( db.query(Artifact).filter(Artifact.id == init_request.expected_hash).first() ) if existing_artifact: - # File already exists - use atomic increment for ref count - _increment_ref_count(db, existing_artifact.id) + # File already exists - deduplicated upload + # NOTE: ref_count is managed by SQL triggers on tag INSERT/DELETE/UPDATE + # We do NOT manually increment here because: + # 1. If a tag is provided, _create_or_update_tag will create/update a tag + # and the SQL trigger will handle ref_count + # 2. If no tag is provided, ref_count shouldn't change (no new reference) # Record the upload upload = Upload( diff --git a/backend/tests/test_ref_count.py b/backend/tests/test_ref_count.py index 1104de9..6a59995 100644 --- a/backend/tests/test_ref_count.py +++ b/backend/tests/test_ref_count.py @@ -174,3 +174,285 @@ class TestRefCountWithDeletion: # Verify ref_count is 0 response = integration_client.get(f"/api/v1/artifact/{expected_hash}") assert response.json()["ref_count"] == 0 + + +class TestRefCountCascadeDelete: + """Tests for ref_count behavior during cascade deletions.""" + + @pytest.mark.integration + def test_ref_count_decrements_on_package_delete( + self, integration_client, unique_test_id + ): + """Test ref_count decrements for all tags when package is deleted.""" + # Create a project and package manually (not using fixtures to control cleanup) + project_name = f"cascade-pkg-{unique_test_id}" + package_name = f"test-pkg-{unique_test_id}" + + # Create project + response = integration_client.post( + "/api/v1/projects", + json={ + "name": project_name, + "description": "Test project", + "is_public": True, + }, + ) + assert response.status_code == 200 + + # Create package + response = integration_client.post( + f"/api/v1/project/{project_name}/packages", + json={"name": package_name, "description": "Test package"}, + ) + assert response.status_code == 200 + + # Upload content with multiple tags + content = f"cascade delete test {unique_test_id}".encode() + expected_hash = compute_sha256(content) + + upload_test_file( + integration_client, project_name, package_name, content, tag="v1" + ) + upload_test_file( + integration_client, project_name, package_name, content, tag="v2" + ) + upload_test_file( + integration_client, project_name, package_name, content, tag="v3" + ) + + # Verify ref_count is 3 + response = integration_client.get(f"/api/v1/artifact/{expected_hash}") + assert response.json()["ref_count"] == 3 + + # Delete the package (should cascade delete all tags and decrement ref_count) + delete_response = integration_client.delete( + f"/api/v1/project/{project_name}/packages/{package_name}" + ) + assert delete_response.status_code == 204 + + # Verify ref_count is 0 (all tags were deleted) + response = integration_client.get(f"/api/v1/artifact/{expected_hash}") + assert response.json()["ref_count"] == 0 + + # Cleanup: delete the project + integration_client.delete(f"/api/v1/projects/{project_name}") + + @pytest.mark.integration + def test_ref_count_decrements_on_project_delete( + self, integration_client, unique_test_id + ): + """Test ref_count decrements for all tags in all packages when project is deleted.""" + # Create a project manually (not using fixtures to control cleanup) + project_name = f"cascade-proj-{unique_test_id}" + package1_name = f"pkg1-{unique_test_id}" + package2_name = f"pkg2-{unique_test_id}" + + # Create project + response = integration_client.post( + "/api/v1/projects", + json={ + "name": project_name, + "description": "Test project", + "is_public": True, + }, + ) + assert response.status_code == 200 + + # Create two packages + for pkg_name in [package1_name, package2_name]: + response = integration_client.post( + f"/api/v1/project/{project_name}/packages", + json={"name": pkg_name, "description": "Test package"}, + ) + assert response.status_code == 200 + + # Upload same content with tags in both packages + content = f"project cascade test {unique_test_id}".encode() + expected_hash = compute_sha256(content) + + upload_test_file( + integration_client, project_name, package1_name, content, tag="v1" + ) + upload_test_file( + integration_client, project_name, package1_name, content, tag="v2" + ) + upload_test_file( + integration_client, project_name, package2_name, content, tag="latest" + ) + upload_test_file( + integration_client, project_name, package2_name, content, tag="stable" + ) + + # Verify ref_count is 4 (2 tags in each of 2 packages) + response = integration_client.get(f"/api/v1/artifact/{expected_hash}") + assert response.json()["ref_count"] == 4 + + # Delete the project (should cascade delete all packages, tags, and decrement ref_count) + delete_response = integration_client.delete(f"/api/v1/projects/{project_name}") + assert delete_response.status_code == 204 + + # Verify ref_count is 0 + response = integration_client.get(f"/api/v1/artifact/{expected_hash}") + assert response.json()["ref_count"] == 0 + + @pytest.mark.integration + def test_shared_artifact_ref_count_partial_decrement( + self, integration_client, unique_test_id + ): + """Test ref_count correctly decrements when artifact is shared across packages.""" + # Create project with two packages + project_name = f"shared-artifact-{unique_test_id}" + package1_name = f"pkg1-{unique_test_id}" + package2_name = f"pkg2-{unique_test_id}" + + # Create project + response = integration_client.post( + "/api/v1/projects", + json={ + "name": project_name, + "description": "Test project", + "is_public": True, + }, + ) + assert response.status_code == 200 + + # Create two packages + for pkg_name in [package1_name, package2_name]: + response = integration_client.post( + f"/api/v1/project/{project_name}/packages", + json={"name": pkg_name, "description": "Test package"}, + ) + assert response.status_code == 200 + + # Upload same content to both packages + content = f"shared artifact {unique_test_id}".encode() + expected_hash = compute_sha256(content) + + upload_test_file( + integration_client, project_name, package1_name, content, tag="v1" + ) + upload_test_file( + integration_client, project_name, package2_name, content, tag="v1" + ) + + # Verify ref_count is 2 + response = integration_client.get(f"/api/v1/artifact/{expected_hash}") + assert response.json()["ref_count"] == 2 + + # Delete only package1 (package2 still references the artifact) + delete_response = integration_client.delete( + f"/api/v1/project/{project_name}/packages/{package1_name}" + ) + assert delete_response.status_code == 204 + + # Verify ref_count is 1 (only package2's tag remains) + response = integration_client.get(f"/api/v1/artifact/{expected_hash}") + assert response.json()["ref_count"] == 1 + + # Cleanup + integration_client.delete(f"/api/v1/projects/{project_name}") + + +class TestRefCountTagUpdate: + """Tests for ref_count behavior when tags are updated to point to different artifacts.""" + + @pytest.mark.integration + def test_ref_count_adjusts_on_tag_update( + self, integration_client, test_package, unique_test_id + ): + """Test ref_count adjusts when a tag is updated to point to a different artifact.""" + project, package = test_package + + # Upload two different artifacts + content1 = f"artifact one {unique_test_id}".encode() + content2 = f"artifact two {unique_test_id}".encode() + hash1 = compute_sha256(content1) + hash2 = compute_sha256(content2) + + # Upload first artifact with tag "latest" + upload_test_file(integration_client, project, package, content1, tag="latest") + + # Verify first artifact has ref_count 1 + response = integration_client.get(f"/api/v1/artifact/{hash1}") + assert response.json()["ref_count"] == 1 + + # Upload second artifact with different tag + upload_test_file(integration_client, project, package, content2, tag="stable") + + # Now update "latest" tag to point to second artifact + # This is done by uploading the same content with the same tag + upload_test_file(integration_client, project, package, content2, tag="latest") + + # Verify first artifact ref_count decreased to 0 (tag moved away) + response = integration_client.get(f"/api/v1/artifact/{hash1}") + assert response.json()["ref_count"] == 0 + + # Verify second artifact ref_count increased to 2 (stable + latest) + response = integration_client.get(f"/api/v1/artifact/{hash2}") + assert response.json()["ref_count"] == 2 + + @pytest.mark.integration + def test_ref_count_unchanged_when_tag_same_artifact( + self, integration_client, test_package, unique_test_id + ): + """Test ref_count doesn't change when tag is 'updated' to same artifact.""" + project, package = test_package + + content = f"same artifact {unique_test_id}".encode() + expected_hash = compute_sha256(content) + + # Upload with tag + upload_test_file(integration_client, project, package, content, tag="v1") + + # Verify ref_count is 1 + response = integration_client.get(f"/api/v1/artifact/{expected_hash}") + assert response.json()["ref_count"] == 1 + + # Upload same content with same tag (no-op) + upload_test_file(integration_client, project, package, content, tag="v1") + + # Verify ref_count is still 1 (no double-counting) + response = integration_client.get(f"/api/v1/artifact/{expected_hash}") + assert response.json()["ref_count"] == 1 + + @pytest.mark.integration + def test_tag_via_post_endpoint_increments_ref_count( + self, integration_client, test_package, unique_test_id + ): + """Test creating tag via POST /tags endpoint increments ref_count.""" + project, package = test_package + + content = f"tag endpoint test {unique_test_id}".encode() + expected_hash = compute_sha256(content) + + # Upload artifact without tag + result = upload_test_file( + integration_client, project, package, content, filename="test.bin", tag=None + ) + artifact_id = result["artifact_id"] + + # Verify ref_count is 0 (no tags yet) + response = integration_client.get(f"/api/v1/artifact/{expected_hash}") + assert response.json()["ref_count"] == 0 + + # Create tag via POST endpoint + tag_response = integration_client.post( + f"/api/v1/project/{project}/{package}/tags", + json={"name": "v1.0.0", "artifact_id": artifact_id}, + ) + assert tag_response.status_code == 200 + + # Verify ref_count is now 1 + response = integration_client.get(f"/api/v1/artifact/{expected_hash}") + assert response.json()["ref_count"] == 1 + + # Create another tag via POST endpoint + tag_response = integration_client.post( + f"/api/v1/project/{project}/{package}/tags", + json={"name": "latest", "artifact_id": artifact_id}, + ) + assert tag_response.status_code == 200 + + # Verify ref_count is now 2 + response = integration_client.get(f"/api/v1/artifact/{expected_hash}") + assert response.json()["ref_count"] == 2