Uploadable: gate imagine to images, guard multi-field storage, add requiredOnPublish (#199, #193)

#199 — imagine no longer runs on non-image files:
- MediaObjectFactory::createMediaObjects() and UploadableFileManager::storeFilesMetadata()
  gate imagine variant generation on an image/* mime AND not SVG (was SVG-only), so a
  non-image (PDF/docx) on a field declaring imagineFilters returns only its primary media
  object instead of invoking Liip Imagine and 500-ing.

Multi-field storage collision guard:
- Two UploadableFields defaulting to the same storage property ('filename') silently shared
  one column — uploading to one populated both. UploadableAttributeReader::getConfiguredProperties()
  now throws UnsupportedAnnotationException at metadata load when two fields share a storage
  property. Each field needs a distinct property: (the bundle auto-maps the column).

#193 — require a file on publish, per field:
- UploadableField gains requiredOnPublish + requiredOnPublishMessage.
- New RequiresUploadedFile class constraint + UploadableLoader validator mapping loader add a
  {ShortName}:published-group rule per flagged field, passing when the transient File or the
  stored filename is present. Per-field, configurable message ({{ property }} placeholder),
  attached atPath the file property. Retires the app-side RequiresUploadedFileTrait.

Tests: DummyMultipleUploadable + DummyUploadableRequiredOnPublish entities, 6 Behat scenarios,
UploadableAttributeReaderTest, a MediaObjectFactoryTest case. Full suite green.

Author: silverbackdan

CLAUDE.md

@@ -572,25 +572,31 @@ Accepts `old-name` and `new-name` arguments (short class names). Derives dtype (
 
 ---
 
-### #193 — Require a file on publish for Uploadable entities — configure via `#[UploadableField(requiredOnPublish: true)]`
+### #193 — Require a file on publish for Uploadable entities — configure via `#[UploadableField(requiredOnPublish: true)]` ✓ **DONE**
 
-An `#[Uploadable]` entity can currently be **published without a file**. Apps work around this by hand-rolling a trait that adds a validation constraint in the `{ShortName}:published` group (see `RequiresUploadedFileTrait` in the `components-web-app` template / srnte). Move this into the bundle and configure it declaratively.
+**Implemented.** `UploadableField` gains `bool $requiredOnPublish = false` and `?string $requiredOnPublishMessage = null` (`src/Annotation/UploadableField.php`). A new validator mapping loader `Validator\MappingLoader\UploadableLoader` (service `silverback.api_components.validator.mapping_loader.uploadable`, wired into `validator.builder` alongside the timestamped loader in `ValidatorCompilerPass`) walks each `#[Uploadable]` class's `UploadableField`s and, for every flagged `requiredOnPublish`, adds a **class-level** `RequiresUploadedFile` constraint (`src/Validator/Constraints/`, validator `silverback.api_components.validator.requires_uploaded_file`) in the `{ShortName}:published` group. The constraint passes when **either** the transient file property (e.g. `$file`) **or** the stored filename property (`UploadableField::$property`) is non-null (read via `PropertyAccess`, so private/public storage both work — no private-property fatal, unlike the old `Assert\Expression` on `this.filename`). The violation is attached `->atPath($fileProperty)` so the front-end maps it to the field. Message is configurable per field via `requiredOnPublishMessage` (supports the `{{ property }}` placeholder); the default fallback is ``A file must be uploaded for the `{{ property }}` field before publishing.`` The bundle-side `RequiresUploadedFileTrait` workaround is retired.
 
-**Sharp edge this fixes:** `$filename` is **private** on `UploadableTrait` (only a public `getFilename()`). An `Assert\Expression` using the natural `this.filename` throws a fatal *"Cannot access private property"* — apps must know to write `this.getFilename()`. A bundle-owned constraint hides this entirely.
+**Multiple files** scale for free — each `UploadableField` gets its own independent `RequiresUploadedFile` constraint keyed to its own file + storage property, each with its own message. Behat: `features/uploads/uploads.feature` (test entity `DummyUploadableRequiredOnPublish`, two required fields — one custom message, one default) covers publish-with-no-files → 422 with a per-field violation each, publish-with-only-one-file → 422 for the missing one, publish-with-all-files → 200.
 
-**Proposed design (mirrors the existing `TimestampedLoader`):**
-- Add `bool $requiredOnPublish = false` to `UploadableField` (`src/Annotation/UploadableField.php`).
-- Add `Validator\MappingLoader\UploadableValidatorMappingLoader` that, for each `#[Uploadable]` class, iterates `UploadableField` properties and — for each flagged `requiredOnPublish` — adds a constraint in the `{ShortName}:published` group passing when **either** the transient file property (e.g. `$file`) **or** the stored filename property (`UploadableField::$property`, default `filename`, read via getter/PropertyAccess) is non-null. Immune to `$filename` only being written at `PRE_WRITE`.
-- Register as `silverback.api_components.validator.mapping_loader.uploadable`, alongside `...mapping_loader.timestamped` in `src/Resources/config/services.php`.
-- Prefer `Assert\Callback` (or a dedicated `RequiresUploadedFile` constraint) over `Assert\Expression` so the private-property trap can't recur.
+**Edge cases → docs, not the attribute:** *"at least N of these"*, *"exactly one of a group"*, conditional requiredness stay app-side via a custom `Assert\Callback` in the `{ShortName}:published` group. File-type / size validation stays on the field via `#[Assert\File(...)]` as the file is uploaded — `requiredOnPublish` only adds the not-blank-on-publish rule.
 
-**Multiple files:** the per-field flag scales for free — each `UploadableField` gets its own independent constraint keyed to its own file property + storage column (`property:`). No extra code.
+References: `src/Validator/Constraints/RequiresUploadedFile.php`, `src/Validator/Constraints/RequiresUploadedFileValidator.php`, `src/Validator/MappingLoader/UploadableLoader.php`, `src/Validator/PublishableValidator.php` (`getShortName() . ':published'`), `src/Annotation/UploadableField.php`.
 
-**Edge cases → docs, not the attribute:** *"at least N of these"*, *"exactly one of a group"*, conditional requiredness stay app-side via a custom `Assert\Callback` in the published group. Add a docs recipe covering (a) `requiredOnPublish: true`, (b) multiple files, (c) hand-rolled conditional rules — including the `getFilename()`-not-`filename` note.
+---
+
+### #199 — Multi-field uploadables: imagine gating + shared-storage collision guard ✓ **DONE**
+
+Surfaced while wiring a two-field uploadable (`file` + `preview`) in an app. Three fixes:
+
+**1. Imagine only runs on raster images (not any non-SVG).** `MediaObjectFactory::createMediaObjects()` previously gated imagine-variant generation on *"not SVG"*, so a non-image (PDF/docx) uploaded to a field that declares `imagineFilters` invoked Liip Imagine on it and 500'd. Now gated on `isImagineProcessable($mimeType)` (contains `image/` **and** not `image/svg+xml`). The same guard is applied to the eager-warm path `UploadableFileManager::storeFilesMetadata()` (the dynamic `ImagineFiltersInterface` route), reading the stored file's mime before warming. Behat: `features/uploads/uploads.feature` — uploading a docx to an `imagineFilters` field → 201 with only the primary media object; an image → still gets the `thumbnail` variant.
+
+**2. Multiple uploadable fields already work — each needs its own storage property.** The Doctrine `UploadableListener::loadClassMetadata` auto-maps a nullable string column per `UploadableField` (keyed off `UploadableField::$property`), which is why `UploadableTrait`'s unmapped `$filename` becomes a column with no `#[ORM\Column]`. The mechanism supports any number of fields; each just needs a **distinct** `property:` plus a matching nullable string entity property (the bundle maps the column). `UploadableTrait` is the single-field convenience (property defaults to `filename`); for extra fields declare e.g. `public ?string $previewFilename = null;` + `#[UploadableField(property: 'previewFilename')]`.
+
+**3. Collision guard (the silent-corruption footgun).** Because `UploadableField::$property` defaults to the constant `'filename'`, two fields that both omit `property:` resolve to the **same** column — uploading to one overwrites the other and both fields report the same file (no error, just corruption). `UploadableAttributeReader::getConfiguredProperties()` now throws `UnsupportedAnnotationException` when two `UploadableField`s on a class share a storage `property`, so the misconfiguration fails loudly at metadata load instead. Unit-tested in `tests/AttributeReader/UploadableAttributeReaderTest.php`; the multi-field behaviour is exercised by test entity `DummyMultipleUploadable` (`file` generic + `preview` with imagine filters, distinct columns) in `features/uploads/uploads.feature`.
 
-**Acceptance:** publishing with no file and no stored filename → 422 grouped under `{ShortName}:published`; `$file` set or existing filename passes; draft writes never require it; multiple fields validated independently; no app-side trait needed; no private-property fatal regardless of storage-property visibility.
+Not implemented (issue #199 item 3, enhancement): a field-level "generic file vs image" flag to default the `/download/{property}` disposition to `attachment` and/or skip image-dimension extraction. Left open — the download disposition is still controllable per request via `?download=true`.
 
-References: `src/Validator/MappingLoader/TimestampedLoader.php`, `src/Validator/PublishableValidator.php` (`getShortName() . ':published'`), `src/Entity/Utility/UploadableTrait.php`, `src/Annotation/UploadableField.php`.
+References: `src/Factory/Uploadable/MediaObjectFactory.php` (`isImagineProcessable`), `src/Helper/Uploadable/UploadableFileManager.php` (`storeFilesMetadata`), `src/AttributeReader/UploadableAttributeReader.php`, `src/EventListener/Doctrine/UploadableListener.php`.
 
 ---
 
@@ -651,7 +657,7 @@ Depends on #194. References: `src/Fixture/CwaFixtureBuilder.php` (flush phases;
 
 ### #197 — Manifest: each depth's payload is a nested resource tree (`NestedJsonStructure[]`) — front-end: cwa-nuxt-module #250 ✓ **DONE (API side)**
 
-**Implemented.** `GET /_/resource_manifest/{id}` now returns `resource_iris` as an array indexed by rendering depth (root first) where **each element is a nested tree node `{ "iri": string, "children": [...] }`** instead of a flat `string[]`. Only `ResourceManifestNormalizer` emits `resource_iris` (via `ManifestDepthGroupTrait`; `RouteNormalizer` does not). The trait was rewritten: `buildDepthGroups` splits depths on the `parentPage`/`parentPageData` boundary (unchanged) and, within each depth, `buildDepthNodes` builds the containment tree instead of flattening — same IRI set as before, same per-depth dedup, same blank-node/`resource_metadatas`/`@`-key/back-reference exclusions (skipped/blank/duplicate resources hoist their children so no noise nodes appear). **Decisions taken:** hard swap (no parallel key — pre-alpha BC break, ships in lockstep with module #250); node key is `iri` (not `@id` — bespoke DTO field); **no per-node metadata yet** (placeholder metadata is a deliberate follow-up — tracked in #198). Tests: `tests/Serializer/Normalizer/ManifestDepthGroupTraitTest.php` asserts exact nested structures; `features/main/route.feature` + `features/main/page.feature` converted to new `DoctrineContext` steps (`the manifest depth :n root IRI should be …`, `… should have :n resource IRIs`, `… should contain the IRI …`, `… should contain/not contain an IRI matching …`) which flatten a depth's tree. Full suite green.
+**Implemented.** `GET /_/resource_manifest/{id}` now returns `resource_iris` as an array indexed by rendering depth (root first) where **each element is a nested tree node `{ "iri": string, "children": [...] }`** instead of a flat `string[]`. Only `ResourceManifestNormalizer` emits `resource_iris` (via `ManifestDepthGroupTrait`; `RouteNormalizer` does not). The trait was rewritten: `buildDepthGroups` splits depths on the `parentPage`/`parentPageData` boundary (unchanged) and, within each depth, `buildDepthNodes` builds the containment tree instead of flattening — same IRI set as before, same per-depth dedup, same blank-node/`resource_metadatas`/`@`-key/back-reference exclusions (skipped/blank/duplicate resources hoist their children so no noise nodes appear). **Decisions taken:** hard swap (no parallel key — pre-alpha BC break, ships in lockstep with module #250); node key is `iri` (not `@id` — bespoke DTO field); **no per-node metadata** — and none is planned: #198 (which proposed it) was **closed as won't-do**. The front-end derives the resource type (incl. specific component type) from the IRI, so manifest metadata would be redundant or would couple the manifest cache to component internals. Placeholder/skeleton rendering is a front-end concern (developer-defined per-type templates) — requested in cwa-nuxt-module, not an API change. Tests: `tests/Serializer/Normalizer/ManifestDepthGroupTraitTest.php` asserts exact nested structures; `features/main/route.feature` + `features/main/page.feature` converted to new `DoctrineContext` steps (`the manifest depth :n root IRI should be …`, `… should have :n resource IRIs`, `… should contain the IRI …`, `… should contain/not contain an IRI matching …`) which flatten a depth's tree. Full suite green.
 
 <details><summary>Original design notes</summary>
 

features/bootstrap/UploadsContext.php

@@ -28,6 +28,7 @@
 use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\DummyUploadable;
 use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\DummyUploadableAndPublishable;
 use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\DummyUploadablePublicUrl;
+use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\DummyUploadableRequiredOnPublish;
 use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\DummyUploadableTemporaryUrl;
 use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\DummyUploadableWithImagineFilters;
 use Symfony\Component\HttpFoundation\File\File;
@@ -118,6 +119,47 @@ public function thereIsADummyUploadableAndPublishable(bool $isDraft = false, boo
         return $object;
     }
 
+    /**
+     * @Given there is a draft DummyUploadableRequiredOnPublish
+     */
+    public function thereIsADraftDummyUploadableRequiredOnPublish(): void
+    {
+        $object = new DummyUploadableRequiredOnPublish();
+        $object->setPublishedAt(null);
+        $this->manager->persist($object);
+        $this->manager->flush();
+        $this->restContext->resources['dummy_uploadable_draft'] = $this->iriConverter->getIriFromResource($object);
+    }
+
+    /**
+     * @Given there is a draft DummyUploadableRequiredOnPublish with all files uploaded
+     */
+    public function thereIsADraftDummyUploadableRequiredOnPublishWithFiles(): void
+    {
+        $object = new DummyUploadableRequiredOnPublish();
+        $object->setPublishedAt(null);
+        $object->file = new File(__DIR__ . '/../assets/files/image.png');
+        $object->preview = new File(__DIR__ . '/../assets/files/image.png');
+        $this->uploadableHelper->persistFiles($object);
+        $this->manager->persist($object);
+        $this->manager->flush();
+        $this->restContext->resources['dummy_uploadable_draft'] = $this->iriConverter->getIriFromResource($object);
+    }
+
+    /**
+     * @Given there is a draft DummyUploadableRequiredOnPublish with only the file uploaded
+     */
+    public function thereIsADraftDummyUploadableRequiredOnPublishWithFileOnly(): void
+    {
+        $object = new DummyUploadableRequiredOnPublish();
+        $object->setPublishedAt(null);
+        $object->file = new File(__DIR__ . '/../assets/files/image.png');
+        $this->uploadableHelper->persistFiles($object);
+        $this->manager->persist($object);
+        $this->manager->flush();
+        $this->restContext->resources['dummy_uploadable_draft'] = $this->iriConverter->getIriFromResource($object);
+    }
+
     /**
      * @Given there is a DummyUploadablePublicUrl
      */

features/uploads/uploads.feature

@@ -249,6 +249,78 @@ Feature: API Resources which can have files uploaded
     Then the response status code should be 200
     And the JSON node "_metadata.mediaObjects.file[0].contentUrl" should be a valid download link for the resource "dummy_uploadable"
 
+  # Multiple independent uploadable fields on one resource.
+  # $file (generic, no imagine filters) and $preview (image, imagine filters) each have their own
+  # storage property, so uploading to one never touches the other, and imagine only ever runs on
+  # an actual image — never on a non-image (docx/pdf) even when the target field declares filters.
+
+  @loginUser
+  Scenario: Uploading a non-image to an imagine-filtered field does not attempt image processing
+    Given I add "Content-Type" header equal to "multipart/form-data"
+    When I send a "POST" request to "/dummy_multiple_uploadables/upload" with parameters:
+      | key     | value           |
+      | preview | @test_file.docx |
+    Then the response status code should be 201
+    And the JSON node "_metadata.mediaObjects.preview[0].imagineFilter" should not exist
+    And the JSON node "_metadata.mediaObjects.preview[1]" should not exist
+
+  @loginUser
+  Scenario: Uploading an image to an imagine-filtered field still produces the imagine variant
+    Given I add "Content-Type" header equal to "multipart/form-data"
+    When I send a "POST" request to "/dummy_multiple_uploadables/upload" with parameters:
+      | key     | value      |
+      | preview | @image.png |
+    Then the response status code should be 201
+    And the JSON node "_metadata.mediaObjects.preview[0].imagineFilter" should not exist
+    And the JSON node "_metadata.mediaObjects.preview[1].imagineFilter" should be equal to the string "thumbnail"
+
+  @loginUser
+  Scenario: Uploading to one uploadable field does not populate the other field
+    Given I add "Content-Type" header equal to "multipart/form-data"
+    When I send a "POST" request to "/dummy_multiple_uploadables/upload" with parameters:
+      | key  | value      |
+      | file | @image.png |
+    Then the response status code should be 201
+    And the JSON node "_metadata.mediaObjects.file[0]" should exist
+    And the JSON node "_metadata.mediaObjects.preview" should not exist
+
+  # requiredOnPublish — a file must be present per flagged field before the resource can be published.
+
+  @loginAdmin
+  Scenario: Publishing is rejected per field when a requiredOnPublish file is missing
+    Given there is a draft DummyUploadableRequiredOnPublish
+    And I add "Content-Type" header equal to "application/merge-patch+json"
+    When I send a "PATCH" request to the resource "dummy_uploadable_draft" with data:
+      | publishedAt |
+      | now         |
+    Then the response status code should be 422
+    And the JSON node "violations" should have 2 elements
+    And the JSON node "violations[0].propertyPath" should be equal to the string "file"
+    And the JSON node "violations[0].message" should be equal to the string "You must upload a file before publishing."
+    And the JSON node "violations[1].propertyPath" should be equal to the string "preview"
+    And the JSON node "violations[1].message" should be equal to the string "A file must be uploaded for the `preview` field before publishing."
+
+  @loginAdmin
+  Scenario: Publishing is still rejected when only some requiredOnPublish files are present
+    Given there is a draft DummyUploadableRequiredOnPublish with only the file uploaded
+    And I add "Content-Type" header equal to "application/merge-patch+json"
+    When I send a "PATCH" request to the resource "dummy_uploadable_draft" with data:
+      | publishedAt |
+      | now         |
+    Then the response status code should be 422
+    And the JSON node "violations" should have 1 element
+    And the JSON node "violations[0].propertyPath" should be equal to the string "preview"
+
+  @loginAdmin
+  Scenario: Publishing succeeds when all requiredOnPublish files are present
+    Given there is a draft DummyUploadableRequiredOnPublish with all files uploaded
+    And I add "Content-Type" header equal to "application/merge-patch+json"
+    When I send a "PATCH" request to the resource "dummy_uploadable_draft" with data:
+      | publishedAt |
+      | now         |
+    Then the response status code should be 200
+    And the JSON node "_metadata.publishable.published" should be true
+
   @loginUser
   Scenario: A multipart file upload fires exactly one Mercure notification
     Given I add "Content-Type" header equal to "multipart/form-data"

src/Annotation/UploadableField.php

@@ -26,6 +26,13 @@ public function __construct(
         public string $property = 'filename',
         public ?string $prefix = null,
         public ?array $imagineFilters = [],
+        // When true, a validation constraint is added in the `{ShortName}:published` group requiring
+        // either the transient file property or its stored filename to be present before the owning
+        // resource can be published. Configured per field, so multiple fields are independent.
+        public bool $requiredOnPublish = false,
+        // Optional override for the violation message (supports the `{{ property }}` placeholder).
+        // Null falls back to the constraint's default message.
+        public ?string $requiredOnPublishMessage = null,
     ) {
     }
 }