diff --git a/stovepipe/controller/read_errors.go b/stovepipe/controller/read_errors.go index 1bbeff6b..127fa436 100644 --- a/stovepipe/controller/read_errors.go +++ b/stovepipe/controller/read_errors.go @@ -41,18 +41,34 @@ func validateHistoryIdentifier(name, value string) error { return nil } -// RequestHistoryNotFoundError indicates that no retained history exists for a selector. -type RequestHistoryNotFoundError struct { +// RequestHistoryByIDNotFoundError indicates that no retained history exists for a request ID. +type RequestHistoryByIDNotFoundError struct { + // RequestID is the selected request identifier. RequestID string } // Error implements error. -func (e *RequestHistoryNotFoundError) Error() string { +func (e *RequestHistoryByIDNotFoundError) Error() string { return fmt.Sprintf("request history not found for request ID %q", e.RequestID) } +// RequestHistoryByURINotFoundError indicates that no retained history exists for a URI. +type RequestHistoryByURINotFoundError struct { + // URI is the selected commit URI. + URI string +} + +// Error implements error. +func (e *RequestHistoryByURINotFoundError) Error() string { + return fmt.Sprintf("request history not found for URI %q", e.URI) +} + // IsRequestHistoryNotFound reports whether err contains a retained-history absence. func IsRequestHistoryNotFound(err error) bool { - var target *RequestHistoryNotFoundError - return errors.As(err, &target) + var byID *RequestHistoryByIDNotFoundError + if errors.As(err, &byID) { + return true + } + var byURI *RequestHistoryByURINotFoundError + return errors.As(err, &byURI) } diff --git a/stovepipe/controller/request_history.go b/stovepipe/controller/request_history.go index 37e2ea6d..cc79cb97 100644 --- a/stovepipe/controller/request_history.go +++ b/stovepipe/controller/request_history.go @@ -29,6 +29,7 @@ import ( // RequestHistoryController handles retained request-history lookups. type RequestHistoryController interface { GetRequestHistoryByID(ctx context.Context, req entity.GetRequestHistoryByIDRequest) ([]entity.RequestLog, error) + GetRequestHistoryByURI(ctx context.Context, req entity.GetRequestHistoryByURIRequest) ([]entity.RequestHistory, error) } var _ RequestHistoryController = (*requestHistoryController)(nil) @@ -81,10 +82,60 @@ func (c *requestHistoryController) readHistoryByID(ctx context.Context, req enti logs, err := stores.GetRequestLogStore().List(ctx, req.ID) if err != nil { if storage.IsNotFound(err) { - return nil, errs.NewUserError(&RequestHistoryNotFoundError{RequestID: req.ID}) + return nil, errs.NewUserError(&RequestHistoryByIDNotFoundError{RequestID: req.ID}) } return nil, fmt.Errorf("GetRequestHistoryByID failed to list request logs request_id=%s: %w", req.ID, err) } return logs, nil } + +// GetRequestHistoryByURI returns the retained history mapped to an exact commit URI. +func (c *requestHistoryController) GetRequestHistoryByURI(ctx context.Context, req entity.GetRequestHistoryByURIRequest) (histories []entity.RequestHistory, retErr error) { + op := metrics.Begin(c.metricsScope, "get_by_uri", metrics.StorageLatencyBuckets, metrics.TagsFromContext(ctx)...) + defer func() { op.Complete(retErr) }() + + history, retErr := c.readHistoryByURI(ctx, req) + if retErr != nil { + return nil, retErr + } + c.logger.Debugw("request history retrieved by URI", + "uri", req.URI, + "request_id", history.RequestID, + "queue", req.Queue, + "event_count", len(history.Events), + ) + return []entity.RequestHistory{history}, nil +} + +func (c *requestHistoryController) readHistoryByURI(ctx context.Context, req entity.GetRequestHistoryByURIRequest) (entity.RequestHistory, error) { + if err := validateHistoryIdentifier("queue", req.Queue); err != nil { + return entity.RequestHistory{}, fmt.Errorf("GetRequestHistoryByURI invalid queue=%q: %w", req.Queue, err) + } + if err := validateHistoryIdentifier("URI", req.URI); err != nil { + return entity.RequestHistory{}, fmt.Errorf("GetRequestHistoryByURI invalid uri=%q queue=%q: %w", req.URI, req.Queue, err) + } + + stores, err := c.stores.For(storage.Config{QueueName: req.Queue}) + if err != nil { + return entity.RequestHistory{}, fmt.Errorf("GetRequestHistoryByURI failed to resolve storage for queue %q: %w", req.Queue, err) + } + + requestID, err := stores.GetRequestURIStore().GetIDByURI(ctx, req.URI) + if err != nil { + if storage.IsNotFound(err) { + return entity.RequestHistory{}, errs.NewUserError(&RequestHistoryByURINotFoundError{URI: req.URI}) + } + return entity.RequestHistory{}, fmt.Errorf("GetRequestHistoryByURI failed to resolve request URI %s: %w", req.URI, err) + } + + logs, err := stores.GetRequestLogStore().List(ctx, requestID) + if err != nil { + if storage.IsNotFound(err) { + return entity.RequestHistory{}, fmt.Errorf("GetRequestHistoryByURI found URI mapping without retained request logs uri=%q request_id=%q: %w", req.URI, requestID, err) + } + return entity.RequestHistory{}, fmt.Errorf("GetRequestHistoryByURI failed to list request logs uri=%s request_id=%s: %w", req.URI, requestID, err) + } + + return entity.RequestHistory{RequestID: requestID, Events: logs}, nil +} diff --git a/stovepipe/controller/request_history_test.go b/stovepipe/controller/request_history_test.go index bd57e66b..866d41a8 100644 --- a/stovepipe/controller/request_history_test.go +++ b/stovepipe/controller/request_history_test.go @@ -102,6 +102,11 @@ func TestGetRequestHistoryByID(t *testing.T) { } assert.Equal(t, tt.wantNotFound, IsRequestHistoryNotFound(err)) assert.Equal(t, tt.wantUser, errs.IsUserError(err)) + if tt.wantNotFound { + var notFound *RequestHistoryByIDNotFoundError + require.ErrorAs(t, err, ¬Found) + assert.Equal(t, requestID, notFound.RequestID) + } if tt.wantCause != nil { assert.ErrorIs(t, err, tt.wantCause) } @@ -125,29 +130,159 @@ func TestGetRequestHistoryByID(t *testing.T) { start, ok := snapshot.Counters()["test.request_history_controller.get_by_id.start+queue=context-queue"] require.True(t, ok) assert.EqualValues(t, 1, start.Value()) - assertOperationFinishIncludesContextTag(t, snapshot, err == nil) + assertOperationFinishIncludesContextTag(t, snapshot, "get_by_id", err == nil) }) } } -func TestRequestHistoryNotFoundError(t *testing.T) { - err := fmt.Errorf("lookup failed: %w", &RequestHistoryNotFoundError{RequestID: "request/queue/1"}) +func TestGetRequestHistoryByURI(t *testing.T) { + const ( + queue = "monorepo/main" + uri = "git://example.com/repo.git/commit/deadbeef" + requestID = "request/monorepo/main/42" + ) + backendErr := errors.New("backend unavailable") + logs := []entity.RequestLog{ + {ID: "state/1", RequestID: requestID, TimestampMs: 10, State: entity.RequestStateAccepted}, + {ID: "event/a", RequestID: requestID, TimestampMs: 20, Event: entity.RequestEventBuildTriggered}, + {ID: "event/a", RequestID: requestID, TimestampMs: 20, Event: entity.RequestEventBuildTriggered}, + } + wantHistory := []entity.RequestHistory{{RequestID: requestID, Events: logs}} + + tests := []struct { + name string + req entity.GetRequestHistoryByURIRequest + mappedID string + factoryErr error + mappingErr error + listErr error + want []entity.RequestHistory + wantInvalid bool + wantNotFound bool + wantCause error + wantLog bool + }{ + {name: "singleton history preserves log order and duplicates", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: uri}, mappedID: requestID, want: wantHistory, wantLog: true}, + {name: "empty queue", req: entity.GetRequestHistoryByURIRequest{URI: uri}, wantInvalid: true}, + {name: "oversized queue", req: entity.GetRequestHistoryByURIRequest{Queue: strings.Repeat("q", maxHistoryIdentifierBytes+1), URI: uri}, wantInvalid: true}, + {name: "empty URI", req: entity.GetRequestHistoryByURIRequest{Queue: queue}, wantInvalid: true}, + {name: "oversized URI", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: strings.Repeat("u", maxHistoryIdentifierBytes+1)}, wantInvalid: true}, + {name: "storage factory failure", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: uri}, factoryErr: backendErr, wantCause: backendErr}, + {name: "URI mapping not found", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: uri}, mappingErr: fmt.Errorf("lookup: %w", storage.ErrNotFound), wantNotFound: true}, + {name: "URI store failure", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: uri}, mappingErr: backendErr, wantCause: backendErr}, + {name: "mapped history absence is internal", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: uri}, mappedID: requestID, listErr: fmt.Errorf("query: %w", storage.ErrNotFound), wantCause: storage.ErrNotFound}, + {name: "log store failure", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: uri}, mappedID: requestID, listErr: backendErr, wantCause: backendErr}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockCtrl := gomock.NewController(t) + factory := storagemock.NewMockFactory(mockCtrl) + stores := storagemock.NewMockStorage(mockCtrl) + uriStore := storagemock.NewMockRequestURIStore(mockCtrl) + logStore := storagemock.NewMockRequestLogStore(mockCtrl) + if !tt.wantInvalid { + factory.EXPECT().For(storage.Config{QueueName: tt.req.Queue}).Return(stores, tt.factoryErr) + if tt.factoryErr == nil { + stores.EXPECT().GetRequestURIStore().Return(uriStore) + uriStore.EXPECT().GetIDByURI(gomock.Any(), tt.req.URI).Return(tt.mappedID, tt.mappingErr) + if tt.mappingErr == nil { + stores.EXPECT().GetRequestLogStore().Return(logStore) + logStore.EXPECT().List(gomock.Any(), tt.mappedID).Return(logs, tt.listErr) + } + } + } + + core, observed := observer.New(zap.DebugLevel) + scope := tally.NewTestScope("test", nil) + controller := NewRequestHistoryController(zap.New(core).Sugar(), scope, factory) + ctx := metrics.WithContextTags(context.Background(), metrics.NewTag("queue", "context-queue")) - assert.True(t, IsRequestHistoryNotFound(err)) + got, err := controller.GetRequestHistoryByURI(ctx, tt.req) + + assert.Equal(t, tt.want, got) + if tt.wantInvalid { + assert.True(t, IsInvalidRequest(err)) + } + assert.Equal(t, tt.wantNotFound, IsRequestHistoryNotFound(err)) + assert.Equal(t, tt.wantInvalid || tt.wantNotFound, errs.IsUserError(err)) + if tt.wantCause != nil { + assert.ErrorIs(t, err, tt.wantCause) + } + if tt.want != nil { + require.NoError(t, err) + } else { + require.Error(t, err) + } + if tt.wantNotFound { + var notFound *RequestHistoryByURINotFoundError + require.ErrorAs(t, err, ¬Found) + assert.Equal(t, uri, notFound.URI) + } + + entries := observed.FilterMessage("request history retrieved by URI").All() + if tt.wantLog { + require.Len(t, entries, 1) + assert.Equal(t, uri, entries[0].ContextMap()["uri"]) + assert.Equal(t, requestID, entries[0].ContextMap()["request_id"]) + assert.Equal(t, queue, entries[0].ContextMap()["queue"]) + assert.Equal(t, int64(len(logs)), entries[0].ContextMap()["event_count"]) + } else { + assert.Empty(t, entries) + } + + snapshot := scope.Snapshot() + start, ok := snapshot.Counters()["test.request_history_controller.get_by_uri.start+queue=context-queue"] + require.True(t, ok) + assert.EqualValues(t, 1, start.Value()) + assertOperationFinishIncludesContextTag(t, snapshot, "get_by_uri", err == nil) + }) + } +} + +func TestRequestHistoryNotFoundErrors(t *testing.T) { + tests := []struct { + name string + err error + assert func(*testing.T, error) + }{ + { + name: "request ID", + err: fmt.Errorf("lookup failed: %w", &RequestHistoryByIDNotFoundError{RequestID: "request/queue/1"}), + assert: func(t *testing.T, err error) { + var notFound *RequestHistoryByIDNotFoundError + require.ErrorAs(t, err, ¬Found) + assert.Equal(t, "request/queue/1", notFound.RequestID) + }, + }, + { + name: "URI", + err: fmt.Errorf("lookup failed: %w", &RequestHistoryByURINotFoundError{URI: "git://repo/commit/1"}), + assert: func(t *testing.T, err error) { + var notFound *RequestHistoryByURINotFoundError + require.ErrorAs(t, err, ¬Found) + assert.Equal(t, "git://repo/commit/1", notFound.URI) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.True(t, IsRequestHistoryNotFound(tt.err)) + tt.assert(t, tt.err) + }) + } assert.False(t, IsRequestHistoryNotFound(errors.New("other"))) - var notFound *RequestHistoryNotFoundError - require.ErrorAs(t, err, ¬Found) - assert.Equal(t, "request/queue/1", notFound.RequestID) } -func assertOperationFinishIncludesContextTag(t *testing.T, snapshot tally.Snapshot, success bool) { +func assertOperationFinishIncludesContextTag(t *testing.T, snapshot tally.Snapshot, operation string, success bool) { t.Helper() wantResult := "error" if success { wantResult = "success" } for _, histogram := range snapshot.Histograms() { - if histogram.Name() == "test.request_history_controller.get_by_id.finish" { + if histogram.Name() == "test.request_history_controller."+operation+".finish" { assert.Equal(t, "context-queue", histogram.Tags()["queue"]) assert.Equal(t, wantResult, histogram.Tags()["result"]) return