Add report deduplication and single-report deletion
Agent-Logs-Url: https://github.com/christianlouis/dmarq/sessions/37d68c2c-7cc8-45e4-bac3-e2e6f1611c6a Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
@@ -178,8 +178,19 @@ async def upload_report(file: UploadFile = File(...)):
|
||||
detail=f"Invalid domain in report: {error_msg}",
|
||||
)
|
||||
|
||||
# Store the report
|
||||
# Check for duplicate report before storing
|
||||
store = ReportStore.get_instance()
|
||||
report_id = report.get("report_id", "")
|
||||
if report_id and store.has_report(domain, report_id):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
detail=(
|
||||
f"Report '{report_id}' for domain '{domain}' has already been uploaded. "
|
||||
"Duplicate reports are not stored to keep statistics accurate."
|
||||
),
|
||||
)
|
||||
|
||||
# Store the report
|
||||
store.add_report(report)
|
||||
|
||||
processed_records = report.get("summary", {}).get("total_count", 0)
|
||||
@@ -330,3 +341,36 @@ async def get_domain_reports_paginated(
|
||||
return PaginatedReportResponse(
|
||||
total=total, page=page, page_size=page_size, total_pages=total_pages, reports=report_entries
|
||||
)
|
||||
|
||||
|
||||
class DeleteReportResponse(BaseModel):
|
||||
"""Response model for report deletion"""
|
||||
|
||||
success: bool
|
||||
message: str
|
||||
|
||||
|
||||
@router.delete(
|
||||
"/domain/{domain}/reports/{report_id}",
|
||||
response_model=DeleteReportResponse,
|
||||
)
|
||||
async def delete_report(domain: str, report_id: str):
|
||||
"""
|
||||
Delete a single DMARC report for a domain.
|
||||
|
||||
Removes the report from the store and recomputes all domain statistics so
|
||||
that aggregated numbers remain accurate after deletion.
|
||||
"""
|
||||
store = ReportStore.get_instance()
|
||||
deleted = store.delete_report(domain, report_id)
|
||||
|
||||
if not deleted:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail=f"Report '{report_id}' not found for domain '{domain}'.",
|
||||
)
|
||||
|
||||
return DeleteReportResponse(
|
||||
success=True,
|
||||
message=f"Report '{report_id}' for domain '{domain}' deleted successfully.",
|
||||
)
|
||||
|
||||
@@ -33,6 +33,69 @@ class ReportStore:
|
||||
# Domain -> sources (sending IPs)
|
||||
self.domain_sources: Dict[str, Dict[str, Dict[str, Any]]] = {}
|
||||
|
||||
def has_report(self, domain: str, report_id: str) -> bool:
|
||||
"""
|
||||
Check whether a report with the given report_id already exists for a domain.
|
||||
|
||||
Args:
|
||||
domain: Domain name
|
||||
report_id: Report identifier from the DMARC report metadata
|
||||
|
||||
Returns:
|
||||
True if the report already exists, False otherwise
|
||||
"""
|
||||
return any(
|
||||
r.get("report_id") == report_id for r in self.domain_reports.get(domain, [])
|
||||
)
|
||||
|
||||
def _recompute_domain_stats(self, domain: str) -> None:
|
||||
"""
|
||||
Recompute summary stats and source data for a domain from its current report list.
|
||||
|
||||
Args:
|
||||
domain: Domain name whose stats should be recalculated
|
||||
"""
|
||||
reports = self.domain_reports.get(domain, [])
|
||||
|
||||
summary: Dict[str, Any] = {
|
||||
"total_count": 0,
|
||||
"passed_count": 0,
|
||||
"failed_count": 0,
|
||||
"reports_processed": len(reports),
|
||||
}
|
||||
sources: Dict[str, Dict[str, Any]] = {}
|
||||
|
||||
for report in reports:
|
||||
report_summary = report.get("summary", {})
|
||||
summary["total_count"] += report_summary.get("total_count", 0)
|
||||
summary["passed_count"] += report_summary.get("passed_count", 0)
|
||||
summary["failed_count"] += report_summary.get("failed_count", 0)
|
||||
|
||||
if "policy" in report:
|
||||
summary["policy"] = report["policy"]
|
||||
|
||||
for record in report.get("records", []):
|
||||
source_ip = record.get("source_ip", "unknown")
|
||||
if source_ip not in sources:
|
||||
sources[source_ip] = {
|
||||
"count": 0,
|
||||
"spf_result": "unknown",
|
||||
"dkim_result": "unknown",
|
||||
"disposition": "none",
|
||||
}
|
||||
sources[source_ip]["count"] += record.get("count", 0)
|
||||
sources[source_ip]["spf_result"] = record.get("spf", "unknown")
|
||||
sources[source_ip]["dkim_result"] = record.get("dkim", "unknown")
|
||||
sources[source_ip]["disposition"] = record.get("disposition", "none")
|
||||
|
||||
total = summary["total_count"]
|
||||
summary["compliance_rate"] = (
|
||||
round(summary["passed_count"] / total * 100, 1) if total > 0 else 0
|
||||
)
|
||||
|
||||
self.domain_summary[domain] = summary
|
||||
self.domain_sources[domain] = sources
|
||||
|
||||
def add_report(self, report: Dict[str, Any]) -> None:
|
||||
"""
|
||||
Add a new report to the store
|
||||
@@ -56,47 +119,8 @@ class ReportStore:
|
||||
# Add the new report
|
||||
self.domain_reports[domain].append(report)
|
||||
|
||||
# Update summary stats for this domain
|
||||
summary = report.get("summary", {})
|
||||
self.domain_summary[domain]["total_count"] += summary.get("total_count", 0)
|
||||
self.domain_summary[domain]["passed_count"] += summary.get("passed_count", 0)
|
||||
self.domain_summary[domain]["failed_count"] += summary.get("failed_count", 0)
|
||||
self.domain_summary[domain]["reports_processed"] += 1
|
||||
|
||||
# Set policy from the latest report
|
||||
if "policy" in report:
|
||||
self.domain_summary[domain]["policy"] = report["policy"]
|
||||
|
||||
# Update source data
|
||||
report_records = report.get("records", [])
|
||||
for record in report_records:
|
||||
source_ip = record.get("source_ip", "unknown")
|
||||
if source_ip not in self.domain_sources[domain]:
|
||||
self.domain_sources[domain][source_ip] = {
|
||||
"count": 0,
|
||||
"spf_result": "unknown",
|
||||
"dkim_result": "unknown",
|
||||
"disposition": "none",
|
||||
}
|
||||
|
||||
# Update source counts and results
|
||||
self.domain_sources[domain][source_ip]["count"] += record.get("count", 0)
|
||||
self.domain_sources[domain][source_ip]["spf_result"] = record.get("spf", "unknown")
|
||||
self.domain_sources[domain][source_ip]["dkim_result"] = record.get("dkim", "unknown")
|
||||
self.domain_sources[domain][source_ip]["disposition"] = record.get(
|
||||
"disposition", "none"
|
||||
)
|
||||
|
||||
# Calculate compliance rate (percentage of passing emails)
|
||||
if self.domain_summary[domain]["total_count"] > 0:
|
||||
pass_rate = (
|
||||
self.domain_summary[domain]["passed_count"]
|
||||
/ self.domain_summary[domain]["total_count"]
|
||||
* 100
|
||||
)
|
||||
self.domain_summary[domain]["compliance_rate"] = round(pass_rate, 1)
|
||||
else:
|
||||
self.domain_summary[domain]["compliance_rate"] = 0
|
||||
# Recompute all summary stats from the full list to keep them consistent
|
||||
self._recompute_domain_stats(domain)
|
||||
|
||||
def get_domains(self) -> List[str]:
|
||||
"""
|
||||
@@ -187,6 +211,38 @@ class ReportStore:
|
||||
self.domain_summary = {}
|
||||
self.domain_sources = {}
|
||||
|
||||
def delete_report(self, domain: str, report_id: str) -> bool:
|
||||
"""
|
||||
Delete a single report from the store and recompute domain statistics.
|
||||
|
||||
If the domain has no remaining reports after deletion, the domain entry
|
||||
is removed entirely from all internal data structures.
|
||||
|
||||
Args:
|
||||
domain: Domain name
|
||||
report_id: Report identifier to delete
|
||||
|
||||
Returns:
|
||||
True if the report was found and deleted, False otherwise
|
||||
"""
|
||||
reports = self.domain_reports.get(domain, [])
|
||||
original_len = len(reports)
|
||||
self.domain_reports[domain] = [r for r in reports if r.get("report_id") != report_id]
|
||||
|
||||
if len(self.domain_reports[domain]) == original_len:
|
||||
# Nothing was removed
|
||||
return False
|
||||
|
||||
if not self.domain_reports[domain]:
|
||||
# Domain has no remaining reports – clean up entirely
|
||||
self.domain_reports.pop(domain, None)
|
||||
self.domain_summary.pop(domain, None)
|
||||
self.domain_sources.pop(domain, None)
|
||||
else:
|
||||
self._recompute_domain_stats(domain)
|
||||
|
||||
return True
|
||||
|
||||
def delete_domain_with_cleanup(self, domain: str) -> bool:
|
||||
"""
|
||||
Delete a domain and all its associated data
|
||||
|
||||
@@ -77,3 +77,67 @@ class TestReportStore:
|
||||
def test_delete_nonexistent_domain(self):
|
||||
store = ReportStore.get_instance()
|
||||
assert store.delete_domain_with_cleanup("nope.com") is False
|
||||
|
||||
def test_has_report_returns_true_for_existing(self):
|
||||
store = ReportStore.get_instance()
|
||||
store.add_report(_sample_report("test.com"))
|
||||
assert store.has_report("test.com", "rpt-001") is True
|
||||
|
||||
def test_has_report_returns_false_for_missing_report_id(self):
|
||||
store = ReportStore.get_instance()
|
||||
store.add_report(_sample_report("test.com"))
|
||||
assert store.has_report("test.com", "rpt-999") is False
|
||||
|
||||
def test_has_report_returns_false_for_unknown_domain(self):
|
||||
store = ReportStore.get_instance()
|
||||
assert store.has_report("nobody.com", "rpt-001") is False
|
||||
|
||||
def test_delete_report_removes_report_and_updates_stats(self):
|
||||
store = ReportStore.get_instance()
|
||||
store.add_report(_sample_report("test.com"))
|
||||
|
||||
result = store.delete_report("test.com", "rpt-001")
|
||||
assert result is True
|
||||
# Domain should be gone entirely when no reports remain
|
||||
assert "test.com" not in store.get_domains()
|
||||
|
||||
def test_delete_report_with_remaining_reports_recomputes_stats(self):
|
||||
store = ReportStore.get_instance()
|
||||
report_a = _sample_report("test.com")
|
||||
report_a["report_id"] = "rpt-001"
|
||||
|
||||
report_b = _sample_report("test.com")
|
||||
report_b["report_id"] = "rpt-002"
|
||||
report_b["summary"] = {"total_count": 3, "passed_count": 1, "failed_count": 2}
|
||||
|
||||
store.add_report(report_a)
|
||||
store.add_report(report_b)
|
||||
|
||||
assert store.get_domain_summary("test.com")["reports_processed"] == 2
|
||||
|
||||
result = store.delete_report("test.com", "rpt-001")
|
||||
assert result is True
|
||||
|
||||
summary = store.get_domain_summary("test.com")
|
||||
assert summary["reports_processed"] == 1
|
||||
# Stats should now reflect only report_b
|
||||
assert summary["total_count"] == 3
|
||||
assert summary["passed_count"] == 1
|
||||
|
||||
def test_delete_report_nonexistent_returns_false(self):
|
||||
store = ReportStore.get_instance()
|
||||
assert store.delete_report("test.com", "rpt-999") is False
|
||||
|
||||
def test_delete_report_unknown_domain_returns_false(self):
|
||||
store = ReportStore.get_instance()
|
||||
assert store.delete_report("nobody.com", "rpt-001") is False
|
||||
|
||||
def test_recompute_stats_after_add(self):
|
||||
"""_recompute_domain_stats is called on add; compliance_rate must be correct."""
|
||||
store = ReportStore.get_instance()
|
||||
report = _sample_report("test.com")
|
||||
report["summary"] = {"total_count": 10, "passed_count": 8, "failed_count": 2}
|
||||
store.add_report(report)
|
||||
|
||||
summary = store.get_domain_summary("test.com")
|
||||
assert summary["compliance_rate"] == 80.0
|
||||
|
||||
@@ -69,3 +69,66 @@ def test_upload_and_get_domain_summary(client: TestClient):
|
||||
assert data["domain"] == "example.com"
|
||||
assert data["total_count"] == 2
|
||||
assert data["reports_processed"] == 1
|
||||
|
||||
|
||||
def test_duplicate_upload_returns_409(client: TestClient):
|
||||
"""Uploading the same report twice returns 409 Conflict."""
|
||||
zip_bytes = _make_zip(SAMPLE_XML)
|
||||
|
||||
first = client.post(
|
||||
"/api/v1/reports/upload",
|
||||
files={"file": ("report.zip", zip_bytes, "application/zip")},
|
||||
)
|
||||
assert first.status_code == 200
|
||||
|
||||
second = client.post(
|
||||
"/api/v1/reports/upload",
|
||||
files={"file": ("report.zip", zip_bytes, "application/zip")},
|
||||
)
|
||||
assert second.status_code == 409
|
||||
assert "already been uploaded" in second.json()["detail"].lower()
|
||||
|
||||
|
||||
def test_delete_report_success(client: TestClient):
|
||||
"""Deleting an existing report returns 200 and removes it from the store."""
|
||||
zip_bytes = _make_zip(SAMPLE_XML)
|
||||
client.post(
|
||||
"/api/v1/reports/upload",
|
||||
files={"file": ("report.zip", zip_bytes, "application/zip")},
|
||||
)
|
||||
|
||||
# Confirm the domain exists first
|
||||
assert client.get("/api/v1/reports/domain/example.com/summary").status_code == 200
|
||||
|
||||
# Delete the report (report_id comes from SAMPLE_XML: "123456789")
|
||||
response = client.delete("/api/v1/reports/domain/example.com/reports/123456789")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert data["success"] is True
|
||||
|
||||
# Domain should be gone now
|
||||
assert client.get("/api/v1/reports/domain/example.com/summary").status_code == 404
|
||||
|
||||
|
||||
def test_delete_nonexistent_report_returns_404(client: TestClient):
|
||||
"""Deleting a report that does not exist returns 404."""
|
||||
response = client.delete("/api/v1/reports/domain/example.com/reports/no-such-id")
|
||||
assert response.status_code == 404
|
||||
|
||||
|
||||
def test_upload_after_delete_succeeds(client: TestClient):
|
||||
"""After deleting a report, the same report can be uploaded again."""
|
||||
zip_bytes = _make_zip(SAMPLE_XML)
|
||||
|
||||
client.post(
|
||||
"/api/v1/reports/upload",
|
||||
files={"file": ("report.zip", zip_bytes, "application/zip")},
|
||||
)
|
||||
client.delete("/api/v1/reports/domain/example.com/reports/123456789")
|
||||
|
||||
response = client.post(
|
||||
"/api/v1/reports/upload",
|
||||
files={"file": ("report.zip", zip_bytes, "application/zip")},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert response.json()["success"] is True
|
||||
|
||||
Reference in New Issue
Block a user