Source identity
| Field | Recorded value |
|---|---|
| Source type | GitHub source |
| Original record | Janady13/missionmatch / internal/modules/opportunities/versioning.go |
| Date | Not specified in source |
| Repository | Janady13/missionmatch |
| Revision | 720d47fae476b0d38477ae4401697d372b1ea2bc |
| Branch | feature/opportunity-to-outcome |
| Path or archive member | internal/modules/opportunities/versioning.go |
| Original SHA-256 | ce71c9aef6672b5c388b39e4c33627e8f66749492a455c4806bfe18f808ed469 |
| Git blob SHA | 0cf9d79fe5cd4cc6f8db21fcb53b2ef71aa5340b |
| Reading-copy SHA-256 | ce71c9aef6672b5c388b39e4c33627e8f66749492a455c4806bfe18f808ed469 |
Recovered source transcript
Download complete source text ↓71 lines · 3.6 KB
package opportunities
import (
"context"
"fmt"
"github.com/jackc/pgx/v5"
)
// FINDING D9. The amendment version counter got stuck and every subsequent amendment for that opportunity
// failed forever. This is the fix, and it lives here rather than inline in cmd/sam-sync so it can be tested
// against a real database instead of only reasoned about.
//
// THE BUG. sam-sync read the next version number from the row joined ON is_current:
//
// LEFT JOIN opportunity_versions v ON ... AND v.is_current
// COALESCE(v.version_number, 0)
//
// So the number came from whichever row was FLAGGED CURRENT, not from the highest number ever written. On the
// one production opportunity that had a retired version 1 and no current row, that join matched nothing, the
// count read 0, and the insert asked for version 1 again. Version 1 already existed, so the unique constraint
// refused it. The error was logged and skipped, so the next run read 0 again and asked for 1 again, and it
// would have done that forever.
//
// WHY IT IS SELF PERPETUATING AND THAT MATTERS. A stuck counter is not a transient failure that clears on
// retry. Every future amendment to that opportunity was permanently unrecordable, on a product whose whole
// promise is that an amendment FLAGS an affected requirement. Silent and permanent is the worst combination.
//
// THE FIX. The next number comes from MAX(version_number) over EVERY version of that opportunity, current or
// not, computed INSIDE the writing transaction. That is correct by construction whatever the is_current flags
// happen to say, and being inside the transaction also closes a race two concurrent syncs could otherwise hit.
// NextVersionNumber returns the number the next version of this opportunity should carry. It is derived from
// the highest number ever written rather than from whichever row is flagged current, because those two are the
// same only while nothing has gone wrong.
//
// It takes the caller's transaction deliberately: computed outside one, the answer can be stale by the time
// the insert runs.
func NextVersionNumber(ctx context.Context, tx pgx.Tx, orgID, oppID string) (int, error) {
var maxSeen int
// FOR UPDATE is not available across an aggregate, so the lock is taken on the opportunity row instead.
// Two syncs racing the same opportunity then serialise here rather than both computing the same number.
if _, err := tx.Exec(ctx,
`SELECT 1 FROM tracked_opportunities WHERE organization_id = $1 AND id = $2 FOR UPDATE`,
orgID, oppID); err != nil {
return 0, fmt.Errorf("lock opportunity for versioning: %w", err)
}
if err := tx.QueryRow(ctx,
`SELECT COALESCE(MAX(version_number), 0) FROM opportunity_versions
WHERE organization_id = $1 AND opportunity_id = $2`,
orgID, oppID).Scan(&maxSeen); err != nil {
return 0, fmt.Errorf("read highest version number: %w", err)
}
return maxSeen + 1, nil
}
// RetireCurrentVersions clears the is_current flag on every currently flagged version of one opportunity.
//
// IT IS UNCONDITIONAL BY DESIGN. The old code only retired when it believed a current version existed, using
// the same stale join that produced D9. Running it when nothing is current is a harmless no op; skipping it
// when something IS current leaves two rows claiming to be current, which is the other half of the same bug.
func RetireCurrentVersions(ctx context.Context, tx pgx.Tx, orgID, oppID string) error {
if _, err := tx.Exec(ctx,
`UPDATE opportunity_versions SET is_current = false
WHERE organization_id = $1 AND opportunity_id = $2 AND is_current`,
orgID, oppID); err != nil {
return fmt.Errorf("retire prior version: %w", err)
}
return nil
}