From d902780df9f80013ba89a6385b13b75c53747c33 Mon Sep 17 00:00:00 2001 From: mnoah1 Date: Thu, 29 Oct 2026 13:45:02 +0000 Subject: [PATCH] fix(stovepipe): complete project results when recording is abandoned ## Why? After record-stage work was abandoned, project_results_complete stayed false and the UI continued to report that project result recording was in progress. ## What? Project record_abandoned events into the existing completion timestamp. Preserve the first projected completion time and any recorded facts and project counts; keep project_facts_recorded distinct from abandonment. Cover event replay, concurrent projection updates, and empty or partial results through both status endpoints. Existing abandoned requests require replaying their retained abandonment event after rollout to repair the summary. ## Test Plan Passed the request-log, record-controller, and RPC-controller unit tests and the Docker-based Stovepipe integration suite. Also passed make fmt, make lint, make check-tidy, and make check-gazelle. ## Issue None. --- doc/rfc/stovepipe/project-status-api.md | 4 +- doc/rfc/stovepipe/request-log.md | 4 +- doc/rfc/stovepipe/steps/record.md | 4 +- stovepipe/core/requestlog/materializer.go | 4 +- .../core/requestlog/materializer_test.go | 1 - stovepipe/core/requestlog/request_summary.go | 15 +- .../core/requestlog/request_summary_test.go | 185 ++++++++++-------- test/integration/stovepipe/suite_test.go | 63 ++++++ 8 files changed, 189 insertions(+), 91 deletions(-) diff --git a/doc/rfc/stovepipe/project-status-api.md b/doc/rfc/stovepipe/project-status-api.md index 30d13632a..21baf3183 100644 --- a/doc/rfc/stovepipe/project-status-api.md +++ b/doc/rfc/stovepipe/project-status-api.md @@ -66,13 +66,13 @@ When project IDs are supplied, the response includes only results for those IDs, With no project IDs supplied, the response lists recorded project results by descending breakage degree, then ascending project ID, excluding the repository fact. If the result for the cursor project is missing, the API returns an error. Results are a live view; restart pagination to discover facts recorded before the cursor. -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. +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. Abandoned recording also completes result production; missing project results remain unknown, and `record_abandoned` history explains the abandonment. 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. ## Implementation -Both endpoints return durable lifecycle summaries, the whole-repository fact when present, and recorded facts for requested projects in paginated request order, or recorded project results by descending breakage degree then ascending project ID when none are requested. `project_results_complete` is true when the durable request summary has a positive `project_results_completed_at_ms`, projected from the retained `project_facts_recorded` event. The record stage writes that event after all project facts and ordering mappings, including a successfully resolved empty set, and before publishing its terminal hook. The materializer persists the completion timestamp before the record stage continues, without changing lifecycle state or request version. Duplicate delivery repairs a failed projection using the original event timestamp, and subsequent state projections preserve completion. The API reads the summary before loading facts and includes its completion timestamp in `updated_at_ms`, without an additional event lookup. Missing completion remains false. 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. +Both endpoints return durable lifecycle summaries, the whole-repository fact when present, and recorded facts for requested projects in paginated request order, or recorded project results by descending breakage degree then ascending project ID when none are requested. `project_results_complete` is true when the durable request summary has a positive `project_results_completed_at_ms`, projected from a retained `project_facts_recorded` or `record_abandoned` event. The first projected completion timestamp is preserved when both events exist. Abandonment leaves recorded facts and project counts unchanged. The record stage writes `project_facts_recorded` after all project facts and ordering mappings, including a successfully resolved empty set. The record DLQ retains `record_abandoned` when production stops. Both paths persist completion before publishing a terminal hook. The materializer persists the completion timestamp before the record stage continues, without changing lifecycle state or request version. Duplicate delivery repairs a failed projection using the original event timestamp, and subsequent state projections preserve completion. The API reads the summary before loading facts and includes its completion timestamp in `updated_at_ms`, without an additional event lookup. Missing completion remains false. 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. A dedicated ordering mapping makes listing a bounded primary-key range read, not a sort over all facts. Listing reads at most `page_size + 1` ordering mappings and hydrates only the returned facts, plus the cursor fact on continuations. The continuation token identifies the last project; its immutable fact supplies the degree needed to resume this ordering. diff --git a/doc/rfc/stovepipe/request-log.md b/doc/rfc/stovepipe/request-log.md index 1dc37cd87..979cc17f6 100644 --- a/doc/rfc/stovepipe/request-log.md +++ b/doc/rfc/stovepipe/request-log.md @@ -208,7 +208,7 @@ For a Request transition, the controller: Request creation, Build changes, and fact creation use the same source-write, log-write, dependent-publish ordering. A request-log outage can leave a source update visible, but it cannot allow dependent processing to move past an unrecorded transition. -The `project_facts_recorded` event also projects its retained timestamp into `RequestSummary.ProjectResultsCompletedAtMs`. This immutable fact is independent of lifecycle state and request version; state updates preserve it, and optimistic locking retries concurrent projection updates. Completion requires an existing lifecycle summary rather than fabricating state from the event. A failed projection fails persistence, and redelivery reconciles the retained occurrence before retrying the summary update. Other non-state events do not update the summary. +The `project_facts_recorded` and `record_abandoned` events project completion of project-result production into `RequestSummary.ProjectResultsCompletedAtMs`. Abandonment ends production without claiming that all project facts were established or changing recorded project counts. The first projected completion timestamp is immutable, including when both events are retained; successful recording still projects its project counts. Completion is independent of lifecycle state and request version; state updates preserve it, and optimistic locking retries concurrent projection updates. Completion requires an existing lifecycle summary rather than fabricating state from the event. A failed projection fails persistence, and redelivery reconciles the retained occurrence before retrying the summary update. Other non-state events do not update the summary. ### Controller ownership @@ -219,7 +219,7 @@ The `project_facts_recorded` event also projects its retained timestamp into `Re | Build | Create Build after runner acceptance, then retain `build_triggered`. | An identical existing Build ensures the event before buildsignal publication. | | Buildsignal | Persist terminal Build and retain `build_finished`; CAS the Request outcome and retain its terminal state. | Existing terminal Build and Request outcome each ensure their own entry before record publication. | | Record | Create or verify the whole-repository fact, then retain `validation_fact_recorded`; after recording project facts, retain one `project_facts_recorded` event. | An identical whole-repository fact and project-fact batch ensure their events before bookmark or promotion work. | -| Record DLQ | Retain `record_abandoned`; for failed Requests, create or verify the repository fact and retain `validation_fact_recorded` before publishing its hook. | Existing facts remain authoritative; stable log and hook IDs make replay idempotent without resolving projects or replaying promotion. | +| Record DLQ | Retain `record_abandoned` and project the end of project-result production; for failed Requests, create or verify the repository fact and retain `validation_fact_recorded` before publishing its hook. | Existing facts remain authoritative; stable log and hook IDs make replay idempotent without resolving projects or replaying promotion. | | Build analysis DLQ | Retain `build_analysis_abandoned` without rerunning detection or changing validation state. | One stable event per build deduplicates recovery deliveries; only state reads and history retention retry. | | Issue tracking DLQ | Protect the current cursor from delayed writes, retain its issue history and `issue_tracking_abandoned`, then mark the pass abandoned. | A stable hash of the notification ID deduplicates the event. Recovery retains accepted issue changes and never processes the remaining range. | | Reconciler | CAS an unrecoverable non-terminal Request to failed, then retain failed. | An existing terminal Request is repaired from its persisted outcome without relabeling it. | diff --git a/doc/rfc/stovepipe/steps/record.md b/doc/rfc/stovepipe/steps/record.md index d0ab56539..8bfa58cef 100644 --- a/doc/rfc/stovepipe/steps/record.md +++ b/doc/rfc/stovepipe/steps/record.md @@ -284,9 +284,9 @@ Every primary subscription comes from `DefaultSubscriptionConfig`, which enables Two different things put a message there, and only one is a poison payload. A delivery that fails with its retry budget spent is dead-lettered by the nack itself, carrying the reason it actually failed. A delivery that never reaches a nack, because it crashed or because its **ack failed** and the visibility timeout redelivered it, is dead-lettered by the poll loop once `retry_count` reaches `MaxAttempts` (3 by default), without the controller running on that final attempt and with only a generic reason recorded. So a missing reconciler exposes more than malformed messages: a fact can be lost to a storage failure that would have succeeded on a later retry, or to an ack that never landed even though the write did. -The reconciler retains a stable `record_abandoned` event. For a Request already in `failed`, it creates a `DegreeBroken` whole-repository fact if absent, retains `validation_fact_recorded` history, and publishes `validation.repository.recorded` after both writes succeed. An existing fact owned by the same Request is adopted unchanged, including its degree and timestamp; a fact owned by another Request is an invariant error. Replays repair missing history and repeat the notification with the same event ID. +The reconciler retains a stable `record_abandoned` event and durably projects completion of project-result production before continuing. For a Request already in `failed`, it creates a `DegreeBroken` whole-repository fact if absent, retains `validation_fact_recorded` history, and publishes `validation.repository.recorded` after both writes succeed. An existing fact owned by the same Request is adopted unchanged, including its degree and timestamp; a fact owned by another Request is an invariant error. Replays repair missing history and repeat the notification with the same event ID. -This fallback does not resolve project attribution or mark project recording complete. A failed build establishes a repository failure even when no individual project verdict is known. Build-issue analysis and detectors remain independent. The reconciler does not change the Request state, advance the bookmark, promote, or release build slots. Other Request states retain abandonment history only, so a succeeded build whose fact could not be recorded still appears unvalidated. +This fallback does not resolve project attribution. Completion means production has stopped; missing project results remain unknown, and abandonment history distinguishes this path from successful project recording. A failed build establishes a repository failure even when no individual project verdict is known. Build-issue analysis and detectors remain independent. The reconciler does not change the Request state, advance the bookmark, promote, or release build slots. Other Request states retain abandonment and project-result completion without creating a repository fact, so a succeeded build whose fact could not be recorded still appears unvalidated. The DLQ consumer uses `errs.AlwaysRetryableProcessor`: Request reads, fact writes, history persistence, and notification publication retry until recovery succeeds. Malformed payloads, invalid or unresolvable queue identities, queue mismatches, and missing Requests are logged, counted, and acknowledged. diff --git a/stovepipe/core/requestlog/materializer.go b/stovepipe/core/requestlog/materializer.go index a90f79348..a31aa90e0 100644 --- a/stovepipe/core/requestlog/materializer.go +++ b/stovepipe/core/requestlog/materializer.go @@ -119,7 +119,9 @@ func (m *materializer) PersistLog(ctx context.Context, stores storage.Storage, l return err } - if retained.State == entity.RequestStateUnknown && retained.Event != entity.RequestEventProjectFactsRecorded { + if retained.State == entity.RequestStateUnknown && + retained.Event != entity.RequestEventProjectFactsRecorded && + retained.Event != entity.RequestEventRecordAbandoned { return nil } if err := materializeRequestProjections(ctx, stores, retained); err != nil { diff --git a/stovepipe/core/requestlog/materializer_test.go b/stovepipe/core/requestlog/materializer_test.go index 46131122e..e12ed0fe5 100644 --- a/stovepipe/core/requestlog/materializer_test.go +++ b/stovepipe/core/requestlog/materializer_test.go @@ -389,7 +389,6 @@ func TestMaterializerRetainsOtherEventsWithoutProjectingSummary(t *testing.T) { entity.RequestEventBuildTriggered, entity.RequestEventBuildFinished, entity.RequestEventValidationFactRecorded, - entity.RequestEventRecordAbandoned, entity.RequestEventBuildAnalysisAbandoned, entity.RequestEventIssueTrackingAbandoned, } { diff --git a/stovepipe/core/requestlog/request_summary.go b/stovepipe/core/requestlog/request_summary.go index 230c6edc4..ec6af16a3 100644 --- a/stovepipe/core/requestlog/request_summary.go +++ b/stovepipe/core/requestlog/request_summary.go @@ -117,12 +117,17 @@ func projectRequestSummaryFacts(summary entity.RequestSummary, log entity.Reques log.RequestID, summary.RequestID, log.Queue, summary.Queue, ) } - if log.Event == entity.RequestEventProjectFactsRecorded { - if log.TimestampMs <= 0 || (summary.ProjectResultsCompletedAtMs != 0 && summary.ProjectResultsCompletedAtMs != log.TimestampMs) { - return entity.RequestSummary{}, fmt.Errorf("request summary completion time conflicts with retained log request_id=%q", log.RequestID) + if log.Event == entity.RequestEventProjectFactsRecorded || log.Event == entity.RequestEventRecordAbandoned { + if log.TimestampMs <= 0 { + return entity.RequestSummary{}, fmt.Errorf("request summary completion time must be positive request_id=%q", log.RequestID) + } + // Recording and abandonment can both be retained; the first projected completion stays authoritative. + if summary.ProjectResultsCompletedAtMs == 0 { + summary.ProjectResultsCompletedAtMs = log.TimestampMs + } + if log.Event == entity.RequestEventProjectFactsRecorded { + return projectRequestSummaryProjectCounts(summary, log) } - summary.ProjectResultsCompletedAtMs = log.TimestampMs - return projectRequestSummaryProjectCounts(summary, log) } if log.State == entity.RequestStateAccepted { if summary.AcceptedAtMs != 0 && summary.AcceptedAtMs != log.TimestampMs { diff --git a/stovepipe/core/requestlog/request_summary_test.go b/stovepipe/core/requestlog/request_summary_test.go index e98aa7b07..82535b424 100644 --- a/stovepipe/core/requestlog/request_summary_test.go +++ b/stovepipe/core/requestlog/request_summary_test.go @@ -514,19 +514,28 @@ func TestMaterializerRetriesAcceptanceRepairAfterPartialWrite(t *testing.T) { func TestMaterializerProjectsProjectCompletion(t *testing.T) { for _, tt := range []struct { name string + event entity.RequestEvent completedAt int64 count string projectCount int32 - wantErr bool }{ - {name: "completed projects", count: "3", projectCount: 3}, - {name: "completed empty set", count: "0"}, - {name: "duplicate completion", completedAt: testNowMs, count: "3", projectCount: 3}, - {name: "conflicting timestamp", completedAt: testNowMs - 1, count: "3", wantErr: true}, + {name: "completed projects", event: entity.RequestEventProjectFactsRecorded, count: "3", projectCount: 3}, + {name: "completed empty set", event: entity.RequestEventProjectFactsRecorded, count: "0"}, + {name: "duplicate completion", event: entity.RequestEventProjectFactsRecorded, completedAt: testNowMs, count: "3", projectCount: 3}, + {name: "recording after abandonment", event: entity.RequestEventProjectFactsRecorded, completedAt: testNowMs - 1, count: "3", projectCount: 3}, + {name: "delayed recording after abandonment", event: entity.RequestEventProjectFactsRecorded, completedAt: testNowMs + 1, count: "3", projectCount: 3}, + {name: "abandoned recording", event: entity.RequestEventRecordAbandoned}, + {name: "duplicate abandonment", event: entity.RequestEventRecordAbandoned, completedAt: testNowMs}, + {name: "abandonment after recording", event: entity.RequestEventRecordAbandoned, completedAt: testNowMs - 1}, + {name: "delayed abandonment after recording", event: entity.RequestEventRecordAbandoned, completedAt: testNowMs + 1}, } { t.Run(tt.name, func(t *testing.T) { materializer, stores, logs, summaries, _ := newTestMaterializer(t) - log := NewRequestEventLog(entity.Request{ID: testRequestID, Queue: testQueue}, entity.RequestEventProjectFactsRecorded, "all", map[string]string{MetadataKeyProjectFactCount: tt.count, MetadataKeyBrokenProjectFactCount: "0"}) + var metadata map[string]string + if tt.event == entity.RequestEventProjectFactsRecorded { + metadata = map[string]string{MetadataKeyProjectFactCount: tt.count, MetadataKeyBrokenProjectFactCount: "0"} + } + log := NewRequestEventLog(entity.Request{ID: testRequestID, Queue: testQueue}, tt.event, "test", metadata) log.TimestampMs = testNowMs current := seededRequestSummary() current.State = entity.RequestStateFailed @@ -536,95 +545,115 @@ func TestMaterializerProjectsProjectCompletion(t *testing.T) { current.AcceptedAtMs = testNowMs - 2000 current.OutcomeReason = entity.RequestOutcomeReasonBuildFailed current.ProjectResultsCompletedAtMs = tt.completedAt + current.ProjectCount = 5 + current.BrokenProjectCount = 2 want := current - want.ProjectResultsCompletedAtMs = log.TimestampMs - want.ProjectCount = tt.projectCount + if want.ProjectResultsCompletedAtMs == 0 { + want.ProjectResultsCompletedAtMs = log.TimestampMs + } + if tt.event == entity.RequestEventProjectFactsRecorded { + want.ProjectCount = tt.projectCount + want.BrokenProjectCount = 0 + } logs.EXPECT().Create(gomock.Any(), log).Return(storage.ErrAlreadyExists) logs.EXPECT().Get(gomock.Any(), testRequestID, log.ID).Return(log, nil) summaries.EXPECT().Get(gomock.Any(), testRequestID).Return(current, nil) - if !tt.wantErr && want != current { + if want != current { summaries.EXPECT().Update(gomock.Any(), want, current.Version, current.Version+1).Return(nil) } - err := materializer.PersistLog(context.Background(), stores, log) - if tt.wantErr { - require.Error(t, err) - } else { - require.NoError(t, err) - } + require.NoError(t, materializer.PersistLog(context.Background(), stores, log)) }) } } func TestMaterializerRepairsCompletionAfterProjectionFailure(t *testing.T) { - materializer, stores, logs, summaries, _ := newTestMaterializer(t) - log := NewRequestEventLog(entity.Request{ID: testRequestID, Queue: testQueue}, entity.RequestEventProjectFactsRecorded, "all", map[string]string{MetadataKeyProjectFactCount: "3", MetadataKeyBrokenProjectFactCount: "1"}) - retained := log - retained.TimestampMs = testNowMs - current := seededRequestSummary() - current.State = entity.RequestStateFailed - current.RequestVersion = 3 - current.StateTimestampMs = testNowMs - 1000 - updated := current - updated.ProjectResultsCompletedAtMs = retained.TimestampMs - updated.ProjectCount = 3 - updated.BrokenProjectCount = 1 - writeErr := errors.New("summary unavailable") - gomock.InOrder( - logs.EXPECT().Create(gomock.Any(), retained).Return(nil), - summaries.EXPECT().Get(gomock.Any(), testRequestID).Return(current, nil), - summaries.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(writeErr), - ) - require.ErrorIs(t, materializer.PersistLog(context.Background(), stores, log), writeErr) - - materializer.now = func() time.Time { return time.UnixMilli(testNowMs + 1000) } - candidate := log - candidate.TimestampMs = testNowMs + 1000 - gomock.InOrder( - logs.EXPECT().Create(gomock.Any(), candidate).Return(storage.ErrAlreadyExists), - logs.EXPECT().Get(gomock.Any(), testRequestID, log.ID).Return(retained, nil), - summaries.EXPECT().Get(gomock.Any(), testRequestID).Return(current, nil), - summaries.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(nil), - ) - require.NoError(t, materializer.PersistLog(context.Background(), stores, log)) + for _, event := range []entity.RequestEvent{entity.RequestEventProjectFactsRecorded, entity.RequestEventRecordAbandoned} { + t.Run(string(event), func(t *testing.T) { + materializer, stores, logs, summaries, _ := newTestMaterializer(t) + log := NewRequestEventLog(entity.Request{ID: testRequestID, Queue: testQueue}, event, "test", map[string]string{MetadataKeyProjectFactCount: "3", MetadataKeyBrokenProjectFactCount: "1"}) + retained := log + retained.TimestampMs = testNowMs + current := seededRequestSummary() + current.State = entity.RequestStateFailed + current.RequestVersion = 3 + current.StateTimestampMs = testNowMs - 1000 + updated := current + updated.ProjectResultsCompletedAtMs = retained.TimestampMs + if event == entity.RequestEventProjectFactsRecorded { + updated.ProjectCount = 3 + updated.BrokenProjectCount = 1 + } + writeErr := errors.New("summary unavailable") + gomock.InOrder( + logs.EXPECT().Create(gomock.Any(), retained).Return(nil), + summaries.EXPECT().Get(gomock.Any(), testRequestID).Return(current, nil), + summaries.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(writeErr), + ) + require.ErrorIs(t, materializer.PersistLog(context.Background(), stores, log), writeErr) + + materializer.now = func() time.Time { return time.UnixMilli(testNowMs + 1000) } + candidate := log + candidate.TimestampMs = testNowMs + 1000 + gomock.InOrder( + logs.EXPECT().Create(gomock.Any(), candidate).Return(storage.ErrAlreadyExists), + logs.EXPECT().Get(gomock.Any(), testRequestID, log.ID).Return(retained, nil), + summaries.EXPECT().Get(gomock.Any(), testRequestID).Return(current, nil), + summaries.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(nil), + ) + require.NoError(t, materializer.PersistLog(context.Background(), stores, log)) + }) + } } func TestMaterializerRetriesCompletionAfterStateProjectionWins(t *testing.T) { - materializer, stores, logs, summaries, _ := newTestMaterializer(t) - log := NewRequestEventLog(entity.Request{ID: testRequestID, Queue: testQueue}, entity.RequestEventProjectFactsRecorded, "all", map[string]string{MetadataKeyProjectFactCount: "3", MetadataKeyBrokenProjectFactCount: "1"}) - log.TimestampMs = testNowMs - current := seededRequestSummary() - current.State = entity.RequestStateProcessing - current.RequestVersion = 2 - updated := current - updated.ProjectResultsCompletedAtMs = log.TimestampMs - updated.ProjectCount = 3 - updated.BrokenProjectCount = 1 - winner := current - winner.State = entity.RequestStateFailed - winner.RequestVersion = 3 - winner.StateTimestampMs = testNowMs + 1 - winner.OutcomeReason = entity.RequestOutcomeReasonBuildFailed - winner.Version++ - updatedWinner := winner - updatedWinner.ProjectResultsCompletedAtMs = log.TimestampMs - updatedWinner.ProjectCount = 3 - updatedWinner.BrokenProjectCount = 1 - gomock.InOrder( - logs.EXPECT().Create(gomock.Any(), log).Return(nil), - summaries.EXPECT().Get(gomock.Any(), testRequestID).Return(current, nil), - summaries.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(storage.ErrVersionMismatch), - summaries.EXPECT().Get(gomock.Any(), testRequestID).Return(winner, nil), - summaries.EXPECT().Update(gomock.Any(), updatedWinner, int32(2), int32(3)).Return(nil), - ) - require.NoError(t, materializer.PersistLog(context.Background(), stores, log)) + for _, event := range []entity.RequestEvent{entity.RequestEventProjectFactsRecorded, entity.RequestEventRecordAbandoned} { + t.Run(string(event), func(t *testing.T) { + materializer, stores, logs, summaries, _ := newTestMaterializer(t) + log := NewRequestEventLog(entity.Request{ID: testRequestID, Queue: testQueue}, event, "test", map[string]string{MetadataKeyProjectFactCount: "3", MetadataKeyBrokenProjectFactCount: "1"}) + log.TimestampMs = testNowMs + current := seededRequestSummary() + current.State = entity.RequestStateProcessing + current.RequestVersion = 2 + updated := current + updated.ProjectResultsCompletedAtMs = log.TimestampMs + if event == entity.RequestEventProjectFactsRecorded { + updated.ProjectCount = 3 + updated.BrokenProjectCount = 1 + } + winner := current + winner.State = entity.RequestStateFailed + winner.RequestVersion = 3 + winner.StateTimestampMs = testNowMs + 1 + winner.OutcomeReason = entity.RequestOutcomeReasonBuildFailed + winner.Version++ + updatedWinner := winner + updatedWinner.ProjectResultsCompletedAtMs = log.TimestampMs + if event == entity.RequestEventProjectFactsRecorded { + updatedWinner.ProjectCount = 3 + updatedWinner.BrokenProjectCount = 1 + } + gomock.InOrder( + logs.EXPECT().Create(gomock.Any(), log).Return(nil), + summaries.EXPECT().Get(gomock.Any(), testRequestID).Return(current, nil), + summaries.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(storage.ErrVersionMismatch), + summaries.EXPECT().Get(gomock.Any(), testRequestID).Return(winner, nil), + summaries.EXPECT().Update(gomock.Any(), updatedWinner, int32(2), int32(3)).Return(nil), + ) + require.NoError(t, materializer.PersistLog(context.Background(), stores, log)) + }) + } } func TestMaterializerCompletionRequiresExistingSummary(t *testing.T) { - materializer, stores, logs, summaries, _ := newTestMaterializer(t) - log := NewRequestEventLog(entity.Request{ID: testRequestID, Queue: testQueue}, entity.RequestEventProjectFactsRecorded, "all", nil) - logs.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil) - summaries.EXPECT().Get(gomock.Any(), testRequestID).Return(entity.RequestSummary{}, storage.ErrNotFound) - require.ErrorIs(t, materializer.PersistLog(context.Background(), stores, log), storage.ErrNotFound) + for _, event := range []entity.RequestEvent{entity.RequestEventProjectFactsRecorded, entity.RequestEventRecordAbandoned} { + t.Run(string(event), func(t *testing.T) { + materializer, stores, logs, summaries, _ := newTestMaterializer(t) + log := NewRequestEventLog(entity.Request{ID: testRequestID, Queue: testQueue}, event, "test", nil) + logs.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil) + summaries.EXPECT().Get(gomock.Any(), testRequestID).Return(entity.RequestSummary{}, storage.ErrNotFound) + require.ErrorIs(t, materializer.PersistLog(context.Background(), stores, log), storage.ErrNotFound) + }) + } } func TestProjectRequestSummaryProjectCounts(t *testing.T) { diff --git a/test/integration/stovepipe/suite_test.go b/test/integration/stovepipe/suite_test.go index a23a7e4b3..3a9084921 100644 --- a/test/integration/stovepipe/suite_test.go +++ b/test/integration/stovepipe/suite_test.go @@ -412,6 +412,69 @@ func (s *StovepipeIntegrationSuite) TestProjectStatusAPI() { } } +func (s *StovepipeIntegrationSuite) TestAbandonedProjectResultsAreComplete() { + for _, recordedProject := range []bool{false, true} { + name := "empty" + if recordedProject { + name = "partial" + } + s.T().Run(name, func(t *testing.T) { + queue, id := "project-status-abandoned/"+name, "1" + uri := "git://project-status-abandoned/" + name + "/commit" + _, err := s.db.Exec(`INSERT INTO request_summary + (queue, request_id, uri, base_uri, state, request_version, state_timestamp_ms, version) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, queue, id, uri, "", "failed", 3, 1000, 1) + require.NoError(t, err) + _, err = s.db.Exec("INSERT INTO request_uri (queue, uri, request_id, version) VALUES (?, ?, ?, ?)", queue, uri, id, 1) + require.NoError(t, err) + backend, err := mysqlstorage.NewStorage(s.db, tally.NoopScope) + require.NoError(t, err) + stores, err := backend.For(queue) + require.NoError(t, err) + require.NoError(t, stores.GetValidationFactStore().Create(s.ctx, entity.ValidationFact{ + URI: uri, RequestID: id, Degree: entity.DegreeBroken, CreatedAt: 2000, + })) + if recordedProject { + require.NoError(t, stores.GetValidationFactStore().Create(s.ctx, entity.ValidationFact{ + URI: uri, RequestID: id, Project: "project-a", Degree: entity.DegreeBroken, CreatedAt: 3000, + })) + } + byIDRequest := &pb.GetProjectStatusByIDRequest{Queue: queue, RequestId: id, Projects: []string{"project-a", "missing"}} + pending, err := s.client.GetProjectStatusByID(s.ctx, byIDRequest) + require.NoError(t, err) + assert.False(t, pending.ProjectResultsComplete) + + abandonment := requestlog.NewRequestEventLog(entity.Request{Queue: queue, ID: id}, entity.RequestEventRecordAbandoned, "repository", nil) + abandonment.TimestampMs = 4000 + materializer := requestlog.NewMaterializer(tally.NoopScope) + require.NoError(t, materializer.PersistLog(s.ctx, stores, abandonment)) + abandonment.TimestampMs = 5000 + require.NoError(t, materializer.PersistLog(s.ctx, stores, abandonment)) + + byID, err := s.client.GetProjectStatusByID(s.ctx, byIDRequest) + require.NoError(t, err) + byURI, err := s.client.GetProjectStatusByURI(s.ctx, &pb.GetProjectStatusByURIRequest{ + Queue: queue, ChangeUri: uri, Projects: byIDRequest.Projects, + }) + require.NoError(t, err) + assert.True(t, byID.ProjectResultsComplete) + assert.True(t, byURI.ProjectResultsComplete) + assert.Equal(t, int64(4000), byID.UpdatedAtMs) + assert.Equal(t, byID.UpdatedAtMs, byURI.UpdatedAtMs) + assert.Equal(t, entity.DegreeBroken, byID.GetRepositoryBreakageDegree()) + assert.Equal(t, pending.Projects, byID.Projects) + assert.Equal(t, byID.Projects, byURI.Projects) + if recordedProject { + require.Len(t, byID.Projects, 1) + assert.Equal(t, "project-a", byID.Projects[0].Project) + assert.Equal(t, entity.DegreeBroken, byID.Projects[0].GetBreakageDegree()) + } else { + assert.Empty(t, byID.Projects) + } + }) + } +} + func (s *StovepipeIntegrationSuite) TestUnindexedFactsRemainAvailableByExplicitProject() { t := s.T() const queue, id, uri = "fresh-index/main", "9", "git://fresh-index/main/older-commit"