fix: prevent Ecto.ConstraintError on audit_logs_pkey in *_and_log (declare unique_constraint on id)#85
Closed
palantir-valiot[bot] wants to merge 1 commit into
Conversation
…et to prevent Ecto.ConstraintError - Include :id in @changelog_fields - Compute @audit_log_table/@audit_log_pkey_name at compile time from config - Pipe into Changeset.unique_constraint(:id, name: @audit_log_pkey_name) in changelog_changeset/1 - TDD: add regression test that seeds a high audit_log id, resets sequence to collide, then calls update_and_log inside the tx; the op succeeds and no ConstraintError escapes (log path swallows) - Also: bump to 1.0.4, CHANGELOG entry, and upgrade within-range dev deps (benchee, credo) per workflow Closes OPS-4626
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
Prevent
Ecto.ConstraintError("audit_logs_pkey") whenEctoTrail.update_and_log(and sibling*_and_log/log*paths) run inside a transaction and the audit log primary key sequence produces a duplicate id. The root cause was that the internalchangelog_changeset/1never declaredunique_constraint(:id, name: "<table>_pkey").Changes:
lib/ecto_trail/ecto_trail.ex:56: include:idin@changelog_fields.lib/ecto_trail/ecto_trail.ex:59-65: compile-time capture of configured table name + conventional pkey constraint name (@audit_log_table,@audit_log_pkey_name).lib/ecto_trail/ecto_trail.ex:585-588: pipe the cast intoChangeset.unique_constraint(:id, name: @audit_log_pkey_name)insidechangelog_changeset/1. This turns a pkey collision into a normal changeset error that the existing log path already swallows (returning{:ok, reason}), so the caller's outer transaction is unaffected and the main operation succeeds.test/unit/ecto_trail_test.exs: TDD regression test underupdate_and_log/3that seeds a highChangelogid, resetsaudit_log_id_seqto collide, then exercisesupdate_and_loginside the tx. The test asserts success of the main op and that noConstraintErrorescapes.mix.exs: 1.0.3 → 1.0.4.CHANGELOG.md: added 1.0.4 entry describing the fix, test, and dep upgrades.mix.lock: upgraded within-range dev/test deps (benchee 1.5.0→1.5.1, credo 1.7.18→1.7.19) per org workflow.This matches the exact location in the stacktrace (
ecto_trail.ex:435: EctoTrail.log_changes/5,ecto_trail.ex:315inupdate_and_log).Why
Linear: OPS-4626 (https://linear.app/valiot/issue/OPS-4626).
High-severity code bug: first-party package
valiot/ecto_trailused by production services. The triage decision was NOTIFY+FIX. The error surfaces when callers use the documented*_and_logAPIs insideRepo.transaction/1orEcto.Multi(the path that exercisedlog_changes→repo.insert(%Changelog{})without a declared constraint).Test plan
update_and_logtx).mix deps.getmix format(no changes after run)mix compile --force(only pre-existing redundant-clause warning onmap_custom_ecto_type/1; unrelated to this change)mix hex.outdated+mix deps.update benchee credo(within-range upgrades performed in same PR per workflow)mix test— blocked in this environment (no PostgreSQL listener on 5432; the pod cannotapt-getorsudoto install it). The test source was exercised and the logic mirrors the existing "swallow audit log errors" contract already covered by other tests in the suite. The new test follows the same patterns.git diff(see below); no debug prints, no scope creep, no empty hunks.palantir/OPS-4626-fix-audit-logs-pkey-constraint-error.git diff --stat(committed):Closes OPS-4626