Skip to content

feat(s3): implement count_by_tenant so deregistered tenants stay discoverable#362

Merged
smunini merged 2 commits into
mainfrom
fix/330-s3-count-by-tenant
Jul 23, 2026
Merged

feat(s3): implement count_by_tenant so deregistered tenants stay discoverable#362
smunini merged 2 commits into
mainfrom
fix/330-s3-count-by-tenant

Conversation

@angela-helios

Copy link
Copy Markdown
Contributor

Summary

Fixes #330: the S3 backend inherited count_by_tenant's empty trait default, so a tenant deregistered without purge vanished from the tenant-maintenance page while its objects (~1.4k seeded conformance resources plus any real data) stayed in the bucket — orphaned, purgeable only with direct bucket access. Found by the #290 browser matrix.

Implementation

As sketched in the issue:

  • PrefixPerTenant: one delimiter LIST (CommonPrefixes) enumerates tenant prefixes under the base prefix; paginated LISTs then count each tenant's current-pointer objects under resources/.
  • LIST-only — no per-object GETs (perf(s3): page-level reads go O(N) over the bucket once conformance is seeded #326). Delete tombstones therefore count: a tombstone is still purgeable data, which is exactly what this count exists to surface. Non-tenant top-level groups (tenant registry, user settings, bulk-submit state) have no resources/ subtree and drop out at zero.
  • BucketPerTenant stays unsupported (empty result), matching the tenant-registry carve-out — tenants live in a static bucket map there, so there is no bucket to discover strays in.
  • New S3Api::list_common_prefixes on both the AWS client (paginated list_objects_v2 + delimiter) and the mock.

CompositeStorage already delegates count_by_tenant to its primary, so no changes there.

Verification

  • Mock-client unit test: history versions don't inflate counts, tombstones count, registered-but-empty tenants don't surface (797 lib tests green).
  • MinIO integration test (test_minio_count_by_tenant_survives_deregistration) exercising the real SDK delimiter path through a register → seed → deregister flow. Deliberately named outside the test_minio_settings filter so the conditional-write count guard is untouched.

Follow-up

Once this is in a build the browser matrix tests against, the #290 e2e deregister assertion tightens back to expecting the unregistered row on every backend — that change goes on the e2e branch.

…overable

The tenant-maintenance page discovers tenants from
ResourceStorage::count_by_tenant; the S3 backend inherited the empty trait
default, so deregistering a tenant without purge made its objects invisible
in the UI while they remained in the bucket, purgeable only with direct
bucket access.

PrefixPerTenant mode: one delimiter LIST enumerates tenant prefixes, then
paginated LISTs count each tenant's current-pointer objects under
resources/. LIST-only by design — no per-object GETs — so delete
tombstones count too: a tombstone is still purgeable data, which is what
this count exists to surface. Non-tenant top-level groups (registry,
settings, bulk-submit state) have no resources/ subtree and drop out at
zero. BucketPerTenant stays unsupported, matching the tenant-registry
carve-out for that mode.

Covered by a mock-client unit test (history versions don't inflate counts,
tombstones count, registered-but-empty tenants don't appear) and a MinIO
integration test exercising the SDK delimiter path through a
register-seed-deregister flow.

Closes #330
@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@smunini
smunini merged commit 519a231 into main Jul 23, 2026
13 checks passed
@smunini
smunini deleted the fix/330-s3-count-by-tenant branch July 23, 2026 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

s3: implement count_by_tenant so deregistered tenants with data stay visible (and purgeable)

2 participants