From a451231e8450b9bb61ace41736dc16adf0dd95f4 Mon Sep 17 00:00:00 2001 From: sergeyb Date: Sat, 14 Feb 2026 02:02:00 +0000 Subject: [PATCH] [feat] Draft showcase implementation of Request entity interacting with null Request Store --- entities/BUILD.bazel | 18 ++++ entities/request.go | 97 +++++++++++++++++++ entities/request_test.go | 112 ++++++++++++++++++++++ entities/storage/BUILD.bazel | 8 -- entities/storage/land_request.go | 5 - extensions/storage/BUILD.bazel | 4 +- extensions/storage/factory.go | 6 -- extensions/storage/mysql/BUILD.bazel | 2 +- extensions/storage/mysql/factory.go | 20 ++-- extensions/storage/mysql/request_store.go | 26 +++-- extensions/storage/request_store.go | 16 +++- extensions/storage/storage.go | 18 ++++ 12 files changed, 289 insertions(+), 43 deletions(-) create mode 100644 entities/BUILD.bazel create mode 100644 entities/request.go create mode 100644 entities/request_test.go delete mode 100644 entities/storage/BUILD.bazel delete mode 100644 entities/storage/land_request.go delete mode 100644 extensions/storage/factory.go create mode 100644 extensions/storage/storage.go diff --git a/entities/BUILD.bazel b/entities/BUILD.bazel new file mode 100644 index 00000000..583cdd26 --- /dev/null +++ b/entities/BUILD.bazel @@ -0,0 +1,18 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "entities", + srcs = ["request.go"], + importpath = "github.com/uber/submitqueue/entities", + visibility = ["//visibility:public"], +) + +go_test( + name = "entities_test", + srcs = ["request_test.go"], + embed = [":entities"], + deps = [ + "@com_github_stretchr_testify//assert", + "@com_github_stretchr_testify//require", + ], +) diff --git a/entities/request.go b/entities/request.go new file mode 100644 index 00000000..eb899464 --- /dev/null +++ b/entities/request.go @@ -0,0 +1,97 @@ +package entities + +import ( + "fmt" + "strconv" + "strings" +) + +// RequestLandStrategy defines the possible source control integration methods. +type RequestLandStrategy int + +const ( + // RequestLandStrategyDefault lets the server decide based on configuration. + RequestLandStrategyDefault RequestLandStrategy = 0 + // RequestLandStrategyRebase rebases commits onto the target branch before landing. + RequestLandStrategyRebase = 1 + // RequestLandStrategySquashRebase squashes commits into a single commit before rebase. + RequestLandStrategySquashRebase = 2 + // RequestLandStrategyMerge merges commits into the target branch by creating a separate merge commit, preserving the commit history along with hashes. + RequestLandStrategyMerge = 3 +) + +type RequestState int + +// TODO: define all states +const ( + // RequestStateUnknown is the unreachable state. It is set by default when the structure is initialized. It should never be seen in the system. + RequestStateUnknown RequestState = 0 + // RequestStateNew is the initial state of a land request. It is confirmed by the system but the processing is not started yet. + RequestStateNew RequestState = 1 + // RequestStateProcessing is the state of a land request that is being processed. + RequestStateProcessing = 2 + // RequestStateLanded is the state of a land request that has been successfully processed and landed. This is the final state. + RequestStateLanded = 3 + // RequestStateError is the state of a land request that has encountered an error. This is the final state. + RequestStateError = 4 +) + +// Change represents a set of related code changes identified by one or more IDs from a particular code change provider, like Github Pull Requests. +// The object is immutable after creation. +type Change struct { + // Source is the code change provider (e.g., "github", "gerrit", "phabricator"). + Source string + // IDs is a list of change IDs, in a format specific to the code change provider, that should be landed together. + IDs []string +} + +// Request defines a request to land (merge into target branch of the source control repository) a set of code changes. +// The object is immutable after creation. +type Request struct { + // **************** + // Immutable fields, fixed at request entity creation + // **************** + + // Queue is the name of the queue processing the land request. Queue name is defined in the configuration and should be unique within the system. + Queue string + // Seq is an autoincrementing integer identifier for the land request. It is unique within the queue. + Seq int64 + // Change is a number of code changes (such as pull requests) to land into the target branch. Target branch is defined by the queue configuration. + Change Change + // LandStrategy is the source control integration strategy to use for this land operation. If not specified, the default queue strategy is used. + LandStrategy RequestLandStrategy + + // **************** + // Following fields could be changed throughout the lifecycle of the request + // **************** + + // State is the current state of the land request. + State RequestState + // Version is the version of the object. It is used for optimistic locking. + // Versioning starts at 1 and is incremented for each change to the object. + Version int32 +} + +// GetID returns the globally unique identifier for the land request. +func (r *Request) GetID() string { + return fmt.Sprintf("%s/%d", r.Queue, r.Seq) +} + +// ParseRequestID parses the globally unique identifier for the land request and returns the queue name and sequence number. +func ParseRequestID(id string) (queue string, seq int64, err error) { + parts := strings.Split(id, "/") + if len(parts) != 2 { + return "", 0, fmt.Errorf("invalid format of the request ID: %s; expected format: /", id) + } + + seq, err = strconv.ParseInt(parts[1], 10, 64) + if err != nil { + return "", 0, fmt.Errorf("invalid sequence number in the request ID: %s; expected format: /; parsing error: %w", id, err) + } + + if seq <= 0 { + return "", 0, fmt.Errorf("invalid sequence number in the request ID: %s; expected format: /; sequence number must be greater than 0 but got %d", id, seq) + } + + return parts[0], seq, nil +} diff --git a/entities/request_test.go b/entities/request_test.go new file mode 100644 index 00000000..7627ea1d --- /dev/null +++ b/entities/request_test.go @@ -0,0 +1,112 @@ +package entities + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRequest_GetID(t *testing.T) { + tests := []struct { + name string + request Request + expected string + }{ + { + name: "standard ID", + request: Request{Queue: "my-queue", Seq: 42}, + expected: "my-queue/42", + }, + { + name: "seq 1", + request: Request{Queue: "q", Seq: 1}, + expected: "q/1", + }, + { + name: "large seq", + request: Request{Queue: "prod", Seq: 9999999}, + expected: "prod/9999999", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, tt.request.GetID()) + }) + } +} + +func TestParseRequestID(t *testing.T) { + tests := []struct { + name string + id string + wantQueue string + wantSeq int64 + expectError bool + }{ + { + name: "valid ID", + id: "my-queue/42", + wantQueue: "my-queue", + wantSeq: 42, + }, + { + name: "seq 1", + id: "q/1", + wantQueue: "q", + wantSeq: 1, + }, + { + name: "missing separator", + id: "no-separator", + expectError: true, + }, + { + name: "too many separators", + id: "a/b/c", + expectError: true, + }, + { + name: "empty string", + id: "", + expectError: true, + }, + { + name: "non-numeric seq", + id: "queue/abc", + expectError: true, + }, + { + name: "zero seq", + id: "queue/0", + expectError: true, + }, + { + name: "negative seq", + id: "queue/-1", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + queue, seq, err := ParseRequestID(tt.id) + if tt.expectError { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantQueue, queue) + assert.Equal(t, tt.wantSeq, seq) + }) + } +} + +func TestGetID_ParseRequestID_Roundtrip(t *testing.T) { + req := &Request{Queue: "test-queue", Seq: 123} + queue, seq, err := ParseRequestID(req.GetID()) + require.NoError(t, err) + assert.Equal(t, req.Queue, queue) + assert.Equal(t, req.Seq, seq) +} diff --git a/entities/storage/BUILD.bazel b/entities/storage/BUILD.bazel deleted file mode 100644 index 73a70e03..00000000 --- a/entities/storage/BUILD.bazel +++ /dev/null @@ -1,8 +0,0 @@ -load("@rules_go//go:def.bzl", "go_library") - -go_library( - name = "storage", - srcs = ["land_request.go"], - importpath = "github.com/uber/submitqueue/entities/storage", - visibility = ["//visibility:public"], -) diff --git a/entities/storage/land_request.go b/entities/storage/land_request.go deleted file mode 100644 index 25afca6b..00000000 --- a/entities/storage/land_request.go +++ /dev/null @@ -1,5 +0,0 @@ -package storage - -// LandRequest TODO: implement land request for storage -type LandRequest struct { -} diff --git a/extensions/storage/BUILD.bazel b/extensions/storage/BUILD.bazel index 675728c9..94afc868 100644 --- a/extensions/storage/BUILD.bazel +++ b/extensions/storage/BUILD.bazel @@ -3,10 +3,10 @@ load("@rules_go//go:def.bzl", "go_library") go_library( name = "storage", srcs = [ - "factory.go", + "storage.go", "request_store.go", ], importpath = "github.com/uber/submitqueue/extensions/storage", visibility = ["//visibility:public"], - deps = ["//entities/storage"], + deps = ["//entities"], ) diff --git a/extensions/storage/factory.go b/extensions/storage/factory.go deleted file mode 100644 index 450a9d90..00000000 --- a/extensions/storage/factory.go +++ /dev/null @@ -1,6 +0,0 @@ -package storage - -// Factory is an interface that defines methods for creating different stores. -type Factory interface { - RequestStore() RequestStore -} diff --git a/extensions/storage/mysql/BUILD.bazel b/extensions/storage/mysql/BUILD.bazel index c2a8e1f2..82ea8937 100644 --- a/extensions/storage/mysql/BUILD.bazel +++ b/extensions/storage/mysql/BUILD.bazel @@ -9,7 +9,7 @@ go_library( importpath = "github.com/uber/submitqueue/extensions/storage/mysql", visibility = ["//visibility:public"], deps = [ - "//entities/storage", + "//entities", "//extensions/storage", ], ) diff --git a/extensions/storage/mysql/factory.go b/extensions/storage/mysql/factory.go index 1e7c47df..311ef45e 100644 --- a/extensions/storage/mysql/factory.go +++ b/extensions/storage/mysql/factory.go @@ -2,22 +2,22 @@ package mysql import "github.com/uber/submitqueue/extensions/storage" -type factory struct { - requestStore storage.RequestStore -} - -type FactoryParams struct { - requestStore storage.RequestStore +// MySQLParameters defines the parameters for the MySQL storage factory. +type MySQLParameters struct { } // NewFactory creates a new MySQL storage factory -func NewFactory(p FactoryParams) (storage.Factory, error) { +func NewFactory(p MySQLParameters) (storage.StoreFactory, error) { return &factory{ - requestStore: p.requestStore, + requestStore: NewRequestStore(), }, nil } -// RequestStore returns the MySQL-backed RequestStore -func (f *factory) RequestStore() storage.RequestStore { +type factory struct { + requestStore storage.RequestStore +} + +// GetRequestStore returns the MySQL-backed RequestStore +func (f *factory) GetRequestStore() storage.RequestStore { return f.requestStore } diff --git a/extensions/storage/mysql/request_store.go b/extensions/storage/mysql/request_store.go index 7aab1edb..b517669c 100644 --- a/extensions/storage/mysql/request_store.go +++ b/extensions/storage/mysql/request_store.go @@ -2,23 +2,35 @@ package mysql import ( "context" + "errors" - entities "github.com/uber/submitqueue/entities/storage" + "github.com/uber/submitqueue/entities" "github.com/uber/submitqueue/extensions/storage" ) type requestStore struct { } -type RequestParam struct { -} - +// NewRequestStore creates a new MySQL-backed RequestStore. func NewRequestStore() storage.RequestStore { return &requestStore{} } -// Get retrieves a land request by ID -func (r *requestStore) Get(ctx context.Context, id string) (*entities.LandRequest, error) { +// Get retrieves a land request by ID. Returns ErrNotFound if the request is not found. +func (r *requestStore) Get(ctx context.Context, id string) (entities.Request, error) { // TODO: implement GET operation - panic("not implemented") + return entities.Request{}, errors.New("not implemented") +} + +// Create creates a new land request. Returns the created request object with generated sequence number. +func (r *requestStore) Create(ctx context.Context, queue string, change entities.Change, strategy entities.RequestLandStrategy, state entities.RequestState) (entities.Request, error) { + // TODO: implement CREATE operation + return entities.Request{}, errors.New("not implemented") +} + +// UpdateState updates the state of a land request if the current version matches the expected version. If versions do not match, returns ErrVersionMismatch. +// The implementation should increment the version by 1 atomically with the state update. +func (r *requestStore) UpdateState(ctx context.Context, id string, version int32, newState entities.RequestState) error { + // TODO: implement UPDATE STATE operation + return errors.New("not implemented") } diff --git a/extensions/storage/request_store.go b/extensions/storage/request_store.go index 70da603a..06e8dab8 100644 --- a/extensions/storage/request_store.go +++ b/extensions/storage/request_store.go @@ -3,11 +3,19 @@ package storage import ( "context" - "github.com/uber/submitqueue/entities/storage" + "github.com/uber/submitqueue/entities" ) -// RequestStore is an interface that defines methods for managing storage requests. +// RequestStore is an interface that defines methods for managing land requests in the database. type RequestStore interface { - // Get retrieves a land request by ID - Get(ctx context.Context, id string) (*storage.LandRequest, error) + // Get retrieves a land request by ID (queue/seq). Returns ErrNotFound if the request is not found. + Get(ctx context.Context, id string) (entities.Request, error) + + // Create creates a new land request. Returns the created request object with generated sequence number. + // The implementation must ensure that the sequence number is unique within the queue. + Create(ctx context.Context, queue string, change entities.Change, strategy entities.RequestLandStrategy, state entities.RequestState) (entities.Request, error) + + // UpdateState updates the state of a land request if the current version matches the expected version. If versions do not match, returns ErrVersionMismatch. + // The implementation should increment the version by 1 atomically with the state update. + UpdateState(ctx context.Context, id string, version int32, newState entities.RequestState) error } diff --git a/extensions/storage/storage.go b/extensions/storage/storage.go new file mode 100644 index 00000000..09d68e4a --- /dev/null +++ b/extensions/storage/storage.go @@ -0,0 +1,18 @@ +package storage + +import "errors" + +// ErrNotFound is returned by storage implementations when the requested record is not found in the database. +var ErrNotFound = errors.New("record not found") + +// ErrVersionMismatch is returned by storage implementations when the expected entity version does not match the current version of the object. +// This is used to implement an optimistic locking mechanism, allowing multiple clients to update the same entity concurrently +// and either retry or implement idempotent operations. +var ErrVersionMismatch = errors.New("version mismatch") + +// StoreFactory is an interface that defines methods for creating different stores.. +// Each store is responsible for performing atomic storage operations for a specific entity type. +type StoreFactory interface { + // GetRequestStore creates a new RequestStore instance. + GetRequestStore() RequestStore +}