spikerj/spikersoft-issues
1
0
Fork 0
Code Issues 196 Pull Requests Actions Packages Projects Releases Wiki Activity

Drop GridFS naming from Art Studio: rename to objectKey, migrate legacy docs, remove the #852 shim #853

New Issue
Closed
opened 2026-07-26 00:29:26 +00:00 by spikerj · 2 comments
spikerj commented 2026-07-26 00:29:26 +00:00
Owner
Copy Link
Copy Source

Follows #852. Nothing has been in GridFS since #783 (epic #413), but the persisted names, the API JSON, the route params and the Angular model all still say gridFsId. That misleading name is what made #849 look safe and took every Art Studio image to a 404.

PR #484 restored production with PropertyNameCaseInsensitive = true. That shim is a hack and this ticket removes it. Goal: zero gridFsId in the Art Studio path.

objectKey is not a new coinage — ArtAssetGenerationParams already persists objectKey. This aligns the rest of Art Studio with it.

Two structural facts that shape the work

  1. ArtAssetArtifactRef is persisted two different ways. ArtAsset.Artifacts is an EF JSON-string column (SpikerDbContext.cs:845) where [BsonElement] is inert and [JsonPropertyName] is live. ArtAsset.QuarantinedArtifacts is a native BSON array where the reverse is true. Both attributes must move in lockstep, and the migration needs two mechanisms — $rename works for the native array, but is impossible for the JSON string, which must be parsed and rewritten per document.
  2. Every write already emits canonical lowercase. All mutations go through EF with default options (primary writer ArtPipeStageOrchestrator.cs:553); there is not one raw-driver write to art-assets. Documents self-heal PascalCase → lowercase on any save, so the migration must handle both casings, not just the PascalCase visible today.

De-risking: there are no Mongo indexes, filters, projections or raw "gridFsId" query literals anywhere in the repo.

Production data (read live)

8 documents, 92 artifact objects, artifacts a JSON string on all 8, all PascalCase today. 69 also persist Url/ThumbnailUrl/DisplayUrl — all null. Those are [BsonIgnore] transient presigned URLs; [BsonIgnore] does not apply to a JSON column, so they are being written. Harmless today, but a save after minting would put a real signed URL in the DB. quarantinedArtifacts all empty; sourceImageGridFsId all null; art-variant-upload-jobs empty.

Scope

Backend

  • ArtAssetArtifactRef.ObjectKey → objectKey (both attributes); SourceArtifactKey → sourceArtifactKey; ArtAsset.SourceImageKey → sourceImageKey
  • Explicit [JsonPropertyName] on every property of ArtAssetArtifactRef — only two are pinned today, so the rest serialize under CLR names by accident
  • [JsonIgnore] on Url/ThumbnailUrl/DisplayUrl
  • [JsonRequired] on ObjectKey so a missing key throws instead of silently defaulting to "" — the guarantee #852 lacked
  • ArtVariantUploadJob: sourceArtifactGridFsId → sourceArtifactKey, stagedGridFsId → stagedObjectKey
  • Delete LegacyTolerantArtifactJson; one shared explicit JsonSerializerOptions for read and write so they can never diverge again
  • Routes: {gridFsId} → {objectKey} on the artifact, gallery and quarantine downloads; {sourceGridFsId} → {sourceObjectKey} on the variant POST
  • Worker contracts (cross RabbitMQ — drain queues in the window): RequestArtAssetStageCommand, ProcessUploadedArtVariantCommand
  • Fix GetArtVariantJobStatusQuery.SourceArtifactKey, which #849 already left serializing as sourceArtifactKey while Angular declares sourceArtifactGridFsId — dormant, but fix rather than perpetuate
  • Fix the broken XML crefs #849 left behind

Migration — mongodb-scripts/2026-07-migrate-art-assets-object-key.js, modelled on 2026-07-migrate-userbookmarks-userid.js. Handles both casings, drops the transient URL fields, $renames the native fields, DRY_RUN defaults true, residual assertions expect 0.

Angular — model rename, plus a key guard in the four URL builders. No call site guards the key today, which is why a missing value became /artifacts//download instead of a visible error.

Tests — ArtStudioStorageKeyNamingTests exists to fail if someone "finishes the job"; it gets inverted, not deleted. Legacy fixtures must use literal JSON, never JsonSerializer.Serialize(currentModel) — that round-trip blind spot is how #849 shipped green.

Cutover (agreed: hard cutover, no dual-emit)

  1. mongodump art-assets
  2. Drain ArtStudio RabbitMQ queues
  3. Maintenance window: run migration → deploy backend → deploy Angular
  4. Hard refresh, verify images

Rollback: restore the dump, redeploy previous images.

Acceptance

Every artifact has a non-empty objectKey; no gridFsId anywhere in the API response; every thumbnail 200; zero /artifacts//download or /artifacts/undefined/download requests; cards render real thumbnails rather than the chair fallback.

Follows #852. Nothing has been in GridFS since #783 (epic #413), but the persisted names, the API JSON, the route params and the Angular model all still say `gridFsId`. That misleading name is what made #849 look safe and took every Art Studio image to a 404. PR #484 restored production with `PropertyNameCaseInsensitive = true`. **That shim is a hack and this ticket removes it.** Goal: zero `gridFsId` in the Art Studio path. `objectKey` is not a new coinage — `ArtAssetGenerationParams` already persists `objectKey`. This aligns the rest of Art Studio with it. ## Two structural facts that shape the work 1. **`ArtAssetArtifactRef` is persisted two different ways.** `ArtAsset.Artifacts` is an EF JSON-string column (`SpikerDbContext.cs:845`) where `[BsonElement]` is **inert** and `[JsonPropertyName]` is live. `ArtAsset.QuarantinedArtifacts` is a **native BSON array** where the reverse is true. Both attributes must move in lockstep, and the migration needs two mechanisms — `$rename` works for the native array, but is impossible for the JSON string, which must be parsed and rewritten per document. 2. **Every write already emits canonical lowercase.** All mutations go through EF with default options (primary writer `ArtPipeStageOrchestrator.cs:553`); there is not one raw-driver write to `art-assets`. Documents self-heal PascalCase → lowercase on any save, so **the migration must handle both casings**, not just the PascalCase visible today. De-risking: there are **no** Mongo indexes, filters, projections or raw `"gridFsId"` query literals anywhere in the repo. ## Production data (read live) 8 documents, 92 artifact objects, `artifacts` a JSON string on all 8, all PascalCase today. 69 also persist `Url`/`ThumbnailUrl`/`DisplayUrl` — **all null**. Those are `[BsonIgnore]` transient presigned URLs; `[BsonIgnore]` does not apply to a JSON column, so they are being written. Harmless today, but a save after minting would put a real signed URL in the DB. `quarantinedArtifacts` all empty; `sourceImageGridFsId` all null; `art-variant-upload-jobs` empty. ## Scope **Backend** - `ArtAssetArtifactRef.ObjectKey` → `objectKey` (both attributes); `SourceArtifactKey` → `sourceArtifactKey`; `ArtAsset.SourceImageKey` → `sourceImageKey` - Explicit `[JsonPropertyName]` on **every** property of `ArtAssetArtifactRef` — only two are pinned today, so the rest serialize under CLR names by accident - `[JsonIgnore]` on `Url`/`ThumbnailUrl`/`DisplayUrl` - `[JsonRequired]` on `ObjectKey` so a missing key **throws** instead of silently defaulting to `""` — the guarantee #852 lacked - `ArtVariantUploadJob`: `sourceArtifactGridFsId` → `sourceArtifactKey`, `stagedGridFsId` → `stagedObjectKey` - Delete `LegacyTolerantArtifactJson`; one shared explicit `JsonSerializerOptions` for read **and** write so they can never diverge again - Routes: `{gridFsId}` → `{objectKey}` on the artifact, gallery and quarantine downloads; `{sourceGridFsId}` → `{sourceObjectKey}` on the variant POST - Worker contracts (**cross RabbitMQ** — drain queues in the window): `RequestArtAssetStageCommand`, `ProcessUploadedArtVariantCommand` - Fix `GetArtVariantJobStatusQuery.SourceArtifactKey`, which #849 already left serializing as `sourceArtifactKey` while Angular declares `sourceArtifactGridFsId` — dormant, but fix rather than perpetuate - Fix the broken XML `cref`s #849 left behind **Migration** — `mongodb-scripts/2026-07-migrate-art-assets-object-key.js`, modelled on `2026-07-migrate-userbookmarks-userid.js`. Handles both casings, drops the transient URL fields, `$renames` the native fields, `DRY_RUN` defaults true, residual assertions expect 0. **Angular** — model rename, plus a **key guard** in the four URL builders. No call site guards the key today, which is why a missing value became `/artifacts//download` instead of a visible error. **Tests** — `ArtStudioStorageKeyNamingTests` exists to fail if someone "finishes the job"; it gets **inverted**, not deleted. Legacy fixtures must use **literal JSON**, never `JsonSerializer.Serialize(currentModel)` — that round-trip blind spot is how #849 shipped green. ## Cutover (agreed: hard cutover, no dual-emit) 1. `mongodump` `art-assets` 2. Drain ArtStudio RabbitMQ queues 3. Maintenance window: run migration → deploy backend → deploy Angular 4. Hard refresh, verify images Rollback: restore the dump, redeploy previous images. ## Acceptance Every artifact has a non-empty `objectKey`; no `gridFsId` anywhere in the API response; every thumbnail 200; zero `/artifacts//download` or `/artifacts/undefined/download` requests; cards render real thumbnails rather than the `chair` fallback.
spikerj commented 2026-07-26 00:50:30 +00:00
Author
Owner
Copy Link
Copy Source

Work complete — both PRs ready, held as WIP pending the cutover

  • backend #485 fix/artstudio-object-key-rename-853
  • angular #579 fix/artstudio-object-key-rename-853

The #852 shim (PropertyNameCaseInsensitive) is deleted.

Migration rehearsed against real production data

mongodump of art-assets restored into a scratch DB, migration run for real there:

documents scanned            : 8
documents needing a rewrite  : 8
transient URL fields dropped : 207     (69 artifacts x 3)
unrecognised keys            : none
every residual assertion     : 0

Then compared prod vs migrated, artifact by artifact:

artifacts compared : 92
key mismatches     : 0
empty objectKeys   : 0

Sibling fields intact. A second run reports 0 changes — idempotent. Scratch DB dropped afterwards.

The proof that images will not break

The migrated documents were fed through the new model and converter options — the exact mapping path production uses:

all 92 artifacts bind with a non-empty objectKey, correct kind, and url/thumbnailUrl/displayUrl all null (the transient presigned URLs are no longer persisted).

That is as close to "images work" as is possible before deploying. The remaining check is the live one in the acceptance criteria, which runs after the cutover.

Also fixed along the way

  • [BsonIgnore] now genuinely means "never persisted" on the JSON-column path. System.Text.Json knew nothing about it, so the transient presigned URLs documented as never persisted were being written into every document. Null today, but a save after minting would have stored an expiring signed URL. Scoped to the storage options — API responses still carry them.
  • The Angular side had no guard at all on the storage key. That is why #852 produced /artifacts//download rather than a visible error. One guard at four builders now covers all twelve call sites.
  • ArtVariantJobStatusDto — a contract #849 had already broken (backend serialized sourceArtifactKey, Angular declared sourceArtifactGridFsId). Dormant; now aligned.
  • tools/BackfillPhotoStackDerivatives referenced the property #849 removed and only compiled because it is not in the solution.

Verification summary
dotnet build SpikerSoft.UnitTests.slnf 0 errors
ArtStudio/ArtPipe backend tests 1055 green
nx test feature-art-studio 432 green
nx test spikersoft 3428 green
nx build spikersoft bundle complete
lint:fix + lint:styles:fix clean

Cutover order (unchanged)

  1. mongodump art-assets
  2. Drain / confirm-empty the ArtStudio RabbitMQ queues — the worker contracts renamed, and a pre-#853 message will not bind (there is a test asserting exactly that, so the requirement can't be forgotten)
  3. Run the migration with DRY_RUN = false
  4. Deploy backend, then Angular
  5. Hard refresh, then the live verification

Migration must precede the backend deploy: ObjectKey is [JsonRequired], so an un-migrated document fails loudly. That is intended.

## Work complete — both PRs ready, held as WIP pending the cutover - backend **#485** `fix/artstudio-object-key-rename-853` - angular **#579** `fix/artstudio-object-key-rename-853` The `#852` shim (`PropertyNameCaseInsensitive`) is **deleted**. ## Migration rehearsed against real production data `mongodump` of `art-assets` restored into a scratch DB, migration run for real there: ``` documents scanned : 8 documents needing a rewrite : 8 transient URL fields dropped : 207 (69 artifacts x 3) unrecognised keys : none every residual assertion : 0 ``` Then compared prod vs migrated, artifact by artifact: ``` artifacts compared : 92 key mismatches : 0 empty objectKeys : 0 ``` Sibling fields intact. A second run reports **0 changes** — idempotent. Scratch DB dropped afterwards. ## The proof that images will not break The migrated documents were fed through the **new** model and converter options — the exact mapping path production uses: > all 92 artifacts bind with a non-empty `objectKey`, correct `kind`, and `url`/`thumbnailUrl`/`displayUrl` all null (the transient presigned URLs are no longer persisted). That is as close to "images work" as is possible before deploying. The remaining check is the live one in the acceptance criteria, which runs after the cutover. ## Also fixed along the way - `[BsonIgnore]` now genuinely means "never persisted" on the JSON-column path. System.Text.Json knew nothing about it, so the transient presigned URLs documented as *never persisted* were being written into every document. Null today, but a save after minting would have stored an expiring signed URL. Scoped to the storage options — API responses still carry them. - The Angular side had **no guard at all** on the storage key. That is why #852 produced `/artifacts//download` rather than a visible error. One guard at four builders now covers all twelve call sites. - `ArtVariantJobStatusDto` — a contract #849 had already broken (backend serialized `sourceArtifactKey`, Angular declared `sourceArtifactGridFsId`). Dormant; now aligned. - `tools/BackfillPhotoStackDerivatives` referenced the property #849 removed and only compiled because it is not in the solution. ## Verification summary | | | |---|---| | `dotnet build SpikerSoft.UnitTests.slnf` | 0 errors | | ArtStudio/ArtPipe backend tests | 1055 green | | `nx test feature-art-studio` | 432 green | | `nx test spikersoft` | 3428 green | | `nx build spikersoft` | bundle complete | | lint:fix + lint:styles:fix | clean | ## Cutover order (unchanged) 1. `mongodump art-assets` 2. Drain / confirm-empty the ArtStudio RabbitMQ queues — the worker contracts renamed, and a pre-#853 message will **not** bind (there is a test asserting exactly that, so the requirement can't be forgotten) 3. Run the migration with `DRY_RUN = false` 4. Deploy backend, then Angular 5. Hard refresh, then the live verification Migration must precede the backend deploy: `ObjectKey` is `[JsonRequired]`, so an un-migrated document fails loudly. That is intended.
spikerj commented 2026-07-29 15:36:24 +00:00
Author
Owner
Copy Link
Copy Source

Resolved in spikersoft-backend PR #485 (8071ff66) + spikersoft-angular PR #579 (d1de0dca), both merged 2026-07-26 and shipped in v2026.07.26b / v2026.07.26a.

Correcting the record: these were previously noted as "held WIP". They are not — they merged, and releases have continued uninterrupted through v2026.07.29a. Since ObjectKey is [JsonRequired], an un-migrated document would hard-fail every Art Studio read, and no follow-up incident was filed, so the cutover evidently completed.

Verified item by item against origin/master:

  • Both attributes pinned to objectKey (ArtAsset.cs:447-450) with [JsonRequired] (:449); sourceArtifactKey (:540); sourceImageKey (:60); explicit [JsonPropertyName] on every persisted property of ArtAssetArtifactRef.
  • ArtVariantUploadJob renamed (:33-38); one shared serializer-options instance; all four routes renamed (ArtStudioController.cs:478,515,699,1436); both worker contracts renamed; GetArtVariantJobStatusQuery.SourceArtifactKey aligned.
  • The #852 shim is gone — SpikerDbContext.cs:884-891 now uses the single shared ArtifactColumnJson (:33-39) for read and write, deliberately case-sensitive. LegacyTolerantArtifactJson no longer exists.
  • Migration mongodb-scripts/2026-07-migrate-art-assets-object-key.js: DRY_RUN = true default (:36), both-casing handling (:87), transient-URL dropping (:88-91), $rename on native fields (:126-129, :195-200), zero-residual assertions.
  • Angular: objectKey on the model (art-studio.models.ts:103), requireObjectKey guard (art-studio.service.ts:680-685) called from all four builders (:689,709,793,886), zero gridFsId left in the Art Studio lib.

One implementation improvement over the plan worth recording: instead of [JsonIgnore] on the transient URLs, it used a storage-scoped TypeInfoResolver honouring [BsonIgnore] (SpikerDbContext.cs:33-39,60). [JsonIgnore] would have stripped those fields from API responses too — this was the right call, and both halves are test-pinned.

Small residuals, none affecting acceptance: no legacy-bind test for ProcessUploadedArtVariantCommand (one exists for RequestArtAssetStageCommand at ArtStudioStorageKeyNamingTests.cs:202-215); stale route shapes in doc comments at GetArtAssetManifestQuery.cs:17-18 and S3ArtAssetFileStore.cs:17. The ~101 remaining gridFsId hits under Art Studio paths are local variables, log templates, test names and intentional legacy fixtures — zero in Mongos/ArtStudio/* or Contracts.Workers/ArtStudio/*.

The live acceptance items ("every thumbnail 200", "zero /artifacts//download") are post-deploy checks not verifiable from a git tree.

Closing.

Resolved in spikersoft-backend PR #485 (`8071ff66`) + spikersoft-angular PR #579 (`d1de0dca`), both merged 2026-07-26 and shipped in `v2026.07.26b` / `v2026.07.26a`. **Correcting the record:** these were previously noted as "held WIP". They are not — they merged, and releases have continued uninterrupted through `v2026.07.29a`. Since `ObjectKey` is `[JsonRequired]`, an un-migrated document would hard-fail every Art Studio read, and no follow-up incident was filed, so the cutover evidently completed. Verified item by item against `origin/master`: - Both attributes pinned to `objectKey` (`ArtAsset.cs:447-450`) with `[JsonRequired]` (`:449`); `sourceArtifactKey` (`:540`); `sourceImageKey` (`:60`); explicit `[JsonPropertyName]` on every persisted property of `ArtAssetArtifactRef`. - `ArtVariantUploadJob` renamed (`:33-38`); one shared serializer-options instance; all four routes renamed (`ArtStudioController.cs:478,515,699,1436`); both worker contracts renamed; `GetArtVariantJobStatusQuery.SourceArtifactKey` aligned. - **The #852 shim is gone** — `SpikerDbContext.cs:884-891` now uses the single shared `ArtifactColumnJson` (`:33-39`) for read and write, deliberately case-sensitive. `LegacyTolerantArtifactJson` no longer exists. - Migration `mongodb-scripts/2026-07-migrate-art-assets-object-key.js`: `DRY_RUN = true` default (`:36`), both-casing handling (`:87`), transient-URL dropping (`:88-91`), `$rename` on native fields (`:126-129`, `:195-200`), zero-residual assertions. - Angular: `objectKey` on the model (`art-studio.models.ts:103`), `requireObjectKey` guard (`art-studio.service.ts:680-685`) called from all four builders (`:689,709,793,886`), zero `gridFsId` left in the Art Studio lib. One implementation improvement over the plan worth recording: instead of `[JsonIgnore]` on the transient URLs, it used a storage-scoped `TypeInfoResolver` honouring `[BsonIgnore]` (`SpikerDbContext.cs:33-39,60`). `[JsonIgnore]` would have stripped those fields from API responses too — this was the right call, and both halves are test-pinned. Small residuals, none affecting acceptance: no legacy-bind test for `ProcessUploadedArtVariantCommand` (one exists for `RequestArtAssetStageCommand` at `ArtStudioStorageKeyNamingTests.cs:202-215`); stale route shapes in doc comments at `GetArtAssetManifestQuery.cs:17-18` and `S3ArtAssetFileStore.cs:17`. The ~101 remaining `gridFsId` hits under Art Studio paths are local variables, log templates, test names and intentional legacy fixtures — zero in `Mongos/ArtStudio/*` or `Contracts.Workers/ArtStudio/*`. The live acceptance items ("every thumbnail 200", "zero `/artifacts//download`") are post-deploy checks not verifiable from a git tree. Closing.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
agentic
Actionable surfaced by the agentic architecture & security review
 bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 sonarqube
Code quality issue surfaced by SonarQube (bug / code smell)
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
spikerj (Joseph Spiker)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#853