[upstream #5007] fix(core): Prevent duplicate translations from concurrent saves - #5
Open
ayim wants to merge 3 commits into
Open
[upstream #5007] fix(core): Prevent duplicate translations from concurrent saves#5ayim wants to merge 3 commits into
ayim wants to merge 3 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirror of vendurehq#5007.
Original author: @brmk
Original head SHA:
4ad3786017cec80dad33e1a33470ccb30caffdb5Original target:
masterDescription
Translatable entities (
Product,Collection,ProductVariant, etc.) could end up with two translation rows for the same(baseId, languageCode)pair.TranslatableSaver.update()loads existing translations, thenTranslationDifferinserts any language not already present — with no locking and no unique constraint. Two concurrent updates that both add the same new language (e.g. a double-click on Update in the Admin UI) can both read "noenrow yet" before either commits, and both insert one. Once that happens, the entity becomes uneditable through the Admin UI.This PR:
@Unique(['languageCode', 'base'])to all 13 translation entities, so the database itself now rejects a second row for the same(baseId, languageCode)pair.isUniqueConstraintViolationError()todb-errors.ts(Postgres/MySQL/SQLite), mirroring the existingisForeignKeyViolationError().TranslationDiffer.applyDiff()so that when an insert hits this constraint (a concurrent request already inserted the row), it re-fetches that row and updates it instead of throwing a 500 — so a losing concurrent save converges instead of leaving the entity stuck.deduplicateTranslations()tomigration-utils, a helper that accepts a single table name or an array of table names, which users can call from their own migration'sup()(before the new unique constraint is created) to remove any duplicate rows that already exist in their database. It also works for any custom translatable entity a plugin defines, not just the 13 core ones.updateProductcalls adding the same new language) and asserting only one translation row results.Fixes vendurehq#4884
Breaking changes
Adds a unique index to the 13
*_translationtables. This only affects databases where duplicate rows already exist for the same(baseId, languageCode)— in that case, runningvendure migrate generatewill produce a migration that fails onup()until the duplicates are removed. No changes to any public API surface otherwise (deduplicateTranslationsis a new, additive export).Migration guide for users
Since existing production databases may already contain the duplicate rows this PR guards against, upgrading needs one extra manual step — this should be called out in the release notes:
@vendure/coreand runvendure migrate generate <name>as usual. If your database has no duplicates, the generated migration only adds the new constraints and needs no further changes.up()), open the generated migration file and add adeduplicateTranslations(queryRunner, [...])call at the very top ofup(), listing every*_translationtable touched by that migration (see the JSDoc example ondeduplicateTranslationsinmigration-utils/translation-deduplication.tsfor a full worked Postgres example with the exact, deterministic constraint names TypeORM will generate). If you have your own plugins with custom translatable entities, include their translation tables in that same array.deduplicateTranslationskeeps the most recently updated row per(baseId, languageCode)and removes the rest, so no translation content is lost — only the redundant duplicate rows are dropped.Worth adding to the upgrade guide / release notes so users hitting the failed-migration case aren't stuck guessing what to do.
Checklist
📌 Always:
👍 Most of the time:
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.