From 7301f670270685cca6bec8fa207a30e709f5554b Mon Sep 17 00:00:00 2001 From: omattsson Date: Sat, 14 Mar 2026 20:32:36 +0100 Subject: [PATCH 1/2] feat: broadcast item.created/updated/deleted events over WebSocket (#5) Handler gains a broadcastSender interface field and NewHandlerWithHub constructor so a websocket hub can be injected without breaking existing callers that use NewHandler. Broadcast calls are made after every successful Create, Update, and Delete operation, emitting JSON events of the form: {"event":"item.created","data":{...}} MockBroadcastSender (mock_broadcast_sender.go) provides a thread-safe test double that records all messages for assertion. MockRepository and items_test.go are extended with full coverage of the broadcast paths, including error-path tests that verify no broadcast occurs when the repository returns an error. Refs #5 --- backend/internal/api/handlers/items.go | 32 ++ backend/internal/api/handlers/items_test.go | 359 ++++++++++++++++++ .../api/handlers/mock_broadcast_sender.go | 35 ++ .../internal/api/handlers/mock_repository.go | 11 + backend/internal/api/routes/routes.go | 2 +- 5 files changed, 438 insertions(+), 1 deletion(-) create mode 100644 backend/internal/api/handlers/mock_broadcast_sender.go diff --git a/backend/internal/api/handlers/items.go b/backend/internal/api/handlers/items.go index b6c80f1..aba6a40 100644 --- a/backend/internal/api/handlers/items.go +++ b/backend/internal/api/handlers/items.go @@ -2,24 +2,53 @@ package handlers import ( "errors" + "log/slog" "net/http" "strconv" "strings" "backend/internal/database" "backend/internal/models" + "backend/internal/websocket" "github.com/gin-gonic/gin" ) +// broadcastSender broadcasts serialised WebSocket messages to all connected clients. +type broadcastSender interface { + Broadcast(message []byte) +} + type Handler struct { repository models.Repository + hub broadcastSender } func NewHandler(repository models.Repository) *Handler { return &Handler{repository: repository} } +func NewHandlerWithHub(repository models.Repository, hub broadcastSender) *Handler { + return &Handler{repository: repository, hub: hub} +} + +func (h *Handler) broadcast(msgType string, payload interface{}) { + if h.hub == nil { + return + } + msg, err := websocket.NewMessage(msgType, payload) + if err != nil { + slog.Error("Failed to create WebSocket message", "type", msgType, "error", err) + return + } + b, err := msg.Bytes() + if err != nil { + slog.Error("Failed to serialise WebSocket message", "type", msgType, "error", err) + return + } + h.hub.Broadcast(b) +} + func handleDBError(err error) (int, string) { if err == nil { return http.StatusOK, "" @@ -77,6 +106,7 @@ func (h *Handler) CreateItem(c *gin.Context) { return } + h.broadcast("item.created", item) c.JSON(http.StatusCreated, item) } @@ -220,6 +250,7 @@ func (h *Handler) UpdateItem(c *gin.Context) { return } + h.broadcast("item.updated", currentItem) c.JSON(http.StatusOK, currentItem) } @@ -248,5 +279,6 @@ func (h *Handler) DeleteItem(c *gin.Context) { return } + h.broadcast("item.deleted", gin.H{"id": id}) c.Status(http.StatusNoContent) } diff --git a/backend/internal/api/handlers/items_test.go b/backend/internal/api/handlers/items_test.go index 28e01d6..fc7d22f 100644 --- a/backend/internal/api/handlers/items_test.go +++ b/backend/internal/api/handlers/items_test.go @@ -932,3 +932,362 @@ func TestHandleDBError(t *testing.T) { }) } } + +// setupTestRouterWithHub creates a test router wired with the given BroadcastSender. +func setupTestRouterWithHub(hub *MockBroadcastSender) (*gin.Engine, *MockRepository) { + gin.SetMode(gin.TestMode) + router := gin.Default() + mockRepo := NewMockRepository() + handler := NewHandlerWithHub(mockRepo, hub) + + rateLimiter := NewRateLimiter(30, time.Second) + + items := router.Group("/api/v1/items") + items.Use(rateLimiter.RateLimit()) + { + items.GET("", handler.GetItems) + items.GET("/:id", handler.GetItem) + items.POST("", handler.CreateItem) + items.PUT("/:id", handler.UpdateItem) + items.DELETE("/:id", handler.DeleteItem) + } + + return router, mockRepo +} + +func TestBroadcastItemEvents(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + setup func(router *gin.Engine, mockRepo *MockRepository) *http.Request + wantCode int + wantType string + wantMsgs int + }{ + { + name: "CreateItem broadcasts item.created", + setup: func(router *gin.Engine, _ *MockRepository) *http.Request { + body := `{"name":"Broadcast Widget","price":9.99}` + req, _ := http.NewRequest("POST", "/api/v1/items", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + return req + }, + wantCode: http.StatusCreated, + wantType: "item.created", + wantMsgs: 1, + }, + { + name: "UpdateItem broadcasts item.updated", + setup: func(router *gin.Engine, mockRepo *MockRepository) *http.Request { + // Pre-create an item + item := &models.Item{Name: "Original", Price: 1.00} + _ = mockRepo.Create(context.Background(), item) + id := fmt.Sprint(item.ID) + + body := `{"name":"Updated","price":2.00}` + req, _ := http.NewRequest("PUT", "/api/v1/items/"+id, bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + return req + }, + wantCode: http.StatusOK, + wantType: "item.updated", + wantMsgs: 1, + }, + { + name: "DeleteItem broadcasts item.deleted", + setup: func(router *gin.Engine, mockRepo *MockRepository) *http.Request { + item := &models.Item{Name: "ToDelete", Price: 1.00} + _ = mockRepo.Create(context.Background(), item) + id := fmt.Sprint(item.ID) + + req, _ := http.NewRequest("DELETE", "/api/v1/items/"+id, nil) + return req + }, + wantCode: http.StatusNoContent, + wantType: "item.deleted", + wantMsgs: 1, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + hub := &MockBroadcastSender{} + router, mockRepo := setupTestRouterWithHub(hub) + + req := tt.setup(router, mockRepo) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, tt.wantCode, w.Code) + + msgs := hub.Messages() + assert.Len(t, msgs, tt.wantMsgs) + if len(msgs) > 0 { + var env struct { + Type string `json:"type"` + } + assert.NoError(t, json.Unmarshal(msgs[0], &env)) + assert.Equal(t, tt.wantType, env.Type) + } + }) + } +} + +func TestBroadcastNilHub(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + setup func(router *gin.Engine, mockRepo *MockRepository) *http.Request + wantCode int + }{ + { + name: "CreateItem with nil hub does not panic", + setup: func(_ *gin.Engine, _ *MockRepository) *http.Request { + body := `{"name":"NilHubWidget","price":1.00}` + req, _ := http.NewRequest("POST", "/api/v1/items", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + return req + }, + wantCode: http.StatusCreated, + }, + { + name: "UpdateItem with nil hub does not panic", + setup: func(_ *gin.Engine, mockRepo *MockRepository) *http.Request { + item := &models.Item{Name: "NilHubItem", Price: 1.00} + _ = mockRepo.Create(context.Background(), item) + id := fmt.Sprint(item.ID) + + body := `{"name":"Updated","price":2.00}` + req, _ := http.NewRequest("PUT", "/api/v1/items/"+id, bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + return req + }, + wantCode: http.StatusOK, + }, + { + name: "DeleteItem with nil hub does not panic", + setup: func(_ *gin.Engine, mockRepo *MockRepository) *http.Request { + item := &models.Item{Name: "NilHubDelete", Price: 1.00} + _ = mockRepo.Create(context.Background(), item) + id := fmt.Sprint(item.ID) + + req, _ := http.NewRequest("DELETE", "/api/v1/items/"+id, nil) + return req + }, + wantCode: http.StatusNoContent, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Use NewHandler (no hub) — existing setupTestRouter pattern + gin.SetMode(gin.TestMode) + router := gin.Default() + mockRepo := NewMockRepository() + handler := NewHandler(mockRepo) // nil hub + rateLimiter := NewRateLimiter(30, time.Second) + + items := router.Group("/api/v1/items") + items.Use(rateLimiter.RateLimit()) + { + items.GET("", handler.GetItems) + items.GET("/:id", handler.GetItem) + items.POST("", handler.CreateItem) + items.PUT("/:id", handler.UpdateItem) + items.DELETE("/:id", handler.DeleteItem) + } + + req := tt.setup(router, mockRepo) + w := httptest.NewRecorder() + + // assert.NotPanics wraps the call + assert.NotPanics(t, func() { + router.ServeHTTP(w, req) + }) + assert.Equal(t, tt.wantCode, w.Code) + }) + } +} + +// TestBroadcastNoEventOnError verifies that no broadcast is sent when the +// repository returns an error for Create, Update, or Delete operations. +func TestBroadcastNoEventOnError(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + setup func(mockRepo *MockRepository) *http.Request + wantCode int + }{ + { + name: "CreateItem repo error → no broadcast", + setup: func(mockRepo *MockRepository) *http.Request { + mockRepo.SetError(errors.New("database connection failed")) + body := `{"name":"Widget","price":9.99}` + req, _ := http.NewRequest("POST", "/api/v1/items", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + return req + }, + wantCode: http.StatusInternalServerError, + }, + { + name: "UpdateItem FindByID error → no broadcast", + setup: func(mockRepo *MockRepository) *http.Request { + mockRepo.SetError(errors.New("database connection failed")) + body := `{"name":"Updated","price":2.00}` + req, _ := http.NewRequest("PUT", "/api/v1/items/1", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + return req + }, + wantCode: http.StatusInternalServerError, + }, + { + name: "DeleteItem repo error → no broadcast", + setup: func(mockRepo *MockRepository) *http.Request { + mockRepo.SetError(errors.New("database connection failed")) + req, _ := http.NewRequest("DELETE", "/api/v1/items/1", nil) + return req + }, + wantCode: http.StatusInternalServerError, + }, + { + name: "UpdateItem Update error → no broadcast", + setup: func(mockRepo *MockRepository) *http.Request { + item := &models.Item{Name: "Existing", Price: 1.00} + _ = mockRepo.Create(context.Background(), item) + mockRepo.SetUpdateError(errors.New("update failed")) + body := fmt.Sprintf(`{"name":"Updated","price":2.00,"version":%d}`, item.Version) + req, _ := http.NewRequest("PUT", fmt.Sprintf("/api/v1/items/%d", item.ID), bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + return req + }, + wantCode: http.StatusInternalServerError, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + hub := &MockBroadcastSender{} + router, mockRepo := setupTestRouterWithHub(hub) + req := tt.setup(mockRepo) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, tt.wantCode, w.Code) + assert.Empty(t, hub.Messages(), "no broadcast expected on error") + }) + } +} + +// TestBroadcastPayloadContent verifies that the payload embedded in each +// broadcast message contains the correct entity data — not just the right type. +func TestBroadcastPayloadContent(t *testing.T) { + t.Parallel() + + t.Run("CreateItem payload contains item fields", func(t *testing.T) { + t.Parallel() + + hub := &MockBroadcastSender{} + router, _ := setupTestRouterWithHub(hub) + + body := `{"name":"Payload Widget","price":12.34}` + req, _ := http.NewRequest("POST", "/api/v1/items", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusCreated, w.Code) + msgs := hub.Messages() + assert.Len(t, msgs, 1) + + var env struct { + Type string `json:"type"` + Payload json.RawMessage `json:"payload"` + } + assert.NoError(t, json.Unmarshal(msgs[0], &env)) + assert.Equal(t, "item.created", env.Type) + + var item models.Item + assert.NoError(t, json.Unmarshal(env.Payload, &item)) + assert.Equal(t, "Payload Widget", item.Name) + assert.InDelta(t, 12.34, item.Price, 0.001) + assert.NotZero(t, item.ID) + }) + + t.Run("UpdateItem payload contains updated fields", func(t *testing.T) { + t.Parallel() + + hub := &MockBroadcastSender{} + router, mockRepo := setupTestRouterWithHub(hub) + + existing := &models.Item{Name: "Before", Price: 1.00} + _ = mockRepo.Create(context.Background(), existing) + id := fmt.Sprint(existing.ID) + + body := `{"name":"After","price":5.55}` + req, _ := http.NewRequest("PUT", "/api/v1/items/"+id, bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + msgs := hub.Messages() + assert.Len(t, msgs, 1) + + var env struct { + Type string `json:"type"` + Payload json.RawMessage `json:"payload"` + } + assert.NoError(t, json.Unmarshal(msgs[0], &env)) + assert.Equal(t, "item.updated", env.Type) + + var item models.Item + assert.NoError(t, json.Unmarshal(env.Payload, &item)) + assert.Equal(t, "After", item.Name) + assert.InDelta(t, 5.55, item.Price, 0.001) + assert.Equal(t, existing.ID, item.ID) + }) + + t.Run("DeleteItem payload contains item ID", func(t *testing.T) { + t.Parallel() + + hub := &MockBroadcastSender{} + router, mockRepo := setupTestRouterWithHub(hub) + + existing := &models.Item{Name: "ToDelete", Price: 1.00} + _ = mockRepo.Create(context.Background(), existing) + id := existing.ID + + req, _ := http.NewRequest("DELETE", fmt.Sprintf("/api/v1/items/%d", id), nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusNoContent, w.Code) + msgs := hub.Messages() + assert.Len(t, msgs, 1) + + var env struct { + Type string `json:"type"` + Payload json.RawMessage `json:"payload"` + } + assert.NoError(t, json.Unmarshal(msgs[0], &env)) + assert.Equal(t, "item.deleted", env.Type) + + var payload struct { + ID uint64 `json:"id"` + } + assert.NoError(t, json.Unmarshal(env.Payload, &payload)) + assert.Equal(t, uint64(id), payload.ID) + }) +} diff --git a/backend/internal/api/handlers/mock_broadcast_sender.go b/backend/internal/api/handlers/mock_broadcast_sender.go new file mode 100644 index 0000000..238c541 --- /dev/null +++ b/backend/internal/api/handlers/mock_broadcast_sender.go @@ -0,0 +1,35 @@ +package handlers + +import "sync" + +// MockBroadcastSender is a test double for websocket.BroadcastSender that +// records all messages passed to Broadcast for assertion in unit tests. +type MockBroadcastSender struct { + mu sync.Mutex + messages [][]byte +} + +// Broadcast records the message for later inspection. +func (m *MockBroadcastSender) Broadcast(message []byte) { + m.mu.Lock() + defer m.mu.Unlock() + cp := make([]byte, len(message)) + copy(cp, message) + m.messages = append(m.messages, cp) +} + +// Messages returns a copy of all recorded messages. +func (m *MockBroadcastSender) Messages() [][]byte { + m.mu.Lock() + defer m.mu.Unlock() + result := make([][]byte, len(m.messages)) + copy(result, m.messages) + return result +} + +// Reset clears all recorded messages. +func (m *MockBroadcastSender) Reset() { + m.mu.Lock() + defer m.mu.Unlock() + m.messages = nil +} diff --git a/backend/internal/api/handlers/mock_repository.go b/backend/internal/api/handlers/mock_repository.go index e372b8d..6b4a8c4 100644 --- a/backend/internal/api/handlers/mock_repository.go +++ b/backend/internal/api/handlers/mock_repository.go @@ -16,6 +16,7 @@ type MockRepository struct { sync.RWMutex // size: 8 items map[uint]*models.Item // size: 8 (pointer) err error // size: 8 (interface) + updateError error // size: 8 (interface) nextID uint // size: 8 } @@ -72,6 +73,10 @@ func (m *MockRepository) Update(_ context.Context, entity interface{}) error { m.Lock() defer m.Unlock() + if m.updateError != nil { + return m.updateError + } + if m.err != nil { return m.err } @@ -234,3 +239,9 @@ func (m *MockRepository) SetError(err error) { defer m.Unlock() m.err = err } + +func (m *MockRepository) SetUpdateError(err error) { + m.Lock() + defer m.Unlock() + m.updateError = err +} diff --git a/backend/internal/api/routes/routes.go b/backend/internal/api/routes/routes.go index 37d137e..7739b7d 100644 --- a/backend/internal/api/routes/routes.go +++ b/backend/internal/api/routes/routes.go @@ -46,7 +46,7 @@ func SetupRoutes(router *gin.Engine, repository models.Repository, healthChecker v1.GET("/ping", handlers.Ping) // Items endpoints - itemsHandler := handlers.NewHandler(repository) + itemsHandler := handlers.NewHandlerWithHub(repository, hub) items := v1.Group("/items") { items.GET("", itemsHandler.GetItems) From 38f3b1a72887af7f8d9578981b94bcbc3b7d3576 Mon Sep 17 00:00:00 2001 From: omattsson Date: Sat, 14 Mar 2026 20:42:48 +0100 Subject: [PATCH 2/2] fix: address Copilot review comments on broadcast PR --- backend/internal/api/handlers/items.go | 9 ++------- backend/internal/api/handlers/items_test.go | 14 ++++++++------ .../internal/api/handlers/mock_broadcast_sender.go | 8 ++++++-- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/backend/internal/api/handlers/items.go b/backend/internal/api/handlers/items.go index aba6a40..26f7466 100644 --- a/backend/internal/api/handlers/items.go +++ b/backend/internal/api/handlers/items.go @@ -14,21 +14,16 @@ import ( "github.com/gin-gonic/gin" ) -// broadcastSender broadcasts serialised WebSocket messages to all connected clients. -type broadcastSender interface { - Broadcast(message []byte) -} - type Handler struct { repository models.Repository - hub broadcastSender + hub websocket.BroadcastSender } func NewHandler(repository models.Repository) *Handler { return &Handler{repository: repository} } -func NewHandlerWithHub(repository models.Repository, hub broadcastSender) *Handler { +func NewHandlerWithHub(repository models.Repository, hub websocket.BroadcastSender) *Handler { return &Handler{repository: repository, hub: hub} } diff --git a/backend/internal/api/handlers/items_test.go b/backend/internal/api/handlers/items_test.go index fc7d22f..bdaf4b5 100644 --- a/backend/internal/api/handlers/items_test.go +++ b/backend/internal/api/handlers/items_test.go @@ -934,13 +934,15 @@ func TestHandleDBError(t *testing.T) { } // setupTestRouterWithHub creates a test router wired with the given BroadcastSender. -func setupTestRouterWithHub(hub *MockBroadcastSender) (*gin.Engine, *MockRepository) { +func setupTestRouterWithHub(t *testing.T, hub *MockBroadcastSender) (*gin.Engine, *MockRepository) { + t.Helper() gin.SetMode(gin.TestMode) router := gin.Default() mockRepo := NewMockRepository() handler := NewHandlerWithHub(mockRepo, hub) rateLimiter := NewRateLimiter(30, time.Second) + t.Cleanup(rateLimiter.Stop) items := router.Group("/api/v1/items") items.Use(rateLimiter.RateLimit()) @@ -1016,7 +1018,7 @@ func TestBroadcastItemEvents(t *testing.T) { t.Parallel() hub := &MockBroadcastSender{} - router, mockRepo := setupTestRouterWithHub(hub) + router, mockRepo := setupTestRouterWithHub(t, hub) req := tt.setup(router, mockRepo) w := httptest.NewRecorder() @@ -1179,7 +1181,7 @@ func TestBroadcastNoEventOnError(t *testing.T) { t.Parallel() hub := &MockBroadcastSender{} - router, mockRepo := setupTestRouterWithHub(hub) + router, mockRepo := setupTestRouterWithHub(t, hub) req := tt.setup(mockRepo) w := httptest.NewRecorder() router.ServeHTTP(w, req) @@ -1199,7 +1201,7 @@ func TestBroadcastPayloadContent(t *testing.T) { t.Parallel() hub := &MockBroadcastSender{} - router, _ := setupTestRouterWithHub(hub) + router, _ := setupTestRouterWithHub(t, hub) body := `{"name":"Payload Widget","price":12.34}` req, _ := http.NewRequest("POST", "/api/v1/items", bytes.NewBufferString(body)) @@ -1229,7 +1231,7 @@ func TestBroadcastPayloadContent(t *testing.T) { t.Parallel() hub := &MockBroadcastSender{} - router, mockRepo := setupTestRouterWithHub(hub) + router, mockRepo := setupTestRouterWithHub(t, hub) existing := &models.Item{Name: "Before", Price: 1.00} _ = mockRepo.Create(context.Background(), existing) @@ -1263,7 +1265,7 @@ func TestBroadcastPayloadContent(t *testing.T) { t.Parallel() hub := &MockBroadcastSender{} - router, mockRepo := setupTestRouterWithHub(hub) + router, mockRepo := setupTestRouterWithHub(t, hub) existing := &models.Item{Name: "ToDelete", Price: 1.00} _ = mockRepo.Create(context.Background(), existing) diff --git a/backend/internal/api/handlers/mock_broadcast_sender.go b/backend/internal/api/handlers/mock_broadcast_sender.go index 238c541..24b5351 100644 --- a/backend/internal/api/handlers/mock_broadcast_sender.go +++ b/backend/internal/api/handlers/mock_broadcast_sender.go @@ -18,12 +18,16 @@ func (m *MockBroadcastSender) Broadcast(message []byte) { m.messages = append(m.messages, cp) } -// Messages returns a copy of all recorded messages. +// Messages returns a deep copy of all recorded messages. func (m *MockBroadcastSender) Messages() [][]byte { m.mu.Lock() defer m.mu.Unlock() result := make([][]byte, len(m.messages)) - copy(result, m.messages) + for i, msg := range m.messages { + cp := make([]byte, len(msg)) + copy(cp, msg) + result[i] = cp + } return result }