Fix double-counting: let SQL triggers manage ref_count
Previously both Python code AND SQL triggers were incrementing/decrementing ref_count, causing inconsistent values. Now: - New artifacts start with ref_count=0 (triggers increment on tag creation) - Tag INSERT/UPDATE/DELETE triggers handle all ref_count changes - Python code only logs operations, doesn't manipulate ref_count - Delete endpoints rely on cascade + triggers for ref_count management Tested: new uploads, duplicates, tag deletion, package deletion (cascade), project deletion (cascade) - all ref_count values now correct.
This commit is contained in:
@@ -186,17 +186,17 @@ def _create_or_update_tag(
|
||||
)
|
||||
db.add(history)
|
||||
|
||||
# Decrement ref_count on old artifact
|
||||
old_ref_count = _decrement_ref_count(db, old_artifact_id)
|
||||
logger.info(
|
||||
f"Tag '{tag_name}' updated: decremented ref_count on artifact "
|
||||
f"{old_artifact_id[:12]}... to {old_ref_count}"
|
||||
)
|
||||
|
||||
# Update tag to point to new artifact
|
||||
# NOTE: SQL trigger (tags_ref_count_update_trigger) handles ref_count:
|
||||
# - Decrements old artifact's ref_count
|
||||
# - Increments new artifact's ref_count
|
||||
existing_tag.artifact_id = new_artifact_id
|
||||
existing_tag.created_by = user_id
|
||||
|
||||
logger.info(
|
||||
f"Tag '{tag_name}' updated: {old_artifact_id[:12]}... -> {new_artifact_id[:12]}..."
|
||||
)
|
||||
|
||||
return existing_tag, False, old_artifact_id
|
||||
else:
|
||||
# Same artifact, no change needed
|
||||
@@ -488,27 +488,29 @@ def delete_project(
|
||||
if not project:
|
||||
raise HTTPException(status_code=404, detail="Project not found")
|
||||
|
||||
# Get all packages in this project
|
||||
# Get counts for logging
|
||||
packages = db.query(Package).filter(Package.project_id == project.id).all()
|
||||
package_count = len(packages)
|
||||
|
||||
# Get all tags across all packages to decrement their artifact ref_counts
|
||||
artifacts_decremented = {}
|
||||
total_tags = 0
|
||||
|
||||
artifact_ids = set()
|
||||
for package in packages:
|
||||
tags = db.query(Tag).filter(Tag.package_id == package.id).all()
|
||||
total_tags += len(tags)
|
||||
|
||||
for tag in tags:
|
||||
if tag.artifact_id not in artifacts_decremented:
|
||||
new_ref_count = _decrement_ref_count(db, tag.artifact_id)
|
||||
artifacts_decremented[tag.artifact_id] = new_ref_count
|
||||
logger.info(
|
||||
f"Project '{project_name}' deletion: decremented ref_count on artifact "
|
||||
f"{tag.artifact_id[:12]}... to {new_ref_count}"
|
||||
)
|
||||
artifact_ids.add(tag.artifact_id)
|
||||
|
||||
# Audit log
|
||||
logger.info(
|
||||
f"Project '{project_name}' deletion: {package_count} packages, "
|
||||
f"{total_tags} tags affecting {len(artifact_ids)} artifacts"
|
||||
)
|
||||
|
||||
# Delete the project (cascade will delete packages, tags, etc.)
|
||||
# NOTE: SQL triggers (tags_ref_count_delete_trigger) handle ref_count automatically
|
||||
db.delete(project)
|
||||
db.commit()
|
||||
|
||||
# Audit log (after commit)
|
||||
_log_audit(
|
||||
db,
|
||||
action="delete_project",
|
||||
@@ -516,14 +518,11 @@ def delete_project(
|
||||
user_id=user_id,
|
||||
source_ip=request.client.host if request.client else None,
|
||||
details={
|
||||
"packages_deleted": len(packages),
|
||||
"packages_deleted": package_count,
|
||||
"tags_deleted": total_tags,
|
||||
"artifacts_affected": list(artifacts_decremented.keys()),
|
||||
"artifacts_affected": list(artifact_ids),
|
||||
},
|
||||
)
|
||||
|
||||
# Delete the project (cascade will delete packages, tags, etc.)
|
||||
db.delete(project)
|
||||
db.commit()
|
||||
|
||||
return None
|
||||
@@ -868,21 +867,22 @@ def delete_package(
|
||||
if not package:
|
||||
raise HTTPException(status_code=404, detail="Package not found")
|
||||
|
||||
# Get all tags in this package to decrement their artifact ref_counts
|
||||
# Get tags count and affected artifacts for logging
|
||||
tags = db.query(Tag).filter(Tag.package_id == package.id).all()
|
||||
artifact_ids = list(set(tag.artifact_id for tag in tags))
|
||||
tag_count = len(tags)
|
||||
|
||||
# Decrement ref_count for each artifact referenced by tags
|
||||
artifacts_decremented = {}
|
||||
for tag in tags:
|
||||
if tag.artifact_id not in artifacts_decremented:
|
||||
new_ref_count = _decrement_ref_count(db, tag.artifact_id)
|
||||
artifacts_decremented[tag.artifact_id] = new_ref_count
|
||||
logger.info(
|
||||
f"Package '{package_name}' deletion: decremented ref_count on artifact "
|
||||
f"{tag.artifact_id[:12]}... to {new_ref_count}"
|
||||
)
|
||||
logger.info(
|
||||
f"Package '{package_name}' deletion: {tag_count} tags affecting "
|
||||
f"{len(artifact_ids)} artifacts"
|
||||
)
|
||||
|
||||
# Audit log
|
||||
# Delete the package (cascade will delete tags, which triggers ref_count decrements)
|
||||
# NOTE: SQL triggers (tags_ref_count_delete_trigger) handle ref_count automatically
|
||||
db.delete(package)
|
||||
db.commit()
|
||||
|
||||
# Audit log (after commit)
|
||||
_log_audit(
|
||||
db,
|
||||
action="delete_package",
|
||||
@@ -890,13 +890,10 @@ def delete_package(
|
||||
user_id=user_id,
|
||||
source_ip=request.client.host if request.client else None,
|
||||
details={
|
||||
"tags_deleted": len(tags),
|
||||
"artifacts_affected": list(artifacts_decremented.keys()),
|
||||
"tags_deleted": tag_count,
|
||||
"artifacts_affected": artifact_ids,
|
||||
},
|
||||
)
|
||||
|
||||
# Delete the package (cascade will delete tags, uploads, consumers)
|
||||
db.delete(package)
|
||||
db.commit()
|
||||
|
||||
return None
|
||||
@@ -982,8 +979,9 @@ def upload_artifact(
|
||||
.first()
|
||||
)
|
||||
if artifact:
|
||||
# Artifact exists - increment ref_count (already locked)
|
||||
artifact.ref_count += 1
|
||||
# Artifact exists - this is a deduplicated upload
|
||||
# NOTE: ref_count is managed by SQL triggers on tag INSERT/DELETE
|
||||
# We don't manually increment here - the tag creation will trigger the increment
|
||||
deduplicated = True
|
||||
saved_bytes = storage_result.size
|
||||
# Merge metadata if new metadata was extracted
|
||||
@@ -1001,6 +999,9 @@ def upload_artifact(
|
||||
# Refresh to get updated ref_count
|
||||
db.refresh(artifact)
|
||||
else:
|
||||
# Create new artifact with ref_count=0
|
||||
# NOTE: ref_count is managed by SQL triggers on tag INSERT/DELETE
|
||||
# When a tag is created for this artifact, the trigger will increment ref_count
|
||||
artifact = Artifact(
|
||||
id=storage_result.sha256,
|
||||
size=storage_result.size,
|
||||
@@ -1012,6 +1013,7 @@ def upload_artifact(
|
||||
created_by=user_id,
|
||||
s3_key=storage_result.s3_key,
|
||||
artifact_metadata=file_metadata or {},
|
||||
ref_count=0, # Triggers will manage this
|
||||
)
|
||||
db.add(artifact)
|
||||
|
||||
@@ -1910,14 +1912,16 @@ def delete_tag(
|
||||
db.add(history)
|
||||
db.flush() # Flush history before deleting tag (cascade will delete history)
|
||||
|
||||
# Decrement ref_count on artifact
|
||||
new_ref_count = _decrement_ref_count(db, artifact_id)
|
||||
logger.info(
|
||||
f"Tag '{tag_name}' deleted: decremented ref_count on artifact "
|
||||
f"{artifact_id[:12]}... to {new_ref_count}"
|
||||
)
|
||||
# NOTE: ref_count decrement is handled by SQL trigger (tags_ref_count_delete_trigger)
|
||||
# when the tag is deleted below
|
||||
logger.info(f"Tag '{tag_name}' deleted for artifact {artifact_id[:12]}...")
|
||||
|
||||
# Audit log
|
||||
# Delete the tag (SQL trigger will decrement ref_count)
|
||||
db.delete(tag)
|
||||
db.commit()
|
||||
|
||||
# Audit log (after commit so we can query the updated ref_count)
|
||||
artifact = db.query(Artifact).filter(Artifact.id == artifact_id).first()
|
||||
_log_audit(
|
||||
db,
|
||||
action="delete_tag",
|
||||
@@ -1926,13 +1930,10 @@ def delete_tag(
|
||||
source_ip=request.client.host if request.client else None,
|
||||
details={
|
||||
"artifact_id": artifact_id,
|
||||
"new_ref_count": new_ref_count,
|
||||
"ref_count_after": artifact.ref_count if artifact else 0,
|
||||
},
|
||||
)
|
||||
|
||||
# Delete the tag
|
||||
db.delete(tag)
|
||||
db.commit()
|
||||
db.commit() # Commit the audit log
|
||||
|
||||
return None
|
||||
|
||||
|
||||
Reference in New Issue
Block a user