docs: capture P3-17 plan and close storage provider task
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
115
docs/plan.md
115
docs/plan.md
@@ -1,38 +1,38 @@
|
||||
# Implementation Plan: backend-test-coverage
|
||||
# Implementation Plan: p3-17-media-processing-s3
|
||||
|
||||
**Branch**: `[test-coverage-t3-t4]` | **Date**: 2026-02-04 | **Spec**: N/A
|
||||
**Input**: Continuation of test coverage tasks (T3/T4) from prior session; no feature spec.
|
||||
**Branch**: `[p3-17-media-processing-s3]` | **Date**: 2026-02-04 | **Spec**: `docs/todo_list.md#17`
|
||||
**Input**: P3-17 “媒体处理管线适配对象存储（S3/MinIO）” + user request to extract a cover from `fixtures/demo.mp4` via ffmpeg.
|
||||
|
||||
## Summary
|
||||
|
||||
Complete backend service test coverage for content access policies (T3) and superadmin write operations (T4), ensuring existing behavior is validated without altering production logic.
|
||||
Adapt the media processing worker to support S3/MinIO storage by downloading source media to a temp directory, running ffmpeg to generate a cover image, uploading derived assets via the storage provider, and preserving the existing local filesystem path. Include fixture-based verification using `fixtures/demo.mp4`.
|
||||
|
||||
## Technical Context
|
||||
|
||||
**Language/Version**: Go 1.x (project standard)
|
||||
**Primary Dependencies**: Fiber, GORM-Gen, Testify
|
||||
**Storage**: PostgreSQL (via GORM)
|
||||
**Testing**: `go test` (service tests under `backend/app/services/*_test.go`)
|
||||
**Target Platform**: Linux server
|
||||
**Project Type**: Web application (frontend + backend)
|
||||
**Performance Goals**: N/A (test-only changes)
|
||||
**Constraints**: No changes to generated files; keep tests aligned with existing service patterns
|
||||
**Scale/Scope**: Backend service tests only (no frontend scope)
|
||||
**Language/Version**: Go 1.x (project standard)
|
||||
**Primary Dependencies**: River (jobs), MinIO SDK (S3 compatible), Fiber
|
||||
**Storage**: PostgreSQL + local filesystem / S3-compatible storage
|
||||
**Testing**: `go test` (service/job tests), ffmpeg CLI for fixture validation
|
||||
**Target Platform**: Linux server
|
||||
**Project Type**: Web application (backend + frontend, backend-only changes)
|
||||
**Performance Goals**: N/A (processing path only)
|
||||
**Constraints**: Preserve local storage behavior; do not edit generated files; follow `backend/llm.txt`; ensure temp files are cleaned
|
||||
**Scale/Scope**: Media processing worker + storage provider only
|
||||
|
||||
## Constitution Check
|
||||
|
||||
- Follow `backend/llm.txt` for backend conventions.
|
||||
- Keep controllers thin; service tests only (no controller edits).
|
||||
- Avoid editing generated files (`routes.gen.go`, `docs.go`).
|
||||
- Run `go test` for impacted service packages.
|
||||
- Follow `backend/llm.txt` conventions and Chinese comments for business logic.
|
||||
- Do not edit generated files (`*.gen.go`, `backend/docs/docs.go`).
|
||||
- Keep controller thin; changes limited to jobs/services/providers.
|
||||
- Preserve local provider behavior; add S3 path without regression.
|
||||
|
||||
## Project Structure
|
||||
|
||||
### Documentation (this feature)
|
||||
|
||||
```text
|
||||
specs/[###-feature]/
|
||||
└── (not used for this task)
|
||||
docs/
|
||||
└── plan.md # This plan
|
||||
```
|
||||
|
||||
### Source Code (repository root)
|
||||
@@ -40,49 +40,68 @@ specs/[###-feature]/
|
||||
```text
|
||||
backend/
|
||||
├── app/
|
||||
│ ├── services/
|
||||
│ │ ├── content_test.go
|
||||
│ │ └── super_test.go
|
||||
└── app/http/v1/dto/
|
||||
└── content.go
|
||||
│ ├── jobs/
|
||||
│ │ ├── media_process_job.go
|
||||
│ │ └── args/media_asset_process.go
|
||||
│ └── services/
|
||||
│ └── common.go
|
||||
└── providers/
|
||||
└── storage/provider.go
|
||||
|
||||
fixtures/
|
||||
└── demo.mp4
|
||||
```
|
||||
|
||||
**Structure Decision**: Web application structure; scope is backend service tests in `backend/app/services`.
|
||||
**Structure Decision**: Web application backend; scope limited to media processing worker and storage provider integration.
|
||||
|
||||
## Plan Phases
|
||||
|
||||
1. Inspect DTO definitions used by content tests to fix T3 assertions.
|
||||
2. Implement remaining content access policy tests (T3) and verify via `go test`.
|
||||
3. Implement superadmin write operation tests (T4) and verify via `go test`.
|
||||
1. **Design & plumbing**: Define temp file conventions and storage download API for S3/local.
|
||||
2. **Implementation**: Add S3 processing flow to worker and cover asset registration; keep local path intact.
|
||||
3. **Verification**: Add tests (or integration checks) and run fixture-based ffmpeg validation.
|
||||
|
||||
## Tasks
|
||||
|
||||
1. Read `backend/app/http/v1/dto/content.go` and update T3 test assertions to match actual DTO fields.
|
||||
2. Extend `backend/app/services/content_test.go` with missing content access policy cases and run targeted tests.
|
||||
3. Extend `backend/app/services/super_test.go` for superadmin write operations; run service test suite.
|
||||
4. Verify all added tests pass without modifying production logic.
|
||||
1. **Define storage download interface**
|
||||
- Add a storage provider helper to download an object to a local temp file (local: copy from `LocalPath/objectKey` without rename; S3: `FGetObject`).
|
||||
- Ensure helper never mutates/deletes the source object, creates parent dirs for destination, and overwrites the destination if it already exists.
|
||||
- Ensure API is used by jobs without leaking provider-specific logic.
|
||||
|
||||
2. **Adapt `MediaProcessWorker` for S3/MinIO**
|
||||
- For non-local providers, download the source object into a temp directory.
|
||||
- Run ffmpeg to extract a cover image from the temp file.
|
||||
- Upload the cover via `storage.PutObject` and register the derived media asset.
|
||||
- Ensure temp directories/files are cleaned on success or failure.
|
||||
|
||||
3. **Update cover asset registration**
|
||||
- For non-local providers, avoid filesystem rename; upload via storage provider and keep object key conventions.
|
||||
- Use `storage.PutObject(ctx, objectKey, coverTempPath, "image/jpeg")`, then cleanup temp files/dirs.
|
||||
- Keep local path move behavior unchanged.
|
||||
|
||||
4. **Add tests / verification hooks**
|
||||
- Add a job/service test for S3 path (gated by MinIO env if needed).
|
||||
- Keep/extend local path test to ensure no regression.
|
||||
- Validate `fixtures/demo.mp4` can produce a cover with ffmpeg.
|
||||
|
||||
## Dependencies
|
||||
|
||||
- Task 1 must complete before Task 2 (DTO fields drive assertions).
|
||||
- Task 2 should complete before Task 3 to isolate failures.
|
||||
- Task 1 must complete before Task 2 (worker needs download API).
|
||||
- Task 2 must complete before Task 3 (cover registration requires new flow).
|
||||
- Task 4 depends on Task 2 & 3 (tests rely on updated pipeline).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- `backend/app/services/content_test.go` has passing T3 coverage for unauthenticated access constraints.
|
||||
- `backend/app/services/super_test.go` includes T4 coverage for create-tenant side effects and superadmin write operations.
|
||||
- `go test ./backend/app/services/...` passes.
|
||||
- No generated files modified.
|
||||
- **Local path unchanged**: `MediaProcessWorker` still generates a cover image for local provider when ffmpeg is available.
|
||||
- **S3/MinIO path works**: For `Storage.Type = s3`, the worker downloads source media, generates cover via ffmpeg, uploads cover to bucket, and creates a derived `media_assets` record with correct object key.
|
||||
- **Fixture validation**: `ffmpeg -y -i fixtures/demo.mp4 -ss 00:00:00.000 -vframes 1 /tmp/demo_cover.jpg` succeeds locally and produces a non-empty file.
|
||||
- **S3/MinIO test config**: tests load Storage config with `Type=s3`, `Endpoint`, `AccessKey`, `SecretKey`, `Bucket`, `PathStyle` (true for MinIO).
|
||||
- **Automated checks** (to be added in this phase):
|
||||
- `go test ./backend/app/jobs -run TestMediaProcessWorkerS3 -count=1` passes (with S3/MinIO config loaded).
|
||||
- `go test ./backend/app/jobs -run TestMediaProcessWorkerLocal -count=1` passes.
|
||||
|
||||
## Risks
|
||||
|
||||
- DTO field changes may require adjusting test assertions; mitigate by verifying struct definitions.
|
||||
- Service behavior may differ from assumptions in prior session; mitigate by aligning with existing tests.
|
||||
|
||||
## Complexity Tracking
|
||||
|
||||
> **Fill ONLY if Constitution Check has violations that must be justified**
|
||||
|
||||
| Violation | Why Needed | Simpler Alternative Rejected Because |
|
||||
|-----------|------------|-------------------------------------|
|
||||
| N/A | N/A | N/A |
|
||||
- **ffmpeg not installed**: Worker should detect and log; processing should fail gracefully.
|
||||
- **Temp storage pressure**: Large media downloads may exceed disk; ensure cleanup on all paths.
|
||||
- **S3 connectivity/transient errors**: Add retry or error propagation to avoid silent failures.
|
||||
- **Access policies**: Misconfigured bucket policies may prevent download/upload; surface clear errors.
|
||||
|
||||
Reference in New Issue
Block a user