From ea2c6cb95a7dfa833c3f2d78382e9b487d864276 Mon Sep 17 00:00:00 2001 From: pj <113072185+prathshenoy@users.noreply.github.com> Date: Mon, 9 Nov 2026 20:55:36 +0000 Subject: [PATCH 1/2] feat(stovepipe): Expose project issue APIs (#1051) ## Summary **What**: - Expose both request-scoped attribution lookups through the service API. **Why**: - Let clients query the issues affecting a project and the projects affected by an issue. The read controller provides this behavior, but service bindings and wiring are needed to make it available through the API. ## Test Plan - [x] Add unit tests. ## Revert Plan - Revert this PR. It adds read-only APIs without changing schemas or stored data. ## Issue Links - [CODEM-632](https://linear.app/uber/issue/CODEM-632/projects-issues) ## Stack 1. #1046 1. #1047 1. #1048 1. #1049 1. #1053 1. #1054 1. #1051 From 0e595b8bd48ba29124b57468ab39ebd80214e87e Mon Sep 17 00:00:00 2001 From: "prath.shenoy" Date: Mon, 9 Nov 2026 20:20:18 +0000 Subject: [PATCH 2/2] refactor(stovepipe): Extract fact persistence ## Summary **What**: - Extract shared validation-fact persistence without changing recording behavior. **Why**: - Allow repository-wide and project results to retain the same retry and ownership checks when their processing moves into separate stages. The existing recording stage owns this shared behavior, preventing another stage from reusing it directly. ## Test Plan - [x] Add unit tests. ## Revert Plan - Revert this PR. It reorganizes existing code without changing schemas or stored data. ## Issue Links - [CODEM-632](https://linear.app/uber/issue/CODEM-632/projects-issues) --- stovepipe/controller/record/BUILD.bazel | 1 + stovepipe/controller/record/facts.go | 44 +--------- stovepipe/controller/record/record.go | 3 +- stovepipe/core/validationfact/BUILD.bazel | 30 +++++++ .../core/validationfact/validationfact.go | 69 +++++++++++++++ .../validationfact/validationfact_test.go | 86 +++++++++++++++++++ 6 files changed, 190 insertions(+), 43 deletions(-) create mode 100644 stovepipe/core/validationfact/BUILD.bazel create mode 100644 stovepipe/core/validationfact/validationfact.go create mode 100644 stovepipe/core/validationfact/validationfact_test.go diff --git a/stovepipe/controller/record/BUILD.bazel b/stovepipe/controller/record/BUILD.bazel index 1f4ee39f6..1d984c2cc 100644 --- a/stovepipe/controller/record/BUILD.bazel +++ b/stovepipe/controller/record/BUILD.bazel @@ -19,6 +19,7 @@ go_library( "//stovepipe/core/loader:go_default_library", "//stovepipe/core/messagequeue:go_default_library", "//stovepipe/core/requestlog:go_default_library", + "//stovepipe/core/validationfact:go_default_library", "//stovepipe/entity:go_default_library", "//stovepipe/extension/projectstatus:go_default_library", "//stovepipe/extension/sourcecontrol:go_default_library", diff --git a/stovepipe/controller/record/facts.go b/stovepipe/controller/record/facts.go index 91db70b75..3612b6d99 100644 --- a/stovepipe/controller/record/facts.go +++ b/stovepipe/controller/record/facts.go @@ -18,7 +18,6 @@ import ( "context" "crypto/sha256" "encoding/hex" - "errors" "fmt" "strconv" "time" @@ -29,6 +28,7 @@ import ( platformhook "github.com/uber/submitqueue/platform/hook" "github.com/uber/submitqueue/platform/metrics" "github.com/uber/submitqueue/stovepipe/core/requestlog" + "github.com/uber/submitqueue/stovepipe/core/validationfact" "github.com/uber/submitqueue/stovepipe/entity" "github.com/uber/submitqueue/stovepipe/extension/projectstatus" "github.com/uber/submitqueue/stovepipe/extension/storage" @@ -43,7 +43,7 @@ func recordFact( request entity.Request, results []projectstatus.Result, ) (entity.ValidationFact, bool, error) { - return recordValidationFact(ctx, logger, scope, store.GetValidationFactStore(), entity.ValidationFact{ + return validationfact.Persist(ctx, logger, scope, store.GetValidationFactStore(), entity.ValidationFact{ URI: request.URI, Project: wholeRepositoryProject, Degree: degreeFor(request.State, results), @@ -52,46 +52,6 @@ func recordFact( }) } -func recordValidationFact( - ctx context.Context, - logger *zap.SugaredLogger, - scope tally.Scope, - factStore storage.ValidationFactStore, - fact entity.ValidationFact, -) (entity.ValidationFact, bool, error) { - err := factStore.Create(ctx, fact) - switch { - case err == nil: - metrics.NamedCounter(scope, _opName, "fact_created", 1, metrics.TagsFromContext(ctx)...) - logger.Infow("recorded validation fact", - "request_id", fact.RequestID, - "uri", fact.URI, - "project", fact.Project, - "degree", fact.Degree, - ) - return fact, true, nil - - case errors.Is(err, storage.ErrAlreadyExists): - stored, getErr := factStore.Get(ctx, fact.URI, fact.Project) - if getErr != nil { - metrics.NamedCounter(scope, _opName, "storage_errors", 1, metrics.TagsFromContext(ctx)...) - return entity.ValidationFact{}, false, fmt.Errorf("failed to load the existing fact for uri %s and project %q: %w", fact.URI, fact.Project, getErr) - } - if stored.RequestID != fact.RequestID { - // Ingest deduplication forbids different requests owning the same immutable fact. - metrics.NamedCounter(scope, _opName, "invariant_errors", 1, metrics.TagsFromContext(ctx)...) - return entity.ValidationFact{}, false, fmt.Errorf( - "fact for uri %s and project %q is owned by request %s, not %s", fact.URI, fact.Project, stored.RequestID, fact.RequestID) - } - metrics.NamedCounter(scope, _opName, "fact_exists", 1, metrics.TagsFromContext(ctx)...) - return stored, false, nil - - default: - metrics.NamedCounter(scope, _opName, "storage_errors", 1, metrics.TagsFromContext(ctx)...) - return entity.ValidationFact{}, false, fmt.Errorf("failed to create the fact for uri %s and project %q: %w", fact.URI, fact.Project, err) - } -} - func persistValidationFactRecordedLog( ctx context.Context, materializer requestlog.Materializer, diff --git a/stovepipe/controller/record/record.go b/stovepipe/controller/record/record.go index 0dc4206e4..d35968f16 100644 --- a/stovepipe/controller/record/record.go +++ b/stovepipe/controller/record/record.go @@ -44,6 +44,7 @@ import ( "github.com/uber/submitqueue/stovepipe/core/loader" stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue" "github.com/uber/submitqueue/stovepipe/core/requestlog" + "github.com/uber/submitqueue/stovepipe/core/validationfact" "github.com/uber/submitqueue/stovepipe/entity" "github.com/uber/submitqueue/stovepipe/extension/projectstatus" "github.com/uber/submitqueue/stovepipe/extension/sourcecontrol" @@ -211,7 +212,7 @@ func (c *Controller) persistProjectFacts(ctx context.Context, store storage.Stor brokenCount := 0 for _, result := range results { - fact, _, err := recordValidationFact(ctx, c.logger, c.metricsScope, store.GetValidationFactStore(), entity.ValidationFact{ + fact, _, err := validationfact.Persist(ctx, c.logger, c.metricsScope, store.GetValidationFactStore(), entity.ValidationFact{ URI: request.URI, Project: result.ProjectInfo.Name, ProjectType: result.ProjectInfo.ProjectType, diff --git a/stovepipe/core/validationfact/BUILD.bazel b/stovepipe/core/validationfact/BUILD.bazel new file mode 100644 index 000000000..cd7fe49ac --- /dev/null +++ b/stovepipe/core/validationfact/BUILD.bazel @@ -0,0 +1,30 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["validationfact.go"], + importpath = "github.com/uber/submitqueue/stovepipe/core/validationfact", + visibility = ["//visibility:public"], + deps = [ + "//platform/metrics:go_default_library", + "//stovepipe/entity:go_default_library", + "//stovepipe/extension/storage:go_default_library", + "@com_github_uber_go_tally//:go_default_library", + "@org_uber_go_zap//:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["validationfact_test.go"], + embed = [":go_default_library"], + deps = [ + "//stovepipe/entity:go_default_library", + "//stovepipe/extension/storage:go_default_library", + "//stovepipe/extension/storage/mock:go_default_library", + "@com_github_stretchr_testify//require:go_default_library", + "@com_github_uber_go_tally//:go_default_library", + "@org_uber_go_mock//gomock:go_default_library", + "@org_uber_go_zap//:go_default_library", + ], +) diff --git a/stovepipe/core/validationfact/validationfact.go b/stovepipe/core/validationfact/validationfact.go new file mode 100644 index 000000000..6f8c427b2 --- /dev/null +++ b/stovepipe/core/validationfact/validationfact.go @@ -0,0 +1,69 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package validationfact retains immutable repository and project validation facts. +package validationfact + +import ( + "context" + "errors" + "fmt" + + "github.com/uber-go/tally" + "github.com/uber/submitqueue/platform/metrics" + "github.com/uber/submitqueue/stovepipe/entity" + "github.com/uber/submitqueue/stovepipe/extension/storage" + "go.uber.org/zap" +) + +// Persist retains an immutable fact, adopting an existing fact only for the same request. +func Persist( + ctx context.Context, + logger *zap.SugaredLogger, + scope tally.Scope, + factStore storage.ValidationFactStore, + fact entity.ValidationFact, +) (entity.ValidationFact, bool, error) { + err := factStore.Create(ctx, fact) + switch { + case err == nil: + metrics.NamedCounter(scope, "record", "fact_created", 1, metrics.TagsFromContext(ctx)...) + logger.Infow("recorded validation fact", + "request_id", fact.RequestID, + "uri", fact.URI, + "project", fact.Project, + "degree", fact.Degree, + ) + return fact, true, nil + + case errors.Is(err, storage.ErrAlreadyExists): + stored, getErr := factStore.Get(ctx, fact.URI, fact.Project) + if getErr != nil { + metrics.NamedCounter(scope, "record", "storage_errors", 1, metrics.TagsFromContext(ctx)...) + return entity.ValidationFact{}, false, fmt.Errorf("failed to load the existing fact for uri %s and project %q: %w", fact.URI, fact.Project, getErr) + } + if stored.RequestID != fact.RequestID { + // Ingest deduplication forbids different requests owning the same immutable fact. + metrics.NamedCounter(scope, "record", "invariant_errors", 1, metrics.TagsFromContext(ctx)...) + return entity.ValidationFact{}, false, fmt.Errorf( + "fact for uri %s and project %q is owned by request %s, not %s", fact.URI, fact.Project, stored.RequestID, fact.RequestID) + } + metrics.NamedCounter(scope, "record", "fact_exists", 1, metrics.TagsFromContext(ctx)...) + return stored, false, nil + + default: + metrics.NamedCounter(scope, "record", "storage_errors", 1, metrics.TagsFromContext(ctx)...) + return entity.ValidationFact{}, false, fmt.Errorf("failed to create the fact for uri %s and project %q: %w", fact.URI, fact.Project, err) + } +} diff --git a/stovepipe/core/validationfact/validationfact_test.go b/stovepipe/core/validationfact/validationfact_test.go new file mode 100644 index 000000000..4ce7d20ca --- /dev/null +++ b/stovepipe/core/validationfact/validationfact_test.go @@ -0,0 +1,86 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package validationfact + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + "github.com/uber-go/tally" + "github.com/uber/submitqueue/stovepipe/entity" + "github.com/uber/submitqueue/stovepipe/extension/storage" + storagemock "github.com/uber/submitqueue/stovepipe/extension/storage/mock" + "go.uber.org/mock/gomock" + "go.uber.org/zap" +) + +func TestPersist(t *testing.T) { + for _, project := range []string{"", "project"} { + t.Run(project, func(t *testing.T) { + for _, tt := range []struct { + name string + createErr error + loadErr error + otherOwner bool + wantCreated bool + wantErr error + }{ + {name: "creates fact", wantCreated: true}, + {name: "adopts existing fact", createErr: storage.ErrAlreadyExists}, + {name: "rejects different owner", createErr: storage.ErrAlreadyExists, otherOwner: true}, + {name: "create fails", createErr: context.DeadlineExceeded, wantErr: context.DeadlineExceeded}, + {name: "read fails", createErr: storage.ErrAlreadyExists, loadErr: context.Canceled, wantErr: context.Canceled}, + } { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + store := storagemock.NewMockValidationFactStore(ctrl) + incoming := entity.ValidationFact{ + URI: "git://repository/main/commit", Project: project, + RequestID: "7", Degree: entity.DegreeBroken, CreatedAt: 20, + } + retained := incoming + retained.Degree, retained.CreatedAt = entity.DegreeGreen, 10 + if tt.otherOwner { + retained.RequestID = "6" + } + store.EXPECT().Create(gomock.Any(), incoming).Return(tt.createErr) + if tt.createErr == storage.ErrAlreadyExists { + store.EXPECT().Get(gomock.Any(), incoming.URI, project).Return(retained, tt.loadErr) + } + + fact, created, err := Persist(context.Background(), zap.NewNop().Sugar(), tally.NoopScope, store, incoming) + + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + } else if tt.otherOwner { + require.Error(t, err) + } else { + require.NoError(t, err) + if tt.wantCreated { + require.Equal(t, incoming, fact) + } else { + require.Equal(t, retained, fact) + } + } + require.Equal(t, tt.wantCreated, created) + if err != nil { + require.Empty(t, fact) + } + }) + } + }) + } +}