Skip to content

Commit 299ba6f

Browse files
PaulDottererPaulDotterer
authored
refactor(store): introduce ErrNotFound sentinel + scrub pgx.ErrNoRows from non-store packages (#243) (#244)
First wave of the storage abstraction tracker (#242). Stops handler, projector, scim, idp, and control packages from reaching for pgx.ErrNoRows directly; centralizes not-found recognition in store.IsNotFound so a future backend (libSQL/Turso as illustrative example, NoSQL as architectural ceiling) registers its own no-rows error in one place rather than in every caller. No behaviour change. errors.Is semantics are preserved end-to-end; the helpers_test bridge test (TestHandleGetError_NotFound) locks both pgx.ErrNoRows and store.ErrNotFound through the recognizer. Adds: server/internal/store/notfound.go Touches: 34 files across api/, scim/, idp/, projectors/, control/. Closes #243.
1 parent 8d12458 commit 299ba6f

35 files changed

Lines changed: 133 additions & 150 deletions

‎internal/api/action_crud.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,8 @@ package api
88
import (
99
"context"
1010
"encoding/json"
11-
"errors"
1211

1312
"connectrpc.com/connect"
14-
"github.com/jackc/pgx/v5"
1513
"github.com/oklog/ulid/v2"
1614

1715
pm "github.com/manchtools/power-manage/sdk/gen/go/pm/v1"
@@ -191,7 +189,7 @@ func (h *ActionHandler) RenameAction(ctx context.Context, req *connect.Request[p
191189

192190
// Verify action exists before appending event
193191
if _, err := h.store.Queries().GetActionByID(ctx, req.Msg.Id); err != nil {
194-
if errors.Is(err, pgx.ErrNoRows) {
192+
if store.IsNotFound(err) {
195193
return nil, apiErrorCtx(ctx, ErrActionNotFound, connect.CodeNotFound, "action not found")
196194
}
197195
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get action")
@@ -233,7 +231,7 @@ func (h *ActionHandler) UpdateActionDescription(ctx context.Context, req *connec
233231

234232
// Verify action exists before appending event
235233
if _, err := h.store.Queries().GetActionByID(ctx, req.Msg.Id); err != nil {
236-
if errors.Is(err, pgx.ErrNoRows) {
234+
if store.IsNotFound(err) {
237235
return nil, apiErrorCtx(ctx, ErrActionNotFound, connect.CodeNotFound, "action not found")
238236
}
239237
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get action")

‎internal/api/action_dispatch.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,12 @@ package api
1111
import (
1212
"context"
1313
"encoding/json"
14-
"errors"
1514
"fmt"
1615
"strings"
1716
"time"
1817

1918
"connectrpc.com/connect"
2019
"github.com/hibiken/asynq"
21-
"github.com/jackc/pgx/v5"
2220
"github.com/oklog/ulid/v2"
2321

2422
pm "github.com/manchtools/power-manage/sdk/gen/go/pm/v1"
@@ -93,7 +91,7 @@ func (h *ActionHandler) DispatchAction(ctx context.Context, req *connect.Request
9391
case *pm.DispatchActionRequest_ActionId:
9492
action, err := h.store.Queries().GetActionByID(ctx, source.ActionId)
9593
if err != nil {
96-
if errors.Is(err, pgx.ErrNoRows) {
94+
if store.IsNotFound(err) {
9795
return nil, apiErrorCtx(ctx, ErrActionNotFound, connect.CodeNotFound, "action not found")
9896
}
9997
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get action")

‎internal/api/assignment_handler.go‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@ package api
22

33
import (
44
"context"
5-
"errors"
65
"log/slog"
76

87
"connectrpc.com/connect"
9-
"github.com/jackc/pgx/v5"
108
"github.com/oklog/ulid/v2"
119
"google.golang.org/protobuf/types/known/timestamppb"
1210

@@ -49,31 +47,31 @@ func (h *AssignmentHandler) CreateAssignment(ctx context.Context, req *connect.R
4947
case pm.AssignmentSourceType_ASSIGNMENT_SOURCE_TYPE_ACTION:
5048
_, err := h.store.Queries().GetActionByID(ctx, req.Msg.SourceId)
5149
if err != nil {
52-
if errors.Is(err, pgx.ErrNoRows) {
50+
if store.IsNotFound(err) {
5351
return nil, apiErrorCtx(ctx, ErrActionNotFound, connect.CodeNotFound, "action not found")
5452
}
5553
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get action")
5654
}
5755
case pm.AssignmentSourceType_ASSIGNMENT_SOURCE_TYPE_ACTION_SET:
5856
_, err := h.store.Queries().GetActionSetByID(ctx, req.Msg.SourceId)
5957
if err != nil {
60-
if errors.Is(err, pgx.ErrNoRows) {
58+
if store.IsNotFound(err) {
6159
return nil, apiErrorCtx(ctx, ErrActionSetNotFound, connect.CodeNotFound, "action set not found")
6260
}
6361
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get action set")
6462
}
6563
case pm.AssignmentSourceType_ASSIGNMENT_SOURCE_TYPE_DEFINITION:
6664
_, err := h.store.Queries().GetDefinitionByID(ctx, req.Msg.SourceId)
6765
if err != nil {
68-
if errors.Is(err, pgx.ErrNoRows) {
66+
if store.IsNotFound(err) {
6967
return nil, apiErrorCtx(ctx, ErrDefinitionNotFound, connect.CodeNotFound, "definition not found")
7068
}
7169
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get definition")
7270
}
7371
case pm.AssignmentSourceType_ASSIGNMENT_SOURCE_TYPE_COMPLIANCE_POLICY:
7472
_, err := h.store.Queries().GetCompliancePolicyByID(ctx, req.Msg.SourceId)
7573
if err != nil {
76-
if errors.Is(err, pgx.ErrNoRows) {
74+
if store.IsNotFound(err) {
7775
return nil, apiErrorCtx(ctx, ErrCompliancePolicyNotFound, connect.CodeNotFound, "compliance policy not found")
7876
}
7977
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get compliance policy")
@@ -85,31 +83,31 @@ func (h *AssignmentHandler) CreateAssignment(ctx context.Context, req *connect.R
8583
case pm.AssignmentTargetType_ASSIGNMENT_TARGET_TYPE_DEVICE:
8684
_, err := h.store.Queries().GetDeviceByID(ctx, db.GetDeviceByIDParams{ID: req.Msg.TargetId})
8785
if err != nil {
88-
if errors.Is(err, pgx.ErrNoRows) {
86+
if store.IsNotFound(err) {
8987
return nil, apiErrorCtx(ctx, ErrDeviceNotFound, connect.CodeNotFound, "device not found")
9088
}
9189
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get device")
9290
}
9391
case pm.AssignmentTargetType_ASSIGNMENT_TARGET_TYPE_DEVICE_GROUP:
9492
_, err := h.store.Queries().GetDeviceGroupByID(ctx, req.Msg.TargetId)
9593
if err != nil {
96-
if errors.Is(err, pgx.ErrNoRows) {
94+
if store.IsNotFound(err) {
9795
return nil, apiErrorCtx(ctx, ErrDeviceGroupNotFound, connect.CodeNotFound, "device group not found")
9896
}
9997
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get device group")
10098
}
10199
case pm.AssignmentTargetType_ASSIGNMENT_TARGET_TYPE_USER:
102100
_, err := h.store.Queries().GetUserByID(ctx, req.Msg.TargetId)
103101
if err != nil {
104-
if errors.Is(err, pgx.ErrNoRows) {
102+
if store.IsNotFound(err) {
105103
return nil, apiErrorCtx(ctx, ErrUserNotFound, connect.CodeNotFound, "user not found")
106104
}
107105
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get user")
108106
}
109107
case pm.AssignmentTargetType_ASSIGNMENT_TARGET_TYPE_USER_GROUP:
110108
_, err := h.store.Queries().GetUserGroupByID(ctx, req.Msg.TargetId)
111109
if err != nil {
112-
if errors.Is(err, pgx.ErrNoRows) {
110+
if store.IsNotFound(err) {
113111
return nil, apiErrorCtx(ctx, ErrUserGroupNotFound, connect.CodeNotFound, "user group not found")
114112
}
115113
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get user group")
@@ -135,7 +133,7 @@ func (h *AssignmentHandler) CreateAssignment(ctx context.Context, req *connect.R
135133
return connect.NewResponse(&pm.CreateAssignmentResponse{
136134
Assignment: h.assignmentToProto(existingAssignment),
137135
}), nil
138-
} else if !errors.Is(err, pgx.ErrNoRows) {
136+
} else if !store.IsNotFound(err) {
139137
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to check existing assignment")
140138
}
141139

‎internal/api/auth_handler.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,9 @@ package api
33

44
import (
55
"context"
6-
"errors"
76
"log/slog"
87

98
"connectrpc.com/connect"
10-
"github.com/jackc/pgx/v5"
119
"google.golang.org/protobuf/types/known/timestamppb"
1210

1311
pm "github.com/manchtools/power-manage/sdk/gen/go/pm/v1"
@@ -57,7 +55,7 @@ func (h *AuthHandler) Login(ctx context.Context, req *connect.Request[pm.LoginRe
5755

5856
user, err := h.store.Queries().GetUserByEmail(ctx, req.Msg.Email)
5957
if err != nil {
60-
if errors.Is(err, pgx.ErrNoRows) {
58+
if store.IsNotFound(err) {
6159
// Perform a dummy bcrypt comparison to prevent timing-based user enumeration
6260
auth.VerifyPassword(req.Msg.Password, auth.DummyHash)
6361
return nil, apiErrorCtx(ctx, ErrInvalidCredentials, connect.CodeUnauthenticated, "invalid credentials")
@@ -183,7 +181,7 @@ func (h *AuthHandler) RefreshToken(ctx context.Context, req *connect.Request[pm.
183181

184182
// Atomically revoke the old refresh token BEFORE generating new tokens.
185183
// RevokeToken uses INSERT ... ON CONFLICT DO NOTHING RETURNING jti, so it
186-
// returns pgx.ErrNoRows when the token was already revoked by a concurrent
184+
// reports not-found (recognized via store.IsNotFound) when the token was already revoked by a concurrent
187185
// request. This prevents a race where two concurrent RefreshToken calls
188186
// with the same token both succeed.
189187
if result.OldJTI != "" {

‎internal/api/certificate_handler.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"time"
77

88
"connectrpc.com/connect"
9-
"github.com/jackc/pgx/v5"
109
"google.golang.org/protobuf/types/known/timestamppb"
1110

1211
pm "github.com/manchtools/power-manage/sdk/gen/go/pm/v1"
@@ -52,7 +51,7 @@ func (h *CertificateHandler) RenewCertificate(ctx context.Context, req *connect.
5251
ID: deviceID,
5352
})
5453
if err != nil {
55-
if err == pgx.ErrNoRows {
54+
if store.IsNotFound(err) {
5655
return nil, apiErrorCtx(ctx, ErrDeviceNotFound, connect.CodeNotFound, "device not found")
5756
}
5857
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to look up device")

‎internal/api/device_group_handler.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@ package api
22

33
import (
44
"context"
5-
"errors"
65
"log/slog"
76

87
"connectrpc.com/connect"
9-
"github.com/jackc/pgx/v5"
108
"github.com/oklog/ulid/v2"
119
"google.golang.org/protobuf/types/known/timestamppb"
1210

@@ -315,7 +313,7 @@ func (h *DeviceGroupHandler) AddDeviceToGroup(ctx context.Context, req *connect.
315313
// Verify device exists
316314
_, err = q.GetDeviceByID(ctx, db.GetDeviceByIDParams{ID: deviceID})
317315
if err != nil {
318-
if errors.Is(err, pgx.ErrNoRows) {
316+
if store.IsNotFound(err) {
319317
return nil, apiErrorCtx(ctx, ErrDeviceNotFound, connect.CodeNotFound, "device not found")
320318
}
321319
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get device")

‎internal/api/device_handler.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"crypto/rand"
66
"encoding/hex"
77
"encoding/json"
8-
"errors"
98
"fmt"
109
"log/slog"
1110
"regexp"
@@ -14,7 +13,6 @@ import (
1413

1514
"connectrpc.com/connect"
1615
"github.com/hibiken/asynq"
17-
"github.com/jackc/pgx/v5"
1816
"google.golang.org/protobuf/encoding/protojson"
1917
"google.golang.org/protobuf/types/known/timestamppb"
2018

@@ -325,7 +323,7 @@ func (h *DeviceHandler) AssignDevice(ctx context.Context, req *connect.Request[p
325323
// Verify user exists
326324
_, err = q.GetUserByID(ctx, userID)
327325
if err != nil {
328-
if errors.Is(err, pgx.ErrNoRows) {
326+
if store.IsNotFound(err) {
329327
return nil, apiErrorCtx(ctx, ErrUserNotFound, connect.CodeNotFound, "user not found")
330328
}
331329
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get user")
@@ -354,7 +352,7 @@ func (h *DeviceHandler) AssignDevice(ctx context.Context, req *connect.Request[p
354352
// Verify user group exists
355353
_, err = q.GetUserGroupByID(ctx, groupID)
356354
if err != nil {
357-
if errors.Is(err, pgx.ErrNoRows) {
355+
if store.IsNotFound(err) {
358356
return nil, apiErrorCtx(ctx, ErrUserGroupNotFound, connect.CodeNotFound, "user group not found")
359357
}
360358
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get user group")

‎internal/api/helpers.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,10 @@ package api
22

33
import (
44
"context"
5-
"errors"
65
"log/slog"
76
"math"
87

98
"connectrpc.com/connect"
10-
"github.com/jackc/pgx/v5"
119

1210
"github.com/manchtools/power-manage/server/internal/auth"
1311
"github.com/manchtools/power-manage/server/internal/middleware"
@@ -30,9 +28,12 @@ func requireAuth(ctx context.Context) (*auth.UserContext, error) {
3028
return userCtx, nil
3129
}
3230

33-
// handleGetError returns a NotFound error for pgx.ErrNoRows, or an Internal error otherwise.
31+
// handleGetError returns a NotFound error when err signals a missing
32+
// row from any supported storage backend, or an Internal error
33+
// otherwise. Backend recognition is centralized in store.IsNotFound;
34+
// see tracker #242 for the abstraction motivation.
3435
func handleGetError(ctx context.Context, err error, notFoundCode, notFoundMsg string) error {
35-
if errors.Is(err, pgx.ErrNoRows) {
36+
if store.IsNotFound(err) {
3637
return apiErrorCtx(ctx, notFoundCode, connect.CodeNotFound, notFoundMsg)
3738
}
3839
return apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get resource")

‎internal/api/helpers_test.go‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/stretchr/testify/require"
1212

1313
"github.com/manchtools/power-manage/server/internal/auth"
14+
"github.com/manchtools/power-manage/server/internal/store"
1415
)
1516

1617
// TestRequireAuth_Success returns the embedded user context unchanged.
@@ -33,16 +34,27 @@ func TestRequireAuth_NoUser(t *testing.T) {
3334
assert.Equal(t, connect.CodeUnauthenticated, cerr.Code())
3435
}
3536

36-
// TestHandleGetError_NotFound maps pgx.ErrNoRows to the supplied
37-
// not-found code with CodeNotFound. Any other error collapses to
38-
// CodeInternal so a database hiccup never leaks "row not found"
39-
// to the client when the row actually exists but the read failed.
37+
// TestHandleGetError_NotFound maps a store-layer not-found error to
38+
// the supplied code with CodeNotFound. Any other error collapses to
39+
// CodeInternal so a database hiccup never leaks "row not found" to
40+
// the client when the row actually exists but the read failed. Both
41+
// pgx.ErrNoRows (the current backend's native sentinel) and
42+
// store.ErrNotFound (the abstract sentinel) must be recognized —
43+
// store.IsNotFound is the bridge.
4044
func TestHandleGetError_NotFound(t *testing.T) {
41-
err := handleGetError(context.Background(), pgx.ErrNoRows, ErrUserNotFound, "user not found")
42-
require.Error(t, err)
43-
cerr := new(connect.Error)
44-
require.True(t, errors.As(err, &cerr))
45-
assert.Equal(t, connect.CodeNotFound, cerr.Code())
45+
cases := map[string]error{
46+
"pgx_native": pgx.ErrNoRows,
47+
"store_abstr": store.ErrNotFound,
48+
}
49+
for name, in := range cases {
50+
t.Run(name, func(t *testing.T) {
51+
err := handleGetError(context.Background(), in, ErrUserNotFound, "user not found")
52+
require.Error(t, err)
53+
cerr := new(connect.Error)
54+
require.True(t, errors.As(err, &cerr))
55+
assert.Equal(t, connect.CodeNotFound, cerr.Code())
56+
})
57+
}
4658
}
4759

4860
func TestHandleGetError_Other(t *testing.T) {

‎internal/api/idp_handler.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,10 @@ import (
55
"crypto/rand"
66
"encoding/hex"
77
"encoding/json"
8-
"errors"
98
"log/slog"
109

1110
"connectrpc.com/connect"
1211

13-
"github.com/jackc/pgx/v5"
1412
"google.golang.org/protobuf/types/known/timestamppb"
1513

1614
pm "github.com/manchtools/power-manage/sdk/gen/go/pm/v1"
@@ -52,7 +50,7 @@ func (h *IDPHandler) CreateIdentityProvider(ctx context.Context, req *connect.Re
5250
if err == nil {
5351
return nil, apiErrorCtx(ctx, ErrProviderSlugExists, connect.CodeAlreadyExists, "provider with this slug already exists")
5452
}
55-
if !errors.Is(err, pgx.ErrNoRows) {
53+
if !store.IsNotFound(err) {
5654
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to check slug")
5755
}
5856

0 commit comments

Comments
 (0)