feat(download): consume dunite-download engine, drop hand-rolled code (BUNYIP-30) #26

Merged
nrupard merged 3 commits from feat/bunyip-30-consume-dunite-download into main 2026-06-01 21:13:17 +02:00
Owner

What

Bunyip now consumes the generic dunite-download engine (dunite PR #2) and the hand-rolled binary-download code in bunyip-domain is deleted. Tracks BUNYIP-30 (subtask of the BUNYIP-28 distribution story, follow-up to BUNYIP-29).

Changes

  • bunyip-domain: dunite-download added as a git dependency. The hand-rolled forgejo.rs, release_cache.rs, download_cache.rs, download_limiter.rs services are deleted; services/mod.rs re-exports the engine types instead, plus a concrete AppDownloadCache = DownloadCache<DownloadCacheRepository> alias. moka / tokio-util / tempfile deps dropped.
  • Repositories: DownloadCacheRepository and DownloadDailyCountRepository rewritten as stateful structs implementing the engine's AssetStore and DownloadCounter traits (same pattern as bunyip-oci's OciBlobCacheRepository). The upsert keeps its SELECT ... FOR UPDATE transaction, satisfying the trait's documented atomicity contract.
  • Models: models/download.rs keeps only the API-facing response types and re-exports the engine's wire/cache models. The public /v1 JSON contract is unchanged (fields still serialize as release_tag); bunyip-web needs no changes.
  • Application model: new artifact_source ('release' | 'generic_package') and forgejo_package columns + a download_source() helper that builds the engine's ArtifactSource. A generic-packages-backed product can now be served through the same endpoints. Admin update queries and UpdateApplication carry the new fields.
  • Migration 20260601000041: renames download_cache.release_tag to version (matches the engine's source-neutral naming) and adds the new application columns.
  • Handlers + wiring: download.rs, admin.rs, and main.rs re-pointed at the engine types (ForgejoAssetClient, ReleaseCache keyed by full ArtifactSource, AppDownloadCache, DownloadLimiter + Postgres counter via app_data).

Dockerfile fix (pre-existing breakage)

just check-docker could not run on main at all: the api Dockerfile still referenced crates/bunyip-core/* paths from before the domain rename, and its dependency-cache layer had a latent stub-caching bug (deps removed but cargo fingerprints kept, so COPY . . with older host mtimes made cargo link the real bunyip-api against stub-built empty libs -> 109 unresolved-import errors). Both fixed here so the builder stage verifies this PR end to end.

Verification (rust-builder containers; no local toolchain on the dev box)

  • cargo check --workspace --all-targets: green
  • 196 lib tests pass (bunyip-domain 18, bunyip-api 171, bunyip-oci 7, bunyip-oidc 0)
  • Zero new clippy violations vs main (branch 31 vs main 33 in bunyip-domain; all remaining are pre-existing under the rust 1.94 toolchain, as are the bunyip-web ones)
  • All 9 changed Rust files rustfmt-clean
  • Docker builder stage (musl) builds the api binary end to end

Notes

  • Pre-existing repo-wide issue (NOT addressed here): cargo clippy -D warnings and cargo fmt --check fail on main under the rust 1.94 builder image (dead code in bunyip-web, older-toolchain formatting). Worth a follow-up issue deciding the canonical toolchain version.
## What Bunyip now consumes the generic `dunite-download` engine (dunite PR #2) and the hand-rolled binary-download code in `bunyip-domain` is deleted. Tracks **BUNYIP-30** (subtask of the BUNYIP-28 distribution story, follow-up to BUNYIP-29). ## Changes * **bunyip-domain**: `dunite-download` added as a git dependency. The hand-rolled `forgejo.rs`, `release_cache.rs`, `download_cache.rs`, `download_limiter.rs` services are deleted; `services/mod.rs` re-exports the engine types instead, plus a concrete `AppDownloadCache = DownloadCache<DownloadCacheRepository>` alias. `moka` / `tokio-util` / `tempfile` deps dropped. * **Repositories**: `DownloadCacheRepository` and `DownloadDailyCountRepository` rewritten as stateful structs implementing the engine's `AssetStore` and `DownloadCounter` traits (same pattern as `bunyip-oci`'s `OciBlobCacheRepository`). The upsert keeps its `SELECT ... FOR UPDATE` transaction, satisfying the trait's documented atomicity contract. * **Models**: `models/download.rs` keeps only the API-facing response types and re-exports the engine's wire/cache models. The public `/v1` JSON contract is unchanged (fields still serialize as `release_tag`); bunyip-web needs no changes. * **Application model**: new `artifact_source` (`'release' | 'generic_package'`) and `forgejo_package` columns + a `download_source()` helper that builds the engine's `ArtifactSource`. A generic-packages-backed product can now be served through the same endpoints. Admin update queries and `UpdateApplication` carry the new fields. * **Migration** `20260601000041`: renames `download_cache.release_tag` to `version` (matches the engine's source-neutral naming) and adds the new application columns. * **Handlers + wiring**: `download.rs`, `admin.rs`, and `main.rs` re-pointed at the engine types (`ForgejoAssetClient`, `ReleaseCache` keyed by full `ArtifactSource`, `AppDownloadCache`, `DownloadLimiter` + Postgres counter via app_data). ## Dockerfile fix (pre-existing breakage) `just check-docker` could not run on main at all: the api Dockerfile still referenced `crates/bunyip-core/*` paths from before the domain rename, and its dependency-cache layer had a latent stub-caching bug (deps removed but cargo fingerprints kept, so `COPY . .` with older host mtimes made cargo link the real bunyip-api against stub-built empty libs -> 109 unresolved-import errors). Both fixed here so the builder stage verifies this PR end to end. ## Verification (rust-builder containers; no local toolchain on the dev box) * `cargo check --workspace --all-targets`: green * 196 lib tests pass (bunyip-domain 18, bunyip-api 171, bunyip-oci 7, bunyip-oidc 0) * Zero new clippy violations vs main (branch 31 vs main 33 in bunyip-domain; all remaining are pre-existing under the rust 1.94 toolchain, as are the bunyip-web ones) * All 9 changed Rust files rustfmt-clean * Docker builder stage (musl) builds the api binary end to end ## Notes * Pre-existing repo-wide issue (NOT addressed here): `cargo clippy -D warnings` and `cargo fmt --check` fail on main under the rust 1.94 builder image (dead code in bunyip-web, older-toolchain formatting). Worth a follow-up issue deciding the canonical toolchain version.
Replaces the hand-rolled binary-download code in bunyip-domain with the generic dunite-download engine. Bunyip now keeps only the consumer-side wiring: Postgres adapters implementing the engine's store traits, actix handlers, and routes.

- bunyip-domain consumes dunite-download as a git dependency; the hand-rolled services (forgejo.rs, release_cache.rs, download_cache.rs, download_limiter.rs) are deleted and their types re-exported from the engine instead (services/mod.rs), including the concrete AppDownloadCache = DownloadCache<DownloadCacheRepository> alias. moka/tokio-util/tempfile deps dropped from bunyip-domain.
- DownloadCacheRepository and DownloadDailyCountRepository are rewritten as stateful structs implementing the engine's AssetStore and DownloadCounter traits (same pattern as bunyip-oci's OciBlobCacheRepository). The upsert keeps its SELECT ... FOR UPDATE transaction, satisfying the trait's atomicity contract.
- models/download.rs keeps only the API-facing response types (DownloadAsset, AppDownloadsResponse, AppDownloadGroup) and re-exports the engine's wire/cache models. The public /v1 JSON contract is unchanged: response fields still serialize as release_tag.
- Application model gains artifact_source ('release' | 'generic_package') and forgejo_package, with a download_source() helper building the engine's ArtifactSource; admin update queries and UpdateApplication carry the new fields. Migration 20260601000041 renames download_cache.release_tag to version and adds the new application columns.
- Handlers (download.rs, admin.rs) and main.rs wiring re-pointed at the engine types: ForgejoAssetClient, ReleaseCache keyed by ArtifactSource, AppDownloadCache, DownloadLimiter backed by the DownloadDailyCountRepository counter via app_data.
- Dockerfile fix (pre-existing breakage on main, required for just check-docker to run at all): stale crates/bunyip-core paths from the domain rename are updated to crates/bunyip-domain, and the dependency-cache layer now purges all workspace-crate artifacts AND cargo fingerprints between the stub build and the real build. Without the fingerprint purge, COPY . . preserves host mtimes and cargo links the real bunyip-api against the stub-built empty libs, failing with 109 unresolved-import errors.

Verification (rust-builder containers; no local toolchain): cargo check workspace green; 196 lib tests pass (bunyip-domain 18, bunyip-api 171, bunyip-oci 7); zero new clippy violations vs main (branch 31 vs main 33 pre-existing under rust 1.94); all 9 changed Rust files rustfmt-clean; docker builder stage (musl) builds the api binary end to end.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
chore: remove accidentally committed Claude session artifact, ignore .claude/
Some checks failed
Check / fmt / clippy / build / test (pull_request) Failing after 6s
9b3cbe6c9d
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fix(download): address code-review findings on the dunite-download adoption
Some checks failed
Check / fmt / clippy / build / test (pull_request) Failing after 4s
Create release / Create release from merged PR (pull_request) Has been skipped
2cc3fd7eae
Fixes from an 8-finding review of PR #26:

- Generic-package apps are now configurable through the admin API: the all-or-nothing Forgejo validation is source-aware (release needs owner + repo + tag; generic_package needs owner + package-or-repo + tag) instead of always demanding forgejo_repo, matching what Application::download_source() accepts.
- artifact_source values are validated in the handler against Application::valid_artifact_source() before the UPDATE, so a bad value is a 400 validation error instead of a Postgres CHECK-constraint 500.
- Asset cache rows are keyed by the configured version (source.version()) instead of the upstream-returned release.version, restoring consistency with admin cache invalidation; upstream tag-name normalization (e.g. case differences) can no longer leak rows that invalidation never matches.
- Integrity failures (ShaMismatch / SizeMismatch) from the engine are logged distinctly before the generic upstream-failure audit, so stale Forgejo metadata is diagnosable from logs.
- download_source() no longer silently treats unknown artifact_source values as a release source: it warns and returns None (not downloadable), with model constants (ARTIFACT_SOURCE_RELEASE / ARTIFACT_SOURCE_GENERIC_PACKAGE) replacing bare string literals; new unit tests cover both.
- forgejo_package can be cleared back to NULL by sending an empty string (NULLIF in the update SQL), since NULL now meaningfully selects the forgejo_repo fallback; documented on UpdateApplication.
- Dead code removed: ApplicationRepository::get_pinned_tag (zero callers); is_downloadable() kept and documented as the public convenience API for upcoming admin/UI work.
- AppDownloadCache alias documented: single definition in bunyip-domain services is deliberate (one home visible to handlers, admin, and main.rs) vs the OCI vertical's per-module duplication, which is tracked separately.

Verification: workspace check green; zero new clippy violations vs the previous branch state; all changed files rustfmt-clean; bunyip-domain/api lib tests pass (the one failure, config::tests::oci_config_defaults, is a pre-existing flaky env-var race between parallel tests, reproducible on main and unrelated to this change).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nrupard deleted branch feat/bunyip-30-consume-dunite-download 2026-06-01 21:13:17 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
psa-systems/bunyip!26
No description provided.