Uh oh!
There was an error while loading. Please reload this page.
feat: add onDuplicate option to DestinationAppwrite - #169
Conversation
CSV / JSON / appwrite-to-appwrite imports that re-run on the same batch (e.g. user re-uploads the same file, or a worker retries a failed chunk) currently throw DuplicateException and abort the whole batch. Wrap the row-buffer flush in the new skipDuplicates() scope guard so duplicate-by-id rows are silently no-op'd at the adapter layer (INSERT IGNORE / ON CONFLICT DO NOTHING / $setOnInsert), letting the rest of the batch proceed. The existing skipRelationshipsExistCheck() wrapper is preserved (FK-target guard); skipDuplicates composes around it. Feature-branch note: depends on utopia-php/database's skipDuplicates() scope guard from PR utopia-php/database#852. composer.json is temporarily pinned to dev-csv-import-upsert-v2 with a 5.99.0 alias so composer can resolve the 5.* constraint transitively. Must be reset to the proper release version (^5.X.Y) once PR #852 merges and utopia-php/database ships.
Per Jake's spec, migration destinations accept two new behavior flags: - overwrite=true → use upsertDocuments() instead of createDocuments() Replaces existing rows with the imported values. Naturally handles duplicate ids. - skip=true → wrap createDocuments() in skipDuplicates() scope guard. Silently no-ops duplicate ids at the adapter layer (INSERT IGNORE equivalent). Existing rows are preserved. Default (both false): plain createDocuments, fails fast on DuplicateException. Original behavior, unchanged for existing callers. Precedence when both set: overwrite wins (upsert subsumes skip). The existing skipRelationshipsExistCheck() FK-guard wrapper is preserved in all three branches.
Greptile SummaryThis PR introduces a PHP backed enum Confidence Score: 5/5Safe to merge — no blocking issues found; default behavior is unchanged and all three dispatch arms are correct. All three No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "Move OnDuplicate enum to Destinations na..." | Re-trigger Greptile |
| @@ -2246,9 +2248,10 @@ | |||
| "ext-pdo": "*", | |||
There was a problem hiding this comment.
skipDuplicates() availability in 5.3.22 needs verification
The PR description states the lock must be pinned to the unreleased dev-csv-import-upsert-v2 branch because skipDuplicates() doesn't exist in any released version of utopia-php/database. However, the actual composer.lock here resolves to the tagged release 5.3.22 — not a dev branch. If skipDuplicates() is absent from 5.3.22, the ON_DUPLICATE_SKIP code path will throw a fatal "Call to undefined method" error at runtime even though static analysis may have passed (PHPStan level 3 may not resolve the method through the scope guard pattern). Please confirm whether 5.3.22 includes skipDuplicates() and update the PR description / draft status accordingly.
Uh oh!
There was an error while loading. Please reload this page.
| public const ON_DUPLICATE_FAIL = 'fail'; | ||
| public const ON_DUPLICATE_SKIP = 'skip'; | ||
| public const ON_DUPLICATE_UPSERT = 'upsert'; | ||
| /** | ||
| * @var array<string> | ||
| */ | ||
| public const ON_DUPLICATES = [ | ||
| self::ON_DUPLICATE_FAIL, | ||
| self::ON_DUPLICATE_SKIP, | ||
| self::ON_DUPLICATE_UPSERT, | ||
| ]; |
There was a problem hiding this comment.
Let's use an enum, push for modern PHP features in new code
Uh oh!
There was an error while loading. Please reload this page.
Summary
Adds an
$onDuplicateconstructor param toDestinationAppwriteso CSV / JSON / appwrite→appwrite imports can control how duplicate-id rows are handled. Three modes:onDuplicate = "fail"(default) — plaincreateDocuments(). Fails fast onDuplicateException. Original behavior, unchanged.onDuplicate = "skip"— wrapscreateDocuments()inskipDuplicates()scope guard. Silently no-ops duplicate ids at the adapter layer (INSERT IGNOREequivalent). Existing rows preserved unchanged.onDuplicate = "upsert"—upsertDocuments()instead ofcreateDocuments(). Replaces existing rows with imported values.The existing
skipRelationshipsExistCheck()FK-guard is preserved in all three branches.Design
Single string param + class constants +
WhiteListvalidator at the REST boundary. This matches:encryption,priority,period,grant_typeetc. acrossapp/controllers/api/*.phpall usenew WhiteList(['a', 'b', 'c'])over raw strings. None of the 92 enum-styleWhiteListparams in Appwrite use PHP backed enums.Resource::STATUS_*,Resource::TYPE_*).Defines
ON_DUPLICATE_FAIL,ON_DUPLICATE_SKIP,ON_DUPLICATE_UPSERTclass constants onDestinations\Appwrite, plus anON_DUPLICATESarray consumers can drop straight intoWhiteList(...). Invalid values throwInvalidArgumentExceptionin the constructor.Changes
src/Migration/Destinations/Appwrite.php— constructor acceptsstring $onDuplicate = self::ON_DUPLICATE_FAIL;createRecord()row-buffer flush dispatches viamatchon the three modes.Dependency
Uses
skipDuplicates()scope guard fromutopia-php/database@5.3.22(already tagged, includes utopia-php/database#852).composer.jsonkeeps the existing"utopia-php/database": "5.*"wildcard;composer.lockresolves to5.3.22.Test plan
testCreateCSVImportSkipDuplicates,testCreateCSVImportOverwrite,testCreateCSVImportDefaultFailsOnDuplicate— all passing locally against a full appwrite stackDownstream PR