[gold] Delete fs_clstore
Bug: skia:10582
Change-Id: I4714cbac8c7f64fd8c4cc2f865a3434a913fb6df
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/373758
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
diff --git a/golden/cmd/gold_frontend/gold_frontend.go b/golden/cmd/gold_frontend/gold_frontend.go
index 6af7c6c..9485e92 100644
--- a/golden/cmd/gold_frontend/gold_frontend.go
+++ b/golden/cmd/gold_frontend/gold_frontend.go
@@ -45,8 +45,6 @@
"go.skia.org/infra/go/vcsinfo/bt_vcs"
"go.skia.org/infra/golden/go/baseline/simple_baseliner"
"go.skia.org/infra/golden/go/clstore"
- "go.skia.org/infra/golden/go/clstore/dualclstore"
- "go.skia.org/infra/golden/go/clstore/fs_clstore"
"go.skia.org/infra/golden/go/clstore/sqlclstore"
"go.skia.org/infra/golden/go/code_review"
"go.skia.org/infra/golden/go/code_review/commenter"
@@ -244,7 +242,7 @@
tjs := mustMakeTryJobStore(fsClient, sqlDB)
- reviewSystems := mustInitializeReviewSystems(fsc, fsClient, client, sqlDB)
+ reviewSystems := mustInitializeReviewSystems(fsc, client, sqlDB)
tileSource := mustMakeTileSource(ctx, fsc, expStore, ignoreStore, traceStore, vcs, publiclyViewableParams, reviewSystems)
@@ -506,7 +504,7 @@
// mustInitializeReviewSystems validates and instantiates one clstore.ReviewSystem for each CRS
// specified via the JSON configuration files.
-func mustInitializeReviewSystems(fsc *frontendServerConfig, fc *firestore.Client, hc *http.Client, sqlDB *pgxpool.Pool) []clstore.ReviewSystem {
+func mustInitializeReviewSystems(fsc *frontendServerConfig, hc *http.Client, sqlDB *pgxpool.Pool) []clstore.ReviewSystem {
rs := make([]clstore.ReviewSystem, 0, len(fsc.CodeReviewSystems))
for _, cfg := range fsc.CodeReviewSystems {
var crs code_review.Client
@@ -539,12 +537,11 @@
sklog.Fatalf("CRS flavor %s not supported.", cfg.Flavor)
return nil
}
- fireCS := fs_clstore.New(fc, cfg.ID)
sqlCS := sqlclstore.New(sqlDB, cfg.ID)
rs = append(rs, clstore.ReviewSystem{
ID: cfg.ID,
Client: crs,
- Store: dualclstore.New(sqlCS, fireCS),
+ Store: sqlCS,
URLTemplate: cfg.URLTemplate,
})
}
diff --git a/golden/go/clstore/dualclstore/BUILD.bazel b/golden/go/clstore/dualclstore/BUILD.bazel
deleted file mode 100644
index afd185b..0000000
--- a/golden/go/clstore/dualclstore/BUILD.bazel
+++ /dev/null
@@ -1,14 +0,0 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
-
-go_library(
- name = "dualclstore",
- srcs = ["dualclstore.go"],
- importpath = "go.skia.org/infra/golden/go/clstore/dualclstore",
- visibility = ["//visibility:public"],
- deps = [
- "//go/skerr",
- "//golden/go/clstore",
- "//golden/go/code_review",
- "@org_golang_x_sync//errgroup",
- ],
-)
diff --git a/golden/go/clstore/dualclstore/dualclstore.go b/golden/go/clstore/dualclstore/dualclstore.go
deleted file mode 100644
index 2e1c194..0000000
--- a/golden/go/clstore/dualclstore/dualclstore.go
+++ /dev/null
@@ -1,77 +0,0 @@
-// Package dualclstore contains an implementation of clstore.Store that reads and writes to
-// a primary implementation and writes to a secondary implementation. It is designed for migrating
-// between two implementations.
-package dualclstore
-
-import (
- "context"
-
- "golang.org/x/sync/errgroup"
-
- "go.skia.org/infra/go/skerr"
- "go.skia.org/infra/golden/go/clstore"
- "go.skia.org/infra/golden/go/code_review"
-)
-
-type StoreImpl struct {
- primary clstore.Store
- secondary clstore.Store
-}
-
-// New returns an implementation of clstore.Store that writes to both provided clstores but only
-// reads from the primary.
-func New(primary, secondary clstore.Store) clstore.Store {
- return &StoreImpl{
- primary: primary,
- secondary: secondary,
- }
-}
-
-// GetChangelist implements clstore.Store by returning data from the primary store.
-func (s *StoreImpl) GetChangelist(ctx context.Context, id string) (code_review.Changelist, error) {
- return s.primary.GetChangelist(ctx, id)
-}
-
-// GetPatchset implements clstore.Store by returning data from the primary store.
-func (s *StoreImpl) GetPatchset(ctx context.Context, clID, psID string) (code_review.Patchset, error) {
- return s.primary.GetPatchset(ctx, clID, psID)
-}
-
-// GetPatchsetByOrder implements clstore.Store by returning data from the primary store.
-func (s *StoreImpl) GetPatchsetByOrder(ctx context.Context, clID string, psOrder int) (code_review.Patchset, error) {
- return s.primary.GetPatchsetByOrder(ctx, clID, psOrder)
-}
-
-// GetChangelists implements clstore.Store by returning data from the primary store.
-func (s *StoreImpl) GetChangelists(ctx context.Context, opts clstore.SearchOptions) ([]code_review.Changelist, int, error) {
- return s.primary.GetChangelists(ctx, opts)
-}
-
-// GetPatchsets implements clstore.Store by returning data from the primary store.
-func (s *StoreImpl) GetPatchsets(ctx context.Context, clID string) ([]code_review.Patchset, error) {
- return s.primary.GetPatchsets(ctx, clID)
-}
-
-// PutChangelist implements clstore.Store by writing data to the primary and secondary in parallel.
-func (s *StoreImpl) PutChangelist(ctx context.Context, cl code_review.Changelist) error {
- eg, ctx := errgroup.WithContext(ctx)
- eg.Go(func() error {
- return skerr.Wrapf(s.primary.PutChangelist(ctx, cl), "primary PutChangelist failed")
- })
- eg.Go(func() error {
- return skerr.Wrapf(s.secondary.PutChangelist(ctx, cl), "secondary PutChangelist failed")
- })
- return skerr.Wrap(eg.Wait())
-}
-
-// PutPatchset implements clstore.Store by writing data to the primary and secondary in parallel.
-func (s *StoreImpl) PutPatchset(ctx context.Context, ps code_review.Patchset) error {
- eg, ctx := errgroup.WithContext(ctx)
- eg.Go(func() error {
- return skerr.Wrapf(s.primary.PutPatchset(ctx, ps), "primary PutPatchset failed")
- })
- eg.Go(func() error {
- return skerr.Wrapf(s.secondary.PutPatchset(ctx, ps), "secondary PutPatchset failed")
- })
- return skerr.Wrap(eg.Wait())
-}
diff --git a/golden/go/clstore/fs_clstore/BUILD.bazel b/golden/go/clstore/fs_clstore/BUILD.bazel
deleted file mode 100644
index 8c7639c..0000000
--- a/golden/go/clstore/fs_clstore/BUILD.bazel
+++ /dev/null
@@ -1,33 +0,0 @@
-load("//bazel/go:go_test.bzl", "go_test")
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
-
-go_library(
- name = "fs_clstore",
- srcs = ["fs_clstore.go"],
- importpath = "go.skia.org/infra/golden/go/clstore/fs_clstore",
- visibility = ["//visibility:public"],
- deps = [
- "//go/firestore",
- "//go/metrics2",
- "//go/skerr",
- "//golden/go/clstore",
- "//golden/go/code_review",
- "@com_google_cloud_go_firestore//:firestore",
- "@org_golang_google_grpc//codes",
- "@org_golang_google_grpc//status",
- ],
-)
-
-go_test(
- name = "fs_clstore_test",
- srcs = ["fs_clstore_test.go"],
- embed = [":fs_clstore"],
- deps = [
- "//go/firestore",
- "//go/testutils/unittest",
- "//golden/go/clstore",
- "//golden/go/code_review",
- "@com_github_stretchr_testify//assert",
- "@com_github_stretchr_testify//require",
- ],
-)
diff --git a/golden/go/clstore/fs_clstore/FIRESTORE.md b/golden/go/clstore/fs_clstore/FIRESTORE.md
deleted file mode 100644
index d3aa3d2..0000000
--- a/golden/go/clstore/fs_clstore/FIRESTORE.md
+++ /dev/null
@@ -1,59 +0,0 @@
-Storing ChangeLists and PatchSets in Firestore
-==============================================
-
-We need to keep track of the ChangeLists and PatchSets that have been associated
-with TryJob data that we have ingested.
-
-See https://docs.google.com/document/d/1d0tOhgx51QOGiSXqTxiwNSlgm1pYHTUSBK3agysX6Iw/edit
-for more context.
-
-Schema
-------
-
-We should have two Firestore Collections (i.e. tables), one for ChangeList
-and a subcollection for PatchSet.
-
- ChangeList
- ID string # SystemID + System
- SystemID string # The id of the CL in, for example, Gerrit
- System string # "gerrit", "github", etc
- Status int # corresponds to code_review.CLStatus
- Owner string # email address
- Updated time.Time
- Subject string
- []PatchSet
- SystemID string # The id of the PS in, for example, Gerrit
- System string # "gerrit", "github", etc
- ChangeListID string # SystemID from ChangeList
- Order int # number of this PS
- GitHash string
- HasUntriagedDigests bool
- CommentedOnCL bool
-
-Indexing
---------
-We need the following composite indexes:
-
-Collection ID | Fields
-------------------------------------------------------------------
-clstore_changelist | status: ASC updated: DESC
-
-
-We should mark ChangeList.Subject as no-index, to save some index space.
-<https://cloud.google.com/firestore/docs/query-data/indexing#exemptions>
-
-Usage
------
-We'll be querying:
- - ChangeLists by SystemID, System
- - PatchSets belonging to a ChangeList ordered by Order.
-
-With this hierarchical setup, we should avoid conflicts
-between multiple systems.
-
-Growth Opportunities
--------------------
-
-We could open up the searching to include "get only Open CLs" or get CLs by
-a given (the logged in?) user. This should just entail adding some more
-indices and a few new functions to the API.
\ No newline at end of file
diff --git a/golden/go/clstore/fs_clstore/fs_clstore.go b/golden/go/clstore/fs_clstore/fs_clstore.go
deleted file mode 100644
index 2e3ef84..0000000
--- a/golden/go/clstore/fs_clstore/fs_clstore.go
+++ /dev/null
@@ -1,301 +0,0 @@
-// Package fs_clstore implements the clstore.Store interface with
-// a FireStore backend.
-package fs_clstore
-
-import (
- "context"
- "fmt"
- "time"
-
- "cloud.google.com/go/firestore"
- ifirestore "go.skia.org/infra/go/firestore"
- "go.skia.org/infra/go/metrics2"
- "go.skia.org/infra/go/skerr"
- "go.skia.org/infra/golden/go/clstore"
- "go.skia.org/infra/golden/go/code_review"
- "google.golang.org/grpc/codes"
- "google.golang.org/grpc/status"
-)
-
-const (
- // These are the collections in Firestore.
- changelistCollection = "clstore_changelist"
- patchsetCollection = "clstore_patchset"
-
- // These are the fields we query by
- orderField = "order"
- statusField = "status"
- updatedField = "updated"
-
- maxReadAttempts = 5
- maxWriteAttempts = 5
- maxOperationTime = time.Minute
-)
-
-// StoreImpl is the Firestore based implementation of clstore.
-type StoreImpl struct {
- client *ifirestore.Client
- crsID string
-}
-
-// New returns a new StoreImpl. The crsID should be distinct enough to separate an internal CRS
-// from an external one, if needed (e.g. "gerrit" vs "gerrit-internal")
-func New(client *ifirestore.Client, crsID string) *StoreImpl {
- return &StoreImpl{
- client: client,
- crsID: crsID,
- }
-}
-
-// changelistEntry represents how a Changelist is stored in Firestore.
-type changelistEntry struct {
- SystemID string `firestore:"systemid"`
- System string `firestore:"system"`
- Owner string `firestore:"owner"`
- Status code_review.CLStatus `firestore:"status"`
- Subject string `firestore:"subject"`
- Updated time.Time `firestore:"updated"`
-}
-
-// patchsetEntry represents how a Patchset is stored in Firestore.
-type patchsetEntry struct {
- SystemID string `firestore:"systemid"`
- System string `firestore:"system"`
- ChangelistID string `firestore:"changelistid"`
- Order int `firestore:"order"`
- GitHash string `firestore:"githash"`
- CommentedOnCL bool `firestore:"did_comment"`
- LastCheckedIfCommentNecessary time.Time `firestore:"last_checked_about_comment"`
-}
-
-// toPatchset converts the Firestore representation of a Patchset to a code_review.Patchset
-func (p patchsetEntry) toPatchset() code_review.Patchset {
- return code_review.Patchset{
- SystemID: p.SystemID,
- ChangelistID: p.ChangelistID,
- Order: p.Order,
- GitHash: p.GitHash,
- LastCheckedIfCommentNecessary: p.LastCheckedIfCommentNecessary,
- CommentedOnCL: p.CommentedOnCL,
- }
-}
-
-// GetChangelist implements the clstore.Store interface.
-func (s *StoreImpl) GetChangelist(ctx context.Context, id string) (code_review.Changelist, error) {
- defer metrics2.FuncTimer().Stop()
- fID := s.changelistFirestoreID(id)
- doc, err := s.client.Collection(changelistCollection).Doc(fID).Get(ctx)
- if err != nil {
- if status.Code(err) == codes.NotFound {
- return code_review.Changelist{}, clstore.ErrNotFound
- }
- return code_review.Changelist{}, skerr.Wrapf(err, "retrieving CL %s from Firestore", fID)
- }
- if doc == nil {
- return code_review.Changelist{}, clstore.ErrNotFound
- }
-
- cle := changelistEntry{}
- if err := doc.DataTo(&cle); err != nil {
- id := doc.Ref.ID
- return code_review.Changelist{}, skerr.Wrapf(err, "corrupt data in Firestore, could not unmarshal %s changelist with id %s", s.crsID, id)
- }
- cl := code_review.Changelist{
- SystemID: cle.SystemID,
- Owner: cle.Owner,
- Status: cle.Status,
- Subject: cle.Subject,
- Updated: cle.Updated,
- }
-
- return cl, nil
-}
-
-// changelistFirestoreID returns the id for a given CL in a given CRS - this allows us to
-// look up a document by id w/o having to perform a query.
-func (s *StoreImpl) changelistFirestoreID(clID string) string {
- return clID + "_" + s.crsID
-}
-
-// GetChangelists implements the clstore.Store interface.
-func (s *StoreImpl) GetChangelists(ctx context.Context, opts clstore.SearchOptions) ([]code_review.Changelist, int, error) {
- defer metrics2.FuncTimer().Stop()
- if opts.Limit <= 0 {
- return nil, 0, skerr.Fmt("must supply a limit")
- }
- q := s.client.Collection(changelistCollection).OrderBy(updatedField, firestore.Desc)
- if !opts.After.IsZero() {
- q = q.Where(updatedField, ">=", opts.After)
- }
- if opts.OpenCLsOnly {
- q = q.Where(statusField, "==", code_review.Open)
- }
- q = q.Limit(opts.Limit).Offset(opts.StartIdx)
-
- var xcl []code_review.Changelist
-
- r := fmt.Sprintf("[%d:%d]", opts.StartIdx, opts.StartIdx+opts.Limit)
- err := s.client.IterDocs(ctx, "GetChangelists", r, q, maxReadAttempts, maxOperationTime, func(doc *firestore.DocumentSnapshot) error {
- if doc == nil {
- return nil
- }
- entry := changelistEntry{}
- if err := doc.DataTo(&entry); err != nil {
- id := doc.Ref.ID
- return skerr.Wrapf(err, "corrupt data in Firestore, could not unmarshal entry with id %s", id)
- }
- xcl = append(xcl, code_review.Changelist{
- SystemID: entry.SystemID,
- Updated: entry.Updated,
- Subject: entry.Subject,
- Status: entry.Status,
- Owner: entry.Owner,
- })
- return nil
- })
- if err != nil {
- return nil, -1, skerr.Wrapf(err, "fetching cls in range %s", r)
- }
- n := len(xcl)
- if n == opts.Limit && n != 0 {
- // We don't know how many there are and it might be too slow to count, so just give
- // the "many" response.
- n = clstore.CountMany
- } else {
- // We know exactly either 1) how many there are (if n > 0) or 2) an upper bound on how many
- // there are (if n == 0)
- n += opts.StartIdx
- }
-
- return xcl, n, nil
-}
-
-// GetPatchset implements the clstore.Store interface.
-func (s *StoreImpl) GetPatchset(ctx context.Context, clID, psID string) (code_review.Patchset, error) {
- defer metrics2.FuncTimer().Stop()
- fID := s.changelistFirestoreID(clID)
- doc, err := s.client.Collection(changelistCollection).Doc(fID).
- Collection(patchsetCollection).Doc(psID).Get(ctx)
- if err != nil {
- if status.Code(err) == codes.NotFound {
- return code_review.Patchset{}, clstore.ErrNotFound
- }
- return code_review.Patchset{}, skerr.Wrapf(err, "retrieving PS %s from Firestore", fID)
- }
- if doc == nil {
- return code_review.Patchset{}, clstore.ErrNotFound
- }
-
- pse := patchsetEntry{}
- if err := doc.DataTo(&pse); err != nil {
- id := doc.Ref.ID
- return code_review.Patchset{}, skerr.Wrapf(err, "corrupt data in Firestore, could not unmarshal %s patchset with id %s", s.crsID, id)
- }
- return pse.toPatchset(), nil
-}
-
-// GetPatchsetByOrder implements the clstore.Store interface.
-func (s *StoreImpl) GetPatchsetByOrder(ctx context.Context, clID string, psOrder int) (code_review.Patchset, error) {
- defer metrics2.FuncTimer().Stop()
- fID := s.changelistFirestoreID(clID)
- q := s.client.Collection(changelistCollection).Doc(fID).
- Collection(patchsetCollection).Where(orderField, "==", psOrder)
-
- ps := code_review.Patchset{}
- found := false
- msg := fmt.Sprintf("%s:%d", clID, psOrder)
- err := s.client.IterDocs(ctx, "GetPatchsetByOrder", msg, q, maxReadAttempts, maxOperationTime, func(doc *firestore.DocumentSnapshot) error {
- if doc == nil || found {
- return nil
- }
- entry := patchsetEntry{}
- if err := doc.DataTo(&entry); err != nil {
- id := doc.Ref.ID
- return skerr.Wrapf(err, "corrupt data in Firestore, could not unmarshal patchsetEntry with id %s", id)
- }
- ps = entry.toPatchset()
- found = true
- return nil
- })
- if err != nil {
- return code_review.Patchset{}, skerr.Wrapf(err, "fetching patchsets for cl %s", clID)
- }
- if !found {
- return code_review.Patchset{}, clstore.ErrNotFound
- }
-
- return ps, nil
-}
-
-// GetPatchsets implements the clstore.Store interface.
-func (s *StoreImpl) GetPatchsets(ctx context.Context, clID string) ([]code_review.Patchset, error) {
- defer metrics2.FuncTimer().Stop()
- fID := s.changelistFirestoreID(clID)
- q := s.client.Collection(changelistCollection).Doc(fID).
- Collection(patchsetCollection).OrderBy(orderField, firestore.Asc)
-
- var xps []code_review.Patchset
-
- err := s.client.IterDocs(ctx, "GetPatchsets", clID, q, maxReadAttempts, maxOperationTime, func(doc *firestore.DocumentSnapshot) error {
- if doc == nil {
- return nil
- }
- entry := patchsetEntry{}
- if err := doc.DataTo(&entry); err != nil {
- id := doc.Ref.ID
- return skerr.Wrapf(err, "corrupt data in Firestore, could not unmarshal entry with id %s", id)
- }
- xps = append(xps, entry.toPatchset())
- return nil
- })
- if err != nil {
- return nil, skerr.Wrapf(err, "fetching patchsets for cl %s", clID)
- }
-
- return xps, nil
-}
-
-// PutChangelist implements the clstore.Store interface.
-func (s *StoreImpl) PutChangelist(ctx context.Context, cl code_review.Changelist) error {
- defer metrics2.FuncTimer().Stop()
- fID := s.changelistFirestoreID(cl.SystemID)
- cd := s.client.Collection(changelistCollection).Doc(fID)
- record := changelistEntry{
- SystemID: cl.SystemID,
- System: s.crsID,
- Owner: cl.Owner,
- Status: cl.Status,
- Subject: cl.Subject,
- Updated: cl.Updated,
- }
- _, err := s.client.Set(ctx, cd, record, maxWriteAttempts, maxOperationTime)
- if err != nil {
- return skerr.Wrapf(err, "could not write CL %v to clstore", cl)
- }
- return nil
-}
-
-// PutPatchset implements the clstore.Store interface.
-func (s *StoreImpl) PutPatchset(ctx context.Context, ps code_review.Patchset) error {
- defer metrics2.FuncTimer().Stop()
- fID := s.changelistFirestoreID(ps.ChangelistID)
- pd := s.client.Collection(changelistCollection).Doc(fID).
- Collection(patchsetCollection).Doc(ps.SystemID)
- record := patchsetEntry{
- SystemID: ps.SystemID,
- System: s.crsID,
- ChangelistID: ps.ChangelistID,
- Order: ps.Order,
- GitHash: ps.GitHash,
- LastCheckedIfCommentNecessary: ps.LastCheckedIfCommentNecessary,
- CommentedOnCL: ps.CommentedOnCL,
- }
- _, err := s.client.Set(ctx, pd, record, maxWriteAttempts, maxOperationTime)
- if err != nil {
- return skerr.Wrapf(err, "could not write PS %v to clstore", ps)
- }
- return nil
-}
-
-// Make sure StoreImpl fulfills the clstore.Store interface.
-var _ clstore.Store = (*StoreImpl)(nil)
diff --git a/golden/go/clstore/fs_clstore/fs_clstore_test.go b/golden/go/clstore/fs_clstore/fs_clstore_test.go
deleted file mode 100644
index f884e5c..0000000
--- a/golden/go/clstore/fs_clstore/fs_clstore_test.go
+++ /dev/null
@@ -1,446 +0,0 @@
-package fs_clstore
-
-import (
- "context"
- "strconv"
- "testing"
- "time"
-
- "github.com/stretchr/testify/assert"
- "github.com/stretchr/testify/require"
- "go.skia.org/infra/go/firestore"
- "go.skia.org/infra/go/testutils/unittest"
- "go.skia.org/infra/golden/go/clstore"
- "go.skia.org/infra/golden/go/code_review"
-)
-
-func TestPutGetChangelist(t *testing.T) {
- unittest.LargeTest(t)
- c, cleanup := firestore.NewClientForTesting(context.Background(), t)
- defer cleanup()
-
- f := New(c, "gerrit")
- ctx := context.Background()
-
- expectedID := "987654"
-
- // Should not exist initially
- _, err := f.GetChangelist(ctx, expectedID)
- require.Error(t, err)
- require.Equal(t, clstore.ErrNotFound, err)
-
- cl := code_review.Changelist{
- SystemID: expectedID,
- Owner: "test@example.com",
- Status: code_review.Abandoned,
- Subject: "some code",
- Updated: time.Date(2019, time.August, 13, 12, 11, 10, 0, time.UTC),
- }
-
- err = f.PutChangelist(ctx, cl)
- require.NoError(t, err)
-
- actual, err := f.GetChangelist(ctx, expectedID)
- require.NoError(t, err)
- require.Equal(t, cl, actual)
-}
-
-func TestPutGetPatchset(t *testing.T) {
- unittest.LargeTest(t)
- c, cleanup := firestore.NewClientForTesting(context.Background(), t)
- defer cleanup()
-
- f := New(c, "gerrit")
- ctx := context.Background()
-
- expectedCLID := "987654"
- expectedPSID := "abcdef012345"
-
- // Should not exist initially
- _, err := f.GetPatchset(ctx, expectedCLID, expectedPSID)
- require.Error(t, err)
- require.Equal(t, clstore.ErrNotFound, err)
-
- ps := code_review.Patchset{
- SystemID: expectedPSID,
- ChangelistID: expectedCLID,
- Order: 3,
- GitHash: "fedcba98765443321",
- LastCheckedIfCommentNecessary: time.Date(2020, time.May, 1, 2, 3, 4, 0, time.UTC),
- CommentedOnCL: true,
- }
-
- err = f.PutPatchset(ctx, ps)
- require.NoError(t, err)
-
- actual, err := f.GetPatchset(ctx, expectedCLID, expectedPSID)
- require.NoError(t, err)
- require.Equal(t, ps, actual)
-}
-
-func TestPutGetPatchsetByOrder(t *testing.T) {
- unittest.LargeTest(t)
- c, cleanup := firestore.NewClientForTesting(context.Background(), t)
- defer cleanup()
-
- f := New(c, "gerrit")
- ctx := context.Background()
-
- expectedCLID := "987654"
- expectedPSOrder := 3
- otherPSOrder := 117
-
- // Should not exist initially
- _, err := f.GetPatchsetByOrder(ctx, expectedCLID, expectedPSOrder)
- require.Error(t, err)
- require.Equal(t, clstore.ErrNotFound, err)
-
- ps := code_review.Patchset{
- SystemID: "abcdef012345",
- ChangelistID: expectedCLID,
- Order: expectedPSOrder,
- GitHash: "fedcba98765443321",
- }
-
- err = f.PutPatchset(ctx, ps)
- require.NoError(t, err)
-
- ps2 := code_review.Patchset{
- SystemID: "zyx9876",
- ChangelistID: expectedCLID,
- Order: otherPSOrder,
- GitHash: "notthisone",
- }
-
- err = f.PutPatchset(ctx, ps2)
- require.NoError(t, err)
-
- actual, err := f.GetPatchsetByOrder(ctx, expectedCLID, expectedPSOrder)
- require.NoError(t, err)
- require.Equal(t, ps, actual)
-
- actual, err = f.GetPatchsetByOrder(ctx, expectedCLID, otherPSOrder)
- require.NoError(t, err)
- require.Equal(t, ps2, actual)
-}
-
-// TestDifferentSystems makes sure that two systems in the same
-// firestore namespace don't overlap.
-func TestDifferentSystems(t *testing.T) {
- unittest.LargeTest(t)
- c, cleanup := firestore.NewClientForTesting(context.Background(), t)
- defer cleanup()
-
- gerrit := New(c, "gerrit")
- github := New(c, "github")
- ctx := context.Background()
-
- expectedCLID := "987654"
-
- gerritCL := code_review.Changelist{
- SystemID: expectedCLID,
- Owner: "test@example.com",
- Status: code_review.Abandoned,
- Subject: "some code on gerrit",
- Updated: time.Date(2019, time.August, 13, 12, 11, 10, 0, time.UTC),
- }
-
- githubCL := code_review.Changelist{
- SystemID: expectedCLID,
- Owner: "test2@example.com",
- Status: code_review.Open,
- Subject: "some code on github",
- Updated: time.Date(2019, time.August, 15, 12, 11, 10, 0, time.UTC),
- }
-
- // Both systems have a CL with the same ID
- err := gerrit.PutChangelist(ctx, gerritCL)
- require.NoError(t, err)
- err = github.PutChangelist(ctx, githubCL)
- require.NoError(t, err)
-
- actualGerrit, err := gerrit.GetChangelist(ctx, expectedCLID)
- require.NoError(t, err)
- actualGithub, err := github.GetChangelist(ctx, expectedCLID)
- require.NoError(t, err)
-
- require.NotEqual(t, actualGerrit, actualGithub)
- require.Equal(t, gerritCL, actualGerrit)
- require.Equal(t, githubCL, actualGithub)
-}
-
-// TestGetPatchsets stores several patchsets and then makes sure we can fetch the ones
-// for a specific CL and they arrive sorted by Order, even if the Patchsets are sparse.
-func TestGetPatchsets(t *testing.T) {
- unittest.LargeTest(t)
- c, cleanup := firestore.NewClientForTesting(context.Background(), t)
- defer cleanup()
-
- f := New(c, "gerrit")
- ctx := context.Background()
-
- expectedID := "987654"
- sparseID := "sparse"
- // None should exist initially
- xps, err := f.GetPatchsets(ctx, expectedID)
- require.NoError(t, err)
- require.Empty(t, xps)
-
- // Create the Changelist, but don't add any Patchsets yet.
- err = f.PutChangelist(ctx, code_review.Changelist{SystemID: expectedID})
- require.NoError(t, err)
-
- // Still no Patchsets
- xps, err = f.GetPatchsets(ctx, expectedID)
- require.NoError(t, err)
- require.Empty(t, xps)
-
- for i := 0; i < 3; i++ {
- ps := code_review.Patchset{
- SystemID: "other_id" + strconv.Itoa(i),
- ChangelistID: "not this CL",
- GitHash: "nope",
- Order: i + 1,
- }
- require.NoError(t, f.PutPatchset(ctx, ps))
- }
- // use random ids to make sure the we are truly sorting on ids
- randIDs := []string{"zkdf", "bkand", "d-sd9f9s3n", "csdfksdfn1"}
- // put them in backwards to make sure they get resorted by order
- for i := 4; i > 0; i-- {
- ps := code_review.Patchset{
- // use an ID
- SystemID: randIDs[i-1],
- ChangelistID: expectedID,
- GitHash: "whatever",
- Order: i,
- }
- require.NoError(t, f.PutPatchset(ctx, ps))
- }
-
- for i := 0; i < 9; i += 3 {
- ps := code_review.Patchset{
- SystemID: "other_other_id" + strconv.Itoa(20-i),
- ChangelistID: sparseID,
- GitHash: "sparse",
- Order: i + 1,
- }
- require.NoError(t, f.PutPatchset(ctx, ps))
- }
-
- // Check that sequential orders work
- xps, err = f.GetPatchsets(ctx, expectedID)
- require.NoError(t, err)
- require.Len(t, xps, 4)
- // Make sure they are in order
- for i, ps := range xps {
- require.Equal(t, i+1, ps.Order)
- require.Equal(t, expectedID, ps.ChangelistID)
- require.Equal(t, "whatever", ps.GitHash)
- }
-
- // Check that sparse patchsets work.
- xps, err = f.GetPatchsets(ctx, sparseID)
- require.NoError(t, err)
- require.Len(t, xps, 3)
- // Make sure they are in order
- for i, ps := range xps {
- require.Equal(t, i*3+1, ps.Order)
- require.Equal(t, sparseID, ps.ChangelistID)
- require.Equal(t, "sparse", ps.GitHash)
- }
-}
-
-func TestGetChangelists(t *testing.T) {
- unittest.LargeTest(t)
- c, cleanup := firestore.NewClientForTesting(context.Background(), t)
- defer cleanup()
-
- f := New(c, "gerrit")
- ctx := context.Background()
-
- // None to start
- cls, total, err := f.GetChangelists(ctx, clstore.SearchOptions{
- StartIdx: 0,
- Limit: 50,
- })
- require.NoError(t, err)
- require.Len(t, cls, 0)
- require.Equal(t, 0, total)
-
- for i := 0; i < 40; i += 2 {
- cl := code_review.Changelist{
- SystemID: "cl" + strconv.Itoa(i),
- Owner: "test@example.com",
- Status: code_review.Open,
- Subject: "blarg",
- Updated: time.Date(2019, time.August, 31, 14, i, i, 0, time.UTC),
- }
- require.NoError(t, f.PutChangelist(ctx, cl))
- }
-
- // Put in a few other ones:
- for i := 1; i < 10; i += 2 {
- cl := code_review.Changelist{
- SystemID: "cl" + strconv.Itoa(i),
- Owner: "test@example.com",
- Status: code_review.Abandoned,
- Subject: "blarg",
- Updated: time.Date(2019, time.September, 1, 4, i, i, 0, time.UTC),
- }
- require.NoError(t, f.PutChangelist(ctx, cl))
- }
-
- for i := 31; i < 40; i += 2 {
- cl := code_review.Changelist{
- SystemID: "cl" + strconv.Itoa(i),
- Owner: "test@example.com",
- Status: code_review.Landed,
- Subject: "blarg",
- Updated: time.Date(2019, time.September, 1, 2, i, i, 0, time.UTC),
- }
- require.NoError(t, f.PutChangelist(ctx, cl))
- }
-
- // Get all of them
- cls, total, err = f.GetChangelists(ctx, clstore.SearchOptions{
- StartIdx: 0,
- Limit: 50,
- })
- require.NoError(t, err)
- require.Len(t, cls, 30)
- require.Equal(t, 30, total)
-
- // Get the first ones
- cls, total, err = f.GetChangelists(ctx, clstore.SearchOptions{
- StartIdx: 0,
- Limit: 3,
- })
- require.NoError(t, err)
- require.Len(t, cls, 3)
- require.Equal(t, clstore.CountMany, total)
- // spot check the dates to make sure the CLs are in the right order.
- require.Equal(t, time.Date(2019, time.September, 1, 4, 9, 9, 0, time.UTC), cls[0].Updated)
- require.Equal(t, time.Date(2019, time.September, 1, 4, 7, 7, 0, time.UTC), cls[1].Updated)
- require.Equal(t, time.Date(2019, time.September, 1, 4, 5, 5, 0, time.UTC), cls[2].Updated)
-
- // Get some in the middle
- cls, total, err = f.GetChangelists(ctx, clstore.SearchOptions{
- StartIdx: 5,
- Limit: 2,
- })
- require.NoError(t, err)
- require.Len(t, cls, 2)
- require.Equal(t, clstore.CountMany, total)
- require.Equal(t, time.Date(2019, time.September, 1, 2, 39, 39, 0, time.UTC), cls[0].Updated)
- require.Equal(t, time.Date(2019, time.September, 1, 2, 37, 37, 0, time.UTC), cls[1].Updated)
-
- // Get some at the end.
- cls, total, err = f.GetChangelists(ctx, clstore.SearchOptions{
- StartIdx: 28,
- Limit: 10,
- })
- require.NoError(t, err)
- require.Len(t, cls, 2)
- require.Equal(t, 30, total)
- require.Equal(t, time.Date(2019, time.August, 31, 14, 2, 2, 0, time.UTC), cls[0].Updated)
- require.Equal(t, time.Date(2019, time.August, 31, 14, 0, 0, 0, time.UTC), cls[1].Updated)
-
- // If we query off the end, we don't know how many there are, so 0 is a fine response.
- cls, total, err = f.GetChangelists(ctx, clstore.SearchOptions{
- StartIdx: 999,
- Limit: 3,
- })
- require.NoError(t, err)
- require.Len(t, cls, 0)
- require.Equal(t, 999, total)
-}
-
-// TestGetChangelistsOptions checks that various search options function.
-func TestGetChangelistsOptions(t *testing.T) {
- unittest.LargeTest(t)
- c, cleanup := firestore.NewClientForTesting(context.Background(), t)
- defer cleanup()
-
- f := New(c, "gerrit")
- ctx := context.Background()
- clA := code_review.Changelist{
- SystemID: "abc",
- Owner: "first@example.com",
- Status: code_review.Open,
- Subject: "Open sesame",
- Updated: time.Date(2019, time.October, 5, 4, 3, 2, 0, time.UTC),
- }
- clB := code_review.Changelist{
- SystemID: "def",
- Owner: "second@example.com",
- Status: code_review.Landed,
- Subject: "Landed Panda",
- Updated: time.Date(2019, time.October, 10, 4, 3, 2, 0, time.UTC),
- }
- clC := code_review.Changelist{
- SystemID: "ghi",
- Owner: "third@example.com",
- Status: code_review.Open,
- Subject: "Open again",
- Updated: time.Date(2019, time.October, 15, 4, 3, 2, 0, time.UTC),
- }
-
- require.NoError(t, f.PutChangelist(ctx, clA))
- require.NoError(t, f.PutChangelist(ctx, clB))
- require.NoError(t, f.PutChangelist(ctx, clC))
-
- xcl, _, err := f.GetChangelists(ctx, clstore.SearchOptions{
- Limit: 10,
- })
- require.NoError(t, err)
- assert.Equal(t, []code_review.Changelist{clC, clB, clA}, xcl)
-
- xcl, _, err = f.GetChangelists(ctx, clstore.SearchOptions{
- OpenCLsOnly: true,
- Limit: 10,
- })
- require.NoError(t, err)
- assert.Equal(t, []code_review.Changelist{clC, clA}, xcl)
-
- xcl, _, err = f.GetChangelists(ctx, clstore.SearchOptions{
- After: time.Date(2019, time.September, 28, 0, 0, 0, 0, time.UTC),
- Limit: 10,
- })
- require.NoError(t, err)
- assert.Equal(t, []code_review.Changelist{clC, clB, clA}, xcl)
-
- xcl, _, err = f.GetChangelists(ctx, clstore.SearchOptions{
- After: time.Date(2019, time.November, 28, 0, 0, 0, 0, time.UTC),
- Limit: 10,
- })
- require.NoError(t, err)
- assert.Empty(t, xcl)
-
- xcl, _, err = f.GetChangelists(ctx, clstore.SearchOptions{
- After: time.Date(2019, time.October, 7, 0, 0, 0, 0, time.UTC),
- Limit: 10,
- })
- require.NoError(t, err)
- assert.Equal(t, []code_review.Changelist{clC, clB}, xcl)
- xcl, _, err = f.GetChangelists(ctx, clstore.SearchOptions{
- OpenCLsOnly: true,
- After: time.Date(2019, time.October, 7, 0, 0, 0, 0, time.UTC),
- Limit: 10,
- })
- require.NoError(t, err)
- assert.Equal(t, []code_review.Changelist{clC}, xcl)
-}
-
-func TestGetChangelistsNoLimit(t *testing.T) {
- unittest.LargeTest(t)
- c, cleanup := firestore.NewClientForTesting(context.Background(), t)
- defer cleanup()
-
- f := New(c, "gerrit")
- ctx := context.Background()
-
- _, _, err := f.GetChangelists(ctx, clstore.SearchOptions{})
- require.Error(t, err)
- assert.Contains(t, err.Error(), "limit")
-}
diff --git a/golden/go/ingestion/fs_ingestionstore/BUILD.bazel b/golden/go/ingestion/fs_ingestionstore/BUILD.bazel
deleted file mode 100644
index e69de29..0000000
--- a/golden/go/ingestion/fs_ingestionstore/BUILD.bazel
+++ /dev/null
diff --git a/golden/go/ingestion_processors/tryjob_ingestion.go b/golden/go/ingestion_processors/tryjob_ingestion.go
index 6d9a715..dfb8a8b 100644
--- a/golden/go/ingestion_processors/tryjob_ingestion.go
+++ b/golden/go/ingestion_processors/tryjob_ingestion.go
@@ -19,8 +19,6 @@
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/vcsinfo"
"go.skia.org/infra/golden/go/clstore"
- "go.skia.org/infra/golden/go/clstore/dualclstore"
- "go.skia.org/infra/golden/go/clstore/fs_clstore"
"go.skia.org/infra/golden/go/clstore/sqlclstore"
"go.skia.org/infra/golden/go/code_review"
"go.skia.org/infra/golden/go/code_review/gerrit_crs"
@@ -119,12 +117,11 @@
if err != nil {
return nil, skerr.Wrapf(err, "could not create client for CRS %q", crsName)
}
- fireCS := fs_clstore.New(fsClient, crsName)
sqlCS := sqlclstore.New(db, crsName)
reviewSystems = append(reviewSystems, clstore.ReviewSystem{
ID: crsName,
Client: crsClient,
- Store: dualclstore.New(sqlCS, fireCS),
+ Store: sqlCS,
})
}
