Refine PgError constraint checks and document PK rule
Remove dead 23505 checks on single-GID primary keys (oauth2_consent, risk_assessment, risk_assessment_scenario, risk_assessment_scope). Add missing constraints to membership_profile and statement_of_applicability. Document composite-PK vs GID-PK rule in cursor rules and contrib guide. Signed-off-by: Émile Ré <emile@probo.com>
This commit is contained in:
@@ -38,3 +38,21 @@ if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok {
|
||||
This applies to all PostgreSQL error codes mapped to sentinel errors:
|
||||
- `"23505"` (unique violation) → `ErrResourceAlreadyExists`
|
||||
- `"23503"` (foreign key violation) → `ErrResourceInUse`
|
||||
|
||||
# Primary key constraint handling
|
||||
|
||||
**Do not** add a 23505 check for a single-column GID primary key (`id TEXT PRIMARY KEY`). GIDs are generated and cannot realistically collide; such a check is dead code.
|
||||
|
||||
**Do** check the composite primary key on junction tables where the PK represents a business uniqueness constraint (e.g. a link between two entities).
|
||||
|
||||
```go
|
||||
// GOOD — composite PK on a junction table (real business constraint)
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == "23505" && pgErr.ConstraintName == "risk_assessment_scenario_threats_pkey" {
|
||||
return ErrResourceAlreadyExists
|
||||
}
|
||||
|
||||
// BAD — single GID PK (can never collide, dead code)
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == "23505" && pgErr.ConstraintName == "risk_assessments_pkey" {
|
||||
return ErrResourceAlreadyExists
|
||||
}
|
||||
```
|
||||
|
||||
@@ -249,6 +249,20 @@ if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok {
|
||||
|
||||
The same applies to foreign key violations (`"23503"`) mapped to `ErrResourceInUse` — always verify the constraint name.
|
||||
|
||||
**Primary key handling:** Do not add a 23505 check for a single-column GID primary key (`id TEXT PRIMARY KEY`). GIDs are generated and cannot realistically collide — such a check is dead code. Only check the primary key constraint on **composite-PK junction tables** where the PK represents a business uniqueness constraint (e.g. linking a scenario to a threat).
|
||||
|
||||
```go
|
||||
// Good — composite PK on junction table (real business constraint)
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == "23505" && pgErr.ConstraintName == "risk_assessment_scenario_threats_pkey" {
|
||||
return ErrResourceAlreadyExists
|
||||
}
|
||||
|
||||
// Bad — single GID PK (cannot collide, dead code)
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == "23505" && pgErr.ConstraintName == "risk_assessments_pkey" {
|
||||
return ErrResourceAlreadyExists
|
||||
}
|
||||
```
|
||||
|
||||
## Filters
|
||||
|
||||
Filters implement `SQLFragment() string` and `SQLArguments() pgx.NamedArgs`. Use double pointers for three-state filtering: `nil` = no filter, `*nil` = IS NULL, `*val` = equals.
|
||||
|
||||
@@ -1219,9 +1219,14 @@ VALUES (
|
||||
|
||||
_, err := conn.Exec(ctx, q, args)
|
||||
if err != nil {
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == "23505" && pgErr.ConstraintName == "idx_profiles_identity_id_organization_id" {
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == "23505" {
|
||||
switch pgErr.ConstraintName {
|
||||
case "idx_profiles_identity_id_organization_id",
|
||||
"idx_profiles_external_id_organization_id",
|
||||
"idx_profiles_user_name_organization_id":
|
||||
return ErrResourceAlreadyExists
|
||||
}
|
||||
}
|
||||
|
||||
return fmt.Errorf("cannot insert profile: %w", err)
|
||||
}
|
||||
|
||||
@@ -22,7 +22,6 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgconn"
|
||||
"go.gearno.de/kit/pg"
|
||||
"go.probo.inc/probo/pkg/gid"
|
||||
"go.probo.inc/probo/pkg/page"
|
||||
@@ -367,10 +366,6 @@ INSERT INTO iam_oauth2_consents (
|
||||
|
||||
_, err := conn.Exec(ctx, q, args)
|
||||
if err != nil {
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == "23505" && pgErr.ConstraintName == "iam_oauth2_consents_pkey" {
|
||||
return ErrResourceAlreadyExists
|
||||
}
|
||||
|
||||
return fmt.Errorf("cannot insert oauth2_consent: %w", err)
|
||||
}
|
||||
|
||||
|
||||
@@ -22,7 +22,6 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgconn"
|
||||
"go.gearno.de/kit/pg"
|
||||
"go.probo.inc/probo/pkg/gid"
|
||||
"go.probo.inc/probo/pkg/page"
|
||||
@@ -191,9 +190,6 @@ VALUES (@id, @tenant_id, @organization_id, @name, @description, @created_at, @up
|
||||
|
||||
_, err := conn.Exec(ctx, q, args)
|
||||
if err != nil {
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == "23505" && pgErr.ConstraintName == "risk_assessments_pkey" {
|
||||
return ErrResourceAlreadyExists
|
||||
}
|
||||
return fmt.Errorf("cannot insert risk assessment: %w", err)
|
||||
}
|
||||
return nil
|
||||
|
||||
@@ -22,7 +22,6 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgconn"
|
||||
"go.gearno.de/kit/pg"
|
||||
"go.probo.inc/probo/pkg/gid"
|
||||
"go.probo.inc/probo/pkg/page"
|
||||
@@ -351,9 +350,6 @@ INSERT INTO risk_assessment_scenarios (
|
||||
}
|
||||
_, err := conn.Exec(ctx, q, args)
|
||||
if err != nil {
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == "23505" && pgErr.ConstraintName == "risk_assessment_scenarios_pkey" {
|
||||
return ErrResourceAlreadyExists
|
||||
}
|
||||
return fmt.Errorf("cannot insert risk scenario: %w", err)
|
||||
}
|
||||
return nil
|
||||
|
||||
@@ -22,7 +22,6 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgconn"
|
||||
"go.gearno.de/kit/pg"
|
||||
"go.probo.inc/probo/pkg/gid"
|
||||
"go.probo.inc/probo/pkg/page"
|
||||
@@ -196,9 +195,6 @@ INSERT INTO risk_assessment_scopes (
|
||||
}
|
||||
_, err := conn.Exec(ctx, q, args)
|
||||
if err != nil {
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == "23505" && pgErr.ConstraintName == "risk_assessment_scopes_pkey" {
|
||||
return ErrResourceAlreadyExists
|
||||
}
|
||||
return fmt.Errorf("cannot insert risk assessment scope: %w", err)
|
||||
}
|
||||
return nil
|
||||
|
||||
@@ -227,8 +227,10 @@ VALUES (
|
||||
|
||||
_, err := conn.Exec(ctx, q, args)
|
||||
if err != nil {
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok {
|
||||
if pgErr.Code == "23505" && pgErr.ConstraintName == "statements_of_applicability_document_id_key" {
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == "23505" {
|
||||
switch pgErr.ConstraintName {
|
||||
case "statements_of_applicability_document_id_key",
|
||||
"states_of_applicability_name_organization_id_uniq":
|
||||
return ErrResourceAlreadyExists
|
||||
}
|
||||
}
|
||||
@@ -267,8 +269,10 @@ WHERE
|
||||
|
||||
result, err := conn.Exec(ctx, q, args)
|
||||
if err != nil {
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok {
|
||||
if pgErr.Code == "23505" && pgErr.ConstraintName == "statements_of_applicability_document_id_key" {
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == "23505" {
|
||||
switch pgErr.ConstraintName {
|
||||
case "statements_of_applicability_document_id_key",
|
||||
"states_of_applicability_name_organization_id_uniq":
|
||||
return ErrResourceAlreadyExists
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user