Skip to content

Commit b56f9f0

Browse files
committed
refactor(broker): narrow health queries, add tests, fix docs
Store: - Add ProviderHealthSummary struct and GetAllHealthStatuses() — narrow 5-column query for /providers/health endpoint - Add GetHealthStatus() to ProfileStorer interface - Add rows.Err() check in GetForHealthCheck Handler: - Replace GetAllProfiles + manual map-building with GetAllHealthStatuses - Fix error message grammar Tests: - Add GetAllHealthStatuses store tests (success, empty) - Update Health handler tests to use new method - Add GetAllHealthStatuses to MockProfileStorer Docs: - Fix duplicate section numbering (5→6) in broker.md - Fix overstated graceful shutdown claim in healthchecks.md
1 parent e8e0bfa commit b56f9f0

11 files changed

Lines changed: 398 additions & 17 deletions

File tree

docs/healthchecks.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ Health workers run inside the standard broker process. For deployments that need
152152
nexus-broker --worker-only
153153
```
154154

155-
In this mode, the HTTP server does not start. The process listens for `SIGINT`/`SIGTERM` and performs a graceful shutdown, draining any in-flight checks before exiting.
155+
In this mode, the HTTP server does not start. The process listens for `SIGINT`/`SIGTERM` and cancels the worker context, signalling in-flight checks to stop. Note: the current implementation does not explicitly wait for worker goroutines to complete before exiting.
156156

157157
The same Docker image and environment variables are used — just override the container command.
158158

docs/services/broker.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ The Broker runs two background workers to continuously monitor integration healt
3535
- Both workers use bounded concurrency (semaphore + WaitGroup) to prevent goroutine exhaustion.
3636
- In `--worker-only` mode, the binary listens for `SIGINT`/`SIGTERM` for graceful shutdown.
3737

38-
### 5. Audit Subsystem
38+
### 6. Audit Subsystem
3939
Every control-plane mutation is recorded in the `audit_events` table via the `audit.Service`:
4040
- **`provider.created`** — logged on every successful `POST /providers` call.
4141
- **`provider.updated`** — logged on `PUT` and `PATCH` mutations.

nexus-broker/internal/repository/postgres/connection.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,10 @@ func (r *connectionRepository) GetForHealthCheck(ctx context.Context, limit int)
142142
rows = append(rows, conn)
143143
}
144144

145+
if err = dbRows.Err(); err != nil {
146+
return nil, err
147+
}
148+
145149
// Returning pointers as per interface
146150
var ptrRows []*domain.ConnectionWithProvider
147151
for i := range rows {

nexus-broker/internal/service/connection_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,14 @@ func (m *MockProfileStorer) GetAllProfiles() ([]provider.Profile, error) {
165165
return nil, args.Error(1)
166166
}
167167

168+
func (m *MockProfileStorer) GetAllHealthStatuses() ([]provider.ProviderHealthSummary, error) {
169+
args := m.Called()
170+
if args.Get(0) != nil {
171+
return args.Get(0).([]provider.ProviderHealthSummary), args.Error(1)
172+
}
173+
return nil, args.Error(1)
174+
}
175+
168176
func (m *MockProfileStorer) GetMetadata() (map[string]map[string]interface{}, error) {
169177
args := m.Called()
170178
if args.Get(0) != nil {

nexus-broker/openapi.yaml

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
openapi: 3.0.3
22
info:
33
title: Nexus Broker API
4-
version: 0.2.2
4+
version: 0.2.4
55
description: |
66
Internal API for the Nexus Broker service.
77
This service handles OAuth 2.0 and OIDC flows, encrypts tokens, and manages provider configurations.
@@ -151,6 +151,62 @@ components:
151151
type: object
152152
description: Decrypted credentials map
153153
additionalProperties: true
154+
health_status:
155+
type: string
156+
enum: [healthy, unhealthy, degraded, expired, unknown]
157+
description: Current health status of the connection
158+
159+
ProviderHealthStatus:
160+
type: object
161+
properties:
162+
id:
163+
type: string
164+
format: uuid
165+
name:
166+
type: string
167+
health_status:
168+
type: string
169+
enum: [healthy, unhealthy, degraded, unknown]
170+
last_health_check_at:
171+
type: string
172+
format: date-time
173+
nullable: true
174+
health_message:
175+
type: string
176+
description: Human-readable detail when status is not healthy
177+
178+
ConnectionSummary:
179+
type: object
180+
properties:
181+
id:
182+
type: string
183+
format: uuid
184+
provider_id:
185+
type: string
186+
format: uuid
187+
provider_name:
188+
type: string
189+
auth_type:
190+
type: string
191+
enum: [oauth2, api_key, basic_auth]
192+
status:
193+
type: string
194+
scopes:
195+
type: array
196+
items: { type: string }
197+
health_status:
198+
type: string
199+
enum: [healthy, unhealthy, degraded, expired, unknown]
200+
last_health_check_at:
201+
type: string
202+
format: date-time
203+
nullable: true
204+
created_at:
205+
type: string
206+
format: date-time
207+
updated_at:
208+
type: string
209+
format: date-time
154210

155211
MetadataResponse:
156212
type: object
@@ -388,6 +444,44 @@ paths:
388444
'404':
389445
description: Connection not found
390446

447+
/providers/health:
448+
get:
449+
summary: Get health status of all providers
450+
description: Returns the current health status of all registered providers. No credentials are included.
451+
security: [{ ApiKeyAuth: [] }]
452+
responses:
453+
'200':
454+
description: Provider health statuses. Returns `[]` when no providers exist.
455+
content:
456+
application/json:
457+
schema:
458+
type: array
459+
items:
460+
$ref: '#/components/schemas/ProviderHealthStatus'
461+
462+
/connections:
463+
get:
464+
summary: List connections for a workspace
465+
description: Returns all non-pending connections for a workspace with their health status. No credentials or tokens are included.
466+
security: [{ ApiKeyAuth: [] }]
467+
parameters:
468+
- in: query
469+
name: workspace_id
470+
required: true
471+
schema: { type: string }
472+
description: Workspace ID to filter connections by
473+
responses:
474+
'200':
475+
description: List of connection summaries
476+
content:
477+
application/json:
478+
schema:
479+
type: array
480+
items:
481+
$ref: '#/components/schemas/ConnectionSummary'
482+
'400':
483+
description: Missing workspace_id parameter
484+
391485
/connections/resolve:
392486
get:
393487
summary: Resolve and retrieve token by workspace and provider
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
package handlers
2+
3+
import (
4+
"errors"
5+
"net/http"
6+
"net/http/httptest"
7+
"testing"
8+
"time"
9+
10+
"github.com/google/uuid"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/mock"
13+
14+
"github.com/Prescott-Data/nexus-framework/nexus-broker/internal/domain"
15+
)
16+
17+
func TestConnectionsList_Success(t *testing.T) {
18+
mockSvc := new(MockConnectionService)
19+
handler := NewConnectionsHandler(mockSvc)
20+
21+
now := time.Now()
22+
expected := []domain.ConnectionSummary{
23+
{
24+
ID: uuid.New(),
25+
ProviderID: uuid.New(),
26+
ProviderName: "google",
27+
AuthType: "oauth2",
28+
Status: "active",
29+
Scopes: []string{"email", "calendar.read"},
30+
HealthStatus: "healthy",
31+
CreatedAt: now,
32+
UpdatedAt: now,
33+
},
34+
{
35+
ID: uuid.New(),
36+
ProviderID: uuid.New(),
37+
ProviderName: "stripe",
38+
AuthType: "api_key",
39+
Status: "active",
40+
HealthStatus: "unhealthy",
41+
CreatedAt: now,
42+
UpdatedAt: now,
43+
},
44+
}
45+
46+
mockSvc.On("ListConnections", mock.Anything, "ws-123").Return(expected, nil).Once()
47+
48+
req := httptest.NewRequest("GET", "/connections?workspace_id=ws-123", nil)
49+
rr := httptest.NewRecorder()
50+
51+
handler.List(rr, req)
52+
53+
assert.Equal(t, http.StatusOK, rr.Code)
54+
assert.Contains(t, rr.Body.String(), "google")
55+
assert.Contains(t, rr.Body.String(), "stripe")
56+
assert.Contains(t, rr.Body.String(), `"health_status":"healthy"`)
57+
assert.Contains(t, rr.Body.String(), `"health_status":"unhealthy"`)
58+
mockSvc.AssertExpectations(t)
59+
}
60+
61+
func TestConnectionsList_EmptyResult(t *testing.T) {
62+
mockSvc := new(MockConnectionService)
63+
handler := NewConnectionsHandler(mockSvc)
64+
65+
mockSvc.On("ListConnections", mock.Anything, "ws-empty").Return([]domain.ConnectionSummary{}, nil).Once()
66+
67+
req := httptest.NewRequest("GET", "/connections?workspace_id=ws-empty", nil)
68+
rr := httptest.NewRecorder()
69+
70+
handler.List(rr, req)
71+
72+
assert.Equal(t, http.StatusOK, rr.Code)
73+
assert.Equal(t, "[]", rr.Body.String())
74+
mockSvc.AssertExpectations(t)
75+
}
76+
77+
func TestConnectionsList_MissingWorkspaceID(t *testing.T) {
78+
mockSvc := new(MockConnectionService)
79+
handler := NewConnectionsHandler(mockSvc)
80+
81+
req := httptest.NewRequest("GET", "/connections", nil)
82+
rr := httptest.NewRecorder()
83+
84+
handler.List(rr, req)
85+
86+
assert.Equal(t, http.StatusBadRequest, rr.Code)
87+
assert.Contains(t, rr.Body.String(), "missing_workspace_id")
88+
// Service should never be called
89+
mockSvc.AssertNotCalled(t, "ListConnections")
90+
}
91+
92+
func TestConnectionsList_ServiceError(t *testing.T) {
93+
mockSvc := new(MockConnectionService)
94+
handler := NewConnectionsHandler(mockSvc)
95+
96+
mockSvc.On("ListConnections", mock.Anything, "ws-broken").Return(nil, errors.New("database unreachable")).Once()
97+
98+
req := httptest.NewRequest("GET", "/connections?workspace_id=ws-broken", nil)
99+
rr := httptest.NewRecorder()
100+
101+
handler.List(rr, req)
102+
103+
assert.Equal(t, http.StatusInternalServerError, rr.Code)
104+
mockSvc.AssertExpectations(t)
105+
}

nexus-broker/pkg/handlers/providers.go

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -205,24 +205,13 @@ func (h *ProvidersHandler) List(w http.ResponseWriter, r *http.Request) {
205205

206206
// Health handles GET /providers/health to list provider health statuses
207207
func (h *ProvidersHandler) Health(w http.ResponseWriter, r *http.Request) {
208-
profiles, err := h.store.GetAllProfiles()
208+
summaries, err := h.store.GetAllHealthStatuses()
209209
if err != nil {
210-
httputil.WriteError(w, http.StatusInternalServerError, "health_failed", "Failed to list providers health")
210+
httputil.WriteError(w, http.StatusInternalServerError, "health_failed", "Failed to retrieve provider health statuses")
211211
return
212212
}
213-
214-
healthData := make([]map[string]interface{}, 0, len(profiles))
215-
for _, p := range profiles {
216-
healthData = append(healthData, map[string]interface{}{
217-
"id": p.ID.String(),
218-
"name": p.Name,
219-
"health_status": p.HealthStatus,
220-
"last_health_check_at": p.LastHealthCheckAt,
221-
"health_message": p.HealthMessage,
222-
})
223-
}
224213

225-
httputil.WriteJSON(w, http.StatusOK, healthData)
214+
httputil.WriteJSON(w, http.StatusOK, summaries)
226215
}
227216

228217
// GetByName handles GET /providers/by-name/{name}

nexus-broker/pkg/handlers/providers_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"net/http"
99
"net/http/httptest"
1010
"testing"
11+
"time"
1112

1213
"github.com/google/uuid"
1314
"github.com/stretchr/testify/assert"
@@ -83,6 +84,14 @@ func (m *MockStore) GetAllProfiles() ([]provider.Profile, error) {
8384
return args.Get(0).([]provider.Profile), args.Error(1)
8485
}
8586

87+
func (m *MockStore) GetAllHealthStatuses() ([]provider.ProviderHealthSummary, error) {
88+
args := m.Called()
89+
if args.Get(0) == nil {
90+
return nil, args.Error(1)
91+
}
92+
return args.Get(0).([]provider.ProviderHealthSummary), args.Error(1)
93+
}
94+
8695
func (m *MockStore) GetMetadata() (map[string]map[string]interface{}, error) {
8796
args := m.Called()
8897
if args.Get(0) == nil {
@@ -278,3 +287,74 @@ func TestPatchProvider_AuditRedactsSecrets(t *testing.T) {
278287
return true
279288
}), mock.AnythingOfType("*http.Request"))
280289
}
290+
291+
func TestHealth_Success(t *testing.T) {
292+
mockStore := new(MockStore)
293+
handler := NewProvidersHandler(mockStore, nil)
294+
295+
now := time.Now()
296+
msg := "token_url returned 503"
297+
summaries := []provider.ProviderHealthSummary{
298+
{
299+
ID: uuid.New(),
300+
Name: "google",
301+
HealthStatus: "healthy",
302+
LastHealthCheckAt: &now,
303+
HealthMessage: nil,
304+
},
305+
{
306+
ID: uuid.New(),
307+
Name: "stripe",
308+
HealthStatus: "unhealthy",
309+
LastHealthCheckAt: &now,
310+
HealthMessage: &msg,
311+
},
312+
}
313+
314+
mockStore.On("GetAllHealthStatuses").Return(summaries, nil).Once()
315+
316+
req := httptest.NewRequest("GET", "/providers/health", nil)
317+
rr := httptest.NewRecorder()
318+
319+
handler.Health(rr, req)
320+
321+
assert.Equal(t, http.StatusOK, rr.Code)
322+
assert.Contains(t, rr.Body.String(), `"health_status":"healthy"`)
323+
assert.Contains(t, rr.Body.String(), `"health_status":"unhealthy"`)
324+
assert.Contains(t, rr.Body.String(), `"health_message":"token_url returned 503"`)
325+
assert.Contains(t, rr.Body.String(), "google")
326+
assert.Contains(t, rr.Body.String(), "stripe")
327+
mockStore.AssertExpectations(t)
328+
}
329+
330+
func TestHealth_EmptyList(t *testing.T) {
331+
mockStore := new(MockStore)
332+
handler := NewProvidersHandler(mockStore, nil)
333+
334+
mockStore.On("GetAllHealthStatuses").Return([]provider.ProviderHealthSummary{}, nil).Once()
335+
336+
req := httptest.NewRequest("GET", "/providers/health", nil)
337+
rr := httptest.NewRecorder()
338+
339+
handler.Health(rr, req)
340+
341+
assert.Equal(t, http.StatusOK, rr.Code)
342+
assert.Equal(t, "[]", rr.Body.String())
343+
mockStore.AssertExpectations(t)
344+
}
345+
346+
func TestHealth_StoreError(t *testing.T) {
347+
mockStore := new(MockStore)
348+
handler := NewProvidersHandler(mockStore, nil)
349+
350+
mockStore.On("GetAllHealthStatuses").Return(nil, errors.New("connection refused")).Once()
351+
352+
req := httptest.NewRequest("GET", "/providers/health", nil)
353+
rr := httptest.NewRecorder()
354+
355+
handler.Health(rr, req)
356+
357+
assert.Equal(t, http.StatusInternalServerError, rr.Code)
358+
assert.Contains(t, rr.Body.String(), "health_failed")
359+
mockStore.AssertExpectations(t)
360+
}

nexus-broker/pkg/provider/interfaces.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,6 @@ type ProfileStorer interface {
2323
DeleteProfileByName(name string) (int64, error)
2424
ListProfiles() ([]ProfileList, error)
2525
GetAllProfiles() ([]Profile, error)
26+
GetAllHealthStatuses() ([]ProviderHealthSummary, error)
2627
GetMetadata() (map[string]map[string]interface{}, error)
2728
}

0 commit comments

Comments
 (0)