From da161e5da8c2366f85db5fa0203740e5aa46cc96 Mon Sep 17 00:00:00 2001 From: Matjaz Debelak Date: Tue, 15 Sep 2026 11:15:50 +0200 Subject: [PATCH 1/3] Drop golang-set, shortid and go-spew, fix typos, revalidate improvements list The subtitle language set is a plain map, the isilon export workflow id uses uuid like the rest of the trigger UI, and the leftover spew debug dumps are gone, so go-spew is only testify's indirect dependency now. Renamed EmtpySRTFile, PlacholderTplData, SubSteams and BmmTargetEnvionment. SubStreams has no JSON tag but nothing reads it, and the BMM field already had the correct tag, so stored histories decode unchanged. potential_improvements.md is revalidated against the code: twelve fixed items removed, stale references corrected. Co-Authored-By: Claude Fable 5.1 --- cmd/fakerclone/main.go | 8 ++-- cmd/trigger_ui/isilon_export.go | 7 +--- go.mod | 4 +- go.sum | 4 -- potential_improvements.md | 56 ++++++++++--------------- services/ffmpeg/progress.go | 4 +- services/vidispine/export.go | 13 +++--- services/vidispine/vsapi/placeholder.go | 4 +- services/vidispine/vsapi/shapes.go | 4 -- workflows/ingest/bmm_simple_upload.go | 4 +- 10 files changed, 41 insertions(+), 67 deletions(-) diff --git a/cmd/fakerclone/main.go b/cmd/fakerclone/main.go index 5c0376df..adca209b 100644 --- a/cmd/fakerclone/main.go +++ b/cmd/fakerclone/main.go @@ -3,8 +3,8 @@ package main import ( "github.com/bcc-code/bcc-media-flows/internal/bootstrap" "github.com/bcc-code/bcc-media-flows/services/rclone" - "github.com/davecgh/go-spew/spew" "github.com/gin-gonic/gin" + "log" "net/http" "time" ) @@ -13,7 +13,7 @@ func jobStatusHandler(c *gin.Context) { req := &rclone.JobStatusRequest{} err := c.BindJSON(req) if err != nil { - spew.Dump(err) + log.Println(err) c.JSON(400, gin.H{"error": err.Error()}) return } @@ -57,7 +57,7 @@ func operationsListHandler(c *gin.Context) { req := &rclone.ListRequest{} err := c.BindJSON(req) if err != nil { - spew.Dump(err) + log.Println(err) c.JSON(400, gin.H{"error": err.Error()}) return } @@ -70,7 +70,7 @@ func operationsStatHandler(c *gin.Context) { req := &rclone.ListRequest{} err := c.BindJSON(req) if err != nil { - spew.Dump(err) + log.Println(err) c.JSON(400, gin.H{"error": err.Error()}) return } diff --git a/cmd/trigger_ui/isilon_export.go b/cmd/trigger_ui/isilon_export.go index 567439c6..e27ebf98 100644 --- a/cmd/trigger_ui/isilon_export.go +++ b/cmd/trigger_ui/isilon_export.go @@ -13,9 +13,8 @@ import ( wfutils "github.com/bcc-code/bcc-media-flows/utils/workflows" "github.com/bcc-code/bcc-media-flows/workflows/export" bccmUtils "github.com/bcc-code/bcc-media-platform/backend/utils" - "github.com/davecgh/go-spew/spew" "github.com/gin-gonic/gin" - "github.com/teris-io/shortid" + "github.com/google/uuid" ) func (s *TriggerServer) isilonExportGET(ctx *gin.Context) { @@ -80,8 +79,6 @@ func (s *TriggerServer) isilonExportPOST(ctx *gin.Context) { selectedResolution := vsResolutions[resolutionIndex] - spew.Dump(ctx.PostForm("exportFormat")) - params := export.IsilonExportParams{ VXID: vxID, WatermarkPath: ctx.PostForm("watermarkPath"), @@ -92,7 +89,7 @@ func (s *TriggerServer) isilonExportPOST(ctx *gin.Context) { } var wfID string - workflowOptions.ID = params.VXID + "-" + shortid.MustGenerate() + workflowOptions.ID = params.VXID + "-" + uuid.NewString() res, err := s.wfClient.ExecuteWorkflow(ctx, workflowOptions, export.IsilonExport, params) if err != nil { renderErrorPage(ctx, http.StatusInternalServerError, err) diff --git a/go.mod b/go.mod index f2a4bc6e..0575d1a8 100644 --- a/go.mod +++ b/go.mod @@ -8,8 +8,6 @@ require ( github.com/bcc-code/bcc-media-platform v0.0.0-20250903091027-11ead5481489 github.com/cloudevents/sdk-go/v2 v2.15.2 github.com/creativeprojects/go-selfupdate v1.1.3 - github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc - github.com/deckarep/golang-set/v2 v2.3.1 github.com/gin-contrib/cors v1.7.2 github.com/gin-gonic/gin v1.10.0 github.com/glebarez/go-sqlite v1.22.0 @@ -22,7 +20,6 @@ require ( github.com/sendgrid/sendgrid-go v3.14.0+incompatible github.com/stretchr/testify v1.11.1 github.com/teamwork/reload v1.4.2 - github.com/teris-io/shortid v0.0.0-20220617161101-71ec9f2aa569 go.temporal.io/api v1.62.14 go.temporal.io/sdk v1.45.0 go.uber.org/mock v0.6.0 @@ -39,6 +36,7 @@ require ( github.com/MicahParks/keyfunc v1.9.0 // indirect github.com/ansel1/merry/v2 v2.2.1 // indirect github.com/cncf/xds/go v0.0.0-20260202195803-dba9d589def2 // indirect + github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/envoyproxy/go-control-plane/envoy v1.37.0 // indirect github.com/envoyproxy/protoc-gen-validate v1.3.3 // indirect github.com/fatih/color v1.18.0 // indirect diff --git a/go.sum b/go.sum index 056b997f..1fd013c4 100644 --- a/go.sum +++ b/go.sum @@ -221,8 +221,6 @@ github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davidmz/go-pageant v1.0.2 h1:bPblRCh5jGU+Uptpz6LgMZGD5hJoOt7otgT454WvHn0= github.com/davidmz/go-pageant v1.0.2/go.mod h1:P2EDDnMqIwG5Rrp05dTRITj9z2zpGcD9efWSkTNKLIE= -github.com/deckarep/golang-set/v2 v2.3.1 h1:vjmkvJt/IV27WXPyYQpAh4bRyWJc5Y435D17XQ9QU5A= -github.com/deckarep/golang-set/v2 v2.3.1/go.mod h1:VAky9rY/yGXJOLEDv3OMci+7wtDpOF4IN+y82NBOac4= github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78= github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc= github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= @@ -662,8 +660,6 @@ github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD github.com/subosito/gotenv v1.4.1/go.mod h1:ayKnFf/c6rvx/2iiLrJUk1e6plDbT3edrFNGqEflhK0= github.com/teamwork/reload v1.4.2 h1:e3U0xXFmhzOSgWNBuyOMOvKS2Q34YNo5bp9Z1uOujYE= github.com/teamwork/reload v1.4.2/go.mod h1:tGCBzttv2CSfSjBTRlIdnQ4kopxrCXPGCTXeOO61SWg= -github.com/teris-io/shortid v0.0.0-20220617161101-71ec9f2aa569 h1:xzABM9let0HLLqFypcxvLmlvEciCHL7+Lv+4vwZqecI= -github.com/teris-io/shortid v0.0.0-20220617161101-71ec9f2aa569/go.mod h1:2Ly+NIftZN4de9zRmENdYbvPQeaVIYKWpLFStLFEBgI= github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY= github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= diff --git a/potential_improvements.md b/potential_improvements.md index 656a472e..4ec103a6 100644 --- a/potential_improvements.md +++ b/potential_improvements.md @@ -1,65 +1,53 @@ # Potential improvements -Condensed 2026-08-21. Items confirmed fixed were removed. Bugs section validated against the code 2026-08-21. +Revalidated against the code 2026-09-15. Items confirmed fixed were removed; the rest were checked to still apply. ## Bugs - The ~60 `wfutils.SendTelegramText` / `telegram.SendText` call sites hand-build legacy Markdown with `fmt.Sprintf`, interpolating filenames, paths and error strings raw. Any unbalanced `_`, `*`, `` ` `` or `[` in those values makes Telegram reject the message ("can't parse entities"); the sender now retries in plain text, so the alert survives but loses its formatting. The root fix is an escaping send helper (`notifications.escapeMarkdown` is the piece to export or mirror) or a move to MarkdownV2, which can also escape inside code entities. - QScan's repository id is an unvalidated default (`QSCAN_REPOSITORY_ID`, 2), and nothing in the code can tell a wrong repository from an unreachable one: the workflow sends only a repository-relative path, so the server/share half of the resolved path lives entirely in QScan's config. A `ListRepositories` call in `QScanActivities.ready()` (or a startup check) asserting that repository's root would turn a silent per-file `file_error` into one clear configuration failure. -- `QScanMaster` turns every non-success terminal status into a non-retryable `QScanAnalysisFailed` (`workflows/misc/qscan_master.go:100`). `file_error` is not always about the file: an SMB outage on the QScan host reports the same status, and that transient infrastructure failure becomes a permanent "QC ERROR" alert with no retry. Worth distinguishing `file_error` (retry a few times, widely spaced) from `analysis_error`/`unsupported` (genuinely permanent). +- `QScanMaster` (`workflows/misc/qscan_master.go`; the raw import QC in PR #509 shares the same code) turns every non-success terminal status into a non-retryable `QScanAnalysisFailed`. `file_error` is not always about the file: an SMB outage on the QScan host reports the same status, and that transient infrastructure failure becomes a permanent "QC ERROR" alert with no retry. Worth distinguishing `file_error` (retry a few times, widely spaced) from `analysis_error`/`unsupported` (genuinely permanent). - `notifications.Simple.RenderMarkdown` emits `# Title`, which legacy Markdown has no heading syntax for — Telegram shows the literal `#`. It also forwards `Message` unescaped, which is deliberate for callers that pass their own markup but means `Simple` cannot be used for text from another system. - - `RawMaterialForm` (`workflows/ingest/raw_material.go`) has had no production caller since commit 0ac6d6c removed the XML order form. The watcher path (`cmd/httpin/watchers.go` `doRawImport`) starts one `RawMaterial` per file event with no metadata, so files uploaded together land in separate runs and output folders, get separate QC mails at best, and carry no uploader address at all. A JSON sidecar form for raw material (like `jsonFormSpecs` has for masters) would restore batching and the uploader. - `utils.IsMedia` (`utils/files.go`) lists only `.mxf`, `.mov` and `.wav`, so an `.mp4` raw upload is imported but gets no ffprobe analysis, thumbnails, previews, transcription or QC. ## Security - `cmd/httpin/main.go:162` — `ExecuteFFmpeg` trigger gives arbitrary ffmpeg argv (read/write/exfil primitive). Delete or gate it. -- No in-process auth on any route; only CORS middleware. Includes admin and state-changing routes. +- No in-process auth on any httpin route; only CORS middleware. Includes state-changing routes. - `cmd/httpin/main.go:266` — `cors.Default()` allows all origins; drop or allowlist. - `cmd/httpin/main.go:269` — workflow triggers reachable over `GET`; no CSRF tokens in any trigger_ui form. - `cmd/httpin/main.go:30` — `triggeredBy` comes from the request, so the audit trail is caller-controlled. -- `GET /schemas` + `POST /trigger-dynamic` expose and run every triggerable workflow, including destructive scheduled ones. -- `cmd/trigger_ui/masters.go` — form paths not confined to `MASTER_TRIGGER_DIR`; watermark paths unvalidated. +- `cmd/trigger_ui/masters.go:157` and `cmd/trigger_ui/main.go:67` — form paths are joined under the configured master-trigger and overlay roots, but `..` segments are not rejected, so a crafted value still escapes the root. - `services/vidispine/vsapi/xml_templates.go` — metadata interpolated into XML via `text/template`, no escaping. Use `encoding/xml`. -- `services/clickup/client.go:27` — hardcoded token in source; other hardcoded internal endpoints (rclone, reaper, baton, emails). rclone sends Basic auth over plain HTTP. +- `services/rclone/upload.go:15` — hardcoded internal endpoint, and rclone sends Basic auth over plain HTTP. (The ClickUp token is a public view share token by design, not a secret.) - `cmd/trigger_ui/templates/*` — 13 templates load Tailwind from a CDN, no SRI/CSP. Vendor the CSS. -- gin runs in debug mode in production (no `SetMode` outside tests); no server timeouts, body size limits, or graceful shutdown. +- gin runs in debug mode in production (no `SetMode` outside tests); `bootstrap.Serve` (`internal/bootstrap/bootstrap.go`) uses `router.Run`, so there are no server timeouts, body size limits, or graceful shutdown. - `cmd/httpin/watchers.go` — hardcoded `/mnt/...` paths bypass the mount-prefix overrides; unknown paths fall through to transcode; fixed `LIVE-INGEST` workflow ID collides on concurrent events. -- `services/transcode/multitrack.go:44` — filenames interpolated unescaped into a `drawtext` filter (ffmpeg filter-grammar injection). Same class in `merge.go` concat lists. -- `worker.Dockerfile` — runs as root (other images set `USER nonroot`), unpinned `alpine:latest`, no ffmpeg (audio/video queues panic in that image). +- `services/transcode/multitrack.go:49` — filenames interpolated unescaped into a `drawtext` filter (ffmpeg filter-grammar injection). +- `worker.Dockerfile` — runs as root (the httpin and trigger_ui images set `USER`), unpinned `alpine:latest`, no ffmpeg (audio/video queues panic in that image). ## CI and tooling -- No CI runs `go test`, `go vet`, `workflowcheck`, or a linter; no `pull_request` trigger. `make test` exists and nothing calls it. -- `workflowcheck` is not pinned; add it as a `go.mod` tool directive. +- CI (`.github/workflows/ci.yml`) runs vet, tests, workflowcheck and golangci-lint, but golangci-lint uses `only-new-issues`, so the pre-existing backlog (errcheck, staticcheck, ineffassign, unused) stays. Burn it down and drop the grandfathering. - No `go mod tidy` check, `govulncheck`, image scanning, or `.dockerignore` (Dockerfiles `COPY . .`). -- golangci-lint (default set) reports ~89 issues: errcheck, staticcheck, ineffassign, unused. -- Low or zero test coverage: `workflows/export`, `activities`, `services/ffmpeg`, and no tests for `directus`, `baton`, `ftp`, `subtrans`, `filecatalyst`. -- Only `vidispine.Client` has an interface + mock; other service clients are concrete structs and untestable without network. -- `services/transcode/testdata/generated/` is partly committed; tests should use `t.TempDir()`. -- Test helpers in `utils/testutils` panic instead of `t.Fatal`/`t.Skip`. -- `services/transcode` tests fail on a current ffmpeg (7.x): `-vsync` was removed and the ProRes `top` AVOption is no longer an encoder option, so `Test_H264Video_WeirdResolutions` and `Test_ProResHyperdeck` are red locally. Either pin the ffmpeg version the tests expect or move to `-fps_mode` / drop `top`. +- Low or zero test coverage: `workflows/export`, `activities`, `services/ffmpeg`, and no tests at all for `services/baton`, `services/ftp`, `services/filecatalyst`. +- Only `vidispine.Client` has an interface + mock; the other service clients expose a `Config` interface but are concrete structs, untestable without network. +- Test helpers in `utils/testutils` (`audio.go`, `video.go`) panic instead of `t.Fatal`/`t.Skip`, so a machine without ffmpeg sees the ingest and activities suites panic rather than skip. +- `services/transcode` tests were seen failing on ffmpeg 7.x (`-vsync` removed, ProRes `top` AVOption gone) in `Test_H264Video_WeirdResolutions` and `Test_ProResHyperdeck`. CI installs Ubuntu's ffmpeg; not re-verified locally (no ffmpeg installed). Either pin the ffmpeg version the tests expect or move to `-fps_mode` / drop `top`. ## Simplification and cleanup -- `workflows/vb_export/` — `bstage` and `gfx` children are near-identical; the same preamble/postamble repeats at ~10 sites. Extract one shared child wrapper. Also: half the children omit `VBExportResult.Title`; `abekas`/`hyperdeck` re-run `AnalyzeFile` although the result is already passed in. -- `common/merge.go` imports `services/vidispine`, inverting the layering; move `AudioStream` down. -- Enum stragglers: watch-folder names, Vidispine job states, shape tags, shorts type/status, `Destinations []string` where enum types exist. (Watch-folder names need a history migration first.) -- JSON tags on workflow payload structs are inconsistent (camelCase vs snake_case vs none); settle one convention — the choice is permanent for replay. -- `interface{}`/`any` where types would do; ~12 activities return `(any, error)` to satisfy `Execute`. -- Dead code: `cmd/fakerclone/`, `vsapi.ListFilesForStorage` + `ListFilesFilter`, assorted unused helpers flagged by `unused`. -- Long functions (150–280 lines) in `incremental_ingest`, `vx_export`, `vx_export_vod`, `generate_short`, `masv_import` decompose naturally. -- `activities/shorts.go` and `activities/reaper.go` still build ad-hoc resty clients outside `internal/httpx`. +- `workflows/vb_export/` — `abekas`, `hyperdeck` re-run `AnalyzeFile` although `VBExport` (`vb_export.go:210`) already analysed the input and could pass the result down. +- Enum stragglers: watch-folder names, Vidispine job states (`"JOB_FAILED"`, `"FINISHED"` string literals), shape tags (literal `ShapeTag: "..."` at 5 sites), Directus shorts type/status (`services/directus/client.go`), and `VBExportParams.Destinations []string` (`vb_export.go:117`) although the `Destinations` enum exists a few lines up. (Watch-folder names need a history migration first.) +- JSON tags on workflow payload structs are inconsistent (~200 snake_case vs ~250 camelCase in `workflows/`); settle one convention — the choice is permanent for replay. +- `interface{}`/`any` where types would do; 15 activities return `(any, error)` to satisfy `Execute`. +- Dead code: `cmd/fakerclone/` (unreferenced), `vsapi.ListFilesForStorage` + `ListFilesFilter` (defined, never called), assorted unused helpers flagged by `unused`. +- Long functions: `doIncremental` (`incremental_ingest.go`), `VXExport` (`vx_export.go`), `GenerateShort` (`generate_short.go`) are each well over 120 lines and decompose naturally. - `emails.Message` carries `CC`/`BCC` that `UtilActivities.SendEmail` never reads; every recipient gets a separate `To`-only mail. Either honour them or drop the fields. -- Typos in exported names: `EmtpySRTFile`, `PlacholderTplData`, `SubSteams`, `BmmTargetEnvionment`, `sanitizeDuplicatdPath`. -- go.mod: two cache libraries, `golang-set` used once next to `lo`, `shortid` overlaps `uuid`, `go-spew` pinned to a pseudo-version, stranded direct deps in `// indirect` blocks. -- Two stray SQLite databases in `cmd/trigger_ui/` (ignored, but invite dev misuse). ## Config -- 55+ env variables read all over; almost none validated at startup, so missing secrets fail late and opaquely. One `Config` struct with required fields, loaded per `main`. -- `godotenv` loads in only 2 of 4 entrypoints; the `.env` files for `httpin` and `trigger_ui` are dead. -- Env read at package-var/`init()` time in several files, so `.env` loading and `t.Setenv` cannot affect them; `vb_export.go` case is a replay hazard. -- `.env.example` files drift from the code (wrong variable names, wrong defaults, missing files); `cmd/worker/readme.md` documents 4 of 6 queues. -- Mount-prefix getters in `environment/` still hand-roll the default-fallback pattern; defaults belong in `Config`. +- `cmd/httpin/watchers.go:128` — the transcode-root regexp is a package-level var built from `environment.Get()`, so it is evaluated before `bootstrap.LoadEnv` runs in `main` and ignores a `.env` override. +- `cmd/worker/readme.md` documents 4 of the 6 queues in `environment/queues.go` (missing `live-ingest` and `debug`). +- Mount-prefix getters in `environment/environment.go` still hand-roll the default-fallback pattern; defaults belong in `Config`. diff --git a/services/ffmpeg/progress.go b/services/ffmpeg/progress.go index 88065c3b..5b48ecd4 100644 --- a/services/ffmpeg/progress.go +++ b/services/ffmpeg/progress.go @@ -28,7 +28,7 @@ type StreamInfo struct { HasAlpha bool VideoStreams []FFProbeStream AudioStreams []FFProbeStream - SubSteams []FFProbeStream + SubStreams []FFProbeStream OtherStreams []FFProbeStream Progressive bool TotalFrames int @@ -58,7 +58,7 @@ func ProbeResultToInfo(info *FFProbeResult) StreamInfo { case "video": streamInfo.VideoStreams = append(streamInfo.VideoStreams, stream) case "subtitle": - streamInfo.SubSteams = append(streamInfo.SubSteams, stream) + streamInfo.SubStreams = append(streamInfo.SubStreams, stream) default: streamInfo.OtherStreams = append(streamInfo.OtherStreams, stream) } diff --git a/services/vidispine/export.go b/services/vidispine/export.go index e446ed73..bf9ffdc5 100644 --- a/services/vidispine/export.go +++ b/services/vidispine/export.go @@ -14,7 +14,6 @@ import ( "github.com/bcc-code/bcc-media-flows/languages" "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "github.com/bcc-code/bcc-media-flows/services/vidispine/vscommon" - mapset "github.com/deckarep/golang-set/v2" "github.com/orsinium-labs/enum" "github.com/samber/lo" ) @@ -78,7 +77,7 @@ var ( ) EmptyWAVFile = environment.GetIsilonPrefix() + "/system/assets/BlankAudio10h.wav" - EmtpySRTFile = environment.GetIsilonPrefix() + "/system/assets/empty.srt" + EmptySRTFile = environment.GetIsilonPrefix() + "/system/assets/empty.srt" ) // GetRelatedAudioPaths returns all related audio paths for a given VXID @@ -559,7 +558,7 @@ func GetDataForExport(client Client, itemVXID string, languagesToExport []string // addSubtitlesAndTranscriptionsToClips modifies the original clips to include subtitles and transcriptions func addSubtitlesAndTranscriptionsToClips(client Client, clips []*Clip, allowAI bool) error { - allSubLanguages := mapset.NewSet[string]() + allSubLanguages := map[string]struct{}{} // Fetch subs for _, clip := range clips { @@ -581,7 +580,7 @@ func addSubtitlesAndTranscriptionsToClips(client Client, clips []*Clip, allowAI clip.SubtitleFiles[langCode] = shape.GetPath() // Collect all languages that any of the clips have subs for - allSubLanguages.Add(langCode) + allSubLanguages[langCode] = struct{}{} } if len(clip.SubtitleFiles) == 0 && allowAI { @@ -589,7 +588,7 @@ func addSubtitlesAndTranscriptionsToClips(client Client, clips []*Clip, allowAI shape := clipShapes.GetShape("Transcribed_Subtitle_SRT") if shape != nil && shape.GetPath() != "" { clip.SubtitleFiles["und"] = shape.GetPath() - allSubLanguages.Add("und") + allSubLanguages["und"] = struct{}{} } } @@ -602,9 +601,9 @@ func addSubtitlesAndTranscriptionsToClips(client Client, clips []*Clip, allowAI for _, clip := range clips { // Add empty subs for all languages that any of the clips have subs for if they are missing // This makes it easier to handle down the line if we always have a sub file for all languages - for langCode := range allSubLanguages.Iter() { + for langCode := range allSubLanguages { if _, ok := clip.SubtitleFiles[langCode]; !ok { - clip.SubtitleFiles[langCode] = EmtpySRTFile + clip.SubtitleFiles[langCode] = EmptySRTFile } } } diff --git a/services/vidispine/vsapi/placeholder.go b/services/vidispine/vsapi/placeholder.go index 5ed6a992..850a5181 100644 --- a/services/vidispine/vsapi/placeholder.go +++ b/services/vidispine/vsapi/placeholder.go @@ -28,7 +28,7 @@ var ( FileStates = enum.New(FileStateClosed, FileStateOpen) ) -type PlacholderTplData struct { +type PlaceholderTplData struct { Title string } @@ -43,7 +43,7 @@ func (c *Client) CreatePlaceholder(ingestType PlaceholderType, title string) (st } var body bytes.Buffer - err := tpl.Execute(&body, PlacholderTplData{ + err := tpl.Execute(&body, PlaceholderTplData{ Title: title, }) if err != nil { diff --git a/services/vidispine/vsapi/shapes.go b/services/vidispine/vsapi/shapes.go index 30578ca7..b8ad2e45 100644 --- a/services/vidispine/vsapi/shapes.go +++ b/services/vidispine/vsapi/shapes.go @@ -6,8 +6,6 @@ import ( "fmt" "net/url" - "github.com/davecgh/go-spew/spew" - "github.com/bcc-code/bcc-media-flows/services/vidispine/vscommon" "github.com/samber/lo" ) @@ -81,8 +79,6 @@ func (c *Client) AddShapeToItem(tag, itemID, fileID string) (string, error) { return "", parseVSError(result.Body(), result.StatusCode(), tag, itemID) } - spew.Dump(result.Result()) - return jobID, nil } diff --git a/workflows/ingest/bmm_simple_upload.go b/workflows/ingest/bmm_simple_upload.go index a2e7c3fd..cc530bfe 100644 --- a/workflows/ingest/bmm_simple_upload.go +++ b/workflows/ingest/bmm_simple_upload.go @@ -22,7 +22,7 @@ type BmmSimpleUploadParams struct { FilePath string `json:"filePath"` Title string `json:"title"` Language string `json:"language"` - BmmTargetEnvionment string `json:"bmmTargetEnvironment"` + BmmTargetEnvironment string `json:"bmmTargetEnvironment"` ForceReplaceTranscription bool `json:"forceReplaceTranscription"` IsPodcast bool `json:"isPodcast"` } @@ -107,7 +107,7 @@ func BmmIngestUpload(ctx workflow.Context, params BmmSimpleUploadParams) (*BmmSi } destinations := []string{export.AssetExportDestinationBMM.Value} - if params.BmmTargetEnvionment == "bmm-int" { + if params.BmmTargetEnvironment == "bmm-int" { destinations = []string{export.AssetExportDestinationBMMIntegration.Value} } From e8b7984ca44353a0d8938f6b537ff86b54809a50 Mon Sep 17 00:00:00 2001 From: Matjaz Debelak Date: Tue, 15 Sep 2026 11:21:02 +0200 Subject: [PATCH 2/3] Add go mod tidy and govulncheck CI jobs, add .dockerignore ci.yml gains a tidy job (go mod tidy -diff) and a govulncheck job. .dockerignore keeps the build context to what go build needs, since every Dockerfile does COPY . .. Update potential_improvements.md. Co-Authored-By: Claude Fable 5.1 --- .dockerignore | 24 ++++++++++++++++++++++++ .github/workflows/ci.yml | 20 ++++++++++++++++++++ potential_improvements.md | 4 ++-- 3 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 .dockerignore diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 00000000..9047f5ed --- /dev/null +++ b/.dockerignore @@ -0,0 +1,24 @@ +# Keep the build context to what `go build` needs. Every Dockerfile does +# `COPY . .`, so anything not listed here ends up in the build stage. +.git +.github +.claude +.idea +.vscode +.local +.DS_Store +.env +.env.* +env.example +*.md +*.Dockerfile +.dockerignore +Makefile +VERSION +coverage.out +coverage.html +*.db +__debug_bin* +build +**/testdata +**/*_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fbb05017..f9e47664 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,6 +23,26 @@ jobs: - name: Run vet, tests and workflowcheck run: make test + tidy: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-go@v6 + with: + go-version-file: go.mod + + - name: go.mod and go.sum are tidy + run: go mod tidy -diff + + govulncheck: + runs-on: ubuntu-latest + steps: + - uses: golang/govulncheck-action@v1 + with: + go-version-file: go.mod + go-package: ./... + lint: runs-on: ubuntu-latest steps: diff --git a/potential_improvements.md b/potential_improvements.md index 4ec103a6..bac748ab 100644 --- a/potential_improvements.md +++ b/potential_improvements.md @@ -6,7 +6,7 @@ Revalidated against the code 2026-09-15. Items confirmed fixed were removed; the - The ~60 `wfutils.SendTelegramText` / `telegram.SendText` call sites hand-build legacy Markdown with `fmt.Sprintf`, interpolating filenames, paths and error strings raw. Any unbalanced `_`, `*`, `` ` `` or `[` in those values makes Telegram reject the message ("can't parse entities"); the sender now retries in plain text, so the alert survives but loses its formatting. The root fix is an escaping send helper (`notifications.escapeMarkdown` is the piece to export or mirror) or a move to MarkdownV2, which can also escape inside code entities. - QScan's repository id is an unvalidated default (`QSCAN_REPOSITORY_ID`, 2), and nothing in the code can tell a wrong repository from an unreachable one: the workflow sends only a repository-relative path, so the server/share half of the resolved path lives entirely in QScan's config. A `ListRepositories` call in `QScanActivities.ready()` (or a startup check) asserting that repository's root would turn a silent per-file `file_error` into one clear configuration failure. -- `QScanMaster` (`workflows/misc/qscan_master.go`; the raw import QC in PR #509 shares the same code) turns every non-success terminal status into a non-retryable `QScanAnalysisFailed`. `file_error` is not always about the file: an SMB outage on the QScan host reports the same status, and that transient infrastructure failure becomes a permanent "QC ERROR" alert with no retry. Worth distinguishing `file_error` (retry a few times, widely spaced) from `analysis_error`/`unsupported` (genuinely permanent). +- `runQScanFile` (`workflows/misc/qscan_master.go`, shared by `QScanMaster` and the `QScanFile` children of `QScanRawImport`) turns every non-success terminal status into a non-retryable `QScanAnalysisFailed`. `file_error` is not always about the file: an SMB outage on the QScan host reports the same status, and that transient infrastructure failure becomes a permanent "QC ERROR" alert with no retry. Worth distinguishing `file_error` (retry a few times, widely spaced) from `analysis_error`/`unsupported` (genuinely permanent). - `notifications.Simple.RenderMarkdown` emits `# Title`, which legacy Markdown has no heading syntax for — Telegram shows the literal `#`. It also forwards `Message` unescaped, which is deliberate for callers that pass their own markup but means `Simple` cannot be used for text from another system. - `RawMaterialForm` (`workflows/ingest/raw_material.go`) has had no production caller since commit 0ac6d6c removed the XML order form. The watcher path (`cmd/httpin/watchers.go` `doRawImport`) starts one `RawMaterial` per file event with no metadata, so files uploaded together land in separate runs and output folders, get separate QC mails at best, and carry no uploader address at all. A JSON sidecar form for raw material (like `jsonFormSpecs` has for masters) would restore batching and the uploader. - `utils.IsMedia` (`utils/files.go`) lists only `.mxf`, `.mov` and `.wav`, so an `.mp4` raw upload is imported but gets no ffprobe analysis, thumbnails, previews, transcription or QC. @@ -30,7 +30,7 @@ Revalidated against the code 2026-09-15. Items confirmed fixed were removed; the ## CI and tooling - CI (`.github/workflows/ci.yml`) runs vet, tests, workflowcheck and golangci-lint, but golangci-lint uses `only-new-issues`, so the pre-existing backlog (errcheck, staticcheck, ineffassign, unused) stays. Burn it down and drop the grandfathering. -- No `go mod tidy` check, `govulncheck`, image scanning, or `.dockerignore` (Dockerfiles `COPY . .`). +- No image scanning in `deploy-images.yml` (a Trivy step was drafted and dropped; revisit once base-image noise is acceptable). - Low or zero test coverage: `workflows/export`, `activities`, `services/ffmpeg`, and no tests at all for `services/baton`, `services/ftp`, `services/filecatalyst`. - Only `vidispine.Client` has an interface + mock; the other service clients expose a `Config` interface but are concrete structs, untestable without network. - Test helpers in `utils/testutils` (`audio.go`, `video.go`) panic instead of `t.Fatal`/`t.Skip`, so a machine without ffmpeg sees the ingest and activities suites panic rather than skip. From 6ffdc4aa2e96de6497763cc69ae6b2884c4fb8b7 Mon Sep 17 00:00:00 2001 From: Matjaz Debelak Date: Tue, 15 Sep 2026 12:19:17 +0200 Subject: [PATCH 3/3] Replace remaining string enums with orsinium-labs/enum types Covers the enum stragglers from potential_improvements.md: watch-folder names, Vidispine job states and shape tags, Directus short status / media item type / image style, and VBExportParams.Destinations. A new internal/enumjson helper encodes every member as its bare string, so workflow payloads look exactly as they did with plain string fields and existing histories replay without migration. Values this codebase produces (watch folders, VB destinations) reject unknown strings; values external systems report (Vidispine, Directus) keep them so an unlisted state still decodes and compares unequal to every named member. The file watcher now rejects an unconfigured transcode folder before starting a workflow instead of letting the workflow fail and move the file to the error folder. The JOB_FAILED and VS_SHAPE_TAG_NOT_FOUND error types are named constants. Co-Authored-By: Claude Fable 5.1 --- activities/directus.go | 6 +- activities/vidispine/client.go | 27 +++++--- activities/vidispine/files.go | 6 +- activities/vidispine/meta.go | 4 +- cmd/httpin/watchers.go | 8 ++- cmd/trigger_ui/vb.go | 13 +++- common/codecs.go | 58 ++++++++++++++--- internal/enumjson/enumjson.go | 58 +++++++++++++++++ internal/enumjson/enumjson_test.go | 52 +++++++++++++++ potential_improvements.md | 2 +- services/cantemo/client.go | 3 +- services/directus/client.go | 46 +++++++------ services/directus/client_test.go | 6 +- services/directus/enums.go | 65 +++++++++++++++++++ services/vidispine/clips.go | 6 +- services/vidispine/export.go | 12 ++-- services/vidispine/vsapi/job_status.go | 58 +++++++++++++++++ services/vidispine/vsapi/jobs.go | 12 ++-- services/vidispine/vsapi/shape_tags.go | 49 ++++++++++++++ services/vidispine/vsapi/shapes.go | 6 +- services/vidispine/vsapi/shapes_test.go | 2 +- utils/workflows/vidispine.go | 2 +- workflows/export/shorts.go | 12 ++-- workflows/ingest/bmm_simple_upload.go | 5 +- workflows/ingest/bmm_track_metadata.go | 3 +- workflows/ingest/common.go | 7 +- workflows/ingest/import_audio_from_reaper.go | 3 +- workflows/ingest/import_subtitles.go | 5 +- workflows/ingest/incremental_ingest.go | 3 +- workflows/ingest/masters.go | 3 +- workflows/ingest/masters_test.go | 4 +- workflows/ingest/mu1_mu2_extract.go | 5 +- workflows/ingest/multitrack.go | 3 +- workflows/ingest/raw_material.go | 3 +- workflows/ingest/sync_fix.go | 3 +- workflows/misc/fix_duration.go | 3 +- workflows/misc/import_subs.go | 3 +- workflows/misc/merge_import_subs.go | 3 +- workflows/misc/slow_move_files.go | 3 +- workflows/misc/transcode_preview-vx.go | 13 ++-- workflows/misc/transcribe-vx.go | 7 +- workflows/misc/watch_folder_transcode.go | 4 +- workflows/misc/watch_folder_transcode_test.go | 38 +++++++++++ workflows/vb_export/vb_export.go | 48 +++++++++----- workflows/vb_export/vb_export_test.go | 42 ++++++++++-- 45 files changed, 580 insertions(+), 144 deletions(-) create mode 100644 internal/enumjson/enumjson.go create mode 100644 internal/enumjson/enumjson_test.go create mode 100644 services/directus/enums.go create mode 100644 services/vidispine/vsapi/job_status.go create mode 100644 services/vidispine/vsapi/shape_tags.go create mode 100644 workflows/misc/watch_folder_transcode_test.go diff --git a/activities/directus.go b/activities/directus.go index d54bb04b..88a3209c 100644 --- a/activities/directus.go +++ b/activities/directus.go @@ -26,7 +26,7 @@ type CreateMediaItemTagInput struct { type CreateMediaItemInput struct { Label string - Type string + Type directus.MediaItemType AssetID string Title string ParentEpisodeID string @@ -37,12 +37,12 @@ type CreateMediaItemInput struct { type CreateShortInput struct { MediaItemID string - Status string + Status directus.ShortStatus } type CreateStyledImageInput struct { ImageID string - Style string + Style directus.ImageStyle } type GetOrCreateTagInput struct { diff --git a/activities/vidispine/client.go b/activities/vidispine/client.go index d1317553..7fd78e51 100644 --- a/activities/vidispine/client.go +++ b/activities/vidispine/client.go @@ -26,9 +26,20 @@ type WaitForJobCompletionParams struct { type MBJobStatusResult struct { JobID string - Status string + Status vsapi.JobStatus } +// Application error types raised by these activities. Workflows match on +// them to decide whether an error is worth retrying or reporting. +const ( + // JobFailedErrorType marks a Vidispine job that ended in a state other + // than FINISHED; retrying the wait cannot help. + JobFailedErrorType = "JOB_FAILED" + // ShapeTagNotFoundErrorType marks an import whose shape-tag Vidispine has + // not configured. + ShapeTagNotFoundErrorType = "VS_SHAPE_TAG_NOT_FOUND" +) + func (a Activities) WaitForJobCompletion(ctx context.Context, params WaitForJobCompletionParams) (*MBJobStatusResult, error) { logger := activity.GetLogger(ctx) logger.Info("Starting WaitForJobCompletionActivity") @@ -43,11 +54,7 @@ func (a Activities) WaitForJobCompletion(ctx context.Context, params WaitForJobC if err != nil { return nil, err } - if job.Status == "FINISHED" { - return &MBJobStatusResult{params.JobID, job.Status}, nil - } - - if job.Status != "STARTED" && job.Status != "READY" && job.Status != "WAITING" { + if job.Status == vsapi.JobStatusFinished || !job.Status.InProgress() { return &MBJobStatusResult{params.JobID, job.Status}, nil } @@ -67,13 +74,13 @@ func (a Activities) JobCompleteOrErr(ctx context.Context, params WaitForJobCompl for { job, err := a.Client.GetJob(params.JobID) if err != nil { - return false, temporal.NewNonRetryableApplicationError("couldn't complete job", "JOB_FAILED", err) + return false, temporal.NewNonRetryableApplicationError("couldn't complete job", JobFailedErrorType, err) } - if job.Status == "FINISHED" { + if job.Status == vsapi.JobStatusFinished { return true, nil } - if job.Status != "STARTED" && job.Status != "READY" && job.Status != "WAITING" { - return false, temporal.NewNonRetryableApplicationError("couldn't complete job", "JOB_FAILED", fmt.Errorf("job failed with status: %s", job.Status), job) + if !job.Status.InProgress() { + return false, temporal.NewNonRetryableApplicationError("couldn't complete job", JobFailedErrorType, fmt.Errorf("job failed with status: %s", job.Status), job) } activity.RecordHeartbeat(ctx, job) diff --git a/activities/vidispine/files.go b/activities/vidispine/files.go index 298d3e44..253ebb96 100644 --- a/activities/vidispine/files.go +++ b/activities/vidispine/files.go @@ -14,7 +14,7 @@ import ( type ImportFileAsShapeParams struct { AssetID string FilePath paths.Path - ShapeTag string + ShapeTag vsapi.ShapeTag Growing bool Replace bool } @@ -53,9 +53,9 @@ func (a Activities) ImportFileAsShapeActivity(ctx context.Context, params Import } } - res, err := a.Client.AddShapeToItem(params.ShapeTag, params.AssetID, fileID) + res, err := a.Client.AddShapeToItem(params.ShapeTag.Value, params.AssetID, fileID) if err != nil && errors.Is(err, vsapi.ErrShapeTagNotFound) { - err = temporal.NewNonRetryableApplicationError(err.Error(), "VS_SHAPE_TAG_NOT_FOUND", err) + err = temporal.NewNonRetryableApplicationError(err.Error(), ShapeTagNotFoundErrorType, err) } return &ImportFileResult{ JobID: res, diff --git a/activities/vidispine/meta.go b/activities/vidispine/meta.go index dcaa062e..670a6741 100644 --- a/activities/vidispine/meta.go +++ b/activities/vidispine/meta.go @@ -18,12 +18,12 @@ type VXOnlyParam struct { type GetFileFromVXParams struct { VXID string - Tags []string + Tags []vsapi.ShapeTag } type GetFileFromVXResult struct { FilePath paths.Path - ShapeTag string + ShapeTag vsapi.ShapeTag } func (a Activities) GetFileFromVXActivity(ctx context.Context, params GetFileFromVXParams) (*GetFileFromVXResult, error) { diff --git a/cmd/httpin/watchers.go b/cmd/httpin/watchers.go index 5e17623e..22f07071 100644 --- a/cmd/httpin/watchers.go +++ b/cmd/httpin/watchers.go @@ -8,6 +8,7 @@ import ( "strings" "time" + "github.com/bcc-code/bcc-media-flows/common" "github.com/bcc-code/bcc-media-flows/environment" "github.com/bcc-code/bcc-media-flows/paths" wfutils "github.com/bcc-code/bcc-media-flows/utils/workflows" @@ -139,13 +140,16 @@ func doTranscode(ctx context.Context, path string) error { } matches := exp.FindStringSubmatch(path) - t := matches[1] + folder := common.WatchFolders.Parse(matches[1]) + if folder == nil { + return fmt.Errorf("%w %q for %s", common.ErrUnknownWatchFolder, matches[1], path) + } workflowOptions := wfutils.NewWorkflowOptions(environment.GetWorkerQueue(), "", "watcher") _, err = c.ExecuteWorkflow(ctx, workflowOptions, miscworkflows.WatchFolderTranscode, miscworkflows.WatchFolderTranscodeInput{ Path: path, - FolderName: t, + FolderName: *folder, }) return err } diff --git a/cmd/trigger_ui/vb.go b/cmd/trigger_ui/vb.go index 74be2799..126f8df0 100644 --- a/cmd/trigger_ui/vb.go +++ b/cmd/trigger_ui/vb.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "github.com/bcc-code/bcc-media-flows/environment" "log" "net/http" @@ -65,9 +66,19 @@ func (s *TriggerServer) vbExportPOST(ctx *gin.Context) { workflowOptions := wfutils.NewWorkflowOptions(environment.GetQueue(), vxID, getTriggeredBy(ctx)) + var destinations []vb_export.Destination + for _, name := range ctx.PostFormArray("destinations[]") { + dest := vb_export.Destinations.Parse(name) + if dest == nil { + renderErrorPage(ctx, http.StatusBadRequest, fmt.Errorf("%w: %q", vb_export.ErrUnknownDestination, name)) + return + } + destinations = append(destinations, *dest) + } + params := vb_export.VBExportParams{ VXID: vxID, - Destinations: ctx.PostFormArray("destinations[]"), + Destinations: destinations, SubtitleShapeTag: ctx.PostForm("subtitleShape"), SubtitleStyle: ctx.PostForm("subtitleStyle"), } diff --git a/common/codecs.go b/common/codecs.go index f33826dc..b1b38b55 100644 --- a/common/codecs.go +++ b/common/codecs.go @@ -1,12 +1,52 @@ package common -const ( - FolderProRes422HQHD = "ProRes422HQ_HD" - FolderProRes422HQNative = "ProRes422HQ_Native" - FolderProRes422HQNative25FPS = "ProRes422HQ_Native_25FPS" - FolderProRes4444K25FPS = "ProRes444_4K-25FPS" - FolderAVCIntra100HD = "AVCintra100_HD" - FolderXDCAMHD422 = "XDCAMHD422" - FolderTranscribe = "Transcribe" - FolderHAP50FPS = "HAP_50FPS" +import ( + "errors" + + "github.com/bcc-code/bcc-media-flows/internal/enumjson" + "github.com/orsinium-labs/enum" +) + +// WatchFolder names a subfolder of the transcode root that the file watcher +// monitors. A file dropped in `//in/` is transcoded (or +// transcribed) according to the folder it landed in. +type WatchFolder enum.Member[string] + +var ( + FolderProRes422HQHD = WatchFolder{Value: "ProRes422HQ_HD"} + FolderProRes422HQNative = WatchFolder{Value: "ProRes422HQ_Native"} + FolderProRes422HQNative25FPS = WatchFolder{Value: "ProRes422HQ_Native_25FPS"} + FolderProRes4444K25FPS = WatchFolder{Value: "ProRes444_4K-25FPS"} + FolderAVCIntra100HD = WatchFolder{Value: "AVCintra100_HD"} + FolderXDCAMHD422 = WatchFolder{Value: "XDCAMHD422"} + FolderTranscribe = WatchFolder{Value: "Transcribe"} + FolderHAP50FPS = WatchFolder{Value: "HAP_50FPS"} + WatchFolders = enum.New( + FolderProRes422HQHD, + FolderProRes422HQNative, + FolderProRes422HQNative25FPS, + FolderProRes4444K25FPS, + FolderAVCIntra100HD, + FolderXDCAMHD422, + FolderTranscribe, + FolderHAP50FPS, + ) + ErrUnknownWatchFolder = errors.New("unknown watch folder") ) + +func (f WatchFolder) String() string { + return f.Value +} + +// MarshalJSON writes the bare folder name, which is what workflow histories +// recorded while FolderName was a plain string contain. +// +//goland:noinspection GoMixedReceiverTypes +func (f WatchFolder) MarshalJSON() ([]byte, error) { + return enumjson.Marshal(f) +} + +//goland:noinspection GoMixedReceiverTypes +func (f *WatchFolder) UnmarshalJSON(data []byte) error { + return enumjson.UnmarshalStrict(data, WatchFolders, f, ErrUnknownWatchFolder) +} diff --git a/internal/enumjson/enumjson.go b/internal/enumjson/enumjson.go new file mode 100644 index 00000000..44fb3cff --- /dev/null +++ b/internal/enumjson/enumjson.go @@ -0,0 +1,58 @@ +// Package enumjson encodes string-valued enum members (types declared as +// `type T enum.Member[string]`) as their bare value in JSON, so a payload +// carries "xdcam" rather than {"Value":"xdcam"}. That keeps Temporal +// workflow histories written when a field was a plain string replayable +// after the field is retyped to an enum. +package enumjson + +import ( + "encoding/json" + "fmt" + + "github.com/orsinium-labs/enum" +) + +// Member is any type whose underlying type is enum.Member[string]. +type Member interface { + ~struct{ Value string } +} + +// member is the underlying type; Go only permits field access through it, +// not through the type parameter. +type member = struct{ Value string } + +// Marshal encodes m as its bare string value. +func Marshal[M Member](m M) ([]byte, error) { + return json.Marshal(member(m).Value) +} + +// UnmarshalStrict decodes a bare string into *dst and rejects values that are +// not members of e, wrapping notFound so callers can errors.Is against it. +// Use it for values this codebase produces itself (workflow inputs, form +// fields), where an unknown value is a bug or a bad request. +func UnmarshalStrict[M Member](data []byte, e enum.Enum[M, string], dst *M, notFound error) error { + var value string + if err := json.Unmarshal(data, &value); err != nil { + return err + } + member := e.Parse(value) + if member == nil { + return fmt.Errorf("%w: %q", notFound, value) + } + *dst = *member + return nil +} + +// UnmarshalOpen decodes a bare string into *dst, keeping values that are not +// among the declared members. Use it for values an external system reports +// (Vidispine job states, Directus statuses), where an unlisted value must +// still round-trip and compare unequal to every named member rather than +// fail decoding. +func UnmarshalOpen[M Member](data []byte, dst *M) error { + var value string + if err := json.Unmarshal(data, &value); err != nil { + return err + } + *dst = M(member{Value: value}) + return nil +} diff --git a/internal/enumjson/enumjson_test.go b/internal/enumjson/enumjson_test.go new file mode 100644 index 00000000..5db0e64b --- /dev/null +++ b/internal/enumjson/enumjson_test.go @@ -0,0 +1,52 @@ +package enumjson + +import ( + "encoding/json" + "errors" + "testing" + + "github.com/orsinium-labs/enum" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type colour enum.Member[string] + +var ( + red = colour{Value: "red"} + blue = colour{Value: "blue"} + colours = enum.New(red, blue) + errNoSuchHue = errors.New("no such colour") + errOtherCause = errors.New("other") +) + +func TestMarshal_BareString(t *testing.T) { + b, err := Marshal(red) + require.NoError(t, err) + assert.Equal(t, `"red"`, string(b)) +} + +func TestUnmarshalStrict(t *testing.T) { + var got colour + require.NoError(t, UnmarshalStrict([]byte(`"blue"`), colours, &got, errNoSuchHue)) + assert.Equal(t, blue, got) + + err := UnmarshalStrict([]byte(`"green"`), colours, &got, errNoSuchHue) + require.Error(t, err) + assert.ErrorIs(t, err, errNoSuchHue) + assert.Contains(t, err.Error(), `"green"`) + assert.NotErrorIs(t, err, errOtherCause) + + var jsonErr *json.SyntaxError + assert.ErrorAs(t, UnmarshalStrict([]byte(`{`), colours, &got, errNoSuchHue), &jsonErr) +} + +func TestUnmarshalOpen_KeepsUnknownValues(t *testing.T) { + var got colour + require.NoError(t, UnmarshalOpen([]byte(`"green"`), &got)) + assert.Equal(t, "green", got.Value) + assert.False(t, colours.Contains(got)) + + require.NoError(t, UnmarshalOpen([]byte(`"red"`), &got)) + assert.Equal(t, red, got) +} diff --git a/potential_improvements.md b/potential_improvements.md index bac748ab..db6b224c 100644 --- a/potential_improvements.md +++ b/potential_improvements.md @@ -39,7 +39,7 @@ Revalidated against the code 2026-09-15. Items confirmed fixed were removed; the ## Simplification and cleanup - `workflows/vb_export/` — `abekas`, `hyperdeck` re-run `AnalyzeFile` although `VBExport` (`vb_export.go:210`) already analysed the input and could pass the result down. -- Enum stragglers: watch-folder names, Vidispine job states (`"JOB_FAILED"`, `"FINISHED"` string literals), shape tags (literal `ShapeTag: "..."` at 5 sites), Directus shorts type/status (`services/directus/client.go`), and `VBExportParams.Destinations []string` (`vb_export.go:117`) although the `Destinations` enum exists a few lines up. (Watch-folder names need a history migration first.) +- Enum stragglers, second round (the first round is done: watch folders, Vidispine job states and shape tags, Directus short status / media item type / image style, `VBExportParams.Destinations`; all encode as bare strings via `internal/enumjson`, so no history migration was needed): `VXExportParams.Destinations []string` (`vx_export.go:42`) although `AssetExportDestinations` exists; `MoveMBFileParams.Shapes []string` and `VBExportParams.SubtitleShapeTag string` could be `vsapi.ShapeTag`; `languages.MBPreviewTag string` is a shape tag too; Cantemo task states `"STARTED"`/`"SUCCESS"` in `activities/cantemo/files.go:54`; `paths.Drive` hand-rolls the JSON methods `internal/enumjson` now provides. - JSON tags on workflow payload structs are inconsistent (~200 snake_case vs ~250 camelCase in `workflows/`); settle one convention — the choice is permanent for replay. - `interface{}`/`any` where types would do; 15 activities return `(any, error)` to satisfy `Execute`. - Dead code: `cmd/fakerclone/` (unreferenced), `vsapi.ListFilesForStorage` + `ListFilesFilter` (defined, never called), assorted unused helpers flagged by `unused`. diff --git a/services/cantemo/client.go b/services/cantemo/client.go index a8c7b1ab..e83490a7 100644 --- a/services/cantemo/client.go +++ b/services/cantemo/client.go @@ -2,6 +2,7 @@ package cantemo import ( "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "strings" "time" @@ -111,7 +112,7 @@ func (c *Client) GetTranscriptionJSON(itemID string) (*Transcription, error) { } for _, format := range formats { - if format.Name != "transcription_json" { + if format.Name != vsapi.ShapeTagTranscriptionJSON.Value { continue } diff --git a/services/directus/client.go b/services/directus/client.go index b47c7c27..c3ab502c 100644 --- a/services/directus/client.go +++ b/services/directus/client.go @@ -6,7 +6,6 @@ import ( "fmt" "os" "path/filepath" - "slices" "time" "github.com/go-resty/resty/v2" @@ -85,7 +84,7 @@ type File struct { // StyledImage represents a styled image in Directus type StyledImage struct { ID string `json:"id"` - Style string `json:"style"` + Style ImageStyle `json:"style"` Language string `json:"language"` File string `json:"file"` DateCreated *time.Time `json:"date_created,omitempty"` @@ -96,28 +95,28 @@ type StyledImage struct { // Short represents a short in Directus type Short struct { - ID string `json:"id"` - MediaItemID string `json:"mediaitem_id"` - Status string `json:"status"` + ID string `json:"id"` + MediaItemID string `json:"mediaitem_id"` + Status ShortStatus `json:"status"` } // ShortCreate is used when creating a new short type ShortCreate struct { - MediaItemID string `json:"mediaitem_id"` - Status string `json:"status"` - Roles []string `json:"roles,omitempty"` + MediaItemID string `json:"mediaitem_id"` + Status ShortStatus `json:"status"` + Roles []string `json:"roles,omitempty"` } // MediaItem represents a media item in Directus" type MediaItem struct { - ID string `json:"id"` - Label string `json:"label"` - Type string `json:"type"` - AssetID int64 `json:"asset_id"` - Title string `json:"title"` - ParentEpisodeID *int `json:"parent_episode_id"` - ParentStartsAt *int64 `json:"parent_starts_at"` - ParentEndsAt *int64 `json:"parent_ends_at"` + ID string `json:"id"` + Label string `json:"label"` + Type MediaItemType `json:"type"` + AssetID int64 `json:"asset_id"` + Title string `json:"title"` + ParentEpisodeID *int `json:"parent_episode_id"` + ParentStartsAt *int64 `json:"parent_starts_at"` + ParentEndsAt *int64 `json:"parent_ends_at"` } // MediaItemCreate is used when creating a new media item @@ -126,7 +125,7 @@ type MediaItem struct { // This prevents sending empty strings for integer fields type MediaItemCreate struct { Label string `json:"label"` - Type string `json:"type"` + Type MediaItemType `json:"type"` AssetID *int64 `json:"asset_id,omitempty"` Title string `json:"title"` ParentEpisodeID *int64 `json:"parent_episode_id,omitempty"` @@ -149,9 +148,9 @@ type MediaItemStyledImageCRUD struct { } type StyledImageCreate struct { - Style string `json:"style"` - Language string `json:"language"` - File string `json:"file"` + Style ImageStyle `json:"style"` + Language string `json:"language"` + File string `json:"file"` } // Tag represents a tag in Directus @@ -213,14 +212,13 @@ func (c *Client) AssetExists(mediabankenID string) (bool, error) { } // CreateStyledImage creates a styled image in Directus and returns the created styled image -func (c *Client) CreateStyledImage(imageID, style string) (*StyledImage, error) { +func (c *Client) CreateStyledImage(imageID string, style ImageStyle) (*StyledImage, error) { if imageID == "" { return nil, errors.New("imageID is required") } - validStyles := []string{"poster", "default", "icon", "album", "featured"} - if !slices.Contains(validStyles, style) { - return nil, fmt.Errorf("invalid style: %s. Valid styles: %v", style, validStyles) + if !ImageStyles.Contains(style) { + return nil, fmt.Errorf("invalid style: %s. Valid styles: %v", style, ImageStyles.Values()) } result := &struct { diff --git a/services/directus/client_test.go b/services/directus/client_test.go index 265726e5..458ce277 100644 --- a/services/directus/client_test.go +++ b/services/directus/client_test.go @@ -53,7 +53,7 @@ func TestClient_ServerErrorsAreErrors(t *testing.T) { }{ {"GetAssetByMediabankenID", func(c *Client) error { _, err := c.GetAssetByMediabankenID("MB-1"); return err }}, {"AssetExists", func(c *Client) error { _, err := c.AssetExists("MB-1"); return err }}, - {"CreateStyledImage", func(c *Client) error { _, err := c.CreateStyledImage("file-1", "poster"); return err }}, + {"CreateStyledImage", func(c *Client) error { _, err := c.CreateStyledImage("file-1", ImageStylePoster); return err }}, {"CreateShort", func(c *Client) error { _, err := c.CreateShort(ShortCreate{MediaItemID: "mi-1"}); return err }}, {"CreateMediaItemStyledImage", func(c *Client) error { return c.CreateMediaItemStyledImage("mi-1", "si-1") }}, {"CreateMediaItem", func(c *Client) error { _, err := c.CreateMediaItem(MediaItemCreate{Label: "l"}); return err }}, @@ -181,7 +181,7 @@ func TestCreateStyledImage_RejectsAnUnknownStyleWithoutCallingDirectus(t *testin t.Cleanup(server.Close) client := NewClient(testConfig{baseURL: server.URL, apiKey: "test-api-key"}) - _, err := client.CreateStyledImage("file-1", "banner") + _, err := client.CreateStyledImage("file-1", ImageStyle{Value: "banner"}) require.Error(t, err) assert.Contains(t, err.Error(), "invalid style") @@ -191,7 +191,7 @@ func TestCreateStyledImage_RejectsAnUnknownStyleWithoutCallingDirectus(t *testin func TestCreateStyledImage_MissingIDIsAnError(t *testing.T) { client := directusServer(t, http.StatusOK, `{"data":{}}`) - _, err := client.CreateStyledImage("file-1", "poster") + _, err := client.CreateStyledImage("file-1", ImageStylePoster) require.Error(t, err) assert.Contains(t, err.Error(), "missing styled image ID") diff --git a/services/directus/enums.go b/services/directus/enums.go new file mode 100644 index 00000000..69b71630 --- /dev/null +++ b/services/directus/enums.go @@ -0,0 +1,65 @@ +package directus + +import ( + "github.com/bcc-code/bcc-media-flows/internal/enumjson" + "github.com/orsinium-labs/enum" +) + +// ShortStatus is the publication state of a short. Directus's status field +// offers the three states below; flows only ever create drafts, editors +// publish in the CMS. +type ShortStatus enum.Member[string] + +var ( + ShortStatusDraft = ShortStatus{Value: "draft"} + ShortStatusPublished = ShortStatus{Value: "published"} + ShortStatusArchived = ShortStatus{Value: "archived"} + ShortStatuses = enum.New(ShortStatusDraft, ShortStatusPublished, ShortStatusArchived) +) + +func (s ShortStatus) String() string { return s.Value } + +//goland:noinspection GoMixedReceiverTypes +func (s ShortStatus) MarshalJSON() ([]byte, error) { return enumjson.Marshal(s) } + +// UnmarshalJSON keeps any state Directus reports, since the CMS owns the list. +// +//goland:noinspection GoMixedReceiverTypes +func (s *ShortStatus) UnmarshalJSON(data []byte) error { return enumjson.UnmarshalOpen(data, s) } + +// MediaItemType is the kind of media item a Directus record describes. Flows +// only create shorts; the type is open because the CMS holds other kinds. +type MediaItemType enum.Member[string] + +var ( + MediaItemTypeShort = MediaItemType{Value: "short"} + MediaItemTypes = enum.New(MediaItemTypeShort) +) + +func (t MediaItemType) String() string { return t.Value } + +//goland:noinspection GoMixedReceiverTypes +func (t MediaItemType) MarshalJSON() ([]byte, error) { return enumjson.Marshal(t) } + +//goland:noinspection GoMixedReceiverTypes +func (t *MediaItemType) UnmarshalJSON(data []byte) error { return enumjson.UnmarshalOpen(data, t) } + +// ImageStyle is the slot a styled image fills on a media item. +type ImageStyle enum.Member[string] + +var ( + ImageStylePoster = ImageStyle{Value: "poster"} + ImageStyleDefault = ImageStyle{Value: "default"} + ImageStyleIcon = ImageStyle{Value: "icon"} + ImageStyleAlbum = ImageStyle{Value: "album"} + ImageStyleFeatured = ImageStyle{Value: "featured"} + ImageStyles = enum.New(ImageStylePoster, ImageStyleDefault, ImageStyleIcon, ImageStyleAlbum, ImageStyleFeatured) +) + +func (s ImageStyle) String() string { return s.Value } + +//goland:noinspection GoMixedReceiverTypes +func (s ImageStyle) MarshalJSON() ([]byte, error) { return enumjson.Marshal(s) } + +//goland:noinspection GoMixedReceiverTypes +func (s *ImageStyle) UnmarshalJSON(data []byte) error { return enumjson.UnmarshalOpen(data, s) } diff --git a/services/vidispine/clips.go b/services/vidispine/clips.go index 6cbf08f1..6790f8ac 100644 --- a/services/vidispine/clips.go +++ b/services/vidispine/clips.go @@ -32,7 +32,7 @@ func SeqToClips(client Client, seq *vsapi.SequenceDocument) ([]*Clip, error) { return nil, err } - shape := shapes.GetShape("original") + shape := shapes.GetShape(vsapi.ShapeTagOriginal) if shape == nil { return nil, fmt.Errorf("no original shape found for item %s", segment.VXID) } @@ -93,7 +93,7 @@ func getClipForAsset( return nil, err } - shape := shapes.GetShape("original") + shape := shapes.GetShape(vsapi.ShapeTagOriginal) if shape == nil { return nil, fmt.Errorf("no original shape found for item %s", itemVXID) } @@ -125,7 +125,7 @@ func getClipForSubclip( return nil, err } - shape := shapes.GetShape("original") + shape := shapes.GetShape(vsapi.ShapeTagOriginal) if shape == nil { return nil, fmt.Errorf("no original shape found for item %s", itemVXID) } diff --git a/services/vidispine/export.go b/services/vidispine/export.go index bf9ffdc5..a7d66d35 100644 --- a/services/vidispine/export.go +++ b/services/vidispine/export.go @@ -105,7 +105,7 @@ func GetRelatedAudioPaths(client Client, vxID string) (map[string]string, error) return nil, err } - shape := shapes.GetShape("original") + shape := shapes.GetShape(vsapi.ShapeTagOriginal) if shape == nil { continue } @@ -182,7 +182,7 @@ func enrichClipWithRelatedAudios(client Client, clip *Clip, oLanguagesToExport [ } // Ok now we can finally get the path to the audio file - relatedAudioShape := relatedAudioShapes.GetShape("original") + relatedAudioShape := relatedAudioShapes.GetShape(vsapi.ShapeTagOriginal) if relatedAudioShape == nil { if languagesToExport[0] == "nor" { // Fall back to "nor" audio and issue a warning *somewhere* @@ -225,7 +225,7 @@ func enrichClipWithEmbeddedAudio(client Client, clip *Clip, languagesToExport [] return nil, err } - shape := shapes.GetShape("original") + shape := shapes.GetShape(vsapi.ShapeTagOriginal) if shape == nil { // The AudioComponent access below dereferences the shape, so a missing original // has to be reported rather than followed. @@ -572,7 +572,7 @@ func addSubtitlesAndTranscriptionsToClips(client Client, clips []*Clip, allowAI for langCode := range languages.LanguagesByISO { // There are also videos with .txt subs... we should support those at some point - shape := clipShapes.GetShape(fmt.Sprintf("sub_%s_srt", langCode)) + shape := clipShapes.GetShape(vsapi.SubtitleShapeTag(langCode)) if shape == nil || shape.GetPath() == "" { continue } @@ -585,14 +585,14 @@ func addSubtitlesAndTranscriptionsToClips(client Client, clips []*Clip, allowAI if len(clip.SubtitleFiles) == 0 && allowAI { // We have no subtitles, so we fall back to transcriptions - shape := clipShapes.GetShape("Transcribed_Subtitle_SRT") + shape := clipShapes.GetShape(vsapi.ShapeTagTranscribedSubtitleSRT) if shape != nil && shape.GetPath() != "" { clip.SubtitleFiles["und"] = shape.GetPath() allSubLanguages["und"] = struct{}{} } } - shape := clipShapes.GetShape("transcription_json") + shape := clipShapes.GetShape(vsapi.ShapeTagTranscriptionJSON) if shape != nil { clip.JSONTranscriptFile = shape.GetPath() } diff --git a/services/vidispine/vsapi/job_status.go b/services/vidispine/vsapi/job_status.go new file mode 100644 index 00000000..2da459a0 --- /dev/null +++ b/services/vidispine/vsapi/job_status.go @@ -0,0 +1,58 @@ +package vsapi + +import ( + "github.com/bcc-code/bcc-media-flows/internal/enumjson" + "github.com/orsinium-labs/enum" +) + +// JobStatus is the state Vidispine reports for a job. The members are the +// states Vidispine documents; a state it adds later still decodes and simply +// compares unequal to every named member, so it is treated as terminal. +type JobStatus enum.Member[string] + +var ( + JobStatusNone = JobStatus{Value: "NONE"} + JobStatusReady = JobStatus{Value: "READY"} + JobStatusStarted = JobStatus{Value: "STARTED"} + JobStatusWaiting = JobStatus{Value: "WAITING"} + JobStatusFinished = JobStatus{Value: "FINISHED"} + JobStatusFinishedWarning = JobStatus{Value: "FINISHED_WARNING"} + JobStatusFailedTotal = JobStatus{Value: "FAILED_TOTAL"} + JobStatusAbortedPending = JobStatus{Value: "ABORTED_PENDING"} + JobStatusAborted = JobStatus{Value: "ABORTED"} + JobStatusDisappeared = JobStatus{Value: "DISAPPEARED"} + JobStatusVidinetJob = JobStatus{Value: "VIDINET_JOB"} + JobStatuses = enum.New( + JobStatusNone, + JobStatusReady, + JobStatusStarted, + JobStatusWaiting, + JobStatusFinished, + JobStatusFinishedWarning, + JobStatusFailedTotal, + JobStatusAbortedPending, + JobStatusAborted, + JobStatusDisappeared, + JobStatusVidinetJob, + ) +) + +// InProgress reports whether Vidispine is still working on the job, i.e. it +// is queued, waiting or running. Every other state is final. +func (s JobStatus) InProgress() bool { + return s == JobStatusStarted || s == JobStatusReady || s == JobStatusWaiting +} + +func (s JobStatus) String() string { + return s.Value +} + +//goland:noinspection GoMixedReceiverTypes +func (s JobStatus) MarshalJSON() ([]byte, error) { + return enumjson.Marshal(s) +} + +//goland:noinspection GoMixedReceiverTypes +func (s *JobStatus) UnmarshalJSON(data []byte) error { + return enumjson.UnmarshalOpen(data, s) +} diff --git a/services/vidispine/vsapi/jobs.go b/services/vidispine/vsapi/jobs.go index 8efc3a53..16ef666b 100644 --- a/services/vidispine/vsapi/jobs.go +++ b/services/vidispine/vsapi/jobs.go @@ -56,10 +56,10 @@ func (c *Client) GetJob(jobID string) (*JobDocument, error) { } type JobDocument struct { - JobID string `json:"jobId"` - User string `json:"user"` - Started *string `json:"started"` - Finished *string `json:"finished"` - Status string `json:"status"` - Type string `json:"type"` + JobID string `json:"jobId"` + User string `json:"user"` + Started *string `json:"started"` + Finished *string `json:"finished"` + Status JobStatus `json:"status"` + Type string `json:"type"` } diff --git a/services/vidispine/vsapi/shape_tags.go b/services/vidispine/vsapi/shape_tags.go new file mode 100644 index 00000000..36bec4d3 --- /dev/null +++ b/services/vidispine/vsapi/shape_tags.go @@ -0,0 +1,49 @@ +package vsapi + +import ( + "github.com/bcc-code/bcc-media-flows/internal/enumjson" + "github.com/orsinium-labs/enum" +) + +// ShapeTag names a Vidispine shape-tag. The members below are the fixed tags +// this codebase reads or writes; Vidispine also carries per-language tags +// (see SubtitleShapeTag and languages' MBPreviewTag), so the set is open and +// an unlisted tag still decodes. +type ShapeTag enum.Member[string] + +var ( + ShapeTagOriginal = ShapeTag{Value: "original"} + ShapeTagLowres = ShapeTag{Value: "lowres"} + ShapeTagLowresWatermarked = ShapeTag{Value: "lowres_watermarked"} + ShapeTagLowAudio = ShapeTag{Value: "lowaudio"} + ShapeTagTranscriptionJSON = ShapeTag{Value: "transcription_json"} + ShapeTagTranscribedSubtitleSRT = ShapeTag{Value: "Transcribed_Subtitle_SRT"} + ShapeTags = enum.New( + ShapeTagOriginal, + ShapeTagLowres, + ShapeTagLowresWatermarked, + ShapeTagLowAudio, + ShapeTagTranscriptionJSON, + ShapeTagTranscribedSubtitleSRT, + ) +) + +// SubtitleShapeTag is the tag of the SRT subtitle shape for an ISO-639-2 +// language code, e.g. "sub_nor_srt". +func SubtitleShapeTag(lang string) ShapeTag { + return ShapeTag{Value: "sub_" + lang + "_srt"} +} + +func (t ShapeTag) String() string { + return t.Value +} + +//goland:noinspection GoMixedReceiverTypes +func (t ShapeTag) MarshalJSON() ([]byte, error) { + return enumjson.Marshal(t) +} + +//goland:noinspection GoMixedReceiverTypes +func (t *ShapeTag) UnmarshalJSON(data []byte) error { + return enumjson.UnmarshalOpen(data, t) +} diff --git a/services/vidispine/vsapi/shapes.go b/services/vidispine/vsapi/shapes.go index b8ad2e45..7935b1a8 100644 --- a/services/vidispine/vsapi/shapes.go +++ b/services/vidispine/vsapi/shapes.go @@ -166,7 +166,7 @@ func (c *Client) GetResolutions(itemVXID string) ([]Resolution, error) { return nil, err } - shape := shapes.GetShape("original") + shape := shapes.GetShape(ShapeTagOriginal) if shape == nil { return nil, errors.New("no original shape found") } @@ -220,9 +220,9 @@ func (c *Client) GetResolutions(itemVXID string) ([]Resolution, error) { return qualities, nil } -func (sr ShapeResult) GetShape(tag string) *Shape { +func (sr ShapeResult) GetShape(tag ShapeTag) *Shape { for _, s := range sr.Shape { - if lo.Contains(s.Tag, tag) { + if lo.Contains(s.Tag, tag.Value) { return &s } } diff --git a/services/vidispine/vsapi/shapes_test.go b/services/vidispine/vsapi/shapes_test.go index c2f3819e..a15c6fbe 100644 --- a/services/vidispine/vsapi/shapes_test.go +++ b/services/vidispine/vsapi/shapes_test.go @@ -58,6 +58,6 @@ func Test_GetPath(t *testing.T) { }, } - path := sr.GetShape("tag1").GetPath() + path := sr.GetShape(ShapeTag{Value: "tag1"}).GetPath() assert.Equal(t, "/path/to/file", path) } diff --git a/utils/workflows/vidispine.go b/utils/workflows/vidispine.go index 5af9502c..e89fce3a 100644 --- a/utils/workflows/vidispine.go +++ b/utils/workflows/vidispine.go @@ -17,7 +17,7 @@ func WaitForVidispineJob(ctx workflow.Context, jobID string) error { BackoffCoefficient: 1.5, InitialInterval: 30 * time.Second, MaximumInterval: 300 * time.Second, - NonRetryableErrorTypes: []string{"JOB_FAILED"}, + NonRetryableErrorTypes: []string{vsactivity.JobFailedErrorType}, } ctx = workflow.WithActivityOptions(ctx, options) return Execute(ctx, activities.Vidispine.JobCompleteOrErr, vsactivity.WaitForJobCompletionParams{ diff --git a/workflows/export/shorts.go b/workflows/export/shorts.go index a8f56f89..b181aadf 100644 --- a/workflows/export/shorts.go +++ b/workflows/export/shorts.go @@ -121,7 +121,7 @@ func ExportShort(ctx workflow.Context, short *ShortsData) error { res, err := wfutils.Execute(ctx, activities.Vidispine.GetFileFromVXActivity, vsactivity.GetFileFromVXParams{ VXID: short.MBMetadata.ID, - Tags: []string{"original"}, + Tags: []vsapi.ShapeTag{vsapi.ShapeTagOriginal}, }).Result(ctx) if err != nil { return err @@ -134,7 +134,7 @@ func ExportShort(ctx workflow.Context, short *ShortsData) error { return fmt.Errorf("failed to generate thumbnail: %w", err) } - _, styledImage, err := uploadImage(ctx, activities.Directus.ShortsFolderID, true, "poster", thumb) + _, styledImage, err := uploadImage(ctx, activities.Directus.ShortsFolderID, true, directus.ImageStylePoster, thumb) if err != nil { return fmt.Errorf("failed to upload thumbnail: %w", err) } @@ -260,7 +260,7 @@ func createShortInPlatform(ctx workflow.Context, short *ShortsData, styledImage // Create media item mediaItemResult, err := wfutils.Execute(ctx, activities.Directus.CreateMediaItem, activities.CreateMediaItemInput{ Label: label, - Type: "short", + Type: directus.MediaItemTypeShort, AssetID: assetID, Title: "", ParentEpisodeID: episodeID, @@ -288,7 +288,7 @@ func createShortInPlatform(ctx workflow.Context, short *ShortsData, styledImage // Create short shortResult, err := wfutils.Execute(ctx, activities.Directus.CreateShort, activities.CreateShortInput{ MediaItemID: mediaItemResult.ID, - Status: "draft", + Status: directus.ShortStatusDraft, }).Result(ctx) if err != nil { @@ -405,7 +405,7 @@ func convertToSeconds(timeStr string) (*int64, error) { return &totalSeconds, nil } -func uploadImage(ctx workflow.Context, directusFolderID string, createStyledImages bool, imageStyle string, image paths.Path) (*directus.File, *directus.StyledImage, error) { +func uploadImage(ctx workflow.Context, directusFolderID string, createStyledImages bool, imageStyle directus.ImageStyle, image paths.Path) (*directus.File, *directus.StyledImage, error) { if !strings.HasSuffix(image.Ext(), ".jpg") { return nil, nil, fmt.Errorf("invalid image extension: %s", image.Ext()) } @@ -420,7 +420,7 @@ func uploadImage(ctx workflow.Context, directusFolderID string, createStyledImag return nil, nil, err } - if createStyledImages && imageStyle != "" { + if createStyledImages && imageStyle.Value != "" { styledImage, err := wfutils.Execute(ctx, activities.Directus.CreateStyledImage, activities.CreateStyledImageInput{ ImageID: res.ID, Style: imageStyle, diff --git a/workflows/ingest/bmm_simple_upload.go b/workflows/ingest/bmm_simple_upload.go index cc530bfe..eb267d78 100644 --- a/workflows/ingest/bmm_simple_upload.go +++ b/workflows/ingest/bmm_simple_upload.go @@ -2,6 +2,7 @@ package ingestworkflows import ( "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "strconv" "github.com/bcc-code/bcc-media-flows/activities" @@ -53,7 +54,7 @@ func BmmIngestUpload(ctx workflow.Context, params BmmSimpleUploadParams) (*BmmSi return nil, err } - res, err := ImportFileAsTag(ctx, "original", newPath, "BMM-"+strconv.Itoa(params.TrackID)+" "+params.Language+" - "+params.Title) + res, err := ImportFileAsTag(ctx, vsapi.ShapeTagOriginal, newPath, "BMM-"+strconv.Itoa(params.TrackID)+" "+params.Language+" - "+params.Title) if err != nil { wfutils.SendTelegramError(ctx, telegram.ChatBMM, "", err) return nil, err @@ -172,7 +173,7 @@ func deliverToSSF(ctx workflow.Context, assetID string, wavPath paths.Path, para // Get the transcription JSON from Vidispine transcriptResult, err := wfutils.Execute(ctx, activities.Vidispine.GetFileFromVXActivity, vsactivity.GetFileFromVXParams{ VXID: assetID, - Tags: []string{"transcription_json"}, + Tags: []vsapi.ShapeTag{vsapi.ShapeTagTranscriptionJSON}, }).Result(ctx) if err != nil { logger.Warn("Failed to get transcription JSON from Vidispine", "error", err) diff --git a/workflows/ingest/bmm_track_metadata.go b/workflows/ingest/bmm_track_metadata.go index 43b8b39b..dd243b09 100644 --- a/workflows/ingest/bmm_track_metadata.go +++ b/workflows/ingest/bmm_track_metadata.go @@ -3,6 +3,7 @@ package ingestworkflows import ( "errors" "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "net/url" "path" "strconv" @@ -128,7 +129,7 @@ func BmmTrackMetadata(ctx workflow.Context, params BmmTrackMetadataParams) (*Bmm } title := fmt.Sprintf("BMM-%d %s - %s", params.BmmTrackID, params.Language, params.Title) - res, err := ImportFileAsTag(ctx, "original", newPath, title) + res, err := ImportFileAsTag(ctx, vsapi.ShapeTagOriginal, newPath, title) if err != nil { wfutils.SendTelegramError(ctx, telegram.ChatBMM, "", err) return nil, err diff --git a/workflows/ingest/common.go b/workflows/ingest/common.go index 3d908a60..f1630b8c 100644 --- a/workflows/ingest/common.go +++ b/workflows/ingest/common.go @@ -2,6 +2,7 @@ package ingestworkflows import ( "errors" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "strconv" "github.com/bcc-code/bcc-media-flows/services/emails" @@ -25,10 +26,10 @@ type ImportTagResult struct { // FilePath and ShapeTag are retained so WaitForImportTag can re-trigger the // import on JOB_FAILED. FilePath paths.Path - ShapeTag string + ShapeTag vsapi.ShapeTag } -func ImportFileAsTag(ctx workflow.Context, tag string, path paths.Path, title string) (*ImportTagResult, error) { +func ImportFileAsTag(ctx workflow.Context, tag vsapi.ShapeTag, path paths.Path, title string) (*ImportTagResult, error) { result, err := wfutils.Execute(ctx, activities.Vidispine.CreatePlaceholderActivity, vsactivity.CreatePlaceholderParams{ Title: title, }).Result(ctx) @@ -67,7 +68,7 @@ func WaitForImportTag(ctx workflow.Context, result *ImportTagResult) error { } var appErr *temporal.ApplicationError - if !errors.As(err, &appErr) || appErr.Type() != "JOB_FAILED" { + if !errors.As(err, &appErr) || appErr.Type() != vsactivity.JobFailedErrorType { return err } diff --git a/workflows/ingest/import_audio_from_reaper.go b/workflows/ingest/import_audio_from_reaper.go index fb68e8cd..7a534c52 100644 --- a/workflows/ingest/import_audio_from_reaper.go +++ b/workflows/ingest/import_audio_from_reaper.go @@ -2,6 +2,7 @@ package ingestworkflows import ( "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "strconv" "strings" "time" @@ -187,7 +188,7 @@ func doImportAudioFileFromReaper(ctx workflow.Context, params ImportAudioFileFro getFileResult, err := wfutils.Execute(ctx, activities.Vidispine.GetFileFromVXActivity, vsactivity.GetFileFromVXParams{ VXID: params.VideoVXID, - Tags: []string{"original"}, + Tags: []vsapi.ShapeTag{vsapi.ShapeTagOriginal}, }).Result(ctx) if err != nil { return err diff --git a/workflows/ingest/import_subtitles.go b/workflows/ingest/import_subtitles.go index af9175ef..c3ddd3ab 100644 --- a/workflows/ingest/import_subtitles.go +++ b/workflows/ingest/import_subtitles.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "strings" vsactivity "github.com/bcc-code/bcc-media-flows/activities/vidispine" @@ -142,7 +143,7 @@ func ImportSubtitles(ctx workflow.Context, input ImportSubtitlesInput) error { vsactivity.ImportFileAsShapeParams{ AssetID: input.VXID, FilePath: srtFilePath, - ShapeTag: "Transcribed_Subtitle_SRT", + ShapeTag: vsapi.ShapeTagTranscribedSubtitleSRT, Replace: true, }) @@ -151,7 +152,7 @@ func ImportSubtitles(ctx workflow.Context, input ImportSubtitlesInput) error { vsactivity.ImportFileAsShapeParams{ AssetID: input.VXID, FilePath: jsonFilePath, - ShapeTag: "transcription_json", + ShapeTag: vsapi.ShapeTagTranscriptionJSON, Replace: true, }) diff --git a/workflows/ingest/incremental_ingest.go b/workflows/ingest/incremental_ingest.go index c466a5c2..de940572 100644 --- a/workflows/ingest/incremental_ingest.go +++ b/workflows/ingest/incremental_ingest.go @@ -3,6 +3,7 @@ package ingestworkflows import ( "errors" "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "path/filepath" "strings" "time" @@ -299,7 +300,7 @@ func startGrowingPreview(ctx workflow.Context, rawPath paths.Path, videoVXID str lowresImportJob, importErr := wfutils.Execute(ctx, activities.Vidispine.ImportFileAsShapeActivity, vsactivity.ImportFileAsShapeParams{ AssetID: videoVXID, FilePath: previewPath, - ShapeTag: "lowres_watermarked", + ShapeTag: vsapi.ShapeTagLowresWatermarked, Growing: true, Replace: false, }).Result(ctx) diff --git a/workflows/ingest/masters.go b/workflows/ingest/masters.go index 146c4b97..b038dfef 100644 --- a/workflows/ingest/masters.go +++ b/workflows/ingest/masters.go @@ -3,6 +3,7 @@ package ingestworkflows import ( "errors" "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "regexp" "strconv" "strings" @@ -64,7 +65,7 @@ func processMaster(ctx workflow.Context, sourceFile paths.Path, destinationFile return "", err } - result, err := ImportFileAsTag(ctx, "original", destinationFile, destinationFile.Base()) + result, err := ImportFileAsTag(ctx, vsapi.ShapeTagOriginal, destinationFile, destinationFile.Base()) if err != nil { return "", err } diff --git a/workflows/ingest/masters_test.go b/workflows/ingest/masters_test.go index c6dc8709..c23cb873 100644 --- a/workflows/ingest/masters_test.go +++ b/workflows/ingest/masters_test.go @@ -89,7 +89,7 @@ func (s *UnitTestSuite) Test_VBBulk_MasterFlow() { s.env.OnActivity(activities.Vidispine.ImportFileAsShapeActivity, mock.Anything, vsactivity.ImportFileAsShapeParams{ AssetID: "VBBulk1", FilePath: paths.MustParse("./testdata/generated/VBBulk_output/VBBulk1.mxf"), - ShapeTag: "original", + ShapeTag: vsapi.ShapeTagOriginal, Growing: false, Replace: false, }).Once().Return(nil, nil) @@ -97,7 +97,7 @@ func (s *UnitTestSuite) Test_VBBulk_MasterFlow() { s.env.OnActivity(activities.Vidispine.ImportFileAsShapeActivity, mock.Anything, vsactivity.ImportFileAsShapeParams{ AssetID: "VBBulk2", FilePath: paths.MustParse("./testdata/generated/VBBulk_output/VBBulk2.mxf"), - ShapeTag: "original", + ShapeTag: vsapi.ShapeTagOriginal, Growing: false, Replace: false, }).Once().Return(nil, nil) diff --git a/workflows/ingest/mu1_mu2_extract.go b/workflows/ingest/mu1_mu2_extract.go index cce12f87..16aaf4d7 100644 --- a/workflows/ingest/mu1_mu2_extract.go +++ b/workflows/ingest/mu1_mu2_extract.go @@ -3,6 +3,7 @@ package ingestworkflows import ( "errors" "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "strings" "time" @@ -28,12 +29,12 @@ func ExtractAudioFromMU1MU2(ctx workflow.Context, input ExtractAudioFromMU1MU2In // Get paths to the original files MU1FileFuture := wfutils.Execute(ctx, activities.Vidispine.GetFileFromVXActivity, vsactivity.GetFileFromVXParams{ VXID: input.MU1ID, - Tags: []string{"original"}, + Tags: []vsapi.ShapeTag{vsapi.ShapeTagOriginal}, }) MU2FileFuture := wfutils.Execute(ctx, activities.Vidispine.GetFileFromVXActivity, vsactivity.GetFileFromVXParams{ VXID: input.MU2ID, - Tags: []string{"original"}, + Tags: []vsapi.ShapeTag{vsapi.ShapeTagOriginal}, }) Mu1Result, err := MU1FileFuture.Result(ctx) diff --git a/workflows/ingest/multitrack.go b/workflows/ingest/multitrack.go index 7e4fcb1b..34112e8d 100644 --- a/workflows/ingest/multitrack.go +++ b/workflows/ingest/multitrack.go @@ -3,6 +3,7 @@ package ingestworkflows import ( "errors" "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "sort" "github.com/bcc-code/bcc-media-flows/activities" @@ -83,7 +84,7 @@ func Multitrack(ctx workflow.Context, params MasterParams) (*MasterResult, error base := files[0].Base() fileName := base[:len(base)-len(muxResult.OutputPath.Ext())] - result, err := ImportFileAsTag(ctx, "original", muxResult.OutputPath, fileName) + result, err := ImportFileAsTag(ctx, vsapi.ShapeTagOriginal, muxResult.OutputPath, fileName) if err != nil { return nil, err } diff --git a/workflows/ingest/raw_material.go b/workflows/ingest/raw_material.go index 9f9f5064..6c00bffc 100644 --- a/workflows/ingest/raw_material.go +++ b/workflows/ingest/raw_material.go @@ -3,6 +3,7 @@ package ingestworkflows import ( "fmt" "github.com/bcc-code/bcc-media-flows/services/rclone" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "strings" "github.com/bcc-code/bcc-media-flows/activities" @@ -102,7 +103,7 @@ func RawMaterial(ctx workflow.Context, params RawMaterialParams) (map[string]pat imported := map[string]paths.Path{} for _, file := range files { var result *ImportTagResult - result, err = ImportFileAsTag(ctx, "original", file, file.Base()) + result, err = ImportFileAsTag(ctx, vsapi.ShapeTagOriginal, file, file.Base()) if err != nil { return imported, err } diff --git a/workflows/ingest/sync_fix.go b/workflows/ingest/sync_fix.go index 3def15b5..97230bdc 100644 --- a/workflows/ingest/sync_fix.go +++ b/workflows/ingest/sync_fix.go @@ -3,6 +3,7 @@ package ingestworkflows import ( "errors" "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" vsactivity "github.com/bcc-code/bcc-media-flows/activities/vidispine" "github.com/bcc-code/bcc-media-flows/common" @@ -135,7 +136,7 @@ func calculateAudioAdjustment(ctx workflow.Context, vxID string, audioPaths map[ return 0, err } - originalShape := shapes.GetShape("original") + originalShape := shapes.GetShape(vsapi.ShapeTagOriginal) if originalShape == nil { return 0, errors.New("original shape not found") } diff --git a/workflows/misc/fix_duration.go b/workflows/misc/fix_duration.go index 98bf9d88..b876eaf2 100644 --- a/workflows/misc/fix_duration.go +++ b/workflows/misc/fix_duration.go @@ -2,6 +2,7 @@ package miscworkflows import ( "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "github.com/bcc-code/bcc-media-flows/activities" vsactivity "github.com/bcc-code/bcc-media-flows/activities/vidispine" @@ -27,7 +28,7 @@ func FixDurationVX( // Get the original file from Vidispine originalFile, err := wfutils.Execute(ctx, activities.Vidispine.GetFileFromVXActivity, vsactivity.GetFileFromVXParams{ VXID: params.VXID, - Tags: []string{"original"}, + Tags: []vsapi.ShapeTag{vsapi.ShapeTagOriginal}, }).Result(ctx) if err != nil { return fmt.Errorf("failed to get original file: %w", err) diff --git a/workflows/misc/import_subs.go b/workflows/misc/import_subs.go index 0fd6976b..b6e04187 100644 --- a/workflows/misc/import_subs.go +++ b/workflows/misc/import_subs.go @@ -2,6 +2,7 @@ package miscworkflows import ( "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "strings" "github.com/bcc-code/bcc-media-flows/services/telegram" @@ -77,7 +78,7 @@ func doImportSubtitlesFromSubtrans(ctx workflow.Context, params ImportSubtitlesF jobRes, err := wfutils.Execute(ctx, activities.Vidispine.ImportFileAsShapeActivity, vsactivity.ImportFileAsShapeParams{ AssetID: params.VXID, FilePath: sub, - ShapeTag: fmt.Sprintf("sub_%s_%s", lang, "srt"), + ShapeTag: vsapi.SubtitleShapeTag(lang), Replace: true, }).Result(ctx) diff --git a/workflows/misc/merge_import_subs.go b/workflows/misc/merge_import_subs.go index 18a1b790..a606e17c 100644 --- a/workflows/misc/merge_import_subs.go +++ b/workflows/misc/merge_import_subs.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/csv" "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "strings" "time" @@ -129,7 +130,7 @@ func MergeAndImportSubtitlesFromCSV(ctx workflow.Context, params MergeAndImportS jobRes, err := wfutils.Execute(ctx, activities.Vidispine.ImportFileAsShapeActivity, vsactivity.ImportFileAsShapeParams{ AssetID: params.TargetVXID, FilePath: sub, - ShapeTag: fmt.Sprintf("sub_%s_%s", lang, "srt"), + ShapeTag: vsapi.SubtitleShapeTag(lang), Replace: true, }).Result(ctx) diff --git a/workflows/misc/slow_move_files.go b/workflows/misc/slow_move_files.go index 94427a19..694a100c 100644 --- a/workflows/misc/slow_move_files.go +++ b/workflows/misc/slow_move_files.go @@ -7,6 +7,7 @@ import ( "github.com/bcc-code/bcc-media-flows/activities/cantemo" vsactivity "github.com/bcc-code/bcc-media-flows/activities/vidispine" "github.com/bcc-code/bcc-media-flows/environment" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" wfutils "github.com/bcc-code/bcc-media-flows/utils/workflows" "go.temporal.io/sdk/client" "go.temporal.io/sdk/temporal" @@ -193,7 +194,7 @@ func MoveFilesWorkerFlow(ctx workflow.Context) error { } for _, shapeTag := range msg.Shapes { - s := meta.GetShape(shapeTag) + s := meta.GetShape(vsapi.ShapeTag{Value: shapeTag}) if s == nil { workflow.GetLogger(ctx).Debug("No shape found for tag", "tag", shapeTag, "vxid", msg.VXID) diff --git a/workflows/misc/transcode_preview-vx.go b/workflows/misc/transcode_preview-vx.go index b138751a..291544ec 100644 --- a/workflows/misc/transcode_preview-vx.go +++ b/workflows/misc/transcode_preview-vx.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "github.com/bcc-code/bcc-media-flows/languages" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "github.com/samber/lo" "path/filepath" "strings" @@ -46,7 +47,7 @@ func TranscodePreviewVX( ctx = workflow.WithActivityOptions(ctx, wfutils.GetDefaultActivityOptions()) shapes, err := wfutils.Execute(ctx, activities.Vidispine.GetFileFromVXActivity, vsactivity.GetFileFromVXParams{ - Tags: []string{"original"}, + Tags: []vsapi.ShapeTag{vsapi.ShapeTagOriginal}, VXID: params.VXID, }).Result(ctx) @@ -73,11 +74,9 @@ func TranscodePreviewVX( return err } - var shapeTag string + shapeTag := vsapi.ShapeTagLowresWatermarked if previewResponse.AudioOnly { - shapeTag = "lowaudio" - } else { - shapeTag = "lowres_watermarked" + shapeTag = vsapi.ShapeTagLowAudio } err = wfutils.Execute(ctx, activities.Vidispine.ImportFileAsShapeActivity, @@ -108,14 +107,14 @@ func TranscodePreviewVX( vsactivity.ImportFileAsShapeParams{ AssetID: params.VXID, FilePath: p, - ShapeTag: tag, + ShapeTag: vsapi.ShapeTag{Value: tag}, }).Wait(ctx) if iterErr != nil { // A shape-tag that isn't configured in Vidispine is expected for some // languages; skip it quietly and continue with the rest instead of // treating it as a failure that alerts. var appErr *temporal.ApplicationError - if errors.As(iterErr, &appErr) && appErr.Type() == "VS_SHAPE_TAG_NOT_FOUND" { + if errors.As(iterErr, &appErr) && appErr.Type() == vsactivity.ShapeTagNotFoundErrorType { logger.Info("Skipping audio preview for unconfigured shape-tag", "language", l, "tag", tag, "vxid", params.VXID) continue diff --git a/workflows/misc/transcribe-vx.go b/workflows/misc/transcribe-vx.go index 68bd8ef1..2105299e 100644 --- a/workflows/misc/transcribe-vx.go +++ b/workflows/misc/transcribe-vx.go @@ -3,6 +3,7 @@ package miscworkflows import ( "errors" "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "github.com/bcc-code/bcc-media-flows/services/telegram" @@ -34,7 +35,7 @@ func TranscribeVX( ctx = workflow.WithActivityOptions(ctx, wfutils.GetDefaultActivityOptions()) shapes, err := wfutils.Execute(ctx, activities.Vidispine.GetFileFromVXActivity, vsactivity.GetFileFromVXParams{ - Tags: []string{"lowres", "lowres_watermarked", "lowaudio", "original"}, + Tags: []vsapi.ShapeTag{vsapi.ShapeTagLowres, vsapi.ShapeTagLowresWatermarked, vsapi.ShapeTagLowAudio, vsapi.ShapeTagOriginal}, VXID: params.VXID, }).Result(ctx) @@ -78,7 +79,7 @@ func TranscribeVX( vsactivity.ImportFileAsShapeParams{ AssetID: params.VXID, FilePath: transcriptionJob.JSONPath, - ShapeTag: "transcription_json", + ShapeTag: vsapi.ShapeTagTranscriptionJSON, Replace: true, }) @@ -86,7 +87,7 @@ func TranscribeVX( vsactivity.ImportFileAsShapeParams{ AssetID: params.VXID, FilePath: transcriptionJob.SRTPath, - ShapeTag: "Transcribed_Subtitle_SRT", + ShapeTag: vsapi.ShapeTagTranscribedSubtitleSRT, Replace: true, }) diff --git a/workflows/misc/watch_folder_transcode.go b/workflows/misc/watch_folder_transcode.go index de142595..13ef579b 100644 --- a/workflows/misc/watch_folder_transcode.go +++ b/workflows/misc/watch_folder_transcode.go @@ -18,12 +18,12 @@ import ( type WatchFolderTranscodeInput struct { Path string - FolderName string + FolderName common.WatchFolder } // watchFolderEncodes are the folders whose transcode is one activity call. // FilePath and OutputDir are filled in per run. -var watchFolderEncodes = map[string]struct { +var watchFolderEncodes = map[common.WatchFolder]struct { activity func(context.Context, activities.EncodeParams) (*activities.EncodeResult, error) params activities.EncodeParams }{ diff --git a/workflows/misc/watch_folder_transcode_test.go b/workflows/misc/watch_folder_transcode_test.go new file mode 100644 index 00000000..6e259a31 --- /dev/null +++ b/workflows/misc/watch_folder_transcode_test.go @@ -0,0 +1,38 @@ +package miscworkflows + +import ( + "encoding/json" + "testing" + + "github.com/bcc-code/bcc-media-flows/common" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Every watch folder must be handled: either as a one-activity encode in +// watchFolderEncodes or by a dedicated branch in WatchFolderTranscode. +func TestEveryWatchFolderIsHandled(t *testing.T) { + branches := map[common.WatchFolder]bool{ + common.FolderTranscribe: true, + common.FolderHAP50FPS: true, + } + for _, folder := range common.WatchFolders.Members() { + _, encoded := watchFolderEncodes[folder] + assert.True(t, encoded || branches[folder], "watch folder %q is not handled", folder.Value) + } +} + +// The input has to keep decoding histories recorded while FolderName was a +// plain string, and reject folder names nothing handles. +func TestWatchFolderTranscodeInput_JSONRoundTrip(t *testing.T) { + var in WatchFolderTranscodeInput + require.NoError(t, json.Unmarshal([]byte(`{"Path":"/x/in/a.mov","FolderName":"ProRes444_4K-25FPS"}`), &in)) + assert.Equal(t, common.FolderProRes4444K25FPS, in.FolderName) + + out, err := json.Marshal(in) + require.NoError(t, err) + assert.JSONEq(t, `{"Path":"/x/in/a.mov","FolderName":"ProRes444_4K-25FPS"}`, string(out)) + + err = json.Unmarshal([]byte(`{"Path":"/x/in/a.mov","FolderName":"DNxHD"}`), &in) + assert.ErrorIs(t, err, common.ErrUnknownWatchFolder) +} diff --git a/workflows/vb_export/vb_export.go b/workflows/vb_export/vb_export.go index 432df2aa..3cdce8d2 100644 --- a/workflows/vb_export/vb_export.go +++ b/workflows/vb_export/vb_export.go @@ -3,6 +3,7 @@ package vb_export import ( "errors" "fmt" + "github.com/bcc-code/bcc-media-flows/services/vidispine/vsapi" "path/filepath" "strings" "time" @@ -11,6 +12,7 @@ import ( "github.com/bcc-code/bcc-media-flows/activities" "github.com/bcc-code/bcc-media-flows/environment" + "github.com/bcc-code/bcc-media-flows/internal/enumjson" "github.com/bcc-code/bcc-media-flows/paths" "github.com/orsinium-labs/enum" "github.com/samber/lo" @@ -46,9 +48,27 @@ var ( DestinationXDCAM, DestinationCasparCG, ) - deliveryFolder = paths.New(paths.BrunstadDrive, "/Delivery/FraMB/") + deliveryFolder = paths.New(paths.BrunstadDrive, "/Delivery/FraMB/") + ErrUnknownDestination = errors.New("unknown VB export destination") ) +func (d Destination) String() string { + return d.Value +} + +// MarshalJSON writes the bare destination name, so payloads look the same as +// they did while Destinations was a []string. +// +//goland:noinspection GoMixedReceiverTypes +func (d Destination) MarshalJSON() ([]byte, error) { + return enumjson.Marshal(d) +} + +//goland:noinspection GoMixedReceiverTypes +func (d *Destination) UnmarshalJSON(data []byte) error { + return enumjson.UnmarshalStrict(data, Destinations, d, ErrUnknownDestination) +} + var destinationDescriptions = map[Destination]string{ DestinationAbekas: "Videoavspilling i bussen", DestinationRawAbekas: "Videoavspilling i bussen (originalfil overføres)", @@ -114,7 +134,7 @@ var ( type VBExportParams struct { VXID string - Destinations []string + Destinations []Destination SubtitleShapeTag string SubtitleStyle string } @@ -159,13 +179,10 @@ func VBExport(ctx workflow.Context, params VBExportParams) ([]wfutils.ResultOrEr return nil, errors.New("vxid is required") } - var destinations []*Destination for _, dest := range params.Destinations { - d := Destinations.Parse(dest) - if d == nil { - return nil, fmt.Errorf("invalid destination: %s", dest) + if _, ok := destinationWorkflows[dest]; !ok { + return nil, fmt.Errorf("%w: %s", ErrUnknownDestination, dest) } - destinations = append(destinations, d) } shapes, err := wfutils.Execute(ctx, activities.Vidispine.GetShapes, avidispine.VXOnlyParam{ @@ -181,14 +198,14 @@ func VBExport(ctx workflow.Context, params VBExportParams) ([]wfutils.ResultOrEr return nil, fmt.Errorf("no clips found for VXID %s", params.VXID) } - videoShape := shapes.GetShape("original") + videoShape := shapes.GetShape(vsapi.ShapeTagOriginal) if videoShape == nil { return nil, fmt.Errorf("no original shape found for item %s", params.VXID) } wfutils.SendTelegramText(ctx, telegram.ChatOslofjord, fmt.Sprintf("🟦 VB Export of %s - `%s` started.\nDestination(s): `%s`\n\nRunID: %s", - params.VXID, filepath.Base(videoShape.GetPath()), strings.Join(params.Destinations, ", "), workflow.GetInfo(ctx).OriginalRunID, + params.VXID, filepath.Base(videoShape.GetPath()), strings.Join(lo.Map(params.Destinations, func(d Destination, _ int) string { return d.Value }), ", "), workflow.GetInfo(ctx).OriginalRunID, ), ) @@ -214,8 +231,8 @@ func VBExport(ctx workflow.Context, params VBExportParams) ([]wfutils.ResultOrEr return nil, err } - destinationsWithAudioOutput := lo.Filter(destinations, func(dest *Destination, _ int) bool { - return *dest != DestinationCasparCG + destinationsWithAudioOutput := lo.Filter(params.Destinations, func(dest Destination, _ int) bool { + return dest != DestinationCasparCG }) if len(destinationsWithAudioOutput) > 0 && analyzeResult.HasAudio && len(analyzeResult.AudioStreams) <= 2 { @@ -257,7 +274,7 @@ func VBExport(ctx workflow.Context, params VBExportParams) ([]wfutils.ResultOrEr } var resultFutures []workflow.Future - for _, dest := range destinations { + for _, dest := range params.Destinations { childParams := VBExportChildWorkflowParams{ ParentParams: params, OriginalFilenameWithoutExt: originalFilenameWithoutExt, @@ -271,18 +288,13 @@ func VBExport(ctx workflow.Context, params VBExportParams) ([]wfutils.ResultOrEr AnalyzeResult: *analyzeResult, } - w, ok := destinationWorkflows[*dest] - if !ok { - return nil, fmt.Errorf("destination not implemented: %s", dest) - } - err = wfutils.CreateFolder(ctx, childParams.OutputDir) if err != nil { return nil, err } ctx = workflow.WithChildOptions(ctx, wfutils.GetVXDefaultWorkflowOptions(ctx, params.VXID)) - future := workflow.ExecuteChildWorkflow(ctx, w, childParams) + future := workflow.ExecuteChildWorkflow(ctx, destinationWorkflows[dest], childParams) resultFutures = append(resultFutures, future) } diff --git a/workflows/vb_export/vb_export_test.go b/workflows/vb_export/vb_export_test.go index 003ae065..8de28435 100644 --- a/workflows/vb_export/vb_export_test.go +++ b/workflows/vb_export/vb_export_test.go @@ -1,6 +1,8 @@ package vb_export import ( + "encoding/json" + "errors" "strings" "testing" @@ -37,7 +39,7 @@ func (s *VBExportTestSuite) Test_VBExport_EmptyVXID() { s.env.ExecuteWorkflow(VBExport, VBExportParams{ VXID: "", - Destinations: []string{"xdcam"}, + Destinations: []Destination{DestinationXDCAM}, }) s.True(s.env.IsWorkflowCompleted()) err := s.env.GetWorkflowError() @@ -50,12 +52,13 @@ func (s *VBExportTestSuite) Test_VBExport_InvalidDestination() { s.env.ExecuteWorkflow(VBExport, VBExportParams{ VXID: "VX-123", - Destinations: []string{"nonexistent"}, + Destinations: []Destination{{Value: "nonexistent"}}, }) s.True(s.env.IsWorkflowCompleted()) err := s.env.GetWorkflowError() s.Error(err) - s.Contains(err.Error(), "invalid destination") + // Rejected while decoding the input payload, before the workflow body runs. + s.Contains(err.Error(), ErrUnknownDestination.Error()) } func (s *VBExportTestSuite) Test_VBExport_NoShapes() { @@ -68,7 +71,7 @@ func (s *VBExportTestSuite) Test_VBExport_NoShapes() { s.env.ExecuteWorkflow(VBExport, VBExportParams{ VXID: "VX-123", - Destinations: []string{"xdcam"}, + Destinations: []Destination{DestinationXDCAM}, }) s.True(s.env.IsWorkflowCompleted()) err := s.env.GetWorkflowError() @@ -88,7 +91,7 @@ func (s *VBExportTestSuite) Test_VBExport_NoOriginalShape() { s.env.ExecuteWorkflow(VBExport, VBExportParams{ VXID: "VX-123", - Destinations: []string{"xdcam"}, + Destinations: []Destination{DestinationXDCAM}, }) s.True(s.env.IsWorkflowCompleted()) err := s.env.GetWorkflowError() @@ -142,7 +145,7 @@ func (s *VBExportTestSuite) Test_VBExport_XDCAM_Success() { s.env.ExecuteWorkflow(VBExport, VBExportParams{ VXID: "VX-123", - Destinations: []string{"xdcam"}, + Destinations: []Destination{DestinationXDCAM}, }) s.True(s.env.IsWorkflowCompleted()) err := s.env.GetWorkflowError() @@ -171,7 +174,7 @@ func (s *VBExportTestSuite) Test_VBExportToXDCAM() { s.env.ExecuteWorkflow(VBExportToXDCAM, VBExportChildWorkflowParams{ ParentParams: VBExportParams{ VXID: "VX-123", - Destinations: []string{"xdcam"}, + Destinations: []Destination{DestinationXDCAM}, }, InputFile: paths.MustParse("/mnt/temp/workflows/test_video.mxf"), // OriginalFile must be set: a zero paths.Path marshals Drive to "", @@ -338,3 +341,28 @@ func childParams() VBExportChildWorkflowParams { func TestVBExportTestSuite(t *testing.T) { suite.Run(t, new(VBExportTestSuite)) } + +// Destinations must decode from the bare names the trigger UI and older +// histories carry, and refuse names no child workflow exists for. +func TestDestination_JSON(t *testing.T) { + var params VBExportParams + if err := json.Unmarshal([]byte(`{"VXID":"VX-1","Destinations":["xdcam","b-stage"]}`), ¶ms); err != nil { + t.Fatal(err) + } + if len(params.Destinations) != 2 || params.Destinations[0] != DestinationXDCAM || params.Destinations[1] != DestinationBStage { + t.Fatalf("unexpected destinations: %v", params.Destinations) + } + + out, err := json.Marshal(params.Destinations) + if err != nil { + t.Fatal(err) + } + if string(out) != `["xdcam","b-stage"]` { + t.Fatalf("unexpected JSON: %s", out) + } + + err = json.Unmarshal([]byte(`["nonexistent"]`), ¶ms.Destinations) + if !errors.Is(err, ErrUnknownDestination) { + t.Fatalf("expected ErrUnknownDestination, got %v", err) + } +}