Skip to content

Modular operations + Asset (asset_id) resource + tag Append mode#6

Open
eitanp461 wants to merge 10 commits into
masterfrom
refactor/modular-operations-and-search
Open

Modular operations + Asset (asset_id) resource + tag Append mode#6
eitanp461 wants to merge 10 commits into
masterfrom
refactor/modular-operations-and-search

Conversation

@eitanp461
Copy link
Copy Markdown

@eitanp461 eitanp461 commented May 27, 2026

Summary

Restructures the Cloudinary node from a single monolithic execute() into a per-operation handler architecture, completes the per-asset CRUD lifecycle using Cloudinary's asset_id, and reorganizes the user-facing resource taxonomy so new workflows pick the safe path by default while saved workflows keep running unchanged.

This is the cumulative work on refactor/modular-operations-and-search — 10 commits — bundled into one review because the pieces are deeply interdependent (descriptor changes co-evolve with handler registrations, the taxonomy shift only makes sense once the new asset_id ops exist, and the test infrastructure rework underpins all of it).

Functional changes (what users see)

New resource: Asset (asset_id-based)

A clean, asset_id-keyed surface for everything per-asset. Operations:

  • Get Asset — fetch a single asset by asset_id with optional info-rich flags (colors, faces, image_metadata, pHash, etc.).
  • Update Asset Tags — with a new Mode selector (see below).
  • Update Asset Structured Metadata — write key/value pairs to a metadata field.
  • Update Asset Display Name — the rename users actually want under dynamic-folder mode (changes the human label, not the delivery URL).
  • Delete Assets — bulk delete by public_id (Cloudinary's bulk endpoint is public_id-keyed; no asset_id variant exists).
  • Search Assets — moved here from the Admin surface; supports pagination, custom expressions, and structured sort.

asset_id is a single immutable global key — callers don't need to juggle the (resource_type, type) coordinate pair that public_id-keyed endpoints require, which removes a whole class of silent no-op bugs (see "the type-field fix" below).

Reshaped resource taxonomy

The dropdown now reads, top to bottom:

  1. UploadUpload File, Upload From URL (unchanged).
  2. Asset — the new asset_id resource above.
  3. Library — account-level reads: Get Tags, Get Metadata Fields.
  4. Asset (Legacy) — pinned to the bottom of the list. Exposes only the two operations that shipped on master (Update Asset Tags, Update Asset Structured Metadata) so existing workflows keep loading and running. Each operation's description is prefixed with a "Legacy form, public_id only" note pointing users at the new Asset resource.

Operation labels within every resource are alphabetized for predictability.

Update Asset Tags: Set vs Append modes

The single biggest behavioral fix. The pre-existing updateTags operation always replaced the tag list — many users expected it to add to existing tags and were silently losing data. The operation now has a Mode selector:

  • Set (Replace All Existing Tags) — default. Preserves the pre-existing behavior exactly; no saved workflow changes meaning.
  • Append (Add to Existing Tags) — hits Cloudinary's POST /:resource_type/tags with command=add and signed auth. Existing tags are preserved.

Append mode exposes a Type field (Upload / Private / Authenticated / Fetch) because the tag-action endpoint scopes lookups by (resource_type, type) and silently returns public_ids: [] for assets stored under a non-default type. The field label deliberately matches the type property on the Cloudinary asset object so users can copy the value straight from upstream JSON.

Present on both Asset and Asset (Legacy) resources, so the BC behavior is identical regardless of which surface a user picks.

Technical changes (what reviewers should look at)

Per-operation handler architecture

Replaces the previous monolithic execute() with:

  • nodes/Cloudinary/operations/<resource>/<operation>.ts — one file per operation, each exporting an OperationHandler of signature (ctx, i, creds) => Promise<IDataObject[]> (operations/types.ts).
  • nodes/Cloudinary/operations/index.ts — a flat ${resource}:${operation} → handler map.
  • Cloudinary.node.ts:execute() — a thin loop over input items that resolves credentials once, looks up the handler, and wraps each returned IDataObject into an INodeExecutionData with the correct pairedItem.

Adding a new operation now means: drop a handler file, add a one-line entry to the map, and add the descriptor fields. No more touching a giant switch.

Two auth flows remain disjoint

The node mixes two Cloudinary auth styles depending on endpoint, and this PR keeps them strictly separated:

  • HTTP Basic auth — Admin API and Update Asset endpoints. Passed via auth: { username, password } on IHttpRequestOptions.
  • Signed Upload API — used by upload/* and now the new appendTags helper. Builds a params object, signs via generateCloudinarySignature (which correctly excludes api_key, file, and signature from the signed payload), and POSTs as application/x-www-form-urlencoded.

Append-tags is the only new code on the signed side. Shared helper at operations/tagAppend.ts is used by both asset:updateTags and updateAsset:updateTags.

Backwards compatibility

Every constraint from CLAUDE.md's "Backwards compatibility" section is respected:

  • Every value string on updateAsset:* is preserved. Saved workflows that reference the legacy resource resolve unchanged.
  • The new asset resource is net-new — no constraints to violate.
  • The new Mode selector on Update Asset Tags defaults to set. Workflows saved before this PR have no stored tagMode → fall back to the default → identical request shape and identical behavior.
  • The new Type field on Append mode defaults to upload — the same implicit value Cloudinary would have used pre-PR for any tag-append-style operation. (There was no append op before, so there's no behavior to preserve here, but the default still matches the most common case.)
  • displayOptions.show clauses on existing fields are only loosened, never narrowed.
  • displayName / description changes (Asset, Asset (Legacy), reorderings) are UI-only metadata, explicitly free-to-change per CLAUDE.md.

No typeVersion bump needed.

Test infrastructure

  • Vitest, not Jest — n8n's ecosystem leans Jest but this package was already on Vitest and the convention is documented in CLAUDE.md.
  • Shared mock builder at operations/testHelpers.ts (excluded from the build per tsconfig.json exclude so it doesn't leak vitest into the published package).
  • vitest.config.ts aliases n8n-workflow to its built CJS entry (n8n-workflow/dist/index.js) — the package's import condition points at raw src/index.ts which Vitest can't load.
  • 106 tests passing. Coverage spans signature generation, URL builders, error extraction, metadata serialization, and per-operation request-contract assertions (URL, method, auth shape, body shape) on the mocked HTTP helper.

The type-field fix worth surfacing explicitly

During end-to-end testing, an input batch where one asset was stored as type: "authenticated" and another as type: "upload" revealed that Cloudinary's tag-action endpoint silently returns public_ids: [] (HTTP 200, no error) when the type body parameter doesn't match the asset's storage type. The handler now reads a tagAppendType parameter and threads it into the signed body. Audited all other public_id-keyed operations — updateAsset:updateTags (set mode), updateAsset:updateMetadata, updateAsset:deleteAssets, and asset:deleteAssets — and all already include type in their requests. The asset_id-keyed operations are immune by design.

Out of scope (deferred to follow-up PRs)

  • asset:moveAsset (dynamic-folder move via asset_folder).
  • asset:renamePublicId (Upload API rename — changes delivery URLs, needs explicit user intent).
  • Other delete variants (by prefix, by tag, by public_ids array).
  • New entity resources (folder, metadataField, uploadPreset).
  • typeVersion: 2 bump that mechanically retires the legacy updateAsset:* surface.
  • A public_id → asset_id resolution op (Get-by-public_id) — closes a workflow ergonomics gap surfaced during testing.

Test plan

  • npm test — 106/106 pass locally.
  • npm run lint — clean.
  • npm run build — clean; icons copied to dist/.
  • npm run n8n-validate — community-package scan passes.
  • Manual smoke in a local n8n instance:
    • Asset → Get Asset by asset_id returns the resource.
    • Asset → Update Asset Tags in Set mode replaces tags (existing BC).
    • Asset → Update Asset Tags in Append mode preserves existing tags.
    • Append mode against an authenticated-type asset succeeds when Type is set correctly; silently no-ops when left at the upload default (documents the failure mode).
    • Asset → Delete Assets removes the targeted public_ids.
    • Asset → Update Asset Display Name updates the human label without changing the delivery URL.
    • Asset (Legacy) → Update Asset Tags still works against public_id exactly as it did on master (load a saved workflow if available).
    • Library → Get Tags and Get Metadata Fields return account-level data.

🤖 Generated with Claude Code

Restructure the Cloudinary node from a single monolithic execute() into a
declarative handler map: one file per operation grouped by resource, with
field definitions split into dedicated description modules. execute() is now
a thin dispatch loop over input items.

Add an Admin "Search Assets" operation that emits one item per matching
asset, auto-injects a resource_type clause from the selected types, supports
automatic pagination ("Return All"), and surfaces rate-limit and invalid
expression errors with actionable messages.

Harden structured-metadata serialization: multi-value fields render as a
bracketed list of quoted strings and delimiter characters are escaped in
every value, matching Cloudinary's documented metadata format. Sanitize the
multipart upload filename so it can't break request framing.

Introduce a Vitest test suite covering the metadata/signature/URL helpers
and the per-operation request contracts, and share common request headers
and auth across handlers.
@eitanp461 eitanp461 requested a review from sveta-slepner May 27, 2026 14:02
eitanp461 and others added 9 commits May 28, 2026 09:25
- vitest.config.ts: update n8n-workflow alias from dist/index.js to
  dist/cjs/index.js. n8n-workflow@2.16 reorganised its build output and
  no longer ships a top-level dist/index.js, so the previous alias
  resolved to nothing and every test file failed at import time with
  "Cannot find package 'n8n-workflow'".
- .nvmrc: bump from v22.11.0 to v24.16.0 (current Active LTS, Krypton).
  Vitest 4 transitively pulls in std-env@4 (ESM-only) and require()s it
  from its CJS config loader; unflagged require(ESM) needs Node >=20.19
  or >=22.12, so v22.11.0 hit ERR_REQUIRE_ESM on `vitest run`. Moving to
  24.16.0 also matches engines.node already declared in package.json.

All 86 tests pass after these changes.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Cloudinary.node.ts: switch inputs/outputs to the 'main' string
  literals and drop the NodeConnectionType import. Change group from
  the bogus ['Cloudinary'] to ['transform'] so the node lands in the
  right category in the n8n node picker.
- package.json: pin the n8n-workflow peer range to ^2.0.0 instead of
  '*'. The previous wildcard let any host install try to load this
  node against incompatible majors; we already build and test against
  the 2.x API.
- package-lock.json: regenerated to match.
- .github/workflows/release.yml: drive setup-node from .nvmrc instead
  of hardcoding '22', and enable npm cache so releases install faster
  and stay in lockstep with the local toolchain.
- CLAUDE.md: rewrite the backwards-compatibility section to spell out
  the frozen-by-string vs frozen-by-meaning vs free-to-change axes,
  call out option values and displayOptions narrowing as breaking,
  document the typeVersion escape hatch, and separate workflow-JSON
  compatibility from runtime-host compatibility. Minor tightening
  elsewhere.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Step 1 of the asset-crud reorganization. UI-only relabel — `value` strings
(`updateAsset`, `admin`) are frozen-by-string per CLAUDE.md so saved
workflows still resolve. `Search Assets` belongs with the entity it
operates on; moved its operation entry, handler-map key, and field
`displayOptions.show.resource` from `admin` to `updateAsset`. Search is
not yet released, so no compatibility shim is needed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds the first new operation in the Asset CRUD effort. Identifies the
asset by its immutable asset_id and hits `GET /resources/:asset_id` —
a single-field form instead of the legacy three-field
(resource_type, type, public_id) shape.

Scope notes:
- Deliberately asset_id-only. No `identifyBy` mode selector — for
  new ops, a one-field form is strictly cleaner UX than gating it
  behind a picker most users wouldn't need.
- Existing `updateTags` / `updateMetadata` are not modified; their
  public_id surface stays exactly as it shipped. asset_id support for
  those ops is the job of the upcoming `asset` resource (see plan).

Adds `buildResourceByAssetIdUrl` utility, the `getOptions` collection
(colors, faces, image_metadata, pages, phash, coordinates,
accessibility_analysis, derived_next_cursor), and per-handler +
URL-builder tests.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds an Asset → Delete Assets op backed by the documented public_id
endpoint:

    DELETE /v1_1/{cloud}/resources/{resource_type}/{type}?public_ids=...

Cloudinary's delete endpoint is public_id-based, not asset_id-based —
an earlier draft assumed otherwise and failed at runtime with
"Illegal ids given". The field accepts:

  - a single public_id (no commas needed),
  - a comma-separated list, or
  - an array from an n8n expression (e.g. `{{ $items().map(...) }}`).

Arrays are pre-joined to a CSV before being placed in `qs` because
n8n's default query serializer turns JS arrays into `public_ids[0]=...`
(bracketed indices), which Cloudinary rejects with "public_ids must be
a list of strings or a comma separated string". Joining to CSV
side-steps the serializer entirely.

New helpers in cloudinary.utils.ts:
  - buildResourceDeleteUrl(cloud, resourceType, type)
  - splitCsvIds(csv) — trims and drops blanks

Adds 8 handler tests covering URL/method, single-string, CSV trimming,
array-from-expression, non-image resource routing, options merging,
and empty-input guards, plus unit tests for the new utils.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Splits the existing Update Asset surface into two resources:
- Asset: asset_id-keyed Get/Update/Search/Delete plus tag and metadata
  operations. asset_id is a single global key, so callers do not need
  to juggle (resource_type, type) coordinates.
- Asset (Legacy): kept at the bottom of the dropdown; only the
  public_id-keyed operations that shipped on master are exposed. Saved
  workflows continue to load unchanged.

Update Asset Tags gains a Mode selector with backward-compatible default:
- Set (default): existing behavior, replaces the tag list via the
  resource endpoint with Basic auth.
- Append: new path hitting POST /:resource_type/tags with command=add
  and signed auth, preserving existing tags. Shared appendTags helper
  lives in operations/tagAppend.ts and is wired into both resources'
  updateTags handlers.

The tag-action endpoint scopes lookups by (resource_type, type) and
silently returns public_ids: [] for assets stored under a non-default
type. Append mode now exposes a Type field (labeled to match the asset
object's "type" property) and threads it into the signed body, so
authenticated/private assets resolve correctly.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@eitanp461 eitanp461 changed the title Refactor node into per-operation handlers and add asset search Modular operations + Asset (asset_id) resource + tag Append mode May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant