Skip to content

Commit 38ee0d7

Browse files
committed
feat(stovepipe): Show most broken projects first
1 parent c06f4a4 commit 38ee0d7

8 files changed

Lines changed: 113 additions & 59 deletions

File tree

‎doc/rfc/stovepipe/project-status-api.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,15 @@ The caller may supply the project IDs for which it wants recorded results. The A
6464

6565
When project IDs are supplied, the response includes only results for those IDs, in the supplied order, and paginates that list. An implementation may record results for every requested project, only requested projects with a non-green result, or another documented subset.
6666

67-
With no project IDs supplied, the response lists all recorded project results in project-ID order, excluding the repository fact. Results are a live view; restart pagination to discover facts recorded behind the cursor.
67+
With no project IDs supplied, the response lists all recorded project results by descending breakage degree, then ascending project ID, excluding the repository fact. The continuation token identifies the last project; its immutable fact supplies the degree needed to resume this ordering. A missing cursor fact returns an error. Results are a live view; restart pagination to discover facts recorded before the cursor.
6868

6969
Each project result is an immutable validation fact keyed by queue, commit URI, and project ID. A result must identify the selected request. `project_results_complete` is false until the implementation has durably finished producing results for the supplied project IDs; a completed empty set is valid.
7070

7171
Callers use `project_results_complete`, rather than the presence or absence of an individual project result, to determine whether the implementation has finished producing results for the requested projects. `next_page_token` is empty only on the final page. A terminal lifecycle notification is emitted only after the repository result and any applicable project completion record are durable. If notification delivery fails, consumers can recover the same result through either endpoint.
7272

7373
## Implementation
7474

75-
Both endpoints return durable lifecycle summaries, the whole-repository fact when present, and recorded facts for requested projects in paginated request order, or all recorded project results in project-ID order when none are requested. `project_results_complete` remains false until a durable completion projection is available. Missing facts are omitted; an explicitly recorded zero degree remains present. `updated_at_ms` is the maximum timestamp of the summary and the facts represented on the page.
75+
Both endpoints return durable lifecycle summaries, the whole-repository fact when present, and recorded facts for requested projects in paginated request order, or all recorded project results by descending breakage degree then ascending project ID when none are requested. `project_results_complete` remains false until a durable completion projection is available. Missing facts are omitted; an explicitly recorded zero degree remains present. `updated_at_ms` is the maximum timestamp of the summary and the facts represented on the page.
7676

7777
## Errors and Authorization
7878

‎stovepipe/controller/project_status_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -300,9 +300,9 @@ func TestGetProjectStatusReturnsProjectFactsInRequestedOrder(t *testing.T) {
300300
summaryStore.EXPECT().Get(gomock.Any(), projectStatusID).Return(requestSummary, nil)
301301
store.EXPECT().GetValidationFactStore().Return(factStore)
302302
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, "").Return(entity.ValidationFact{}, storage.ErrNotFound)
303-
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, "project-b").Return(entity.ValidationFact{URI: projectStatusURI, RequestID: projectStatusID, Project: "project-b", Degree: entity.DegreeBroken, CreatedAt: 20}, nil)
303+
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, "project-b").Return(entity.ValidationFact{URI: projectStatusURI, RequestID: projectStatusID, Project: "project-b", Degree: entity.DegreeGreen, CreatedAt: 20}, nil)
304304
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, "missing").Return(entity.ValidationFact{}, storage.ErrNotFound)
305-
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, "project-a").Return(entity.ValidationFact{URI: projectStatusURI, RequestID: projectStatusID, Project: "project-a", Degree: entity.DegreeGreen, CreatedAt: 30}, nil)
305+
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, "project-a").Return(entity.ValidationFact{URI: projectStatusURI, RequestID: projectStatusID, Project: "project-a", Degree: entity.DegreeBroken, CreatedAt: 30}, nil)
306306

307307
controller := NewProjectStatusController(zap.NewNop().Sugar(), tally.NoopScope, factory)
308308
got, err := controller.GetProjectStatus(context.Background(), entity.GetProjectStatusRequest{Queue: projectStatusQueue, ChangeURI: projectStatusURI, Projects: []string{"project-b", "missing", "project-a"}})
@@ -452,9 +452,9 @@ func TestParseProjectStatusPageTokenRejectsMismatches(t *testing.T) {
452452
func TestGetProjectStatusPaginatesStoredProjects(t *testing.T) {
453453
summary := entity.RequestSummary{RequestID: projectStatusID, Queue: projectStatusQueue, URI: projectStatusURI, StateTimestampMs: 10}
454454
stored := []entity.ValidationFact{
455+
{URI: projectStatusURI, RequestID: projectStatusID, Project: "c", Degree: 1, CreatedAt: 60},
455456
{URI: projectStatusURI, RequestID: projectStatusID, Project: "a", Degree: 0.5, CreatedAt: 20},
456457
{URI: projectStatusURI, RequestID: projectStatusID, Project: "b", Degree: 0, CreatedAt: 50},
457-
{URI: projectStatusURI, RequestID: projectStatusID, Project: "c", Degree: 1, CreatedAt: 60},
458458
}
459459

460460
ctrl := gomock.NewController(t)
@@ -469,15 +469,15 @@ func TestGetProjectStatusPaginatesStoredProjects(t *testing.T) {
469469
store.EXPECT().GetValidationFactStore().Return(facts).Times(2)
470470
facts.EXPECT().Get(gomock.Any(), projectStatusURI, "").Return(entity.ValidationFact{}, storage.ErrNotFound).Times(2)
471471
facts.EXPECT().ListProjects(gomock.Any(), projectStatusURI, "", 3).Return(stored, nil)
472-
facts.EXPECT().ListProjects(gomock.Any(), projectStatusURI, "b", defaultProjectStatusPageSize+1).Return(stored[2:], nil)
472+
facts.EXPECT().ListProjects(gomock.Any(), projectStatusURI, "a", defaultProjectStatusPageSize+1).Return(stored[2:], nil)
473473

474474
controller := NewProjectStatusController(zap.NewNop().Sugar(), tally.NoopScope, factory)
475475
req := entity.GetProjectStatusRequest{Queue: projectStatusQueue, RequestID: projectStatusID, PageSize: 2}
476476

477477
first, err := controller.GetProjectStatus(context.Background(), req)
478478
require.NoError(t, err)
479479
assert.Equal(t, stored[:2], first.ProjectValidationFacts)
480-
assert.Equal(t, int64(50), first.UpdatedAtMs)
480+
assert.Equal(t, int64(60), first.UpdatedAtMs)
481481
require.NotEmpty(t, first.NextPageToken)
482482

483483
req.PageToken = first.NextPageToken
@@ -487,7 +487,7 @@ func TestGetProjectStatusPaginatesStoredProjects(t *testing.T) {
487487
second, err := controller.GetProjectStatus(context.Background(), req)
488488
require.NoError(t, err)
489489
assert.Equal(t, stored[2:], second.ProjectValidationFacts)
490-
assert.Equal(t, int64(60), second.UpdatedAtMs)
490+
assert.Equal(t, int64(50), second.UpdatedAtMs)
491491
assert.Empty(t, second.NextPageToken)
492492
}
493493

‎stovepipe/entity/project_status.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ type GetProjectStatusResult struct {
4040
HasRepositoryValidationFact bool
4141
// ProjectResultsComplete reports whether the implementation has finished its project-result set.
4242
ProjectResultsComplete bool
43-
// ProjectValidationFacts contains recorded results in request order, or project-ID order when Projects is empty.
43+
// ProjectValidationFacts contains recorded results in request order, or descending degree then ascending project ID when Projects is empty.
4444
ProjectValidationFacts []ValidationFact
4545
// NextPageToken continues requested-project result pagination when another page exists.
4646
NextPageToken string

‎stovepipe/extension/storage/mysql/validation_fact_store.go‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,21 @@ func (v *validationFactStore) ListProjects(ctx context.Context, uri, afterProjec
101101
if limit <= 0 {
102102
return nil, fmt.Errorf("validation fact list limit must be positive")
103103
}
104-
rows, err := v.db.QueryContext(ctx, `
104+
query := `
105105
SELECT uri, project, degree, request_id, created_at FROM validation_fact
106-
WHERE queue = ? AND uri = ? AND project > ?
107-
ORDER BY project ASC LIMIT ?`,
108-
v.queue, uri, afterProject, limit,
109-
)
106+
WHERE queue = ? AND uri = ? AND project <> ''`
107+
args := []any{v.queue, uri}
108+
if afterProject != "" {
109+
cursor, err := v.Get(ctx, uri, afterProject)
110+
if err != nil {
111+
return nil, fmt.Errorf("failed to load project cursor queue=%s uri=%s project=%s: %w", v.queue, uri, afterProject, err)
112+
}
113+
query += ` AND (degree < ? OR (degree = ? AND project > ?))`
114+
args = append(args, cursor.Degree, cursor.Degree, afterProject)
115+
}
116+
query += ` ORDER BY degree DESC, project ASC LIMIT ?`
117+
args = append(args, limit)
118+
rows, err := v.db.QueryContext(ctx, query, args...)
110119
if err != nil {
111120
return nil, fmt.Errorf("failed to list project facts queue=%s uri=%s: %w", v.queue, uri, err)
112121
}

‎stovepipe/extension/storage/mysql/validation_fact_store_test.go‎

Lines changed: 62 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -184,38 +184,71 @@ func TestValidationFactStore_ListProjects(t *testing.T) {
184184
readErr := fmt.Errorf("connection reset")
185185
columns := []string{"uri", "project", "degree", "request_id", "created_at"}
186186
want := entity.ValidationFact{URI: testFactURI, Project: "project-b", Degree: 0.5, RequestID: "2", CreatedAt: 20}
187-
tests := []struct {
188-
name string
189-
rows *sqlmock.Rows
190-
queryErr error
191-
wantErr bool
192-
want []entity.ValidationFact
193-
}{
194-
{name: "found", rows: sqlmock.NewRows(columns).AddRow(want.URI, want.Project, want.Degree, want.RequestID, want.CreatedAt), want: []entity.ValidationFact{want}},
195-
{name: "empty", rows: sqlmock.NewRows(columns)},
196-
{name: "query failure", queryErr: readErr, wantErr: true},
197-
{name: "scan failure", rows: sqlmock.NewRows(columns).AddRow(want.URI, want.Project, "invalid", want.RequestID, want.CreatedAt), wantErr: true},
198-
{name: "iteration failure", rows: sqlmock.NewRows(columns).AddRow(want.URI, want.Project, want.Degree, want.RequestID, want.CreatedAt).RowError(0, readErr), wantErr: true},
187+
for _, afterProject := range []string{"", "project-z"} {
188+
tests := []struct {
189+
name string
190+
rows *sqlmock.Rows
191+
queryErr error
192+
wantErr bool
193+
want []entity.ValidationFact
194+
}{
195+
{name: "found", rows: sqlmock.NewRows(columns).AddRow(want.URI, want.Project, want.Degree, want.RequestID, want.CreatedAt), want: []entity.ValidationFact{want}},
196+
{name: "empty", rows: sqlmock.NewRows(columns)},
197+
{name: "query failure", queryErr: readErr, wantErr: true},
198+
{name: "scan failure", rows: sqlmock.NewRows(columns).AddRow(want.URI, want.Project, "invalid", want.RequestID, want.CreatedAt), wantErr: true},
199+
{name: "iteration failure", rows: sqlmock.NewRows(columns).AddRow(want.URI, want.Project, want.Degree, want.RequestID, want.CreatedAt).RowError(0, readErr), wantErr: true},
200+
}
201+
for _, tt := range tests {
202+
t.Run(tt.name+"/"+afterProject, func(t *testing.T) {
203+
db, mock, store := setupValidationFactStoreTest(t)
204+
defer db.Close()
205+
queryPattern := `SELECT uri, project, degree, request_id, created_at FROM validation_fact\s+WHERE queue = \? AND uri = \? AND project <> ''`
206+
if afterProject != "" {
207+
mock.ExpectQuery("SELECT uri, project, degree, request_id, created_at FROM validation_fact").
208+
WithArgs(testFactQueue, testFactURI, afterProject).
209+
WillReturnRows(sqlmock.NewRows(columns).AddRow(testFactURI, afterProject, 1.0, "2", 10))
210+
queryPattern += ` AND \(degree < \? OR \(degree = \? AND project > \?\)\)`
211+
}
212+
query := mock.ExpectQuery(queryPattern + ` ORDER BY degree DESC, project ASC LIMIT \?`)
213+
if afterProject == "" {
214+
query.WithArgs(testFactQueue, testFactURI, 3)
215+
} else {
216+
query.WithArgs(testFactQueue, testFactURI, 1.0, 1.0, afterProject, 3)
217+
}
218+
if tt.queryErr != nil {
219+
query.WillReturnError(tt.queryErr)
220+
} else {
221+
query.WillReturnRows(tt.rows)
222+
}
223+
got, err := store.ListProjects(context.Background(), testFactURI, afterProject, 3)
224+
if tt.wantErr {
225+
require.Error(t, err)
226+
if tt.queryErr != nil {
227+
assert.ErrorIs(t, err, tt.queryErr)
228+
}
229+
} else {
230+
require.NoError(t, err)
231+
assert.Equal(t, tt.want, got)
232+
}
233+
require.NoError(t, mock.ExpectationsWereMet())
234+
})
235+
}
199236
}
200-
for _, tt := range tests {
201-
t.Run(tt.name, func(t *testing.T) {
237+
}
238+
239+
func TestValidationFactStore_ListProjectsCursorFailure(t *testing.T) {
240+
for _, lookupErr := range []error{sql.ErrNoRows, fmt.Errorf("connection reset")} {
241+
t.Run(lookupErr.Error(), func(t *testing.T) {
202242
db, mock, store := setupValidationFactStoreTest(t)
203243
defer db.Close()
204-
query := mock.ExpectQuery(`SELECT uri, project, degree, request_id, created_at FROM validation_fact\s+WHERE queue = \? AND uri = \? AND project > \?\s+ORDER BY project ASC LIMIT \?`).WithArgs(testFactQueue, testFactURI, "project-a", 3)
205-
if tt.queryErr != nil {
206-
query.WillReturnError(tt.queryErr)
207-
} else {
208-
query.WillReturnRows(tt.rows)
209-
}
210-
got, err := store.ListProjects(context.Background(), testFactURI, "project-a", 3)
211-
if tt.wantErr {
212-
require.Error(t, err)
213-
if tt.queryErr != nil {
214-
assert.ErrorIs(t, err, tt.queryErr)
215-
}
216-
} else {
217-
require.NoError(t, err)
218-
assert.Equal(t, tt.want, got)
244+
mock.ExpectQuery("SELECT uri, project, degree, request_id, created_at FROM validation_fact").
245+
WithArgs(testFactQueue, testFactURI, "cursor").
246+
WillReturnError(lookupErr)
247+
248+
_, err := store.ListProjects(context.Background(), testFactURI, "cursor", 3)
249+
require.ErrorIs(t, err, lookupErr)
250+
if lookupErr == sql.ErrNoRows {
251+
assert.ErrorIs(t, err, storage.ErrNotFound)
219252
}
220253
require.NoError(t, mock.ExpectationsWereMet())
221254
})

‎stovepipe/extension/storage/validation_fact_store.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,10 @@ type ValidationFactStore interface {
3636
// fact exists, which means the scope was never validated rather than validated green.
3737
Get(ctx context.Context, uri, project string) (entity.ValidationFact, error)
3838

39-
// ListProjects returns at most limit project facts for uri in ascending project-ID
40-
// order, excluding the repository fact. afterProject is an exclusive cursor (empty
41-
// for the first page); limit must be positive. No matches returns an empty slice.
39+
// ListProjects returns at most limit project facts for uri by descending degree,
40+
// then ascending project ID, excluding the repository fact. afterProject identifies
41+
// the exclusive cursor fact (empty for the first page); its immutable degree is
42+
// resolved internally. A missing cursor returns ErrNotFound. limit must be positive;
43+
// no matches returns an empty slice.
4244
ListProjects(ctx context.Context, uri, afterProject string, limit int) ([]entity.ValidationFact, error)
4345
}

‎test/integration/stovepipe/extension/storage/suite.go‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -688,29 +688,39 @@ func (s *ValidationFactStoreContractSuite) TestValidationFactStore_ListProjects(
688688
store := s.storeFor(queue)
689689
failedA := entity.ValidationFact{URI: uri, Project: "a", Degree: 0.25, RequestID: "1", CreatedAt: 10}
690690
failedC := entity.ValidationFact{URI: uri, Project: "c", Degree: 1, RequestID: "1", CreatedAt: 20}
691+
failedD := entity.ValidationFact{URI: uri, Project: "d", Degree: 1, RequestID: "1", CreatedAt: 25}
691692
greenB := entity.ValidationFact{URI: uri, Project: "b", Degree: 0, RequestID: "1", CreatedAt: 30}
692693
for _, fact := range []entity.ValidationFact{
693-
failedC, failedA,
694+
failedD, failedC, failedA,
694695
greenB,
695696
{URI: uri, Degree: 1, RequestID: "1", CreatedAt: 40},
696697
{URI: uri + "-other", Project: "other-uri", Degree: 1, RequestID: "2", CreatedAt: 50},
697698
} {
698699
require.NoError(t, store.Create(s.ctx, fact))
699700
}
700701
require.NoError(t, s.storeFor(queue+"-other").Create(s.ctx, entity.ValidationFact{URI: uri, Project: "other-queue", Degree: 1, RequestID: "1", CreatedAt: 60}))
701-
first, err := store.ListProjects(s.ctx, uri, "", 1)
702-
require.NoError(t, err)
703-
assert.Equal(t, []entity.ValidationFact{failedA}, first)
702+
want := []entity.ValidationFact{failedC, failedD, failedA, greenB}
703+
afterProject := ""
704+
for _, fact := range want {
705+
page, err := store.ListProjects(s.ctx, uri, afterProject, 1)
706+
require.NoError(t, err)
707+
require.Equal(t, []entity.ValidationFact{fact}, page)
708+
afterProject = page[0].Project
709+
}
704710
second, err := store.ListProjects(s.ctx, uri, "a", 10)
705711
require.NoError(t, err)
706-
assert.Equal(t, []entity.ValidationFact{greenB, failedC}, second)
712+
assert.Equal(t, []entity.ValidationFact{greenB}, second)
707713
all, err := store.ListProjects(s.ctx, uri, "", 10)
708714
require.NoError(t, err)
709-
assert.Equal(t, []entity.ValidationFact{failedA, greenB, failedC}, all)
710-
empty, err := store.ListProjects(s.ctx, uri, "c", 10)
715+
assert.Equal(t, want, all)
716+
empty, err := store.ListProjects(s.ctx, uri, "b", 10)
711717
require.NoError(t, err)
712718
assert.Empty(t, empty)
713719
missing, err := store.ListProjects(s.ctx, "missing", "", 10)
714720
require.NoError(t, err)
715721
assert.Empty(t, missing)
722+
for _, cursor := range []string{"missing", "other-uri", "other-queue"} {
723+
_, err := store.ListProjects(s.ctx, uri, cursor, 10)
724+
assert.ErrorIs(t, err, storage.ErrNotFound)
725+
}
716726
}

‎test/integration/stovepipe/suite_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -352,20 +352,20 @@ func (s *StovepipeIntegrationSuite) TestProjectStatusAPI() {
352352
})
353353
require.NoError(t, err)
354354
require.Len(t, firstPage.Projects, 1)
355-
assert.Equal(t, "project-a", firstPage.Projects[0].GetProject())
356-
assert.Equal(t, 0.5, firstPage.Projects[0].GetBreakageDegree())
357-
assert.Equal(t, int64(3000), firstPage.GetUpdatedAtMs())
355+
assert.Equal(t, "project-b", firstPage.Projects[0].GetProject())
356+
assert.Equal(t, 1.0, firstPage.Projects[0].GetBreakageDegree())
357+
assert.Equal(t, int64(5000), firstPage.GetUpdatedAtMs())
358358
require.NotEmpty(t, firstPage.NextPageToken)
359359
remaining, err := s.client.GetProjectStatusByID(s.ctx, &pb.GetProjectStatusByIDRequest{
360360
Queue: queue, RequestId: id, PageToken: firstPage.NextPageToken,
361361
})
362362
require.NoError(t, err)
363363
require.Len(t, remaining.Projects, 2)
364-
assert.Equal(t, "project-b", remaining.Projects[0].GetProject())
365-
assert.Equal(t, 1.0, remaining.Projects[0].GetBreakageDegree())
364+
assert.Equal(t, "project-a", remaining.Projects[0].GetProject())
365+
assert.Equal(t, 0.5, remaining.Projects[0].GetBreakageDegree())
366366
assert.Equal(t, "project-green", remaining.Projects[1].GetProject())
367367
assert.Zero(t, remaining.Projects[1].GetBreakageDegree())
368-
assert.Equal(t, int64(5000), remaining.GetUpdatedAtMs())
368+
assert.Equal(t, int64(4000), remaining.GetUpdatedAtMs())
369369
assert.Empty(t, remaining.NextPageToken)
370370
assert.False(t, remaining.ProjectResultsComplete)
371371

0 commit comments

Comments
 (0)