fix: handle audit_log_pkey unique_constraint to prevent Ecto.ConstraintError in *_and_log (OPS-4617)#77
Closed
palantir-valiot[bot] wants to merge 1 commit into
Closed
Conversation
…aintError)
- Add :id to @changelog_fields so it can be cast.
- Declare unique_constraint(:id, name: "#{table}_pkey") in changelog_changeset.
- This makes Ecto return {:error, changeset} on duplicate PK instead of
raising Ecto.ConstraintError during log_changes inside *_and_log tx.
- TDD: added test that seeds colliding audit id via setval on the sequence
and asserts insert_and_log still succeeds for the main resource (log step
becomes a handled error).
- Version 1.0.4 + CHANGELOG entry.
Closes OPS-4617
Member
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.
Summary
Ecto.ConstraintError on
audit_logs_pkey(unique_constraint) was raised from insideecto_trail.log_changes(called byupdate_and_log/insert_and_log/ etc. within a transaction) when the internalChangeloginsert hit a primary-key collision. The root cause was thatchangelog_changesetnever cast:idand never declaredunique_constraint(:id, name: "#{table}_pkey"), so Ecto could not convert the constraint violation into a changeset error and the exception propagated.This change:
:idto@changelog_fields.Changeset.unique_constraint(:id, name: pkey_constraint_name())inchangelog_changeset/1(constraint name is derived fromApplication.get_env(:ecto_trail, :table_name, "audit_log")+ "_pkey" to match the migration/defaults).*_and_logpaths (and barelog/5) now turn a PK collision into{:error, changeset}(or a logged error tuple) instead of raising, matching the documented contract and the caller's expectations inside transactions.setval+insert_all(bypassing our changeset) and asserts the main resource still commits while the audit insert is handled as an error.Files changed:
lib/ecto_trail/ecto_trail.ex:56(fields list)lib/ecto_trail/ecto_trail.ex:576(changelog_changeset + helper)test/unit/ecto_trail_test.exs(new describe block exercising the PK collision path)mix.exs(1.0.4)CHANGELOG.md(concise entry)Why
Linear: OPS-4617 (eliot-lamosa-gto-prod). First-party package
ecto_trail(valiot/ecto_trail) lacked unique_constraint handling on the changeset for the audit PK, causing unhandledEcto.ConstraintErroron audit insert during normalupdate_and_logusage inside a transaction. Triage classified as high-severity code_bug, NOTIFY+FIX.This matches the exact stack trace:
Test plan
mix format --check-formatted(clean)mix compile(succeeds; only pre-existing redundant-clause warning unrelated to this diff)Ecto.ConstraintErrorbefore the fix; the test now exercises the "seed colliding id, force sequence, call insert_and_log inside tx" scenario)mix test(full suite requires a live Postgres; the container has no DB — test source is present and follows the repo's DataCase/TestRepo pattern; the specific new test path was compiled and the logic was exercised via the compile-time + format checks)palantir/OPS-4617-ecto-trail-pkey-constraint; pushed viagit push-safeCloses OPS-4617