From 30b76c263333650572fdfb25b20c1fd9dec79b11 Mon Sep 17 00:00:00 2001 From: Cezary Bartoszuk Date: Thu, 20 Apr 2023 14:13:59 -0500 Subject: [PATCH 01/30] Pull migration from #50727 --- .../1681300431_ownership_signals/down.sql | 13 +++ .../metadata.yaml | 2 + .../1681300431_ownership_signals/up.sql | 88 +++++++++++++++++++ 3 files changed, 103 insertions(+) create mode 100644 migrations/frontend/1681300431_ownership_signals/down.sql create mode 100644 migrations/frontend/1681300431_ownership_signals/metadata.yaml create mode 100644 migrations/frontend/1681300431_ownership_signals/up.sql diff --git a/migrations/frontend/1681300431_ownership_signals/down.sql b/migrations/frontend/1681300431_ownership_signals/down.sql new file mode 100644 index 0000000000000..ba43cc0bf88b7 --- /dev/null +++ b/migrations/frontend/1681300431_ownership_signals/down.sql @@ -0,0 +1,13 @@ +DROP TRIGGER IF EXISTS update_own_aggregate_recent_contribution ON own_signal_recent_contribution; +DROP FUNCTION IF EXISTS update_own_aggregate_recent_contribution(); + +DROP INDEX IF EXISTS own_aggregate_recent_contribution_author_file; +DROP TABLE IF EXISTS own_aggregate_recent_contribution; + +DROP TABLE IF EXISTS own_signal_recent_contribution; + +DROP INDEX IF EXISTS commit_authors_email_name; +DROP TABLE IF EXISTS commit_authors; + +DROP INDEX IF EXISTS repo_paths_index_absolute_path; +DROP TABLE IF EXISTS repo_paths; diff --git a/migrations/frontend/1681300431_ownership_signals/metadata.yaml b/migrations/frontend/1681300431_ownership_signals/metadata.yaml new file mode 100644 index 0000000000000..0b6f69923c40c --- /dev/null +++ b/migrations/frontend/1681300431_ownership_signals/metadata.yaml @@ -0,0 +1,2 @@ +name: ownership_signals +parents: [1680707560, 1680088638] diff --git a/migrations/frontend/1681300431_ownership_signals/up.sql b/migrations/frontend/1681300431_ownership_signals/up.sql new file mode 100644 index 0000000000000..9f4d5950b5e48 --- /dev/null +++ b/migrations/frontend/1681300431_ownership_signals/up.sql @@ -0,0 +1,88 @@ +CREATE TABLE IF NOT EXISTS repo_paths ( + id SERIAL PRIMARY KEY, + repo_id INTEGER NOT NULL REFERENCES repo(id) ON DELETE CASCADE DEFERRABLE, + absolute_path TEXT NOT NULL, + is_dir BOOLEAN NOT NULL, + parent_id INTEGER NULL REFERENCES repo_paths(id) +); + +COMMENT ON COLUMN repo_paths.absolute_path +IS 'Absolute path does not start or end with forward slash. Example: "a/b/c". Root directory is empty path "".'; + +CREATE UNIQUE INDEX IF NOT EXISTS repo_paths_index_absolute_path +ON repo_paths +USING btree (repo_id, absolute_path); + +CREATE TABLE IF NOT EXISTS commit_authors ( + id SERIAL PRIMARY KEY, + email TEXT NOT NULL, + name TEXT NOT NULL +); + +CREATE UNIQUE INDEX IF NOT EXISTS commit_authors_email_name +ON commit_authors +USING btree (email, name); + +CREATE TABLE IF NOT EXISTS own_signal_recent_contribution ( + id SERIAL PRIMARY KEY, + commit_author_id INTEGER NOT NULL REFERENCES commit_authors(id), + changed_file_path_id INTEGER NOT NULL REFERENCES repo_paths(id), + commit_timestamp TIMESTAMP NOT NULL, + commit_id_hash INTEGER NOT NULL +); + +COMMENT ON TABLE own_signal_recent_contribution +IS 'One entry per file changed in every commit that classifies as a contribution signal.'; + +CREATE TABLE IF NOT EXISTS own_aggregate_recent_contribution ( + id SERIAL PRIMARY KEY, + commit_author_id INTEGER NOT NULL REFERENCES commit_authors(id), + changed_file_path_id INTEGER NOT NULL REFERENCES repo_paths(id), + contributions_count INTEGER DEFAULT 0 +); + +CREATE UNIQUE INDEX IF NOT EXISTS own_aggregate_recent_contribution_author_file +ON own_aggregate_recent_contribution +USING btree (commit_author_id, changed_file_path_id); + +CREATE OR REPLACE FUNCTION update_own_aggregate_recent_contribution() RETURNS TRIGGER AS $$ +BEGIN + WITH RECURSIVE ancestors AS ( + SELECT id, parent_id, 1 AS level + FROM repo_paths + WHERE id = NEW.changed_file_path_id + UNION ALL + SELECT p.id, p.parent_id, a.level + 1 + FROM repo_paths p + JOIN ancestors a ON p.id = a.parent_id + ) + UPDATE own_aggregate_recent_contribution + SET contributions_count = contributions_count + 1 + WHERE commit_author_id = NEW.commit_author_id AND changed_file_path_id IN ( + SELECT id FROM ancestors + ); + + WITH RECURSIVE ancestors AS ( + SELECT id, parent_id, 1 AS level + FROM repo_paths + WHERE id = NEW.changed_file_path_id + UNION ALL + SELECT p.id, p.parent_id, a.level + 1 + FROM repo_paths p + JOIN ancestors a ON p.id = a.parent_id + ) + INSERT INTO own_aggregate_recent_contribution (commit_author_id, changed_file_path_id, contributions_count) + SELECT NEW.commit_author_id, id, 1 + FROM ancestors + WHERE NOT EXISTS ( + SELECT 1 FROM own_aggregate_recent_contribution + WHERE commit_author_id = NEW.commit_author_id AND changed_file_path_id = ancestors.id + ); + + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER update_own_aggregate_recent_contribution +AFTER INSERT ON own_signal_recent_contribution +FOR EACH ROW EXECUTE PROCEDURE update_own_aggregate_recent_contribution(); From 7b85767fc02e6ae07006e0ac9a4b147da263c921 Mon Sep 17 00:00:00 2001 From: Cezary Bartoszuk Date: Thu, 20 Apr 2023 19:51:07 -0500 Subject: [PATCH 02/30] Migrations and slightly massaged stores --- enterprise/internal/database/mocks_temp.go | 115 ++++++ internal/database/commit_signals.go | 163 ++++++++ internal/database/commit_signals_test.go | 93 +++++ internal/database/database.go | 5 + internal/database/mocks_temp.go | 114 ++++++ internal/database/schema.json | 421 +++++++++++++++++++++ internal/database/schema.md | 79 ++++ migrations/frontend/squashed.sql | 159 ++++++++ 8 files changed, 1149 insertions(+) create mode 100644 internal/database/commit_signals.go create mode 100644 internal/database/commit_signals_test.go diff --git a/enterprise/internal/database/mocks_temp.go b/enterprise/internal/database/mocks_temp.go index 27a6acfb7b14c..ba1048c1c3538 100644 --- a/enterprise/internal/database/mocks_temp.go +++ b/enterprise/internal/database/mocks_temp.go @@ -7920,6 +7920,9 @@ type MockEnterpriseDB struct { // OutboundWebhooksFunc is an instance of a mock function object // controlling the behavior of the method OutboundWebhooks. OutboundWebhooksFunc *EnterpriseDBOutboundWebhooksFunc + // OwnSignalsFunc is an instance of a mock function object controlling + // the behavior of the method OwnSignals. + OwnSignalsFunc *EnterpriseDBOwnSignalsFunc // PermissionSyncJobsFunc is an instance of a mock function object // controlling the behavior of the method PermissionSyncJobs. PermissionSyncJobsFunc *EnterpriseDBPermissionSyncJobsFunc @@ -8145,6 +8148,11 @@ func NewMockEnterpriseDB() *MockEnterpriseDB { return }, }, + OwnSignalsFunc: &EnterpriseDBOwnSignalsFunc{ + defaultHook: func() (r0 database.OwnSignalStore) { + return + }, + }, PermissionSyncJobsFunc: &EnterpriseDBPermissionSyncJobsFunc{ defaultHook: func() (r0 database.PermissionSyncJobStore) { return @@ -8427,6 +8435,11 @@ func NewStrictMockEnterpriseDB() *MockEnterpriseDB { panic("unexpected invocation of MockEnterpriseDB.OutboundWebhooks") }, }, + OwnSignalsFunc: &EnterpriseDBOwnSignalsFunc{ + defaultHook: func() database.OwnSignalStore { + panic("unexpected invocation of MockEnterpriseDB.OwnSignals") + }, + }, PermissionSyncJobsFunc: &EnterpriseDBPermissionSyncJobsFunc{ defaultHook: func() database.PermissionSyncJobStore { panic("unexpected invocation of MockEnterpriseDB.PermissionSyncJobs") @@ -8656,6 +8669,9 @@ func NewMockEnterpriseDBFrom(i EnterpriseDB) *MockEnterpriseDB { OutboundWebhooksFunc: &EnterpriseDBOutboundWebhooksFunc{ defaultHook: i.OutboundWebhooks, }, + OwnSignalsFunc: &EnterpriseDBOwnSignalsFunc{ + defaultHook: i.OwnSignals, + }, PermissionSyncJobsFunc: &EnterpriseDBPermissionSyncJobsFunc{ defaultHook: i.PermissionSyncJobs, }, @@ -11466,6 +11482,105 @@ func (c EnterpriseDBOutboundWebhooksFuncCall) Results() []interface{} { return []interface{}{c.Result0} } +// EnterpriseDBOwnSignalsFunc describes the behavior when the OwnSignals +// method of the parent MockEnterpriseDB instance is invoked. +type EnterpriseDBOwnSignalsFunc struct { + defaultHook func() database.OwnSignalStore + hooks []func() database.OwnSignalStore + history []EnterpriseDBOwnSignalsFuncCall + mutex sync.Mutex +} + +// OwnSignals delegates to the next hook function in the queue and stores +// the parameter and result values of this invocation. +func (m *MockEnterpriseDB) OwnSignals() database.OwnSignalStore { + r0 := m.OwnSignalsFunc.nextHook()() + m.OwnSignalsFunc.appendCall(EnterpriseDBOwnSignalsFuncCall{r0}) + return r0 +} + +// SetDefaultHook sets function that is called when the OwnSignals method of +// the parent MockEnterpriseDB instance is invoked and the hook queue is +// empty. +func (f *EnterpriseDBOwnSignalsFunc) SetDefaultHook(hook func() database.OwnSignalStore) { + f.defaultHook = hook +} + +// PushHook adds a function to the end of hook queue. Each invocation of the +// OwnSignals method of the parent MockEnterpriseDB instance invokes the +// hook at the front of the queue and discards it. After the queue is empty, +// the default hook function is invoked for any future action. +func (f *EnterpriseDBOwnSignalsFunc) PushHook(hook func() database.OwnSignalStore) { + f.mutex.Lock() + f.hooks = append(f.hooks, hook) + f.mutex.Unlock() +} + +// SetDefaultReturn calls SetDefaultHook with a function that returns the +// given values. +func (f *EnterpriseDBOwnSignalsFunc) SetDefaultReturn(r0 database.OwnSignalStore) { + f.SetDefaultHook(func() database.OwnSignalStore { + return r0 + }) +} + +// PushReturn calls PushHook with a function that returns the given values. +func (f *EnterpriseDBOwnSignalsFunc) PushReturn(r0 database.OwnSignalStore) { + f.PushHook(func() database.OwnSignalStore { + return r0 + }) +} + +func (f *EnterpriseDBOwnSignalsFunc) nextHook() func() database.OwnSignalStore { + f.mutex.Lock() + defer f.mutex.Unlock() + + if len(f.hooks) == 0 { + return f.defaultHook + } + + hook := f.hooks[0] + f.hooks = f.hooks[1:] + return hook +} + +func (f *EnterpriseDBOwnSignalsFunc) appendCall(r0 EnterpriseDBOwnSignalsFuncCall) { + f.mutex.Lock() + f.history = append(f.history, r0) + f.mutex.Unlock() +} + +// History returns a sequence of EnterpriseDBOwnSignalsFuncCall objects +// describing the invocations of this function. +func (f *EnterpriseDBOwnSignalsFunc) History() []EnterpriseDBOwnSignalsFuncCall { + f.mutex.Lock() + history := make([]EnterpriseDBOwnSignalsFuncCall, len(f.history)) + copy(history, f.history) + f.mutex.Unlock() + + return history +} + +// EnterpriseDBOwnSignalsFuncCall is an object that describes an invocation +// of method OwnSignals on an instance of MockEnterpriseDB. +type EnterpriseDBOwnSignalsFuncCall struct { + // Result0 is the value of the 1st result returned from this method + // invocation. + Result0 database.OwnSignalStore +} + +// Args returns an interface slice containing the arguments of this +// invocation. +func (c EnterpriseDBOwnSignalsFuncCall) Args() []interface{} { + return []interface{}{} +} + +// Results returns an interface slice containing the results of this +// invocation. +func (c EnterpriseDBOwnSignalsFuncCall) Results() []interface{} { + return []interface{}{c.Result0} +} + // EnterpriseDBPermissionSyncJobsFunc describes the behavior when the // PermissionSyncJobs method of the parent MockEnterpriseDB instance is // invoked. diff --git a/internal/database/commit_signals.go b/internal/database/commit_signals.go new file mode 100644 index 0000000000000..d3e9bb918fc77 --- /dev/null +++ b/internal/database/commit_signals.go @@ -0,0 +1,163 @@ +package database + +import ( + "context" + "database/sql" + "hash/fnv" + "strings" + "time" + + "github.com/keegancsmith/sqlf" + + "github.com/sourcegraph/log" + + "github.com/sourcegraph/sourcegraph/internal/api" + "github.com/sourcegraph/sourcegraph/internal/database/basestore" + "github.com/sourcegraph/sourcegraph/lib/errors" +) + +///// +// TODO: There is no support for directory root now. The fix is simple though. + +type OwnSignalStore interface { + AddCommit(ctx context.Context, commit Commit) error + FindRecentAuthors(ctx context.Context, repoID api.RepoID, path string) ([]RecentAuthor, error) +} + +type Commit struct { + RepoID api.RepoID + AuthorName string + AuthorEmail string + Timestamp time.Time + CommitSHA string + FilesChanged []string +} + +type ownSignalStore struct { + logger log.Logger + *basestore.Store +} + +func ensureAuthor(ctx context.Context, tx *basestore.Store, commit Commit) (int, error) { + var authorID int + err := tx.QueryRow( + ctx, + sqlf.Sprintf(`SELECT id FROM commit_authors WHERE email = %s AND name = %s`, commit.AuthorEmail, commit.AuthorName), + ).Scan(&authorID) + if err == sql.ErrNoRows { + err = tx.QueryRow( + ctx, + sqlf.Sprintf(`INSERT INTO commit_authors (email, name) VALUES (%s, %s) RETURNING id`, commit.AuthorEmail, commit.AuthorName), + ).Scan(&authorID) + } + if err != nil { + return 0, err + } + return authorID, nil +} + +func ensureRepoPaths(ctx context.Context, tx *basestore.Store, commit Commit) ([]int, error) { + pathIDs := make([]int, len(commit.FilesChanged)) + for i, path := range commit.FilesChanged { + pathPrefixes := strings.Split(path, "/") + var parentPathID *int + var pathID int + for j := range pathPrefixes { + pathPrefix := strings.Join(pathPrefixes[:j+1], "/") + isDir := j < len(pathPrefixes)-1 + // Get or create repo path + err := tx.QueryRow( + ctx, + sqlf.Sprintf(` + SELECT id FROM repo_paths + WHERE repo_id = %s + AND absolute_path = %s + `, commit.RepoID, pathPrefix), + ).Scan(&pathID) + if err == sql.ErrNoRows { + err = tx.QueryRow( + ctx, + sqlf.Sprintf(` + INSERT INTO repo_paths (repo_id, absolute_path, is_dir, parent_id) + VALUES (%s, %s, %s, %s) RETURNING id`, commit.RepoID, pathPrefix, isDir, parentPathID), + ).Scan(&pathID) + } + if err != nil { + return nil, err + } + parentPathID = &pathID + } + pathIDs[i] = pathID + } + return pathIDs, nil +} + +func (s *ownSignalStore) AddCommit(ctx context.Context, commit Commit) error { + tx := s.Store + // Get or create commit author + authorID, err := ensureAuthor(ctx, tx, commit) + if err != nil { + return errors.Wrap(err, "cannot insert commit author") + } + // Get or create repo paths + pathIDs, err := ensureRepoPaths(ctx, tx, commit) + if err != nil { + return errors.Wrap(err, "cannot insert repo paths") + } + // Insert into own_signal_recent_contribution + for _, pathID := range pathIDs { + commitID := fnv.New32a() + commitID.Write([]byte(commit.CommitSHA)) + q := sqlf.Sprintf(`INSERT INTO own_signal_recent_contribution (commit_author_id, changed_file_path_id, + commit_timestamp, commit_id_hash) VALUES (%s, %s, %s, %s)`, + authorID, + pathID, + commit.Timestamp, + commitID.Sum32(), + ) + err = tx.Exec(ctx, q) + if err != nil { + return err + } + } + + return nil +} + +type RecentAuthor struct { + AuthorName string + AuthorEmail string + ContributionCount int +} + +func (s *ownSignalStore) FindRecentAuthors(ctx context.Context, repoID api.RepoID, path string) ([]RecentAuthor, error) { + var authors []RecentAuthor + q := sqlf.Sprintf(` + SELECT a.name, a.email, g.contributions_count + FROM commit_authors AS a + INNER JOIN own_aggregate_recent_contribution AS g + ON a.id = g.commit_author_id + INNER JOIN repo_paths AS p + ON p.id = g.changed_file_path_id + WHERE p.repo_id = %s + AND p.absolute_path = %s + ORDER BY 3 DESC + `, repoID, path) + rows, err := s.Query(ctx, q) + if err != nil { + return nil, err + } + defer rows.Close() + for rows.Next() { + var a RecentAuthor + if err := rows.Scan(&a.AuthorName, &a.AuthorEmail, &a.ContributionCount); err != nil { + return nil, err + } + authors = append(authors, a) + } + return authors, nil +} + +func OwnSignalsStoreWith(other basestore.ShareableStore) OwnSignalStore { + return &ownSignalStore{Store: basestore.NewWithHandle(other.Handle())} +} diff --git a/internal/database/commit_signals_test.go b/internal/database/commit_signals_test.go new file mode 100644 index 0000000000000..c4508b8a1862f --- /dev/null +++ b/internal/database/commit_signals_test.go @@ -0,0 +1,93 @@ +package database + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/sourcegraph/log/logtest" + "github.com/stretchr/testify/assert" + + "github.com/sourcegraph/sourcegraph/internal/database/dbtest" + "github.com/sourcegraph/sourcegraph/internal/types" +) + +func TestOwnSignalStore_AddCommit(t *testing.T) { + if testing.Short() { + t.Skip() + } + + t.Parallel() + logger := logtest.Scoped(t) + db := NewDB(logger, dbtest.NewDB(logger, t)) + store := OwnSignalsStoreWith(db) + + ctx := context.Background() + repo := mustCreate(ctx, t, db, &types.Repo{Name: "a/b"}) + + for i, commit := range []Commit{ + { + RepoID: repo.ID, + AuthorName: "alice", + AuthorEmail: "alice@example.com", + FilesChanged: []string{"file1.txt", "dir/file2.txt"}, + }, + { + RepoID: repo.ID, + AuthorName: "alice", + AuthorEmail: "alice@example.com", + FilesChanged: []string{"file1.txt", "dir/file3.txt"}, + }, + { + RepoID: repo.ID, + AuthorName: "alice", + AuthorEmail: "alice@example.com", + FilesChanged: []string{"file1.txt", "dir/file2.txt", "dir/subdir/file.txt"}, + }, + { + RepoID: repo.ID, + AuthorName: "bob", + AuthorEmail: "bob@example.com", + FilesChanged: []string{"file1.txt", "dir2/file2.txt", "dir2/subdir/file.txt"}, + }, + } { + commit.Timestamp = time.Now() + commit.CommitSHA = fmt.Sprintf("sha%d", i) + if err := store.AddCommit(ctx, commit); err != nil { + t.Fatal(err) + } + } + + for p, w := range map[string][]RecentAuthor{ + "dir": { + { + AuthorName: "alice", + AuthorEmail: "alice@example.com", + ContributionCount: 4, + }, + }, + "file1.txt": { + { + AuthorName: "alice", + AuthorEmail: "alice@example.com", + ContributionCount: 3, + }, + { + AuthorName: "bob", + AuthorEmail: "bob@example.com", + ContributionCount: 1, + }, + }, + } { + path := p + want := w + t.Run(path, func(t *testing.T) { + got, err := store.FindRecentAuthors(ctx, repo.ID, path) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, want, got) + }) + } +} diff --git a/internal/database/database.go b/internal/database/database.go index abcd305e33f0d..d5e288c4747af 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -40,6 +40,7 @@ type DB interface { OutboundWebhooks(encryption.Key) OutboundWebhookStore OutboundWebhookJobs(encryption.Key) OutboundWebhookJobStore OutboundWebhookLogs(encryption.Key) OutboundWebhookLogStore + OwnSignals() OwnSignalStore Permissions() PermissionStore PermissionSyncJobs() PermissionSyncJobStore Phabricator() PhabricatorStore @@ -199,6 +200,10 @@ func (d *db) OutboundWebhookLogs(key encryption.Key) OutboundWebhookLogStore { return OutboundWebhookLogsWith(d.Store, key) } +func (d *db) OwnSignals() OwnSignalStore { + return OwnSignalsStoreWith(d.Store) +} + func (d *db) Permissions() PermissionStore { return PermissionsWith(d.Store) } diff --git a/internal/database/mocks_temp.go b/internal/database/mocks_temp.go index a761638972121..bae50dbd9f795 100644 --- a/internal/database/mocks_temp.go +++ b/internal/database/mocks_temp.go @@ -4954,6 +4954,9 @@ type MockDB struct { // OutboundWebhooksFunc is an instance of a mock function object // controlling the behavior of the method OutboundWebhooks. OutboundWebhooksFunc *DBOutboundWebhooksFunc + // OwnSignalsFunc is an instance of a mock function object controlling + // the behavior of the method OwnSignals. + OwnSignalsFunc *DBOwnSignalsFunc // PermissionSyncJobsFunc is an instance of a mock function object // controlling the behavior of the method PermissionSyncJobs. PermissionSyncJobsFunc *DBPermissionSyncJobsFunc @@ -5158,6 +5161,11 @@ func NewMockDB() *MockDB { return }, }, + OwnSignalsFunc: &DBOwnSignalsFunc{ + defaultHook: func() (r0 OwnSignalStore) { + return + }, + }, PermissionSyncJobsFunc: &DBPermissionSyncJobsFunc{ defaultHook: func() (r0 PermissionSyncJobStore) { return @@ -5415,6 +5423,11 @@ func NewStrictMockDB() *MockDB { panic("unexpected invocation of MockDB.OutboundWebhooks") }, }, + OwnSignalsFunc: &DBOwnSignalsFunc{ + defaultHook: func() OwnSignalStore { + panic("unexpected invocation of MockDB.OwnSignals") + }, + }, PermissionSyncJobsFunc: &DBPermissionSyncJobsFunc{ defaultHook: func() PermissionSyncJobStore { panic("unexpected invocation of MockDB.PermissionSyncJobs") @@ -5624,6 +5637,9 @@ func NewMockDBFrom(i DB) *MockDB { OutboundWebhooksFunc: &DBOutboundWebhooksFunc{ defaultHook: i.OutboundWebhooks, }, + OwnSignalsFunc: &DBOwnSignalsFunc{ + defaultHook: i.OwnSignals, + }, PermissionSyncJobsFunc: &DBPermissionSyncJobsFunc{ defaultHook: i.PermissionSyncJobs, }, @@ -8102,6 +8118,104 @@ func (c DBOutboundWebhooksFuncCall) Results() []interface{} { return []interface{}{c.Result0} } +// DBOwnSignalsFunc describes the behavior when the OwnSignals method of the +// parent MockDB instance is invoked. +type DBOwnSignalsFunc struct { + defaultHook func() OwnSignalStore + hooks []func() OwnSignalStore + history []DBOwnSignalsFuncCall + mutex sync.Mutex +} + +// OwnSignals delegates to the next hook function in the queue and stores +// the parameter and result values of this invocation. +func (m *MockDB) OwnSignals() OwnSignalStore { + r0 := m.OwnSignalsFunc.nextHook()() + m.OwnSignalsFunc.appendCall(DBOwnSignalsFuncCall{r0}) + return r0 +} + +// SetDefaultHook sets function that is called when the OwnSignals method of +// the parent MockDB instance is invoked and the hook queue is empty. +func (f *DBOwnSignalsFunc) SetDefaultHook(hook func() OwnSignalStore) { + f.defaultHook = hook +} + +// PushHook adds a function to the end of hook queue. Each invocation of the +// OwnSignals method of the parent MockDB instance invokes the hook at the +// front of the queue and discards it. After the queue is empty, the default +// hook function is invoked for any future action. +func (f *DBOwnSignalsFunc) PushHook(hook func() OwnSignalStore) { + f.mutex.Lock() + f.hooks = append(f.hooks, hook) + f.mutex.Unlock() +} + +// SetDefaultReturn calls SetDefaultHook with a function that returns the +// given values. +func (f *DBOwnSignalsFunc) SetDefaultReturn(r0 OwnSignalStore) { + f.SetDefaultHook(func() OwnSignalStore { + return r0 + }) +} + +// PushReturn calls PushHook with a function that returns the given values. +func (f *DBOwnSignalsFunc) PushReturn(r0 OwnSignalStore) { + f.PushHook(func() OwnSignalStore { + return r0 + }) +} + +func (f *DBOwnSignalsFunc) nextHook() func() OwnSignalStore { + f.mutex.Lock() + defer f.mutex.Unlock() + + if len(f.hooks) == 0 { + return f.defaultHook + } + + hook := f.hooks[0] + f.hooks = f.hooks[1:] + return hook +} + +func (f *DBOwnSignalsFunc) appendCall(r0 DBOwnSignalsFuncCall) { + f.mutex.Lock() + f.history = append(f.history, r0) + f.mutex.Unlock() +} + +// History returns a sequence of DBOwnSignalsFuncCall objects describing the +// invocations of this function. +func (f *DBOwnSignalsFunc) History() []DBOwnSignalsFuncCall { + f.mutex.Lock() + history := make([]DBOwnSignalsFuncCall, len(f.history)) + copy(history, f.history) + f.mutex.Unlock() + + return history +} + +// DBOwnSignalsFuncCall is an object that describes an invocation of method +// OwnSignals on an instance of MockDB. +type DBOwnSignalsFuncCall struct { + // Result0 is the value of the 1st result returned from this method + // invocation. + Result0 OwnSignalStore +} + +// Args returns an interface slice containing the arguments of this +// invocation. +func (c DBOwnSignalsFuncCall) Args() []interface{} { + return []interface{}{} +} + +// Results returns an interface slice containing the results of this +// invocation. +func (c DBOwnSignalsFuncCall) Results() []interface{} { + return []interface{}{c.Result0} +} + // DBPermissionSyncJobsFunc describes the behavior when the // PermissionSyncJobs method of the parent MockDB instance is invoked. type DBPermissionSyncJobsFunc struct { diff --git a/internal/database/schema.json b/internal/database/schema.json index 2bfd9a60e73fd..271d5a67878f5 100755 --- a/internal/database/schema.json +++ b/internal/database/schema.json @@ -194,6 +194,10 @@ "Name": "update_codeintel_path_ranks_updated_at_column", "Definition": "CREATE OR REPLACE FUNCTION public.update_codeintel_path_ranks_updated_at_column()\n RETURNS trigger\n LANGUAGE plpgsql\nAS $function$ BEGIN\n NEW.updated_at = NOW();\n RETURN NEW;\nEND;\n$function$\n" }, + { + "Name": "update_own_aggregate_recent_contribution", + "Definition": "CREATE OR REPLACE FUNCTION public.update_own_aggregate_recent_contribution()\n RETURNS trigger\n LANGUAGE plpgsql\nAS $function$\nBEGIN\n WITH RECURSIVE ancestors AS (\n SELECT id, parent_id, 1 AS level\n FROM repo_paths\n WHERE id = NEW.changed_file_path_id\n UNION ALL\n SELECT p.id, p.parent_id, a.level + 1\n FROM repo_paths p\n JOIN ancestors a ON p.id = a.parent_id\n )\n UPDATE own_aggregate_recent_contribution\n SET contributions_count = contributions_count + 1\n WHERE commit_author_id = NEW.commit_author_id AND changed_file_path_id IN (\n SELECT id FROM ancestors\n );\n\n WITH RECURSIVE ancestors AS (\n SELECT id, parent_id, 1 AS level\n FROM repo_paths\n WHERE id = NEW.changed_file_path_id\n UNION ALL\n SELECT p.id, p.parent_id, a.level + 1\n FROM repo_paths p\n JOIN ancestors a ON p.id = a.parent_id\n )\n INSERT INTO own_aggregate_recent_contribution (commit_author_id, changed_file_path_id, contributions_count)\n SELECT NEW.commit_author_id, id, 1\n FROM ancestors\n WHERE NOT EXISTS (\n SELECT 1 FROM own_aggregate_recent_contribution\n WHERE commit_author_id = NEW.commit_author_id AND changed_file_path_id = ancestors.id\n );\n\n RETURN NEW;\nEND;\n$function$\n" + }, { "Name": "versions_insert_row_trigger", "Definition": "CREATE OR REPLACE FUNCTION public.versions_insert_row_trigger()\n RETURNS trigger\n LANGUAGE plpgsql\nAS $function$\nBEGIN\n NEW.first_version = NEW.version;\n RETURN NEW;\nEND $function$\n" @@ -506,6 +510,15 @@ "Increment": 1, "CycleOption": "NO" }, + { + "Name": "commit_authors_id_seq", + "TypeName": "integer", + "StartValue": 1, + "MinimumValue": 1, + "MaximumValue": 2147483647, + "Increment": 1, + "CycleOption": "NO" + }, { "Name": "configuration_policies_audit_logs_seq", "TypeName": "bigint", @@ -911,6 +924,24 @@ "Increment": 1, "CycleOption": "NO" }, + { + "Name": "own_aggregate_recent_contribution_id_seq", + "TypeName": "integer", + "StartValue": 1, + "MinimumValue": 1, + "MaximumValue": 2147483647, + "Increment": 1, + "CycleOption": "NO" + }, + { + "Name": "own_signal_recent_contribution_id_seq", + "TypeName": "integer", + "StartValue": 1, + "MinimumValue": 1, + "MaximumValue": 2147483647, + "Increment": 1, + "CycleOption": "NO" + }, { "Name": "package_repo_filters_id_seq", "TypeName": "integer", @@ -992,6 +1023,15 @@ "Increment": 1, "CycleOption": "NO" }, + { + "Name": "repo_paths_id_seq", + "TypeName": "integer", + "StartValue": 1, + "MinimumValue": 1, + "MaximumValue": 2147483647, + "Increment": 1, + "CycleOption": "NO" + }, { "Name": "roles_id_seq", "TypeName": "integer", @@ -8052,6 +8092,75 @@ ], "Triggers": [] }, + { + "Name": "commit_authors", + "Comment": "", + "Columns": [ + { + "Name": "email", + "Index": 2, + "TypeName": "text", + "IsNullable": false, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "id", + "Index": 1, + "TypeName": "integer", + "IsNullable": false, + "Default": "nextval('commit_authors_id_seq'::regclass)", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "name", + "Index": 3, + "TypeName": "text", + "IsNullable": false, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + } + ], + "Indexes": [ + { + "Name": "commit_authors_email_name", + "IsPrimaryKey": false, + "IsUnique": true, + "IsExclusion": false, + "IsDeferrable": false, + "IndexDefinition": "CREATE UNIQUE INDEX commit_authors_email_name ON commit_authors USING btree (email, name)", + "ConstraintType": "", + "ConstraintDefinition": "" + }, + { + "Name": "commit_authors_pkey", + "IsPrimaryKey": true, + "IsUnique": true, + "IsExclusion": false, + "IsDeferrable": false, + "IndexDefinition": "CREATE UNIQUE INDEX commit_authors_pkey ON commit_authors USING btree (id)", + "ConstraintType": "p", + "ConstraintDefinition": "PRIMARY KEY (id)" + } + ], + "Constraints": null, + "Triggers": [] + }, { "Name": "configuration_policies_audit_logs", "Comment": "", @@ -18374,6 +18483,208 @@ ], "Triggers": [] }, + { + "Name": "own_aggregate_recent_contribution", + "Comment": "", + "Columns": [ + { + "Name": "changed_file_path_id", + "Index": 3, + "TypeName": "integer", + "IsNullable": false, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "commit_author_id", + "Index": 2, + "TypeName": "integer", + "IsNullable": false, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "contributions_count", + "Index": 4, + "TypeName": "integer", + "IsNullable": true, + "Default": "0", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "id", + "Index": 1, + "TypeName": "integer", + "IsNullable": false, + "Default": "nextval('own_aggregate_recent_contribution_id_seq'::regclass)", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + } + ], + "Indexes": [ + { + "Name": "own_aggregate_recent_contribution_author_file", + "IsPrimaryKey": false, + "IsUnique": true, + "IsExclusion": false, + "IsDeferrable": false, + "IndexDefinition": "CREATE UNIQUE INDEX own_aggregate_recent_contribution_author_file ON own_aggregate_recent_contribution USING btree (commit_author_id, changed_file_path_id)", + "ConstraintType": "", + "ConstraintDefinition": "" + }, + { + "Name": "own_aggregate_recent_contribution_pkey", + "IsPrimaryKey": true, + "IsUnique": true, + "IsExclusion": false, + "IsDeferrable": false, + "IndexDefinition": "CREATE UNIQUE INDEX own_aggregate_recent_contribution_pkey ON own_aggregate_recent_contribution USING btree (id)", + "ConstraintType": "p", + "ConstraintDefinition": "PRIMARY KEY (id)" + } + ], + "Constraints": [ + { + "Name": "own_aggregate_recent_contribution_changed_file_path_id_fkey", + "ConstraintType": "f", + "RefTableName": "repo_paths", + "IsDeferrable": false, + "ConstraintDefinition": "FOREIGN KEY (changed_file_path_id) REFERENCES repo_paths(id)" + }, + { + "Name": "own_aggregate_recent_contribution_commit_author_id_fkey", + "ConstraintType": "f", + "RefTableName": "commit_authors", + "IsDeferrable": false, + "ConstraintDefinition": "FOREIGN KEY (commit_author_id) REFERENCES commit_authors(id)" + } + ], + "Triggers": [] + }, + { + "Name": "own_signal_recent_contribution", + "Comment": "One entry per file changed in every commit that classifies as a contribution signal.", + "Columns": [ + { + "Name": "changed_file_path_id", + "Index": 3, + "TypeName": "integer", + "IsNullable": false, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "commit_author_id", + "Index": 2, + "TypeName": "integer", + "IsNullable": false, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "commit_id_hash", + "Index": 5, + "TypeName": "integer", + "IsNullable": false, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "commit_timestamp", + "Index": 4, + "TypeName": "timestamp without time zone", + "IsNullable": false, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "id", + "Index": 1, + "TypeName": "integer", + "IsNullable": false, + "Default": "nextval('own_signal_recent_contribution_id_seq'::regclass)", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + } + ], + "Indexes": [ + { + "Name": "own_signal_recent_contribution_pkey", + "IsPrimaryKey": true, + "IsUnique": true, + "IsExclusion": false, + "IsDeferrable": false, + "IndexDefinition": "CREATE UNIQUE INDEX own_signal_recent_contribution_pkey ON own_signal_recent_contribution USING btree (id)", + "ConstraintType": "p", + "ConstraintDefinition": "PRIMARY KEY (id)" + } + ], + "Constraints": [ + { + "Name": "own_signal_recent_contribution_changed_file_path_id_fkey", + "ConstraintType": "f", + "RefTableName": "repo_paths", + "IsDeferrable": false, + "ConstraintDefinition": "FOREIGN KEY (changed_file_path_id) REFERENCES repo_paths(id)" + }, + { + "Name": "own_signal_recent_contribution_commit_author_id_fkey", + "ConstraintType": "f", + "RefTableName": "commit_authors", + "IsDeferrable": false, + "ConstraintDefinition": "FOREIGN KEY (commit_author_id) REFERENCES commit_authors(id)" + } + ], + "Triggers": [ + { + "Name": "update_own_aggregate_recent_contribution", + "Definition": "CREATE TRIGGER update_own_aggregate_recent_contribution AFTER INSERT ON own_signal_recent_contribution FOR EACH ROW EXECUTE FUNCTION update_own_aggregate_recent_contribution()" + } + ] + }, { "Name": "package_repo_filters", "Comment": "", @@ -20800,6 +21111,116 @@ ], "Triggers": [] }, + { + "Name": "repo_paths", + "Comment": "", + "Columns": [ + { + "Name": "absolute_path", + "Index": 3, + "TypeName": "text", + "IsNullable": false, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "Absolute path does not start or end with forward slash. Example: \"a/b/c\". Root directory is empty path \"\"." + }, + { + "Name": "id", + "Index": 1, + "TypeName": "integer", + "IsNullable": false, + "Default": "nextval('repo_paths_id_seq'::regclass)", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "is_dir", + "Index": 4, + "TypeName": "boolean", + "IsNullable": false, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "parent_id", + "Index": 5, + "TypeName": "integer", + "IsNullable": true, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "repo_id", + "Index": 2, + "TypeName": "integer", + "IsNullable": false, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + } + ], + "Indexes": [ + { + "Name": "repo_paths_index_absolute_path", + "IsPrimaryKey": false, + "IsUnique": true, + "IsExclusion": false, + "IsDeferrable": false, + "IndexDefinition": "CREATE UNIQUE INDEX repo_paths_index_absolute_path ON repo_paths USING btree (repo_id, absolute_path)", + "ConstraintType": "", + "ConstraintDefinition": "" + }, + { + "Name": "repo_paths_pkey", + "IsPrimaryKey": true, + "IsUnique": true, + "IsExclusion": false, + "IsDeferrable": false, + "IndexDefinition": "CREATE UNIQUE INDEX repo_paths_pkey ON repo_paths USING btree (id)", + "ConstraintType": "p", + "ConstraintDefinition": "PRIMARY KEY (id)" + } + ], + "Constraints": [ + { + "Name": "repo_paths_parent_id_fkey", + "ConstraintType": "f", + "RefTableName": "repo_paths", + "IsDeferrable": false, + "ConstraintDefinition": "FOREIGN KEY (parent_id) REFERENCES repo_paths(id)" + }, + { + "Name": "repo_paths_repo_id_fkey", + "ConstraintType": "f", + "RefTableName": "repo", + "IsDeferrable": true, + "ConstraintDefinition": "FOREIGN KEY (repo_id) REFERENCES repo(id) ON DELETE CASCADE DEFERRABLE" + } + ], + "Triggers": [] + }, { "Name": "repo_pending_permissions", "Comment": "", diff --git a/internal/database/schema.md b/internal/database/schema.md index a39d468954e5e..5dc3aa6f97ccb 100755 --- a/internal/database/schema.md +++ b/internal/database/schema.md @@ -1002,6 +1002,22 @@ Foreign-key constraints: ``` +# Table "public.commit_authors" +``` + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+-------------------------------------------- + id | integer | | not null | nextval('commit_authors_id_seq'::regclass) + email | text | | not null | + name | text | | not null | +Indexes: + "commit_authors_pkey" PRIMARY KEY, btree (id) + "commit_authors_email_name" UNIQUE, btree (email, name) +Referenced by: + TABLE "own_aggregate_recent_contribution" CONSTRAINT "own_aggregate_recent_contribution_commit_author_id_fkey" FOREIGN KEY (commit_author_id) REFERENCES commit_authors(id) + TABLE "own_signal_recent_contribution" CONSTRAINT "own_signal_recent_contribution_commit_author_id_fkey" FOREIGN KEY (commit_author_id) REFERENCES commit_authors(id) + +``` + # Table "public.configuration_policies_audit_logs" ``` Column | Type | Collation | Nullable | Default @@ -2766,6 +2782,44 @@ Referenced by: ``` +# Table "public.own_aggregate_recent_contribution" +``` + Column | Type | Collation | Nullable | Default +----------------------+---------+-----------+----------+--------------------------------------------------------------- + id | integer | | not null | nextval('own_aggregate_recent_contribution_id_seq'::regclass) + commit_author_id | integer | | not null | + changed_file_path_id | integer | | not null | + contributions_count | integer | | | 0 +Indexes: + "own_aggregate_recent_contribution_pkey" PRIMARY KEY, btree (id) + "own_aggregate_recent_contribution_author_file" UNIQUE, btree (commit_author_id, changed_file_path_id) +Foreign-key constraints: + "own_aggregate_recent_contribution_changed_file_path_id_fkey" FOREIGN KEY (changed_file_path_id) REFERENCES repo_paths(id) + "own_aggregate_recent_contribution_commit_author_id_fkey" FOREIGN KEY (commit_author_id) REFERENCES commit_authors(id) + +``` + +# Table "public.own_signal_recent_contribution" +``` + Column | Type | Collation | Nullable | Default +----------------------+-----------------------------+-----------+----------+------------------------------------------------------------ + id | integer | | not null | nextval('own_signal_recent_contribution_id_seq'::regclass) + commit_author_id | integer | | not null | + changed_file_path_id | integer | | not null | + commit_timestamp | timestamp without time zone | | not null | + commit_id_hash | integer | | not null | +Indexes: + "own_signal_recent_contribution_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + "own_signal_recent_contribution_changed_file_path_id_fkey" FOREIGN KEY (changed_file_path_id) REFERENCES repo_paths(id) + "own_signal_recent_contribution_commit_author_id_fkey" FOREIGN KEY (commit_author_id) REFERENCES commit_authors(id) +Triggers: + update_own_aggregate_recent_contribution AFTER INSERT ON own_signal_recent_contribution FOR EACH ROW EXECUTE FUNCTION update_own_aggregate_recent_contribution() + +``` + +One entry per file changed in every commit that classifies as a contribution signal. + # Table "public.package_repo_filters" ``` Column | Type | Collation | Nullable | Default @@ -3071,6 +3125,7 @@ Referenced by: TABLE "lsif_retention_configuration" CONSTRAINT "lsif_retention_configuration_repository_id_fkey" FOREIGN KEY (repository_id) REFERENCES repo(id) ON DELETE CASCADE TABLE "permission_sync_jobs" CONSTRAINT "permission_sync_jobs_repository_id_fkey" FOREIGN KEY (repository_id) REFERENCES repo(id) ON DELETE CASCADE TABLE "repo_kvps" CONSTRAINT "repo_kvps_repo_id_fkey" FOREIGN KEY (repo_id) REFERENCES repo(id) ON DELETE CASCADE + TABLE "repo_paths" CONSTRAINT "repo_paths_repo_id_fkey" FOREIGN KEY (repo_id) REFERENCES repo(id) ON DELETE CASCADE DEFERRABLE TABLE "search_context_repos" CONSTRAINT "search_context_repos_repo_id_fk" FOREIGN KEY (repo_id) REFERENCES repo(id) ON DELETE CASCADE TABLE "sub_repo_permissions" CONSTRAINT "sub_repo_permissions_repo_id_fk" FOREIGN KEY (repo_id) REFERENCES repo(id) ON DELETE CASCADE TABLE "user_public_repos" CONSTRAINT "user_public_repos_repo_id_fkey" FOREIGN KEY (repo_id) REFERENCES repo(id) ON DELETE CASCADE @@ -3125,6 +3180,30 @@ Foreign-key constraints: ``` +# Table "public.repo_paths" +``` + Column | Type | Collation | Nullable | Default +---------------+---------+-----------+----------+---------------------------------------- + id | integer | | not null | nextval('repo_paths_id_seq'::regclass) + repo_id | integer | | not null | + absolute_path | text | | not null | + is_dir | boolean | | not null | + parent_id | integer | | | +Indexes: + "repo_paths_pkey" PRIMARY KEY, btree (id) + "repo_paths_index_absolute_path" UNIQUE, btree (repo_id, absolute_path) +Foreign-key constraints: + "repo_paths_parent_id_fkey" FOREIGN KEY (parent_id) REFERENCES repo_paths(id) + "repo_paths_repo_id_fkey" FOREIGN KEY (repo_id) REFERENCES repo(id) ON DELETE CASCADE DEFERRABLE +Referenced by: + TABLE "own_aggregate_recent_contribution" CONSTRAINT "own_aggregate_recent_contribution_changed_file_path_id_fkey" FOREIGN KEY (changed_file_path_id) REFERENCES repo_paths(id) + TABLE "own_signal_recent_contribution" CONSTRAINT "own_signal_recent_contribution_changed_file_path_id_fkey" FOREIGN KEY (changed_file_path_id) REFERENCES repo_paths(id) + TABLE "repo_paths" CONSTRAINT "repo_paths_parent_id_fkey" FOREIGN KEY (parent_id) REFERENCES repo_paths(id) + +``` + +**absolute_path**: Absolute path does not start or end with forward slash. Example: "a/b/c". Root directory is empty path "". + # Table "public.repo_pending_permissions" ``` Column | Type | Collation | Nullable | Default diff --git a/migrations/frontend/squashed.sql b/migrations/frontend/squashed.sql index 1a48b154aae81..f2f64a6370725 100755 --- a/migrations/frontend/squashed.sql +++ b/migrations/frontend/squashed.sql @@ -819,6 +819,46 @@ CREATE FUNCTION update_codeintel_path_ranks_updated_at_column() RETURNS trigger END; $$; +CREATE FUNCTION update_own_aggregate_recent_contribution() RETURNS trigger + LANGUAGE plpgsql + AS $$ +BEGIN + WITH RECURSIVE ancestors AS ( + SELECT id, parent_id, 1 AS level + FROM repo_paths + WHERE id = NEW.changed_file_path_id + UNION ALL + SELECT p.id, p.parent_id, a.level + 1 + FROM repo_paths p + JOIN ancestors a ON p.id = a.parent_id + ) + UPDATE own_aggregate_recent_contribution + SET contributions_count = contributions_count + 1 + WHERE commit_author_id = NEW.commit_author_id AND changed_file_path_id IN ( + SELECT id FROM ancestors + ); + + WITH RECURSIVE ancestors AS ( + SELECT id, parent_id, 1 AS level + FROM repo_paths + WHERE id = NEW.changed_file_path_id + UNION ALL + SELECT p.id, p.parent_id, a.level + 1 + FROM repo_paths p + JOIN ancestors a ON p.id = a.parent_id + ) + INSERT INTO own_aggregate_recent_contribution (commit_author_id, changed_file_path_id, contributions_count) + SELECT NEW.commit_author_id, id, 1 + FROM ancestors + WHERE NOT EXISTS ( + SELECT 1 FROM own_aggregate_recent_contribution + WHERE commit_author_id = NEW.commit_author_id AND changed_file_path_id = ancestors.id + ); + + RETURN NEW; +END; +$$; + CREATE FUNCTION versions_insert_row_trigger() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -1785,6 +1825,22 @@ CREATE SEQUENCE codeowners_id_seq ALTER SEQUENCE codeowners_id_seq OWNED BY codeowners.id; +CREATE TABLE commit_authors ( + id integer NOT NULL, + email text NOT NULL, + name text NOT NULL +); + +CREATE SEQUENCE commit_authors_id_seq + AS integer + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE commit_authors_id_seq OWNED BY commit_authors.id; + CREATE TABLE configuration_policies_audit_logs ( log_timestamp timestamp with time zone DEFAULT clock_timestamp(), record_deleted_at timestamp with time zone, @@ -3465,6 +3521,43 @@ CREATE VIEW outbound_webhooks_with_event_types AS WHERE (outbound_webhook_event_types.outbound_webhook_id = outbound_webhooks.id))) AS event_types FROM outbound_webhooks; +CREATE TABLE own_aggregate_recent_contribution ( + id integer NOT NULL, + commit_author_id integer NOT NULL, + changed_file_path_id integer NOT NULL, + contributions_count integer DEFAULT 0 +); + +CREATE SEQUENCE own_aggregate_recent_contribution_id_seq + AS integer + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE own_aggregate_recent_contribution_id_seq OWNED BY own_aggregate_recent_contribution.id; + +CREATE TABLE own_signal_recent_contribution ( + id integer NOT NULL, + commit_author_id integer NOT NULL, + changed_file_path_id integer NOT NULL, + commit_timestamp timestamp without time zone NOT NULL, + commit_id_hash integer NOT NULL +); + +COMMENT ON TABLE own_signal_recent_contribution IS 'One entry per file changed in every commit that classifies as a contribution signal.'; + +CREATE SEQUENCE own_signal_recent_contribution_id_seq + AS integer + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE own_signal_recent_contribution_id_seq OWNED BY own_signal_recent_contribution.id; + CREATE TABLE package_repo_filters ( id integer NOT NULL, behaviour text NOT NULL, @@ -3789,6 +3882,26 @@ CREATE TABLE repo_kvps ( value text ); +CREATE TABLE repo_paths ( + id integer NOT NULL, + repo_id integer NOT NULL, + absolute_path text NOT NULL, + is_dir boolean NOT NULL, + parent_id integer +); + +COMMENT ON COLUMN repo_paths.absolute_path IS 'Absolute path does not start or end with forward slash. Example: "a/b/c". Root directory is empty path "".'; + +CREATE SEQUENCE repo_paths_id_seq + AS integer + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE repo_paths_id_seq OWNED BY repo_paths.id; + CREATE TABLE repo_pending_permissions ( repo_id integer NOT NULL, permission text NOT NULL, @@ -4449,6 +4562,8 @@ ALTER TABLE ONLY codeintel_ranking_references_processed ALTER COLUMN id SET DEFA ALTER TABLE ONLY codeowners ALTER COLUMN id SET DEFAULT nextval('codeowners_id_seq'::regclass); +ALTER TABLE ONLY commit_authors ALTER COLUMN id SET DEFAULT nextval('commit_authors_id_seq'::regclass); + ALTER TABLE ONLY configuration_policies_audit_logs ALTER COLUMN sequence SET DEFAULT nextval('configuration_policies_audit_logs_seq'::regclass); ALTER TABLE ONLY context_detection_embedding_jobs ALTER COLUMN id SET DEFAULT nextval('context_detection_embedding_jobs_id_seq'::regclass); @@ -4535,6 +4650,10 @@ ALTER TABLE ONLY outbound_webhook_logs ALTER COLUMN id SET DEFAULT nextval('outb ALTER TABLE ONLY outbound_webhooks ALTER COLUMN id SET DEFAULT nextval('outbound_webhooks_id_seq'::regclass); +ALTER TABLE ONLY own_aggregate_recent_contribution ALTER COLUMN id SET DEFAULT nextval('own_aggregate_recent_contribution_id_seq'::regclass); + +ALTER TABLE ONLY own_signal_recent_contribution ALTER COLUMN id SET DEFAULT nextval('own_signal_recent_contribution_id_seq'::regclass); + ALTER TABLE ONLY package_repo_filters ALTER COLUMN id SET DEFAULT nextval('package_repo_filters_id_seq'::regclass); ALTER TABLE ONLY package_repo_versions ALTER COLUMN id SET DEFAULT nextval('package_repo_versions_id_seq'::regclass); @@ -4553,6 +4672,8 @@ ALTER TABLE ONLY repo ALTER COLUMN id SET DEFAULT nextval('repo_id_seq'::regclas ALTER TABLE ONLY repo_embedding_jobs ALTER COLUMN id SET DEFAULT nextval('repo_embedding_jobs_id_seq'::regclass); +ALTER TABLE ONLY repo_paths ALTER COLUMN id SET DEFAULT nextval('repo_paths_id_seq'::regclass); + ALTER TABLE ONLY roles ALTER COLUMN id SET DEFAULT nextval('roles_id_seq'::regclass); ALTER TABLE ONLY saved_searches ALTER COLUMN id SET DEFAULT nextval('saved_searches_id_seq'::regclass); @@ -4723,6 +4844,9 @@ ALTER TABLE ONLY codeowners ALTER TABLE ONLY codeowners ADD CONSTRAINT codeowners_repo_id_key UNIQUE (repo_id); +ALTER TABLE ONLY commit_authors + ADD CONSTRAINT commit_authors_pkey PRIMARY KEY (id); + ALTER TABLE ONLY context_detection_embedding_jobs ADD CONSTRAINT context_detection_embedding_jobs_pkey PRIMARY KEY (id); @@ -4912,6 +5036,12 @@ ALTER TABLE ONLY outbound_webhook_logs ALTER TABLE ONLY outbound_webhooks ADD CONSTRAINT outbound_webhooks_pkey PRIMARY KEY (id); +ALTER TABLE ONLY own_aggregate_recent_contribution + ADD CONSTRAINT own_aggregate_recent_contribution_pkey PRIMARY KEY (id); + +ALTER TABLE ONLY own_signal_recent_contribution + ADD CONSTRAINT own_signal_recent_contribution_pkey PRIMARY KEY (id); + ALTER TABLE ONLY package_repo_filters ADD CONSTRAINT package_repo_filters_pkey PRIMARY KEY (id); @@ -4954,6 +5084,9 @@ ALTER TABLE ONLY repo_kvps ALTER TABLE ONLY repo ADD CONSTRAINT repo_name_unique UNIQUE (name) DEFERRABLE; +ALTER TABLE ONLY repo_paths + ADD CONSTRAINT repo_paths_pkey PRIMARY KEY (id); + ALTER TABLE ONLY repo_pending_permissions ADD CONSTRAINT repo_pending_permissions_perm_unique UNIQUE (repo_id, permission); @@ -5191,6 +5324,8 @@ CREATE INDEX codeintel_ranking_references_processed_reference_id ON codeintel_ra CREATE INDEX codeintel_ranking_references_upload_id ON codeintel_ranking_references USING btree (upload_id); +CREATE UNIQUE INDEX commit_authors_email_name ON commit_authors USING btree (email, name); + CREATE INDEX configuration_policies_audit_logs_policy_id ON configuration_policies_audit_logs USING btree (policy_id); CREATE INDEX configuration_policies_audit_logs_timestamp ON configuration_policies_audit_logs USING brin (log_timestamp); @@ -5395,6 +5530,8 @@ CREATE INDEX outbound_webhook_payload_process_after_idx ON outbound_webhook_jobs CREATE INDEX outbound_webhooks_logs_status_code_idx ON outbound_webhook_logs USING btree (status_code); +CREATE UNIQUE INDEX own_aggregate_recent_contribution_author_file ON own_aggregate_recent_contribution USING btree (commit_author_id, changed_file_path_id); + CREATE UNIQUE INDEX package_repo_filters_unique_matcher_per_scheme ON package_repo_filters USING btree (scheme, matcher); CREATE INDEX package_repo_versions_blocked ON package_repo_versions USING btree (blocked); @@ -5455,6 +5592,8 @@ CREATE INDEX repo_name_trgm ON repo USING gin (lower((name)::text) gin_trgm_ops) CREATE INDEX repo_non_deleted_id_name_idx ON repo USING btree (id, name) WHERE (deleted_at IS NULL); +CREATE UNIQUE INDEX repo_paths_index_absolute_path ON repo_paths USING btree (repo_id, absolute_path); + CREATE INDEX repo_permissions_unrestricted_true_idx ON repo_permissions USING btree (unrestricted) WHERE unrestricted; CREATE INDEX repo_private ON repo USING btree (private); @@ -5591,6 +5730,8 @@ CREATE TRIGGER update_codeintel_path_ranks_statistics BEFORE UPDATE ON codeintel CREATE TRIGGER update_codeintel_path_ranks_updated_at BEFORE UPDATE ON codeintel_path_ranks FOR EACH ROW WHEN ((new.* IS DISTINCT FROM old.*)) EXECUTE FUNCTION update_codeintel_path_ranks_updated_at_column(); +CREATE TRIGGER update_own_aggregate_recent_contribution AFTER INSERT ON own_signal_recent_contribution FOR EACH ROW EXECUTE FUNCTION update_own_aggregate_recent_contribution(); + CREATE TRIGGER versions_insert BEFORE INSERT ON versions FOR EACH ROW EXECUTE FUNCTION versions_insert_row_trigger(); ALTER TABLE ONLY access_requests @@ -5950,6 +6091,18 @@ ALTER TABLE ONLY outbound_webhooks ALTER TABLE ONLY outbound_webhooks ADD CONSTRAINT outbound_webhooks_updated_by_fkey FOREIGN KEY (updated_by) REFERENCES users(id) ON DELETE SET NULL; +ALTER TABLE ONLY own_aggregate_recent_contribution + ADD CONSTRAINT own_aggregate_recent_contribution_changed_file_path_id_fkey FOREIGN KEY (changed_file_path_id) REFERENCES repo_paths(id); + +ALTER TABLE ONLY own_aggregate_recent_contribution + ADD CONSTRAINT own_aggregate_recent_contribution_commit_author_id_fkey FOREIGN KEY (commit_author_id) REFERENCES commit_authors(id); + +ALTER TABLE ONLY own_signal_recent_contribution + ADD CONSTRAINT own_signal_recent_contribution_changed_file_path_id_fkey FOREIGN KEY (changed_file_path_id) REFERENCES repo_paths(id); + +ALTER TABLE ONLY own_signal_recent_contribution + ADD CONSTRAINT own_signal_recent_contribution_commit_author_id_fkey FOREIGN KEY (commit_author_id) REFERENCES commit_authors(id); + ALTER TABLE ONLY package_repo_versions ADD CONSTRAINT package_id_fk FOREIGN KEY (package_id) REFERENCES lsif_dependency_repos(id) ON DELETE CASCADE; @@ -5983,6 +6136,12 @@ ALTER TABLE ONLY registry_extensions ALTER TABLE ONLY repo_kvps ADD CONSTRAINT repo_kvps_repo_id_fkey FOREIGN KEY (repo_id) REFERENCES repo(id) ON DELETE CASCADE; +ALTER TABLE ONLY repo_paths + ADD CONSTRAINT repo_paths_parent_id_fkey FOREIGN KEY (parent_id) REFERENCES repo_paths(id); + +ALTER TABLE ONLY repo_paths + ADD CONSTRAINT repo_paths_repo_id_fkey FOREIGN KEY (repo_id) REFERENCES repo(id) ON DELETE CASCADE DEFERRABLE; + ALTER TABLE ONLY role_permissions ADD CONSTRAINT role_permissions_permission_id_fkey FOREIGN KEY (permission_id) REFERENCES permissions(id) ON DELETE CASCADE DEFERRABLE; From 73e0ff97202f02d2afe7bb633394759fd1ec5a4d Mon Sep 17 00:00:00 2001 From: Cezary Bartoszuk Date: Fri, 21 Apr 2023 09:44:49 -0500 Subject: [PATCH 03/30] Experimenting with inserting paths --- internal/database/commit_signals.go | 29 +++++++++++++++++-- .../1681300431_ownership_signals/up.sql | 1 - 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/internal/database/commit_signals.go b/internal/database/commit_signals.go index d3e9bb918fc77..bd872cdd58340 100644 --- a/internal/database/commit_signals.go +++ b/internal/database/commit_signals.go @@ -64,7 +64,6 @@ func ensureRepoPaths(ctx context.Context, tx *basestore.Store, commit Commit) ([ var pathID int for j := range pathPrefixes { pathPrefix := strings.Join(pathPrefixes[:j+1], "/") - isDir := j < len(pathPrefixes)-1 // Get or create repo path err := tx.QueryRow( ctx, @@ -78,8 +77,8 @@ func ensureRepoPaths(ctx context.Context, tx *basestore.Store, commit Commit) ([ err = tx.QueryRow( ctx, sqlf.Sprintf(` - INSERT INTO repo_paths (repo_id, absolute_path, is_dir, parent_id) - VALUES (%s, %s, %s, %s) RETURNING id`, commit.RepoID, pathPrefix, isDir, parentPathID), + INSERT INTO repo_paths (repo_id, absolute_path, parent_id) + VALUES (%s, %s, %s) RETURNING id`, commit.RepoID, pathPrefix, parentPathID), ).Scan(&pathID) } if err != nil { @@ -92,6 +91,30 @@ func ensureRepoPaths(ctx context.Context, tx *basestore.Store, commit Commit) ([ return pathIDs, nil } +const pathInsertFmtstr = ` + inderted__ AS ( + INSERT INTO repo_paths (repo_id, absolute_path, is_dir, parent_id) + VALUES (%%s, %%s, %%s, (SELECT id FROM selected_previous)) + ON CONFLICT DO NOTHING + RETURNING id + ), + selected__ AS ( + SELECT id + FROM inserted__ + + UNION ALL + + SELECT id + FROM repo_paths + WHERE repo_id = %%s + AND absolute_path = %%s + ) +` + +func ensureRepoPaths2(ctx context.Context, tx *basestore.Store, commit Commit) ([]int, error) { + return nil, nil +} + func (s *ownSignalStore) AddCommit(ctx context.Context, commit Commit) error { tx := s.Store // Get or create commit author diff --git a/migrations/frontend/1681300431_ownership_signals/up.sql b/migrations/frontend/1681300431_ownership_signals/up.sql index 9f4d5950b5e48..cd6fff2b449db 100644 --- a/migrations/frontend/1681300431_ownership_signals/up.sql +++ b/migrations/frontend/1681300431_ownership_signals/up.sql @@ -2,7 +2,6 @@ CREATE TABLE IF NOT EXISTS repo_paths ( id SERIAL PRIMARY KEY, repo_id INTEGER NOT NULL REFERENCES repo(id) ON DELETE CASCADE DEFERRABLE, absolute_path TEXT NOT NULL, - is_dir BOOLEAN NOT NULL, parent_id INTEGER NULL REFERENCES repo_paths(id) ); From e5bfdc9ccfdee590ca47e57f41eb8e7100c2b24d Mon Sep 17 00:00:00 2001 From: Cezary Bartoszuk Date: Fri, 21 Apr 2023 12:27:41 -0500 Subject: [PATCH 04/30] Clean up commit_signals.go --- internal/database/commit_signals.go | 227 +++++++++++++++++----------- 1 file changed, 137 insertions(+), 90 deletions(-) diff --git a/internal/database/commit_signals.go b/internal/database/commit_signals.go index bd872cdd58340..4b284f56acb25 100644 --- a/internal/database/commit_signals.go +++ b/internal/database/commit_signals.go @@ -4,7 +4,7 @@ import ( "context" "database/sql" "hash/fnv" - "strings" + "path" "time" "github.com/keegancsmith/sqlf" @@ -16,14 +16,15 @@ import ( "github.com/sourcegraph/sourcegraph/lib/errors" ) -///// -// TODO: There is no support for directory root now. The fix is simple though. - type OwnSignalStore interface { AddCommit(ctx context.Context, commit Commit) error FindRecentAuthors(ctx context.Context, repoID api.RepoID, path string) ([]RecentAuthor, error) } +func OwnSignalsStoreWith(other basestore.ShareableStore) OwnSignalStore { + return &ownSignalStore{Store: basestore.NewWithHandle(other.Handle())} +} + type Commit struct { RepoID api.RepoID AuthorName string @@ -33,19 +34,28 @@ type Commit struct { FilesChanged []string } +type RecentAuthor struct { + AuthorName string + AuthorEmail string + ContributionCount int +} + type ownSignalStore struct { logger log.Logger *basestore.Store } -func ensureAuthor(ctx context.Context, tx *basestore.Store, commit Commit) (int, error) { +// TODO: Use one query for author. + +func (s *ownSignalStore) ensureAuthor(ctx context.Context, commit Commit) (int, error) { + db := s.Store var authorID int - err := tx.QueryRow( + err := db.QueryRow( ctx, sqlf.Sprintf(`SELECT id FROM commit_authors WHERE email = %s AND name = %s`, commit.AuthorEmail, commit.AuthorName), ).Scan(&authorID) if err == sql.ErrNoRows { - err = tx.QueryRow( + err = db.QueryRow( ctx, sqlf.Sprintf(`INSERT INTO commit_authors (email, name) VALUES (%s, %s) RETURNING id`, commit.AuthorEmail, commit.AuthorName), ).Scan(&authorID) @@ -56,116 +66,157 @@ func ensureAuthor(ctx context.Context, tx *basestore.Store, commit Commit) (int, return authorID, nil } -func ensureRepoPaths(ctx context.Context, tx *basestore.Store, commit Commit) ([]int, error) { - pathIDs := make([]int, len(commit.FilesChanged)) - for i, path := range commit.FilesChanged { - pathPrefixes := strings.Split(path, "/") - var parentPathID *int - var pathID int - for j := range pathPrefixes { - pathPrefix := strings.Join(pathPrefixes[:j+1], "/") - // Get or create repo path - err := tx.QueryRow( - ctx, - sqlf.Sprintf(` - SELECT id FROM repo_paths - WHERE repo_id = %s - AND absolute_path = %s - `, commit.RepoID, pathPrefix), - ).Scan(&pathID) - if err == sql.ErrNoRows { - err = tx.QueryRow( - ctx, - sqlf.Sprintf(` - INSERT INTO repo_paths (repo_id, absolute_path, parent_id) - VALUES (%s, %s, %s) RETURNING id`, commit.RepoID, pathPrefix, parentPathID), - ).Scan(&pathID) - } - if err != nil { - return nil, err - } - parentPathID = &pathID - } - pathIDs[i] = pathID - } - return pathIDs, nil -} - const pathInsertFmtstr = ` - inderted__ AS ( - INSERT INTO repo_paths (repo_id, absolute_path, is_dir, parent_id) - VALUES (%%s, %%s, %%s, (SELECT id FROM selected_previous)) - ON CONFLICT DO NOTHING - RETURNING id - ), - selected__ AS ( - SELECT id - FROM inserted__ - - UNION ALL - + WITH already_exists (id) AS ( SELECT id FROM repo_paths - WHERE repo_id = %%s - AND absolute_path = %%s + WHERE repo_id = %s + AND absolute_path = %s + ), + need_to_insert (id) AS ( + INSERT INTO repo_paths (repo_id, absolute_path, parent_id) + VALUES (%s, %s, %s) + ON CONFLICT (repo_id, absolute_path) DO NOTHING + RETURNING id ) -` - -func ensureRepoPaths2(ctx context.Context, tx *basestore.Store, commit Commit) ([]int, error) { - return nil, nil + SELECT id FROM already_exists + UNION ALL + SELECT id FROM need_to_insert` + +// ensureRepoPaths takes paths of files changed in the given commit +// and makes sure they all exist in the database (alongside with their ancestor paths) +// as per the schema. +// +// The operation makes a number of queries to the database that is comparable +// to the size of the given file tree. In other words, every directory mentioned +// in the `commit.FilesChanged` (including parents and ancestors) will be queried +// or inserted with a single query (no repetitions though). +// Optimizing this into fewer queries seems to make the implementation very hard to read. +// +// The result int slice is guaranteed to be in order corresponding to the order +// of `commit.FilesChanged`. +func (s *ownSignalStore) ensureRepoPaths(ctx context.Context, commit Commit) ([]int, error) { + db := s.Store + // compute all the ancestor paths for all changed files in a commit + var paths []string + for _, file := range commit.FilesChanged { + for p := file; p != "."; p = path.Dir(p) { + paths = append(paths, p) + } + } + // add empty string which references the repo root directory + paths = append(paths, "") + // reverse paths so we start at the root + for i := 0; i < len(paths)/2; i++ { + j := len(paths) - i - 1 + paths[i], paths[j] = paths[j], paths[i] + } + // remove duplicates from paths, to avoid extra query, + // especially if many files within the same directory structure + // are referenced + seen := make(map[string]bool) + j := 0 + for i := 0; i < len(paths); i++ { + if !seen[paths[i]] { + seen[paths[i]] = true + paths[j] = paths[i] + j++ + } + } + paths = paths[:j] + // insert all directories one query each and note the IDs + ids := map[string]int{} + for _, p := range paths { + var parentID *int + parent := path.Dir(p) + if parent == "." { + parent = "" + } + if id, ok := ids[parent]; p != "" && ok { + parentID = &id + } else if p != "" { + return nil, errors.Newf("cannot find parent id of %q: this is a bug", p) + } + r := db.QueryRow(ctx, sqlf.Sprintf(pathInsertFmtstr, commit.RepoID, p, commit.RepoID, p, parentID)) + var id int + if err := r.Scan(&id); err != nil { + return nil, errors.Wrapf(err, "failed to insert or retrieve %q", p) + } + ids[p] = id + } + // return the IDs of inserted files changed, in order + fIDs := make([]int, len(commit.FilesChanged)) + for i, f := range commit.FilesChanged { + id, ok := ids[f] + if !ok { + return nil, errors.Newf("cannot find id of %q which should have been inserted, this is a bug", f) + } + fIDs[i] = id + } + return fIDs, nil } +const insertRecentContributorSignalFmtstr = ` + INSERT INTO own_signal_recent_contribution ( + commit_author_id, + changed_file_path_id, + commit_timestamp, + commit_id_hash + ) VALUES (%s, %s, %s, %s) +` + +// AddCommit inserts a recent contribution signal for each file changed by given commit. +// +// As per schema, `commit_id_hash` is just a int hash of the git SHA. +// This is used for the purpose of removing old recent contributor signals. +// The aggregate signals in `own_aggregate_recent_contribution` are updated atomically +// for each new signal appearing in `own_signal_recent_contribution` by using +// a trigger: `update_own_aggregate_recent_contribution`. func (s *ownSignalStore) AddCommit(ctx context.Context, commit Commit) error { - tx := s.Store - // Get or create commit author - authorID, err := ensureAuthor(ctx, tx, commit) + db := s.Store + // Get or create commit author: + authorID, err := s.ensureAuthor(ctx, commit) if err != nil { return errors.Wrap(err, "cannot insert commit author") } - // Get or create repo paths - pathIDs, err := ensureRepoPaths(ctx, tx, commit) + // Get or create necessary repo paths: + pathIDs, err := s.ensureRepoPaths(ctx, commit) if err != nil { return errors.Wrap(err, "cannot insert repo paths") } - // Insert into own_signal_recent_contribution + // Insert individual signals into own_signal_recent_contribution: for _, pathID := range pathIDs { commitID := fnv.New32a() commitID.Write([]byte(commit.CommitSHA)) - q := sqlf.Sprintf(`INSERT INTO own_signal_recent_contribution (commit_author_id, changed_file_path_id, - commit_timestamp, commit_id_hash) VALUES (%s, %s, %s, %s)`, + q := sqlf.Sprintf(insertRecentContributorSignalFmtstr, authorID, pathID, commit.Timestamp, commitID.Sum32(), ) - err = tx.Exec(ctx, q) + err = db.Exec(ctx, q) if err != nil { return err } } - return nil } -type RecentAuthor struct { - AuthorName string - AuthorEmail string - ContributionCount int -} +const findRecentContributorsFmtstr = ` + SELECT a.name, a.email, g.contributions_count + FROM commit_authors AS a + INNER JOIN own_aggregate_recent_contribution AS g + ON a.id = g.commit_author_id + INNER JOIN repo_paths AS p + ON p.id = g.changed_file_path_id + WHERE p.repo_id = %s + AND p.absolute_path = %s + ORDER BY 3 DESC +` func (s *ownSignalStore) FindRecentAuthors(ctx context.Context, repoID api.RepoID, path string) ([]RecentAuthor, error) { var authors []RecentAuthor - q := sqlf.Sprintf(` - SELECT a.name, a.email, g.contributions_count - FROM commit_authors AS a - INNER JOIN own_aggregate_recent_contribution AS g - ON a.id = g.commit_author_id - INNER JOIN repo_paths AS p - ON p.id = g.changed_file_path_id - WHERE p.repo_id = %s - AND p.absolute_path = %s - ORDER BY 3 DESC - `, repoID, path) + q := sqlf.Sprintf(findRecentContributorsFmtstr, repoID, path) rows, err := s.Query(ctx, q) if err != nil { return nil, err @@ -180,7 +231,3 @@ func (s *ownSignalStore) FindRecentAuthors(ctx context.Context, repoID api.RepoI } return authors, nil } - -func OwnSignalsStoreWith(other basestore.ShareableStore) OwnSignalStore { - return &ownSignalStore{Store: basestore.NewWithHandle(other.Handle())} -} From 64806368680fa13cd103332f33c7831f6491f2e8 Mon Sep 17 00:00:00 2001 From: Cezary Bartoszuk Date: Fri, 21 Apr 2023 13:16:23 -0500 Subject: [PATCH 05/30] rename RecentContributionSignalStore --- enterprise/internal/database/mocks_temp.go | 235 +++++++++++---------- internal/database/commit_signals.go | 4 +- internal/database/commit_signals_test.go | 2 +- internal/database/database.go | 6 +- internal/database/mocks_temp.go | 231 ++++++++++---------- internal/database/schema.json | 15 +- internal/database/schema.md | 1 - migrations/frontend/squashed.sql | 1 - 8 files changed, 244 insertions(+), 251 deletions(-) diff --git a/enterprise/internal/database/mocks_temp.go b/enterprise/internal/database/mocks_temp.go index ba1048c1c3538..1220244ceaad2 100644 --- a/enterprise/internal/database/mocks_temp.go +++ b/enterprise/internal/database/mocks_temp.go @@ -7920,9 +7920,6 @@ type MockEnterpriseDB struct { // OutboundWebhooksFunc is an instance of a mock function object // controlling the behavior of the method OutboundWebhooks. OutboundWebhooksFunc *EnterpriseDBOutboundWebhooksFunc - // OwnSignalsFunc is an instance of a mock function object controlling - // the behavior of the method OwnSignals. - OwnSignalsFunc *EnterpriseDBOwnSignalsFunc // PermissionSyncJobsFunc is an instance of a mock function object // controlling the behavior of the method PermissionSyncJobs. PermissionSyncJobsFunc *EnterpriseDBPermissionSyncJobsFunc @@ -7941,6 +7938,10 @@ type MockEnterpriseDB struct { // QueryRowContextFunc is an instance of a mock function object // controlling the behavior of the method QueryRowContext. QueryRowContextFunc *EnterpriseDBQueryRowContextFunc + // RecentContributionSignalsFunc is an instance of a mock function + // object controlling the behavior of the method + // RecentContributionSignals. + RecentContributionSignalsFunc *EnterpriseDBRecentContributionSignalsFunc // RedisKeyValueFunc is an instance of a mock function object // controlling the behavior of the method RedisKeyValue. RedisKeyValueFunc *EnterpriseDBRedisKeyValueFunc @@ -8148,11 +8149,6 @@ func NewMockEnterpriseDB() *MockEnterpriseDB { return }, }, - OwnSignalsFunc: &EnterpriseDBOwnSignalsFunc{ - defaultHook: func() (r0 database.OwnSignalStore) { - return - }, - }, PermissionSyncJobsFunc: &EnterpriseDBPermissionSyncJobsFunc{ defaultHook: func() (r0 database.PermissionSyncJobStore) { return @@ -8183,6 +8179,11 @@ func NewMockEnterpriseDB() *MockEnterpriseDB { return }, }, + RecentContributionSignalsFunc: &EnterpriseDBRecentContributionSignalsFunc{ + defaultHook: func() (r0 database.RecentContributionSignalStore) { + return + }, + }, RedisKeyValueFunc: &EnterpriseDBRedisKeyValueFunc{ defaultHook: func() (r0 database.RedisKeyValueStore) { return @@ -8435,11 +8436,6 @@ func NewStrictMockEnterpriseDB() *MockEnterpriseDB { panic("unexpected invocation of MockEnterpriseDB.OutboundWebhooks") }, }, - OwnSignalsFunc: &EnterpriseDBOwnSignalsFunc{ - defaultHook: func() database.OwnSignalStore { - panic("unexpected invocation of MockEnterpriseDB.OwnSignals") - }, - }, PermissionSyncJobsFunc: &EnterpriseDBPermissionSyncJobsFunc{ defaultHook: func() database.PermissionSyncJobStore { panic("unexpected invocation of MockEnterpriseDB.PermissionSyncJobs") @@ -8470,6 +8466,11 @@ func NewStrictMockEnterpriseDB() *MockEnterpriseDB { panic("unexpected invocation of MockEnterpriseDB.QueryRowContext") }, }, + RecentContributionSignalsFunc: &EnterpriseDBRecentContributionSignalsFunc{ + defaultHook: func() database.RecentContributionSignalStore { + panic("unexpected invocation of MockEnterpriseDB.RecentContributionSignals") + }, + }, RedisKeyValueFunc: &EnterpriseDBRedisKeyValueFunc{ defaultHook: func() database.RedisKeyValueStore { panic("unexpected invocation of MockEnterpriseDB.RedisKeyValue") @@ -8669,9 +8670,6 @@ func NewMockEnterpriseDBFrom(i EnterpriseDB) *MockEnterpriseDB { OutboundWebhooksFunc: &EnterpriseDBOutboundWebhooksFunc{ defaultHook: i.OutboundWebhooks, }, - OwnSignalsFunc: &EnterpriseDBOwnSignalsFunc{ - defaultHook: i.OwnSignals, - }, PermissionSyncJobsFunc: &EnterpriseDBPermissionSyncJobsFunc{ defaultHook: i.PermissionSyncJobs, }, @@ -8690,6 +8688,9 @@ func NewMockEnterpriseDBFrom(i EnterpriseDB) *MockEnterpriseDB { QueryRowContextFunc: &EnterpriseDBQueryRowContextFunc{ defaultHook: i.QueryRowContext, }, + RecentContributionSignalsFunc: &EnterpriseDBRecentContributionSignalsFunc{ + defaultHook: i.RecentContributionSignals, + }, RedisKeyValueFunc: &EnterpriseDBRedisKeyValueFunc{ defaultHook: i.RedisKeyValue, }, @@ -11482,105 +11483,6 @@ func (c EnterpriseDBOutboundWebhooksFuncCall) Results() []interface{} { return []interface{}{c.Result0} } -// EnterpriseDBOwnSignalsFunc describes the behavior when the OwnSignals -// method of the parent MockEnterpriseDB instance is invoked. -type EnterpriseDBOwnSignalsFunc struct { - defaultHook func() database.OwnSignalStore - hooks []func() database.OwnSignalStore - history []EnterpriseDBOwnSignalsFuncCall - mutex sync.Mutex -} - -// OwnSignals delegates to the next hook function in the queue and stores -// the parameter and result values of this invocation. -func (m *MockEnterpriseDB) OwnSignals() database.OwnSignalStore { - r0 := m.OwnSignalsFunc.nextHook()() - m.OwnSignalsFunc.appendCall(EnterpriseDBOwnSignalsFuncCall{r0}) - return r0 -} - -// SetDefaultHook sets function that is called when the OwnSignals method of -// the parent MockEnterpriseDB instance is invoked and the hook queue is -// empty. -func (f *EnterpriseDBOwnSignalsFunc) SetDefaultHook(hook func() database.OwnSignalStore) { - f.defaultHook = hook -} - -// PushHook adds a function to the end of hook queue. Each invocation of the -// OwnSignals method of the parent MockEnterpriseDB instance invokes the -// hook at the front of the queue and discards it. After the queue is empty, -// the default hook function is invoked for any future action. -func (f *EnterpriseDBOwnSignalsFunc) PushHook(hook func() database.OwnSignalStore) { - f.mutex.Lock() - f.hooks = append(f.hooks, hook) - f.mutex.Unlock() -} - -// SetDefaultReturn calls SetDefaultHook with a function that returns the -// given values. -func (f *EnterpriseDBOwnSignalsFunc) SetDefaultReturn(r0 database.OwnSignalStore) { - f.SetDefaultHook(func() database.OwnSignalStore { - return r0 - }) -} - -// PushReturn calls PushHook with a function that returns the given values. -func (f *EnterpriseDBOwnSignalsFunc) PushReturn(r0 database.OwnSignalStore) { - f.PushHook(func() database.OwnSignalStore { - return r0 - }) -} - -func (f *EnterpriseDBOwnSignalsFunc) nextHook() func() database.OwnSignalStore { - f.mutex.Lock() - defer f.mutex.Unlock() - - if len(f.hooks) == 0 { - return f.defaultHook - } - - hook := f.hooks[0] - f.hooks = f.hooks[1:] - return hook -} - -func (f *EnterpriseDBOwnSignalsFunc) appendCall(r0 EnterpriseDBOwnSignalsFuncCall) { - f.mutex.Lock() - f.history = append(f.history, r0) - f.mutex.Unlock() -} - -// History returns a sequence of EnterpriseDBOwnSignalsFuncCall objects -// describing the invocations of this function. -func (f *EnterpriseDBOwnSignalsFunc) History() []EnterpriseDBOwnSignalsFuncCall { - f.mutex.Lock() - history := make([]EnterpriseDBOwnSignalsFuncCall, len(f.history)) - copy(history, f.history) - f.mutex.Unlock() - - return history -} - -// EnterpriseDBOwnSignalsFuncCall is an object that describes an invocation -// of method OwnSignals on an instance of MockEnterpriseDB. -type EnterpriseDBOwnSignalsFuncCall struct { - // Result0 is the value of the 1st result returned from this method - // invocation. - Result0 database.OwnSignalStore -} - -// Args returns an interface slice containing the arguments of this -// invocation. -func (c EnterpriseDBOwnSignalsFuncCall) Args() []interface{} { - return []interface{}{} -} - -// Results returns an interface slice containing the results of this -// invocation. -func (c EnterpriseDBOwnSignalsFuncCall) Results() []interface{} { - return []interface{}{c.Result0} -} - // EnterpriseDBPermissionSyncJobsFunc describes the behavior when the // PermissionSyncJobs method of the parent MockEnterpriseDB instance is // invoked. @@ -12212,6 +12114,109 @@ func (c EnterpriseDBQueryRowContextFuncCall) Results() []interface{} { return []interface{}{c.Result0} } +// EnterpriseDBRecentContributionSignalsFunc describes the behavior when the +// RecentContributionSignals method of the parent MockEnterpriseDB instance +// is invoked. +type EnterpriseDBRecentContributionSignalsFunc struct { + defaultHook func() database.RecentContributionSignalStore + hooks []func() database.RecentContributionSignalStore + history []EnterpriseDBRecentContributionSignalsFuncCall + mutex sync.Mutex +} + +// RecentContributionSignals delegates to the next hook function in the +// queue and stores the parameter and result values of this invocation. +func (m *MockEnterpriseDB) RecentContributionSignals() database.RecentContributionSignalStore { + r0 := m.RecentContributionSignalsFunc.nextHook()() + m.RecentContributionSignalsFunc.appendCall(EnterpriseDBRecentContributionSignalsFuncCall{r0}) + return r0 +} + +// SetDefaultHook sets function that is called when the +// RecentContributionSignals method of the parent MockEnterpriseDB instance +// is invoked and the hook queue is empty. +func (f *EnterpriseDBRecentContributionSignalsFunc) SetDefaultHook(hook func() database.RecentContributionSignalStore) { + f.defaultHook = hook +} + +// PushHook adds a function to the end of hook queue. Each invocation of the +// RecentContributionSignals method of the parent MockEnterpriseDB instance +// invokes the hook at the front of the queue and discards it. After the +// queue is empty, the default hook function is invoked for any future +// action. +func (f *EnterpriseDBRecentContributionSignalsFunc) PushHook(hook func() database.RecentContributionSignalStore) { + f.mutex.Lock() + f.hooks = append(f.hooks, hook) + f.mutex.Unlock() +} + +// SetDefaultReturn calls SetDefaultHook with a function that returns the +// given values. +func (f *EnterpriseDBRecentContributionSignalsFunc) SetDefaultReturn(r0 database.RecentContributionSignalStore) { + f.SetDefaultHook(func() database.RecentContributionSignalStore { + return r0 + }) +} + +// PushReturn calls PushHook with a function that returns the given values. +func (f *EnterpriseDBRecentContributionSignalsFunc) PushReturn(r0 database.RecentContributionSignalStore) { + f.PushHook(func() database.RecentContributionSignalStore { + return r0 + }) +} + +func (f *EnterpriseDBRecentContributionSignalsFunc) nextHook() func() database.RecentContributionSignalStore { + f.mutex.Lock() + defer f.mutex.Unlock() + + if len(f.hooks) == 0 { + return f.defaultHook + } + + hook := f.hooks[0] + f.hooks = f.hooks[1:] + return hook +} + +func (f *EnterpriseDBRecentContributionSignalsFunc) appendCall(r0 EnterpriseDBRecentContributionSignalsFuncCall) { + f.mutex.Lock() + f.history = append(f.history, r0) + f.mutex.Unlock() +} + +// History returns a sequence of +// EnterpriseDBRecentContributionSignalsFuncCall objects describing the +// invocations of this function. +func (f *EnterpriseDBRecentContributionSignalsFunc) History() []EnterpriseDBRecentContributionSignalsFuncCall { + f.mutex.Lock() + history := make([]EnterpriseDBRecentContributionSignalsFuncCall, len(f.history)) + copy(history, f.history) + f.mutex.Unlock() + + return history +} + +// EnterpriseDBRecentContributionSignalsFuncCall is an object that describes +// an invocation of method RecentContributionSignals on an instance of +// MockEnterpriseDB. +type EnterpriseDBRecentContributionSignalsFuncCall struct { + // Result0 is the value of the 1st result returned from this method + // invocation. + Result0 database.RecentContributionSignalStore +} + +// Args returns an interface slice containing the arguments of this +// invocation. +func (c EnterpriseDBRecentContributionSignalsFuncCall) Args() []interface{} { + return []interface{}{} +} + +// Results returns an interface slice containing the results of this +// invocation. +func (c EnterpriseDBRecentContributionSignalsFuncCall) Results() []interface{} { + return []interface{}{c.Result0} +} + // EnterpriseDBRedisKeyValueFunc describes the behavior when the // RedisKeyValue method of the parent MockEnterpriseDB instance is invoked. type EnterpriseDBRedisKeyValueFunc struct { diff --git a/internal/database/commit_signals.go b/internal/database/commit_signals.go index 4b284f56acb25..772459270b830 100644 --- a/internal/database/commit_signals.go +++ b/internal/database/commit_signals.go @@ -16,12 +16,12 @@ import ( "github.com/sourcegraph/sourcegraph/lib/errors" ) -type OwnSignalStore interface { +type RecentContributionSignalStore interface { AddCommit(ctx context.Context, commit Commit) error FindRecentAuthors(ctx context.Context, repoID api.RepoID, path string) ([]RecentAuthor, error) } -func OwnSignalsStoreWith(other basestore.ShareableStore) OwnSignalStore { +func RecentContributionSignalStoreWith(other basestore.ShareableStore) RecentContributionSignalStore { return &ownSignalStore{Store: basestore.NewWithHandle(other.Handle())} } diff --git a/internal/database/commit_signals_test.go b/internal/database/commit_signals_test.go index c4508b8a1862f..13345cbccf1e3 100644 --- a/internal/database/commit_signals_test.go +++ b/internal/database/commit_signals_test.go @@ -21,7 +21,7 @@ func TestOwnSignalStore_AddCommit(t *testing.T) { t.Parallel() logger := logtest.Scoped(t) db := NewDB(logger, dbtest.NewDB(logger, t)) - store := OwnSignalsStoreWith(db) + store := RecentContributionSignalStoreWith(db) ctx := context.Background() repo := mustCreate(ctx, t, db, &types.Repo{Name: "a/b"}) diff --git a/internal/database/database.go b/internal/database/database.go index d5e288c4747af..6ac27057773c3 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -40,7 +40,7 @@ type DB interface { OutboundWebhooks(encryption.Key) OutboundWebhookStore OutboundWebhookJobs(encryption.Key) OutboundWebhookJobStore OutboundWebhookLogs(encryption.Key) OutboundWebhookLogStore - OwnSignals() OwnSignalStore + RecentContributionSignals() RecentContributionSignalStore Permissions() PermissionStore PermissionSyncJobs() PermissionSyncJobStore Phabricator() PhabricatorStore @@ -200,8 +200,8 @@ func (d *db) OutboundWebhookLogs(key encryption.Key) OutboundWebhookLogStore { return OutboundWebhookLogsWith(d.Store, key) } -func (d *db) OwnSignals() OwnSignalStore { - return OwnSignalsStoreWith(d.Store) +func (d *db) RecentContributionSignals() RecentContributionSignalStore { + return RecentContributionSignalStoreWith(d.Store) } func (d *db) Permissions() PermissionStore { diff --git a/internal/database/mocks_temp.go b/internal/database/mocks_temp.go index bae50dbd9f795..8dea3a1aeac66 100644 --- a/internal/database/mocks_temp.go +++ b/internal/database/mocks_temp.go @@ -4954,9 +4954,6 @@ type MockDB struct { // OutboundWebhooksFunc is an instance of a mock function object // controlling the behavior of the method OutboundWebhooks. OutboundWebhooksFunc *DBOutboundWebhooksFunc - // OwnSignalsFunc is an instance of a mock function object controlling - // the behavior of the method OwnSignals. - OwnSignalsFunc *DBOwnSignalsFunc // PermissionSyncJobsFunc is an instance of a mock function object // controlling the behavior of the method PermissionSyncJobs. PermissionSyncJobsFunc *DBPermissionSyncJobsFunc @@ -4972,6 +4969,10 @@ type MockDB struct { // QueryRowContextFunc is an instance of a mock function object // controlling the behavior of the method QueryRowContext. QueryRowContextFunc *DBQueryRowContextFunc + // RecentContributionSignalsFunc is an instance of a mock function + // object controlling the behavior of the method + // RecentContributionSignals. + RecentContributionSignalsFunc *DBRecentContributionSignalsFunc // RedisKeyValueFunc is an instance of a mock function object // controlling the behavior of the method RedisKeyValue. RedisKeyValueFunc *DBRedisKeyValueFunc @@ -5161,11 +5162,6 @@ func NewMockDB() *MockDB { return }, }, - OwnSignalsFunc: &DBOwnSignalsFunc{ - defaultHook: func() (r0 OwnSignalStore) { - return - }, - }, PermissionSyncJobsFunc: &DBPermissionSyncJobsFunc{ defaultHook: func() (r0 PermissionSyncJobStore) { return @@ -5191,6 +5187,11 @@ func NewMockDB() *MockDB { return }, }, + RecentContributionSignalsFunc: &DBRecentContributionSignalsFunc{ + defaultHook: func() (r0 RecentContributionSignalStore) { + return + }, + }, RedisKeyValueFunc: &DBRedisKeyValueFunc{ defaultHook: func() (r0 RedisKeyValueStore) { return @@ -5423,11 +5424,6 @@ func NewStrictMockDB() *MockDB { panic("unexpected invocation of MockDB.OutboundWebhooks") }, }, - OwnSignalsFunc: &DBOwnSignalsFunc{ - defaultHook: func() OwnSignalStore { - panic("unexpected invocation of MockDB.OwnSignals") - }, - }, PermissionSyncJobsFunc: &DBPermissionSyncJobsFunc{ defaultHook: func() PermissionSyncJobStore { panic("unexpected invocation of MockDB.PermissionSyncJobs") @@ -5453,6 +5449,11 @@ func NewStrictMockDB() *MockDB { panic("unexpected invocation of MockDB.QueryRowContext") }, }, + RecentContributionSignalsFunc: &DBRecentContributionSignalsFunc{ + defaultHook: func() RecentContributionSignalStore { + panic("unexpected invocation of MockDB.RecentContributionSignals") + }, + }, RedisKeyValueFunc: &DBRedisKeyValueFunc{ defaultHook: func() RedisKeyValueStore { panic("unexpected invocation of MockDB.RedisKeyValue") @@ -5637,9 +5638,6 @@ func NewMockDBFrom(i DB) *MockDB { OutboundWebhooksFunc: &DBOutboundWebhooksFunc{ defaultHook: i.OutboundWebhooks, }, - OwnSignalsFunc: &DBOwnSignalsFunc{ - defaultHook: i.OwnSignals, - }, PermissionSyncJobsFunc: &DBPermissionSyncJobsFunc{ defaultHook: i.PermissionSyncJobs, }, @@ -5655,6 +5653,9 @@ func NewMockDBFrom(i DB) *MockDB { QueryRowContextFunc: &DBQueryRowContextFunc{ defaultHook: i.QueryRowContext, }, + RecentContributionSignalsFunc: &DBRecentContributionSignalsFunc{ + defaultHook: i.RecentContributionSignals, + }, RedisKeyValueFunc: &DBRedisKeyValueFunc{ defaultHook: i.RedisKeyValue, }, @@ -8118,104 +8119,6 @@ func (c DBOutboundWebhooksFuncCall) Results() []interface{} { return []interface{}{c.Result0} } -// DBOwnSignalsFunc describes the behavior when the OwnSignals method of the -// parent MockDB instance is invoked. -type DBOwnSignalsFunc struct { - defaultHook func() OwnSignalStore - hooks []func() OwnSignalStore - history []DBOwnSignalsFuncCall - mutex sync.Mutex -} - -// OwnSignals delegates to the next hook function in the queue and stores -// the parameter and result values of this invocation. -func (m *MockDB) OwnSignals() OwnSignalStore { - r0 := m.OwnSignalsFunc.nextHook()() - m.OwnSignalsFunc.appendCall(DBOwnSignalsFuncCall{r0}) - return r0 -} - -// SetDefaultHook sets function that is called when the OwnSignals method of -// the parent MockDB instance is invoked and the hook queue is empty. -func (f *DBOwnSignalsFunc) SetDefaultHook(hook func() OwnSignalStore) { - f.defaultHook = hook -} - -// PushHook adds a function to the end of hook queue. Each invocation of the -// OwnSignals method of the parent MockDB instance invokes the hook at the -// front of the queue and discards it. After the queue is empty, the default -// hook function is invoked for any future action. -func (f *DBOwnSignalsFunc) PushHook(hook func() OwnSignalStore) { - f.mutex.Lock() - f.hooks = append(f.hooks, hook) - f.mutex.Unlock() -} - -// SetDefaultReturn calls SetDefaultHook with a function that returns the -// given values. -func (f *DBOwnSignalsFunc) SetDefaultReturn(r0 OwnSignalStore) { - f.SetDefaultHook(func() OwnSignalStore { - return r0 - }) -} - -// PushReturn calls PushHook with a function that returns the given values. -func (f *DBOwnSignalsFunc) PushReturn(r0 OwnSignalStore) { - f.PushHook(func() OwnSignalStore { - return r0 - }) -} - -func (f *DBOwnSignalsFunc) nextHook() func() OwnSignalStore { - f.mutex.Lock() - defer f.mutex.Unlock() - - if len(f.hooks) == 0 { - return f.defaultHook - } - - hook := f.hooks[0] - f.hooks = f.hooks[1:] - return hook -} - -func (f *DBOwnSignalsFunc) appendCall(r0 DBOwnSignalsFuncCall) { - f.mutex.Lock() - f.history = append(f.history, r0) - f.mutex.Unlock() -} - -// History returns a sequence of DBOwnSignalsFuncCall objects describing the -// invocations of this function. -func (f *DBOwnSignalsFunc) History() []DBOwnSignalsFuncCall { - f.mutex.Lock() - history := make([]DBOwnSignalsFuncCall, len(f.history)) - copy(history, f.history) - f.mutex.Unlock() - - return history -} - -// DBOwnSignalsFuncCall is an object that describes an invocation of method -// OwnSignals on an instance of MockDB. -type DBOwnSignalsFuncCall struct { - // Result0 is the value of the 1st result returned from this method - // invocation. - Result0 OwnSignalStore -} - -// Args returns an interface slice containing the arguments of this -// invocation. -func (c DBOwnSignalsFuncCall) Args() []interface{} { - return []interface{}{} -} - -// Results returns an interface slice containing the results of this -// invocation. -func (c DBOwnSignalsFuncCall) Results() []interface{} { - return []interface{}{c.Result0} -} - // DBPermissionSyncJobsFunc describes the behavior when the // PermissionSyncJobs method of the parent MockDB instance is invoked. type DBPermissionSyncJobsFunc struct { @@ -8743,6 +8646,106 @@ func (c DBQueryRowContextFuncCall) Results() []interface{} { return []interface{}{c.Result0} } +// DBRecentContributionSignalsFunc describes the behavior when the +// RecentContributionSignals method of the parent MockDB instance is +// invoked. +type DBRecentContributionSignalsFunc struct { + defaultHook func() RecentContributionSignalStore + hooks []func() RecentContributionSignalStore + history []DBRecentContributionSignalsFuncCall + mutex sync.Mutex +} + +// RecentContributionSignals delegates to the next hook function in the +// queue and stores the parameter and result values of this invocation. +func (m *MockDB) RecentContributionSignals() RecentContributionSignalStore { + r0 := m.RecentContributionSignalsFunc.nextHook()() + m.RecentContributionSignalsFunc.appendCall(DBRecentContributionSignalsFuncCall{r0}) + return r0 +} + +// SetDefaultHook sets function that is called when the +// RecentContributionSignals method of the parent MockDB instance is invoked +// and the hook queue is empty. +func (f *DBRecentContributionSignalsFunc) SetDefaultHook(hook func() RecentContributionSignalStore) { + f.defaultHook = hook +} + +// PushHook adds a function to the end of hook queue. Each invocation of the +// RecentContributionSignals method of the parent MockDB instance invokes +// the hook at the front of the queue and discards it. After the queue is +// empty, the default hook function is invoked for any future action. +func (f *DBRecentContributionSignalsFunc) PushHook(hook func() RecentContributionSignalStore) { + f.mutex.Lock() + f.hooks = append(f.hooks, hook) + f.mutex.Unlock() +} + +// SetDefaultReturn calls SetDefaultHook with a function that returns the +// given values. +func (f *DBRecentContributionSignalsFunc) SetDefaultReturn(r0 RecentContributionSignalStore) { + f.SetDefaultHook(func() RecentContributionSignalStore { + return r0 + }) +} + +// PushReturn calls PushHook with a function that returns the given values. +func (f *DBRecentContributionSignalsFunc) PushReturn(r0 RecentContributionSignalStore) { + f.PushHook(func() RecentContributionSignalStore { + return r0 + }) +} + +func (f *DBRecentContributionSignalsFunc) nextHook() func() RecentContributionSignalStore { + f.mutex.Lock() + defer f.mutex.Unlock() + + if len(f.hooks) == 0 { + return f.defaultHook + } + + hook := f.hooks[0] + f.hooks = f.hooks[1:] + return hook +} + +func (f *DBRecentContributionSignalsFunc) appendCall(r0 DBRecentContributionSignalsFuncCall) { + f.mutex.Lock() + f.history = append(f.history, r0) + f.mutex.Unlock() +} + +// History returns a sequence of DBRecentContributionSignalsFuncCall objects +// describing the invocations of this function. +func (f *DBRecentContributionSignalsFunc) History() []DBRecentContributionSignalsFuncCall { + f.mutex.Lock() + history := make([]DBRecentContributionSignalsFuncCall, len(f.history)) + copy(history, f.history) + f.mutex.Unlock() + + return history +} + +// DBRecentContributionSignalsFuncCall is an object that describes an +// invocation of method RecentContributionSignals on an instance of MockDB. +type DBRecentContributionSignalsFuncCall struct { + // Result0 is the value of the 1st result returned from this method + // invocation. + Result0 RecentContributionSignalStore +} + +// Args returns an interface slice containing the arguments of this +// invocation. +func (c DBRecentContributionSignalsFuncCall) Args() []interface{} { + return []interface{}{} +} + +// Results returns an interface slice containing the results of this +// invocation. +func (c DBRecentContributionSignalsFuncCall) Results() []interface{} { + return []interface{}{c.Result0} +} + // DBRedisKeyValueFunc describes the behavior when the RedisKeyValue method // of the parent MockDB instance is invoked. type DBRedisKeyValueFunc struct { diff --git a/internal/database/schema.json b/internal/database/schema.json index 271d5a67878f5..991df6f987c3f 100755 --- a/internal/database/schema.json +++ b/internal/database/schema.json @@ -21141,22 +21141,9 @@ "GenerationExpression": "", "Comment": "" }, - { - "Name": "is_dir", - "Index": 4, - "TypeName": "boolean", - "IsNullable": false, - "Default": "", - "CharacterMaximumLength": 0, - "IsIdentity": false, - "IdentityGeneration": "", - "IsGenerated": "NEVER", - "GenerationExpression": "", - "Comment": "" - }, { "Name": "parent_id", - "Index": 5, + "Index": 4, "TypeName": "integer", "IsNullable": true, "Default": "", diff --git a/internal/database/schema.md b/internal/database/schema.md index 5dc3aa6f97ccb..7af42ee04cb7a 100755 --- a/internal/database/schema.md +++ b/internal/database/schema.md @@ -3187,7 +3187,6 @@ Foreign-key constraints: id | integer | | not null | nextval('repo_paths_id_seq'::regclass) repo_id | integer | | not null | absolute_path | text | | not null | - is_dir | boolean | | not null | parent_id | integer | | | Indexes: "repo_paths_pkey" PRIMARY KEY, btree (id) diff --git a/migrations/frontend/squashed.sql b/migrations/frontend/squashed.sql index f2f64a6370725..a16ca3f50bd97 100755 --- a/migrations/frontend/squashed.sql +++ b/migrations/frontend/squashed.sql @@ -3886,7 +3886,6 @@ CREATE TABLE repo_paths ( id integer NOT NULL, repo_id integer NOT NULL, absolute_path text NOT NULL, - is_dir boolean NOT NULL, parent_id integer ); From 98c919c4e543a21dd6dc379fa72c4633f75469bd Mon Sep 17 00:00:00 2001 From: Cezary Bartoszuk Date: Fri, 21 Apr 2023 13:42:05 -0500 Subject: [PATCH 06/30] Tidy and comment commit_signals.go, reverse index on aggregate --- internal/database/commit_signals.go | 77 ++++++++++++------- internal/database/commit_signals_test.go | 16 +++- .../1681300431_ownership_signals/down.sql | 7 +- .../1681300431_ownership_signals/up.sql | 4 +- 4 files changed, 67 insertions(+), 37 deletions(-) diff --git a/internal/database/commit_signals.go b/internal/database/commit_signals.go index 772459270b830..cf96133b0ef32 100644 --- a/internal/database/commit_signals.go +++ b/internal/database/commit_signals.go @@ -2,15 +2,12 @@ package database import ( "context" - "database/sql" "hash/fnv" "path" "time" "github.com/keegancsmith/sqlf" - "github.com/sourcegraph/log" - "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/database/basestore" "github.com/sourcegraph/sourcegraph/lib/errors" @@ -18,11 +15,11 @@ import ( type RecentContributionSignalStore interface { AddCommit(ctx context.Context, commit Commit) error - FindRecentAuthors(ctx context.Context, repoID api.RepoID, path string) ([]RecentAuthor, error) + FindRecentAuthors(ctx context.Context, repoID api.RepoID, path string) ([]RecentContributorSummary, error) } func RecentContributionSignalStoreWith(other basestore.ShareableStore) RecentContributionSignalStore { - return &ownSignalStore{Store: basestore.NewWithHandle(other.Handle())} + return &recentContributionSignalStore{Store: basestore.NewWithHandle(other.Handle())} } type Commit struct { @@ -34,33 +31,49 @@ type Commit struct { FilesChanged []string } -type RecentAuthor struct { +type RecentContributorSummary struct { AuthorName string AuthorEmail string ContributionCount int } -type ownSignalStore struct { - logger log.Logger +type recentContributionSignalStore struct { *basestore.Store } -// TODO: Use one query for author. +const commitAuthorInsertFmtstr = ` + WITH already_exists (id) AS ( + SELECT id + FROM commit_authors + WHERE name = %s + AND email = %s + ), + need_to_insert (id) AS ( + INSERT INTO commit_authors (name, email) + VALUES (%s, %s) + ON CONFLICT (name, email) DO NOTHING + RETURNING id + ) + SELECT id FROM already_exists + UNION ALL + SELECT id FROM need_to_insert +` -func (s *ownSignalStore) ensureAuthor(ctx context.Context, commit Commit) (int, error) { +// ensureAuthor makes sure the that commit author designated by name and email +// exists in the `commit_authors` table, and returns its ID. +func (s *recentContributionSignalStore) ensureAuthor(ctx context.Context, commit Commit) (int, error) { db := s.Store var authorID int - err := db.QueryRow( + if err := db.QueryRow( ctx, - sqlf.Sprintf(`SELECT id FROM commit_authors WHERE email = %s AND name = %s`, commit.AuthorEmail, commit.AuthorName), - ).Scan(&authorID) - if err == sql.ErrNoRows { - err = db.QueryRow( - ctx, - sqlf.Sprintf(`INSERT INTO commit_authors (email, name) VALUES (%s, %s) RETURNING id`, commit.AuthorEmail, commit.AuthorName), - ).Scan(&authorID) - } - if err != nil { + sqlf.Sprintf( + commitAuthorInsertFmtstr, + commit.AuthorName, + commit.AuthorEmail, + commit.AuthorName, + commit.AuthorEmail, + ), + ).Scan(&authorID); err != nil { return 0, err } return authorID, nil @@ -95,7 +108,7 @@ const pathInsertFmtstr = ` // // The result int slice is guaranteed to be in order corresponding to the order // of `commit.FilesChanged`. -func (s *ownSignalStore) ensureRepoPaths(ctx context.Context, commit Commit) ([]int, error) { +func (s *recentContributionSignalStore) ensureRepoPaths(ctx context.Context, commit Commit) ([]int, error) { db := s.Store // compute all the ancestor paths for all changed files in a commit var paths []string @@ -172,7 +185,7 @@ const insertRecentContributorSignalFmtstr = ` // The aggregate signals in `own_aggregate_recent_contribution` are updated atomically // for each new signal appearing in `own_signal_recent_contribution` by using // a trigger: `update_own_aggregate_recent_contribution`. -func (s *ownSignalStore) AddCommit(ctx context.Context, commit Commit) error { +func (s *recentContributionSignalStore) AddCommit(ctx context.Context, commit Commit) error { db := s.Store // Get or create commit author: authorID, err := s.ensureAuthor(ctx, commit) @@ -214,8 +227,16 @@ const findRecentContributorsFmtstr = ` ORDER BY 3 DESC ` -func (s *ownSignalStore) FindRecentAuthors(ctx context.Context, repoID api.RepoID, path string) ([]RecentAuthor, error) { - var authors []RecentAuthor +// FindRecentAuthors returns all recent authors for given `repoID` and `path`. +// Since the recent contributor signal aggregate is computed within `AddCommit` +// This just looks up `own_aggregate_recent_contribution` associated with given +// repo and path, and pulls all the related authors. +// Notes: +// - `path` has not forward slash at the beginning, example: "dir1/dir2/file.go", "file2.go". +// - Empty string `path` designates repo root (so all contributions for the whole repo). +// - TODO: Need to support limit & offset here. +func (s *recentContributionSignalStore) FindRecentAuthors(ctx context.Context, repoID api.RepoID, path string) ([]RecentContributorSummary, error) { + var contributions []RecentContributorSummary q := sqlf.Sprintf(findRecentContributorsFmtstr, repoID, path) rows, err := s.Query(ctx, q) if err != nil { @@ -223,11 +244,11 @@ func (s *ownSignalStore) FindRecentAuthors(ctx context.Context, repoID api.RepoI } defer rows.Close() for rows.Next() { - var a RecentAuthor - if err := rows.Scan(&a.AuthorName, &a.AuthorEmail, &a.ContributionCount); err != nil { + var rcs RecentContributorSummary + if err := rows.Scan(&rcs.AuthorName, &rcs.AuthorEmail, &rcs.ContributionCount); err != nil { return nil, err } - authors = append(authors, a) + contributions = append(contributions, rcs) } - return authors, nil + return contributions, nil } diff --git a/internal/database/commit_signals_test.go b/internal/database/commit_signals_test.go index 13345cbccf1e3..5efabb16a8ec6 100644 --- a/internal/database/commit_signals_test.go +++ b/internal/database/commit_signals_test.go @@ -13,7 +13,7 @@ import ( "github.com/sourcegraph/sourcegraph/internal/types" ) -func TestOwnSignalStore_AddCommit(t *testing.T) { +func TestRecentContributorsSignalStore(t *testing.T) { if testing.Short() { t.Skip() } @@ -59,7 +59,7 @@ func TestOwnSignalStore_AddCommit(t *testing.T) { } } - for p, w := range map[string][]RecentAuthor{ + for p, w := range map[string][]RecentContributorSummary{ "dir": { { AuthorName: "alice", @@ -79,6 +79,18 @@ func TestOwnSignalStore_AddCommit(t *testing.T) { ContributionCount: 1, }, }, + "": { + { + AuthorName: "alice", + AuthorEmail: "alice@example.com", + ContributionCount: 7, + }, + { + AuthorName: "bob", + AuthorEmail: "bob@example.com", + ContributionCount: 3, + }, + }, } { path := p want := w diff --git a/migrations/frontend/1681300431_ownership_signals/down.sql b/migrations/frontend/1681300431_ownership_signals/down.sql index ba43cc0bf88b7..befe9c109b449 100644 --- a/migrations/frontend/1681300431_ownership_signals/down.sql +++ b/migrations/frontend/1681300431_ownership_signals/down.sql @@ -1,13 +1,10 @@ DROP TRIGGER IF EXISTS update_own_aggregate_recent_contribution ON own_signal_recent_contribution; DROP FUNCTION IF EXISTS update_own_aggregate_recent_contribution(); - +DROP INDEX IF EXISTS own_aggregate_recent_contribution_file_author; DROP INDEX IF EXISTS own_aggregate_recent_contribution_author_file; DROP TABLE IF EXISTS own_aggregate_recent_contribution; - DROP TABLE IF EXISTS own_signal_recent_contribution; - DROP INDEX IF EXISTS commit_authors_email_name; DROP TABLE IF EXISTS commit_authors; - DROP INDEX IF EXISTS repo_paths_index_absolute_path; -DROP TABLE IF EXISTS repo_paths; +DROP TABLE IF EXISTS repo_paths; \ No newline at end of file diff --git a/migrations/frontend/1681300431_ownership_signals/up.sql b/migrations/frontend/1681300431_ownership_signals/up.sql index cd6fff2b449db..1f300ccce6bc9 100644 --- a/migrations/frontend/1681300431_ownership_signals/up.sql +++ b/migrations/frontend/1681300431_ownership_signals/up.sql @@ -40,9 +40,9 @@ CREATE TABLE IF NOT EXISTS own_aggregate_recent_contribution ( contributions_count INTEGER DEFAULT 0 ); -CREATE UNIQUE INDEX IF NOT EXISTS own_aggregate_recent_contribution_author_file +CREATE UNIQUE INDEX IF NOT EXISTS own_aggregate_recent_contribution_file_author ON own_aggregate_recent_contribution -USING btree (commit_author_id, changed_file_path_id); +USING btree (changed_file_path_id, commit_author_id); CREATE OR REPLACE FUNCTION update_own_aggregate_recent_contribution() RETURNS TRIGGER AS $$ BEGIN From be338f1835dc4da67dccadbcc9cc8d8f313b71d3 Mon Sep 17 00:00:00 2001 From: Cezary Bartoszuk Date: Fri, 21 Apr 2023 14:34:10 -0500 Subject: [PATCH 07/30] Make up.sql idempotent --- internal/database/schema.json | 4 +-- internal/database/schema.md | 2 +- .../1681300431_ownership_signals/down.sql | 7 ++++-- .../1681300431_ownership_signals/up.sql | 25 ++++++++++++++++--- migrations/frontend/squashed.sql | 2 +- 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/internal/database/schema.json b/internal/database/schema.json index 991df6f987c3f..b818ab8bcc97b 100755 --- a/internal/database/schema.json +++ b/internal/database/schema.json @@ -18542,12 +18542,12 @@ ], "Indexes": [ { - "Name": "own_aggregate_recent_contribution_author_file", + "Name": "own_aggregate_recent_contribution_file_author", "IsPrimaryKey": false, "IsUnique": true, "IsExclusion": false, "IsDeferrable": false, - "IndexDefinition": "CREATE UNIQUE INDEX own_aggregate_recent_contribution_author_file ON own_aggregate_recent_contribution USING btree (commit_author_id, changed_file_path_id)", + "IndexDefinition": "CREATE UNIQUE INDEX own_aggregate_recent_contribution_file_author ON own_aggregate_recent_contribution USING btree (changed_file_path_id, commit_author_id)", "ConstraintType": "", "ConstraintDefinition": "" }, diff --git a/internal/database/schema.md b/internal/database/schema.md index 7af42ee04cb7a..00dacd97d66ba 100755 --- a/internal/database/schema.md +++ b/internal/database/schema.md @@ -2792,7 +2792,7 @@ Referenced by: contributions_count | integer | | | 0 Indexes: "own_aggregate_recent_contribution_pkey" PRIMARY KEY, btree (id) - "own_aggregate_recent_contribution_author_file" UNIQUE, btree (commit_author_id, changed_file_path_id) + "own_aggregate_recent_contribution_file_author" UNIQUE, btree (changed_file_path_id, commit_author_id) Foreign-key constraints: "own_aggregate_recent_contribution_changed_file_path_id_fkey" FOREIGN KEY (changed_file_path_id) REFERENCES repo_paths(id) "own_aggregate_recent_contribution_commit_author_id_fkey" FOREIGN KEY (commit_author_id) REFERENCES commit_authors(id) diff --git a/migrations/frontend/1681300431_ownership_signals/down.sql b/migrations/frontend/1681300431_ownership_signals/down.sql index befe9c109b449..b03b507c6c0ab 100644 --- a/migrations/frontend/1681300431_ownership_signals/down.sql +++ b/migrations/frontend/1681300431_ownership_signals/down.sql @@ -1,10 +1,13 @@ DROP TRIGGER IF EXISTS update_own_aggregate_recent_contribution ON own_signal_recent_contribution; DROP FUNCTION IF EXISTS update_own_aggregate_recent_contribution(); + DROP INDEX IF EXISTS own_aggregate_recent_contribution_file_author; -DROP INDEX IF EXISTS own_aggregate_recent_contribution_author_file; DROP TABLE IF EXISTS own_aggregate_recent_contribution; + DROP TABLE IF EXISTS own_signal_recent_contribution; + DROP INDEX IF EXISTS commit_authors_email_name; DROP TABLE IF EXISTS commit_authors; + DROP INDEX IF EXISTS repo_paths_index_absolute_path; -DROP TABLE IF EXISTS repo_paths; \ No newline at end of file +DROP TABLE IF EXISTS repo_paths; diff --git a/migrations/frontend/1681300431_ownership_signals/up.sql b/migrations/frontend/1681300431_ownership_signals/up.sql index 1f300ccce6bc9..bd1bb8b0e0702 100644 --- a/migrations/frontend/1681300431_ownership_signals/up.sql +++ b/migrations/frontend/1681300431_ownership_signals/up.sql @@ -82,6 +82,25 @@ BEGIN END; $$ LANGUAGE plpgsql; -CREATE TRIGGER update_own_aggregate_recent_contribution -AFTER INSERT ON own_signal_recent_contribution -FOR EACH ROW EXECUTE PROCEDURE update_own_aggregate_recent_contribution(); +DO $$ +DECLARE + trigger_exists INTEGER; +BEGIN + -- Check if the trigger already exists + SELECT COUNT(*) + INTO trigger_exists + FROM pg_trigger + WHERE tgname = 'update_own_aggregate_recent_contribution'; + + -- If the trigger exists, drop it + IF trigger_exists > 0 THEN + EXECUTE 'DROP TRIGGER update_own_aggregate_recent_contribution ON own_signal_recent_contribution'; + END IF; + + -- Create the trigger + EXECUTE 'CREATE TRIGGER update_own_aggregate_recent_contribution + AFTER INSERT + ON own_signal_recent_contribution + FOR EACH ROW + EXECUTE FUNCTION update_own_aggregate_recent_contribution()'; +END $$; diff --git a/migrations/frontend/squashed.sql b/migrations/frontend/squashed.sql index a16ca3f50bd97..e0d11e999d39b 100755 --- a/migrations/frontend/squashed.sql +++ b/migrations/frontend/squashed.sql @@ -5529,7 +5529,7 @@ CREATE INDEX outbound_webhook_payload_process_after_idx ON outbound_webhook_jobs CREATE INDEX outbound_webhooks_logs_status_code_idx ON outbound_webhook_logs USING btree (status_code); -CREATE UNIQUE INDEX own_aggregate_recent_contribution_author_file ON own_aggregate_recent_contribution USING btree (commit_author_id, changed_file_path_id); +CREATE UNIQUE INDEX own_aggregate_recent_contribution_file_author ON own_aggregate_recent_contribution USING btree (changed_file_path_id, commit_author_id); CREATE UNIQUE INDEX package_repo_filters_unique_matcher_per_scheme ON package_repo_filters USING btree (scheme, matcher); From ee0dd162b64b84eb3cc995c7728769ba1b241ff3 Mon Sep 17 00:00:00 2001 From: Cezary Bartoszuk Date: Fri, 21 Apr 2023 14:35:37 -0500 Subject: [PATCH 08/30] Rename commit_signals to recent_contribution_signal --- ...commit_signals.go => recent_contribution_signal.go} | 0 ...nals_test.go => recent_contribution_signal_test.go} | 10 +++++----- 2 files changed, 5 insertions(+), 5 deletions(-) rename internal/database/{commit_signals.go => recent_contribution_signal.go} (100%) rename internal/database/{commit_signals_test.go => recent_contribution_signal_test.go} (91%) diff --git a/internal/database/commit_signals.go b/internal/database/recent_contribution_signal.go similarity index 100% rename from internal/database/commit_signals.go rename to internal/database/recent_contribution_signal.go diff --git a/internal/database/commit_signals_test.go b/internal/database/recent_contribution_signal_test.go similarity index 91% rename from internal/database/commit_signals_test.go rename to internal/database/recent_contribution_signal_test.go index 5efabb16a8ec6..093b269001e72 100644 --- a/internal/database/commit_signals_test.go +++ b/internal/database/recent_contribution_signal_test.go @@ -13,7 +13,7 @@ import ( "github.com/sourcegraph/sourcegraph/internal/types" ) -func TestRecentContributorsSignalStore(t *testing.T) { +func TestRecentContributionSignalStore(t *testing.T) { if testing.Short() { t.Skip() } @@ -81,13 +81,13 @@ func TestRecentContributorsSignalStore(t *testing.T) { }, "": { { - AuthorName: "alice", - AuthorEmail: "alice@example.com", + AuthorName: "alice", + AuthorEmail: "alice@example.com", ContributionCount: 7, }, { - AuthorName: "bob", - AuthorEmail: "bob@example.com", + AuthorName: "bob", + AuthorEmail: "bob@example.com", ContributionCount: 3, }, }, From 321358be201f9d715d3dd58978b824368669f2c6 Mon Sep 17 00:00:00 2001 From: Cezary Bartoszuk Date: Fri, 21 Apr 2023 14:47:18 -0500 Subject: [PATCH 09/30] Update BUILD files --- dev/linters/bodyclose/BUILD.bazel | 5 +---- dev/linters/depguard/BUILD.bazel | 1 - dev/linters/exportloopref/BUILD.bazel | 5 +---- dev/linters/unparam/BUILD.bazel | 1 - internal/database/BUILD.bazel | 2 ++ migrations/BUILD.bazel | 3 +++ 6 files changed, 7 insertions(+), 10 deletions(-) diff --git a/dev/linters/bodyclose/BUILD.bazel b/dev/linters/bodyclose/BUILD.bazel index 93228f234266b..9d6c52923f385 100644 --- a/dev/linters/bodyclose/BUILD.bazel +++ b/dev/linters/bodyclose/BUILD.bazel @@ -5,8 +5,5 @@ go_library( srcs = ["bodyclose.go"], importpath = "github.com/sourcegraph/sourcegraph/dev/linters/bodyclose", visibility = ["//visibility:public"], - deps = [ - "//dev/linters/nolint", - "@com_github_timakin_bodyclose//passes/bodyclose:go_default_library", - ], + deps = ["//dev/linters/nolint"], ) diff --git a/dev/linters/depguard/BUILD.bazel b/dev/linters/depguard/BUILD.bazel index 7b5fca9789e01..6cfb17e89c987 100644 --- a/dev/linters/depguard/BUILD.bazel +++ b/dev/linters/depguard/BUILD.bazel @@ -7,7 +7,6 @@ go_library( visibility = ["//visibility:public"], deps = [ "//dev/linters/nolint", - "@com_github_openpeedeep_depguard_v2//:go_default_library", "@org_golang_x_tools//go/analysis", ], ) diff --git a/dev/linters/exportloopref/BUILD.bazel b/dev/linters/exportloopref/BUILD.bazel index a49fce5970269..cfab09d407860 100644 --- a/dev/linters/exportloopref/BUILD.bazel +++ b/dev/linters/exportloopref/BUILD.bazel @@ -5,8 +5,5 @@ go_library( srcs = ["exportloopref.go"], importpath = "github.com/sourcegraph/sourcegraph/dev/linters/exportloopref", visibility = ["//visibility:public"], - deps = [ - "//dev/linters/nolint", - "@com_github_kyoh86_exportloopref//:go_default_library", - ], + deps = ["//dev/linters/nolint"], ) diff --git a/dev/linters/unparam/BUILD.bazel b/dev/linters/unparam/BUILD.bazel index 39aaebe7497f7..becc863acebdd 100644 --- a/dev/linters/unparam/BUILD.bazel +++ b/dev/linters/unparam/BUILD.bazel @@ -7,7 +7,6 @@ go_library( visibility = ["//visibility:public"], deps = [ "//dev/linters/nolint", - "@cc_mvdan_unparam//check:go_default_library", "@org_golang_x_tools//go/analysis", "@org_golang_x_tools//go/analysis/passes/buildssa", "@org_golang_x_tools//go/packages", diff --git a/internal/database/BUILD.bazel b/internal/database/BUILD.bazel index f6902b8f48d7f..b05a359df2a2f 100644 --- a/internal/database/BUILD.bazel +++ b/internal/database/BUILD.bazel @@ -42,6 +42,7 @@ go_library( "permission_sync_jobs.go", "permissions.go", "phabricator.go", + "recent_contribution_signal.go", "redis_key_value.go", "repo_kvps.go", "repo_statistics.go", @@ -179,6 +180,7 @@ go_test( "permission_sync_jobs_test.go", "permissions_test.go", "phabricator_test.go", + "recent_contribution_signal_test.go", "redis_key_value_test.go", "repo_kvps_test.go", "repo_statistics_test.go", diff --git a/migrations/BUILD.bazel b/migrations/BUILD.bazel index 056992fe2e369..dd4c65cb8ab7b 100644 --- a/migrations/BUILD.bazel +++ b/migrations/BUILD.bazel @@ -889,6 +889,9 @@ go_library( "frontend/1681807446_add_github_apps_table/down.sql", "frontend/1681807446_add_github_apps_table/metadata.yaml", "frontend/1681807446_add_github_apps_table/up.sql", + "frontend/1681300431_ownership_signals/down.sql", + "frontend/1681300431_ownership_signals/metadata.yaml", + "frontend/1681300431_ownership_signals/up.sql", ], importpath = "github.com/sourcegraph/sourcegraph/migrations", visibility = ["//visibility:public"], From 543a16a55e08af1395593736b18704a8a48999cb Mon Sep 17 00:00:00 2001 From: Cezary Bartoszuk Date: Fri, 21 Apr 2023 14:58:56 -0500 Subject: [PATCH 10/30] bazel configure again --- dev/linters/bodyclose/BUILD.bazel | 5 +- dev/linters/depguard/BUILD.bazel | 1 + dev/linters/exportloopref/BUILD.bazel | 5 +- dev/linters/unparam/BUILD.bazel | 1 + migrations/BUILD.bazel | 179 +++++++++++++------------- 5 files changed, 98 insertions(+), 93 deletions(-) diff --git a/dev/linters/bodyclose/BUILD.bazel b/dev/linters/bodyclose/BUILD.bazel index 9d6c52923f385..93228f234266b 100644 --- a/dev/linters/bodyclose/BUILD.bazel +++ b/dev/linters/bodyclose/BUILD.bazel @@ -5,5 +5,8 @@ go_library( srcs = ["bodyclose.go"], importpath = "github.com/sourcegraph/sourcegraph/dev/linters/bodyclose", visibility = ["//visibility:public"], - deps = ["//dev/linters/nolint"], + deps = [ + "//dev/linters/nolint", + "@com_github_timakin_bodyclose//passes/bodyclose:go_default_library", + ], ) diff --git a/dev/linters/depguard/BUILD.bazel b/dev/linters/depguard/BUILD.bazel index 6cfb17e89c987..7b5fca9789e01 100644 --- a/dev/linters/depguard/BUILD.bazel +++ b/dev/linters/depguard/BUILD.bazel @@ -7,6 +7,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//dev/linters/nolint", + "@com_github_openpeedeep_depguard_v2//:go_default_library", "@org_golang_x_tools//go/analysis", ], ) diff --git a/dev/linters/exportloopref/BUILD.bazel b/dev/linters/exportloopref/BUILD.bazel index cfab09d407860..a49fce5970269 100644 --- a/dev/linters/exportloopref/BUILD.bazel +++ b/dev/linters/exportloopref/BUILD.bazel @@ -5,5 +5,8 @@ go_library( srcs = ["exportloopref.go"], importpath = "github.com/sourcegraph/sourcegraph/dev/linters/exportloopref", visibility = ["//visibility:public"], - deps = ["//dev/linters/nolint"], + deps = [ + "//dev/linters/nolint", + "@com_github_kyoh86_exportloopref//:go_default_library", + ], ) diff --git a/dev/linters/unparam/BUILD.bazel b/dev/linters/unparam/BUILD.bazel index becc863acebdd..39aaebe7497f7 100644 --- a/dev/linters/unparam/BUILD.bazel +++ b/dev/linters/unparam/BUILD.bazel @@ -7,6 +7,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//dev/linters/nolint", + "@cc_mvdan_unparam//check:go_default_library", "@org_golang_x_tools//go/analysis", "@org_golang_x_tools//go/analysis/passes/buildssa", "@org_golang_x_tools//go/packages", diff --git a/migrations/BUILD.bazel b/migrations/BUILD.bazel index 266bc8823968f..8f1c5822ef8cf 100644 --- a/migrations/BUILD.bazel +++ b/migrations/BUILD.bazel @@ -67,6 +67,18 @@ go_library( "codeinsights/1672921606_data_retention_jobs_series_metadata/down.sql", "codeinsights/1672921606_data_retention_jobs_series_metadata/metadata.yaml", "codeinsights/1672921606_data_retention_jobs_series_metadata/up.sql", + "codeinsights/1674474174_remove_dirty_queries_table/down.sql", + "codeinsights/1674474174_remove_dirty_queries_table/metadata.yaml", + "codeinsights/1674474174_remove_dirty_queries_table/up.sql", + "codeinsights/1675113463_backfill_repo_query_selector/down.sql", + "codeinsights/1675113463_backfill_repo_query_selector/metadata.yaml", + "codeinsights/1675113463_backfill_repo_query_selector/up.sql", + "codeinsights/1675347548_add_insight_view_series_num_samples/down.sql", + "codeinsights/1675347548_add_insight_view_series_num_samples/metadata.yaml", + "codeinsights/1675347548_add_insight_view_series_num_samples/up.sql", + "codeinsights/1679051112_remove_commit_index_tables/down.sql", + "codeinsights/1679051112_remove_commit_index_tables/metadata.yaml", + "codeinsights/1679051112_remove_commit_index_tables/up.sql", "codeinsights/squashed.sql", "codeintel/1000000033_squashed_migrations_privileged/down.sql", "codeintel/1000000033_squashed_migrations_privileged/metadata.yaml", @@ -113,6 +125,21 @@ go_library( "codeintel/1671059396_remove_duplicate_trigger/down.sql", "codeintel/1671059396_remove_duplicate_trigger/metadata.yaml", "codeintel/1671059396_remove_duplicate_trigger/up.sql", + "codeintel/1676423214_remove_lsif_data/down.sql", + "codeintel/1676423214_remove_lsif_data/metadata.yaml", + "codeintel/1676423214_remove_lsif_data/up.sql", + "codeintel/1678041507_cleanup_unused_functions/down.sql", + "codeintel/1678041507_cleanup_unused_functions/metadata.yaml", + "codeintel/1678041507_cleanup_unused_functions/up.sql", + "codeintel/1678898749_make_unreferenced_documents_index_usable/down.sql", + "codeintel/1678898749_make_unreferenced_documents_index_usable/metadata.yaml", + "codeintel/1678898749_make_unreferenced_documents_index_usable/up.sql", + "codeintel/1678899132_remove_unused_unreferenced_documents_index/down.sql", + "codeintel/1678899132_remove_unused_unreferenced_documents_index/metadata.yaml", + "codeintel/1678899132_remove_unused_unreferenced_documents_index/up.sql", + "codeintel/1679010276_add_missing_index/down.sql", + "codeintel/1679010276_add_missing_index/metadata.yaml", + "codeintel/1679010276_add_missing_index/up.sql", "codeintel/squashed.sql", "frontend/1648051770_squashed_migrations_privileged/down.sql", "frontend/1648051770_squashed_migrations_privileged/metadata.yaml", @@ -582,6 +609,15 @@ go_library( "frontend/1670934184_add_gitserver_corruption_columns/down.sql", "frontend/1670934184_add_gitserver_corruption_columns/metadata.yaml", "frontend/1670934184_add_gitserver_corruption_columns/up.sql", + "frontend/1671159453_outbound_webhooks/down.sql", + "frontend/1671159453_outbound_webhooks/metadata.yaml", + "frontend/1671159453_outbound_webhooks/up.sql", + "frontend/1671463799_teams/down.sql", + "frontend/1671463799_teams/metadata.yaml", + "frontend/1671463799_teams/up.sql", + "frontend/1671543381_add_default_roles/down.sql", + "frontend/1671543381_add_default_roles/metadata.yaml", + "frontend/1671543381_add_default_roles/up.sql", "frontend/1672884222_create_namespace_permissions_table/down.sql", "frontend/1672884222_create_namespace_permissions_table/metadata.yaml", "frontend/1672884222_create_namespace_permissions_table/up.sql", @@ -591,67 +627,60 @@ go_library( "frontend/1673019611_lsif_uploads_audit_logs_bigint_upload_size/down.sql", "frontend/1673019611_lsif_uploads_audit_logs_bigint_upload_size/metadata.yaml", "frontend/1673019611_lsif_uploads_audit_logs_bigint_upload_size/up.sql", - "frontend/1673405886_make_batch_spec_of_batch_change_not_nullable_again/down.sql", - "frontend/1673405886_make_batch_spec_of_batch_change_not_nullable_again/metadata.yaml", - "frontend/1673405886_make_batch_spec_of_batch_change_not_nullable_again/up.sql", - "frontend/squashed.sql", - "frontend/1671159453_outbound_webhooks/down.sql", - "frontend/1671159453_outbound_webhooks/metadata.yaml", - "frontend/1671159453_outbound_webhooks/up.sql", - "frontend/1671543381_add_default_roles/down.sql", - "frontend/1671543381_add_default_roles/metadata.yaml", - "frontend/1671543381_add_default_roles/up.sql", "frontend/1673351808_add_repo_corruption_stat/down.sql", "frontend/1673351808_add_repo_corruption_stat/metadata.yaml", "frontend/1673351808_add_repo_corruption_stat/up.sql", + "frontend/1673405886_make_batch_spec_of_batch_change_not_nullable_again/down.sql", + "frontend/1673405886_make_batch_spec_of_batch_change_not_nullable_again/metadata.yaml", + "frontend/1673405886_make_batch_spec_of_batch_change_not_nullable_again/up.sql", "frontend/1673871310_add_columns_to_permission_sync_jobs_table/down.sql", "frontend/1673871310_add_columns_to_permission_sync_jobs_table/metadata.yaml", "frontend/1673871310_add_columns_to_permission_sync_jobs_table/up.sql", "frontend/1673897709_add_cascade_batch_spec_resolution_jobs/down.sql", "frontend/1673897709_add_cascade_batch_spec_resolution_jobs/metadata.yaml", "frontend/1673897709_add_cascade_batch_spec_resolution_jobs/up.sql", - "frontend/1674047296_rename_roles_readonly_column_to_system/down.sql", - "frontend/1674047296_rename_roles_readonly_column_to_system/metadata.yaml", - "frontend/1674047296_rename_roles_readonly_column_to_system/up.sql", + "frontend/1674035302_remove_webhook_build_jobs_table/down.sql", + "frontend/1674035302_remove_webhook_build_jobs_table/metadata.yaml", + "frontend/1674035302_remove_webhook_build_jobs_table/up.sql", "frontend/1674041632_add_constraints_to_permission_sync_jobs_table/down.sql", "frontend/1674041632_add_constraints_to_permission_sync_jobs_table/metadata.yaml", "frontend/1674041632_add_constraints_to_permission_sync_jobs_table/up.sql", + "frontend/1674047296_rename_roles_readonly_column_to_system/down.sql", + "frontend/1674047296_rename_roles_readonly_column_to_system/metadata.yaml", + "frontend/1674047296_rename_roles_readonly_column_to_system/up.sql", "frontend/1674455760_add_cancellation_reason_to_permission_sync_jobs_table/down.sql", "frontend/1674455760_add_cancellation_reason_to_permission_sync_jobs_table/metadata.yaml", "frontend/1674455760_add_cancellation_reason_to_permission_sync_jobs_table/up.sql", - "frontend/1674035302_remove_webhook_build_jobs_table/down.sql", - "frontend/1674035302_remove_webhook_build_jobs_table/metadata.yaml", - "frontend/1674035302_remove_webhook_build_jobs_table/up.sql", - "codeinsights/1674474174_remove_dirty_queries_table/down.sql", - "codeinsights/1674474174_remove_dirty_queries_table/metadata.yaml", - "codeinsights/1674474174_remove_dirty_queries_table/up.sql", "frontend/1674480050_add_column_redacted_contents_to_critical_and_site_config/down.sql", "frontend/1674480050_add_column_redacted_contents_to_critical_and_site_config/metadata.yaml", "frontend/1674480050_add_column_redacted_contents_to_critical_and_site_config/up.sql", "frontend/1674642349_add_priority_to_permission_sync_jobs/down.sql", "frontend/1674642349_add_priority_to_permission_sync_jobs/metadata.yaml", "frontend/1674642349_add_priority_to_permission_sync_jobs/up.sql", + "frontend/1674669326_package_repos_separate_versions_table/down.sql", + "frontend/1674669326_package_repos_separate_versions_table/metadata.yaml", + "frontend/1674669326_package_repos_separate_versions_table/up.sql", "frontend/1674669794_add_foreign_keys_to_permission_sync_jobs/down.sql", "frontend/1674669794_add_foreign_keys_to_permission_sync_jobs/metadata.yaml", "frontend/1674669794_add_foreign_keys_to_permission_sync_jobs/up.sql", - "frontend/1671463799_teams/down.sql", - "frontend/1671463799_teams/metadata.yaml", - "frontend/1671463799_teams/up.sql", - "codeinsights/1675113463_backfill_repo_query_selector/down.sql", - "codeinsights/1675113463_backfill_repo_query_selector/metadata.yaml", - "codeinsights/1675113463_backfill_repo_query_selector/up.sql", + "frontend/1674754280_executor_job_tokens/down.sql", + "frontend/1674754280_executor_job_tokens/metadata.yaml", + "frontend/1674754280_executor_job_tokens/up.sql", + "frontend/1674814035_add_unified_source_perms_table/down.sql", + "frontend/1674814035_add_unified_source_perms_table/metadata.yaml", + "frontend/1674814035_add_unified_source_perms_table/up.sql", "frontend/1674952295_make_user_id_namespace_permissions_non_nullable/down.sql", "frontend/1674952295_make_user_id_namespace_permissions_non_nullable/metadata.yaml", "frontend/1674952295_make_user_id_namespace_permissions_non_nullable/up.sql", "frontend/1675155867_add_no_perms_column_to_permission_sync_jobs_table/down.sql", "frontend/1675155867_add_no_perms_column_to_permission_sync_jobs_table/metadata.yaml", "frontend/1675155867_add_no_perms_column_to_permission_sync_jobs_table/up.sql", - "frontend/1674814035_add_unified_source_perms_table/down.sql", - "frontend/1674814035_add_unified_source_perms_table/metadata.yaml", - "frontend/1674814035_add_unified_source_perms_table/up.sql", "frontend/1675194688_fix_should_reindex_in_views/down.sql", "frontend/1675194688_fix_should_reindex_in_views/metadata.yaml", "frontend/1675194688_fix_should_reindex_in_views/up.sql", + "frontend/1675257827_redis_key_value/down.sql", + "frontend/1675257827_redis_key_value/metadata.yaml", + "frontend/1675257827_redis_key_value/up.sql", "frontend/1675277218_add_lsif_uploads_uploaded_at_id/down.sql", "frontend/1675277218_add_lsif_uploads_uploaded_at_id/metadata.yaml", "frontend/1675277218_add_lsif_uploads_uploaded_at_id/up.sql", @@ -661,27 +690,12 @@ go_library( "frontend/1675277968_drop_lsif_indexes_queued_at/down.sql", "frontend/1675277968_drop_lsif_indexes_queued_at/metadata.yaml", "frontend/1675277968_drop_lsif_indexes_queued_at/up.sql", - "frontend/1674669326_package_repos_separate_versions_table/down.sql", - "frontend/1674669326_package_repos_separate_versions_table/metadata.yaml", - "frontend/1674669326_package_repos_separate_versions_table/up.sql", - "frontend/1675257827_redis_key_value/down.sql", - "frontend/1675257827_redis_key_value/metadata.yaml", - "frontend/1675257827_redis_key_value/up.sql", - "frontend/1675367314_add_results_to_permission_sync_jobs/down.sql", - "frontend/1675367314_add_results_to_permission_sync_jobs/metadata.yaml", - "frontend/1675367314_add_results_to_permission_sync_jobs/up.sql", - "codeinsights/1675347548_add_insight_view_series_num_samples/down.sql", - "codeinsights/1675347548_add_insight_view_series_num_samples/metadata.yaml", - "codeinsights/1675347548_add_insight_view_series_num_samples/up.sql", - "codeintel/1676423214_remove_lsif_data/down.sql", - "codeintel/1676423214_remove_lsif_data/metadata.yaml", - "codeintel/1676423214_remove_lsif_data/up.sql", - "frontend/1674754280_executor_job_tokens/down.sql", - "frontend/1674754280_executor_job_tokens/metadata.yaml", - "frontend/1674754280_executor_job_tokens/up.sql", "frontend/1675296942_add_column_to_changesets_for_external_fork_name/down.sql", "frontend/1675296942_add_column_to_changesets_for_external_fork_name/metadata.yaml", "frontend/1675296942_add_column_to_changesets_for_external_fork_name/up.sql", + "frontend/1675367314_add_results_to_permission_sync_jobs/down.sql", + "frontend/1675367314_add_results_to_permission_sync_jobs/metadata.yaml", + "frontend/1675367314_add_results_to_permission_sync_jobs/up.sql", "frontend/1675647612_remove_roles_deleted_at/down.sql", "frontend/1675647612_remove_roles_deleted_at/metadata.yaml", "frontend/1675647612_remove_roles_deleted_at/up.sql", @@ -700,6 +714,9 @@ go_library( "frontend/1676328864_add_cached_available_indexers/down.sql", "frontend/1676328864_add_cached_available_indexers/metadata.yaml", "frontend/1676328864_add_cached_available_indexers/up.sql", + "frontend/1676420496_frontend/down.sql", + "frontend/1676420496_frontend/metadata.yaml", + "frontend/1676420496_frontend/up.sql", "frontend/1676584791_add_index/down.sql", "frontend/1676584791_add_index/metadata.yaml", "frontend/1676584791_add_index/up.sql", @@ -721,6 +738,9 @@ go_library( "frontend/1677104938_move_processed_flag_out_of_references_table/down.sql", "frontend/1677104938_move_processed_flag_out_of_references_table/metadata.yaml", "frontend/1677104938_move_processed_flag_out_of_references_table/up.sql", + "frontend/1677166643_package_repos_allowblock_lists/down.sql", + "frontend/1677166643_package_repos_allowblock_lists/metadata.yaml", + "frontend/1677166643_package_repos_allowblock_lists/up.sql", "frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/down.sql", "frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/metadata.yaml", "frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/up.sql", @@ -763,15 +783,15 @@ go_library( "frontend/1677811663_make_team_creator_nullable/down.sql", "frontend/1677811663_make_team_creator_nullable/metadata.yaml", "frontend/1677811663_make_team_creator_nullable/up.sql", - "codeintel/1678041507_cleanup_unused_functions/down.sql", - "codeintel/1678041507_cleanup_unused_functions/metadata.yaml", - "codeintel/1678041507_cleanup_unused_functions/up.sql", "frontend/1677878270_eric_is_bad_at_math/down.sql", "frontend/1677878270_eric_is_bad_at_math/metadata.yaml", "frontend/1677878270_eric_is_bad_at_math/up.sql", "frontend/1677944569_drop_unused_types/down.sql", "frontend/1677944569_drop_unused_types/metadata.yaml", "frontend/1677944569_drop_unused_types/up.sql", + "frontend/1677944752_drop_unused_lockfiles_tables/down.sql", + "frontend/1677944752_drop_unused_lockfiles_tables/metadata.yaml", + "frontend/1677944752_drop_unused_lockfiles_tables/up.sql", "frontend/1677945580_drop_unused_ranking_table/down.sql", "frontend/1677945580_drop_unused_ranking_table/metadata.yaml", "frontend/1677945580_drop_unused_ranking_table/up.sql", @@ -784,18 +804,21 @@ go_library( "frontend/1678091683_add_migrated_column_to_user_permissions/down.sql", "frontend/1678091683_add_migrated_column_to_user_permissions/metadata.yaml", "frontend/1678091683_add_migrated_column_to_user_permissions/up.sql", - "frontend/1677944752_drop_unused_lockfiles_tables/down.sql", - "frontend/1677944752_drop_unused_lockfiles_tables/metadata.yaml", - "frontend/1677944752_drop_unused_lockfiles_tables/up.sql", "frontend/1678112318_add_external_accounts_scim_index/down.sql", "frontend/1678112318_add_external_accounts_scim_index/metadata.yaml", "frontend/1678112318_add_external_accounts_scim_index/up.sql", + "frontend/1678175532_index_github_topics/down.sql", + "frontend/1678175532_index_github_topics/metadata.yaml", + "frontend/1678175532_index_github_topics/up.sql", + "frontend/1678213774_sg_telemetry_allowlist/down.sql", + "frontend/1678213774_sg_telemetry_allowlist/metadata.yaml", + "frontend/1678213774_sg_telemetry_allowlist/up.sql", "frontend/1678214530_fix_indexes_on_ranking_tables/down.sql", "frontend/1678214530_fix_indexes_on_ranking_tables/metadata.yaml", "frontend/1678214530_fix_indexes_on_ranking_tables/up.sql", - "frontend/1677166643_package_repos_allowblock_lists/down.sql", - "frontend/1677166643_package_repos_allowblock_lists/metadata.yaml", - "frontend/1677166643_package_repos_allowblock_lists/up.sql", + "frontend/1678220614_sg_telemetry_allowlist/down.sql", + "frontend/1678220614_sg_telemetry_allowlist/metadata.yaml", + "frontend/1678220614_sg_telemetry_allowlist/up.sql", "frontend/1678290792_drop_unused_index_event_logs_user_id/down.sql", "frontend/1678290792_drop_unused_index_event_logs_user_id/metadata.yaml", "frontend/1678290792_drop_unused_index_event_logs_user_id/up.sql", @@ -808,15 +831,6 @@ go_library( "frontend/1678291831_drop_unused_index_package_repo_versions_fk_idx/down.sql", "frontend/1678291831_drop_unused_index_package_repo_versions_fk_idx/metadata.yaml", "frontend/1678291831_drop_unused_index_package_repo_versions_fk_idx/up.sql", - "frontend/1678175532_index_github_topics/down.sql", - "frontend/1678175532_index_github_topics/metadata.yaml", - "frontend/1678175532_index_github_topics/up.sql", - "frontend/1678213774_sg_telemetry_allowlist/down.sql", - "frontend/1678213774_sg_telemetry_allowlist/metadata.yaml", - "frontend/1678213774_sg_telemetry_allowlist/up.sql", - "frontend/1678220614_sg_telemetry_allowlist/down.sql", - "frontend/1678220614_sg_telemetry_allowlist/metadata.yaml", - "frontend/1678220614_sg_telemetry_allowlist/up.sql", "frontend/1678320579_normalize_webhook_urns/down.sql", "frontend/1678320579_normalize_webhook_urns/metadata.yaml", "frontend/1678320579_normalize_webhook_urns/up.sql", @@ -826,9 +840,6 @@ go_library( "frontend/1678409821_package_repos_missing_indexes/down.sql", "frontend/1678409821_package_repos_missing_indexes/metadata.yaml", "frontend/1678409821_package_repos_missing_indexes/up.sql", - "frontend/1676420496_frontend/down.sql", - "frontend/1676420496_frontend/metadata.yaml", - "frontend/1676420496_frontend/up.sql", "frontend/1678456448_make_role_name_citext/down.sql", "frontend/1678456448_make_role_name_citext/metadata.yaml", "frontend/1678456448_make_role_name_citext/up.sql", @@ -838,15 +849,6 @@ go_library( "frontend/1678832491_remove_sg_jsonb_concat_agg/down.sql", "frontend/1678832491_remove_sg_jsonb_concat_agg/metadata.yaml", "frontend/1678832491_remove_sg_jsonb_concat_agg/up.sql", - "codeintel/1678898749_make_unreferenced_documents_index_usable/down.sql", - "codeintel/1678898749_make_unreferenced_documents_index_usable/metadata.yaml", - "codeintel/1678898749_make_unreferenced_documents_index_usable/up.sql", - "codeintel/1678899132_remove_unused_unreferenced_documents_index/down.sql", - "codeintel/1678899132_remove_unused_unreferenced_documents_index/metadata.yaml", - "codeintel/1678899132_remove_unused_unreferenced_documents_index/up.sql", - "codeintel/1679010276_add_missing_index/down.sql", - "codeintel/1679010276_add_missing_index/metadata.yaml", - "codeintel/1679010276_add_missing_index/up.sql", "frontend/1678899992_add_codeintel_ranking_paths/down.sql", "frontend/1678899992_add_codeintel_ranking_paths/metadata.yaml", "frontend/1678899992_add_codeintel_ranking_paths/up.sql", @@ -859,18 +861,15 @@ go_library( "frontend/1679404397_add_missing_index_for_vacuum/down.sql", "frontend/1679404397_add_missing_index_for_vacuum/metadata.yaml", "frontend/1679404397_add_missing_index_for_vacuum/up.sql", + "frontend/1679426934_soft_deleted_repository_name_reconciling_names/down.sql", + "frontend/1679426934_soft_deleted_repository_name_reconciling_names/metadata.yaml", + "frontend/1679426934_soft_deleted_repository_name_reconciling_names/up.sql", "frontend/1679428966_speed_up_deletes_from_ranking_table/down.sql", "frontend/1679428966_speed_up_deletes_from_ranking_table/metadata.yaml", "frontend/1679428966_speed_up_deletes_from_ranking_table/up.sql", "frontend/1679432506_speed_up_deletes_from_cm_trigger_jobs/down.sql", "frontend/1679432506_speed_up_deletes_from_cm_trigger_jobs/metadata.yaml", "frontend/1679432506_speed_up_deletes_from_cm_trigger_jobs/up.sql", - "frontend/1679426934_soft_deleted_repository_name_reconciling_names/down.sql", - "frontend/1679426934_soft_deleted_repository_name_reconciling_names/metadata.yaml", - "frontend/1679426934_soft_deleted_repository_name_reconciling_names/up.sql", - "codeinsights/1679051112_remove_commit_index_tables/down.sql", - "codeinsights/1679051112_remove_commit_index_tables/metadata.yaml", - "codeinsights/1679051112_remove_commit_index_tables/up.sql", "frontend/1679603787_keep_cloning_progress_in_gitserver_repos/down.sql", "frontend/1679603787_keep_cloning_progress_in_gitserver_repos/metadata.yaml", "frontend/1679603787_keep_cloning_progress_in_gitserver_repos/up.sql", @@ -883,21 +882,19 @@ go_library( "frontend/1680707560_sg_telemetry_allowlist/down.sql", "frontend/1680707560_sg_telemetry_allowlist/metadata.yaml", "frontend/1680707560_sg_telemetry_allowlist/up.sql", - "frontend/1681923094_sg_telemetry_allowlist/down.sql", - "frontend/1681923094_sg_telemetry_allowlist/metadata.yaml", - "frontend/1681923094_sg_telemetry_allowlist/up.sql", - "frontend/1681807446_add_github_apps_table/down.sql", - "frontend/1681807446_add_github_apps_table/metadata.yaml", - "frontend/1681807446_add_github_apps_table/up.sql", -<<<<<<< HEAD "frontend/1681300431_ownership_signals/down.sql", "frontend/1681300431_ownership_signals/metadata.yaml", "frontend/1681300431_ownership_signals/up.sql", -======= + "frontend/1681807446_add_github_apps_table/down.sql", + "frontend/1681807446_add_github_apps_table/metadata.yaml", + "frontend/1681807446_add_github_apps_table/up.sql", + "frontend/1681923094_sg_telemetry_allowlist/down.sql", + "frontend/1681923094_sg_telemetry_allowlist/metadata.yaml", + "frontend/1681923094_sg_telemetry_allowlist/up.sql", "frontend/1681982430_user_completions_quota/down.sql", "frontend/1681982430_user_completions_quota/metadata.yaml", "frontend/1681982430_user_completions_quota/up.sql", ->>>>>>> main + "frontend/squashed.sql", ], importpath = "github.com/sourcegraph/sourcegraph/migrations", visibility = ["//visibility:public"], From 81bbaaba35aec0bdff4a0e9e928e5df72d572e9c Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Tue, 25 Apr 2023 13:26:00 -0700 Subject: [PATCH 11/30] pr comments --- .../database/recent_contribution_signal.go | 52 +++-- internal/database/schema.json | 200 +++++++++--------- internal/database/schema.md | 42 ++-- migrations/frontend/squashed.sql | 52 ++--- 4 files changed, 182 insertions(+), 164 deletions(-) diff --git a/internal/database/recent_contribution_signal.go b/internal/database/recent_contribution_signal.go index cf96133b0ef32..70221b0900168 100644 --- a/internal/database/recent_contribution_signal.go +++ b/internal/database/recent_contribution_signal.go @@ -2,6 +2,7 @@ package database import ( "context" + "fmt" "hash/fnv" "path" "time" @@ -10,6 +11,7 @@ import ( "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/database/basestore" + "github.com/sourcegraph/sourcegraph/internal/database/dbutil" "github.com/sourcegraph/sourcegraph/lib/errors" ) @@ -41,6 +43,15 @@ type recentContributionSignalStore struct { *basestore.Store } +func (s *recentContributionSignalStore) With(other basestore.ShareableStore) *recentContributionSignalStore { + return &recentContributionSignalStore{Store: s.Store.With(other)} +} + +func (s *recentContributionSignalStore) Transact(ctx context.Context) (*recentContributionSignalStore, error) { + txBase, err := s.Store.Transact(ctx) + return &recentContributionSignalStore{Store: txBase}, err +} + const commitAuthorInsertFmtstr = ` WITH already_exists (id) AS ( SELECT id @@ -94,7 +105,8 @@ const pathInsertFmtstr = ` ) SELECT id FROM already_exists UNION ALL - SELECT id FROM need_to_insert` + SELECT id FROM need_to_insert +` // ensureRepoPaths takes paths of files changed in the given commit // and makes sure they all exist in the database (alongside with their ancestor paths) @@ -157,7 +169,7 @@ func (s *recentContributionSignalStore) ensureRepoPaths(ctx context.Context, com } ids[p] = id } - // return the IDs of inserted files changed, in order + // return the IDs of inserted files changed, in order of `commit.FilesChanged` fIDs := make([]int, len(commit.FilesChanged)) for i, f := range commit.FilesChanged { id, ok := ids[f] @@ -185,18 +197,24 @@ const insertRecentContributorSignalFmtstr = ` // The aggregate signals in `own_aggregate_recent_contribution` are updated atomically // for each new signal appearing in `own_signal_recent_contribution` by using // a trigger: `update_own_aggregate_recent_contribution`. -func (s *recentContributionSignalStore) AddCommit(ctx context.Context, commit Commit) error { - db := s.Store +func (s *recentContributionSignalStore) AddCommit(ctx context.Context, commit Commit) (err error) { + tx, err := s.Transact(ctx) + if err != nil { + return err + } + defer func() { err = tx.Done(err) }() + // Get or create commit author: - authorID, err := s.ensureAuthor(ctx, commit) + authorID, err := tx.ensureAuthor(ctx, commit) if err != nil { return errors.Wrap(err, "cannot insert commit author") } // Get or create necessary repo paths: - pathIDs, err := s.ensureRepoPaths(ctx, commit) + pathIDs, err := tx.ensureRepoPaths(ctx, commit) if err != nil { return errors.Wrap(err, "cannot insert repo paths") } + fmt.Println(fmt.Sprintf("commit: %s", commit.CommitSHA)) // Insert individual signals into own_signal_recent_contribution: for _, pathID := range pathIDs { commitID := fnv.New32a() @@ -207,7 +225,7 @@ func (s *recentContributionSignalStore) AddCommit(ctx context.Context, commit Co commit.Timestamp, commitID.Sum32(), ) - err = db.Exec(ctx, q) + err = tx.Exec(ctx, q) if err != nil { return err } @@ -236,19 +254,19 @@ const findRecentContributorsFmtstr = ` // - Empty string `path` designates repo root (so all contributions for the whole repo). // - TODO: Need to support limit & offset here. func (s *recentContributionSignalStore) FindRecentAuthors(ctx context.Context, repoID api.RepoID, path string) ([]RecentContributorSummary, error) { - var contributions []RecentContributorSummary q := sqlf.Sprintf(findRecentContributorsFmtstr, repoID, path) - rows, err := s.Query(ctx, q) - if err != nil { - return nil, err - } - defer rows.Close() - for rows.Next() { + + contributionsScanner := basestore.NewSliceScanner(func(scanner dbutil.Scanner) (RecentContributorSummary, error) { var rcs RecentContributorSummary - if err := rows.Scan(&rcs.AuthorName, &rcs.AuthorEmail, &rcs.ContributionCount); err != nil { - return nil, err + if err := scanner.Scan(&rcs.AuthorName, &rcs.AuthorEmail, &rcs.ContributionCount); err != nil { + return RecentContributorSummary{}, err } - contributions = append(contributions, rcs) + return rcs, nil + }) + + contributions, err := contributionsScanner(s.Query(ctx, q)) + if err != nil { + return nil, err } return contributions, nil } diff --git a/internal/database/schema.json b/internal/database/schema.json index 7da061bc9383a..d01c323fa3ce8 100755 --- a/internal/database/schema.json +++ b/internal/database/schema.json @@ -934,7 +934,7 @@ "CycleOption": "NO" }, { - "Name": "own_signal_recent_contribution_id_seq", + "Name": "own_background_jobs_id_seq", "TypeName": "integer", "StartValue": 1, "MinimumValue": 1, @@ -943,7 +943,7 @@ "CycleOption": "NO" }, { - "Name": "own_background_jobs_id_seq", + "Name": "own_signal_recent_contribution_id_seq", "TypeName": "integer", "StartValue": 1, "MinimumValue": 1, @@ -18492,6 +18492,103 @@ ], "Triggers": [] }, + { + "Name": "own_aggregate_recent_contribution", + "Comment": "", + "Columns": [ + { + "Name": "changed_file_path_id", + "Index": 3, + "TypeName": "integer", + "IsNullable": false, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "commit_author_id", + "Index": 2, + "TypeName": "integer", + "IsNullable": false, + "Default": "", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "contributions_count", + "Index": 4, + "TypeName": "integer", + "IsNullable": true, + "Default": "0", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + }, + { + "Name": "id", + "Index": 1, + "TypeName": "integer", + "IsNullable": false, + "Default": "nextval('own_aggregate_recent_contribution_id_seq'::regclass)", + "CharacterMaximumLength": 0, + "IsIdentity": false, + "IdentityGeneration": "", + "IsGenerated": "NEVER", + "GenerationExpression": "", + "Comment": "" + } + ], + "Indexes": [ + { + "Name": "own_aggregate_recent_contribution_file_author", + "IsPrimaryKey": false, + "IsUnique": true, + "IsExclusion": false, + "IsDeferrable": false, + "IndexDefinition": "CREATE UNIQUE INDEX own_aggregate_recent_contribution_file_author ON own_aggregate_recent_contribution USING btree (changed_file_path_id, commit_author_id)", + "ConstraintType": "", + "ConstraintDefinition": "" + }, + { + "Name": "own_aggregate_recent_contribution_pkey", + "IsPrimaryKey": true, + "IsUnique": true, + "IsExclusion": false, + "IsDeferrable": false, + "IndexDefinition": "CREATE UNIQUE INDEX own_aggregate_recent_contribution_pkey ON own_aggregate_recent_contribution USING btree (id)", + "ConstraintType": "p", + "ConstraintDefinition": "PRIMARY KEY (id)" + } + ], + "Constraints": [ + { + "Name": "own_aggregate_recent_contribution_changed_file_path_id_fkey", + "ConstraintType": "f", + "RefTableName": "repo_paths", + "IsDeferrable": false, + "ConstraintDefinition": "FOREIGN KEY (changed_file_path_id) REFERENCES repo_paths(id)" + }, + { + "Name": "own_aggregate_recent_contribution_commit_author_id_fkey", + "ConstraintType": "f", + "RefTableName": "commit_authors", + "IsDeferrable": false, + "ConstraintDefinition": "FOREIGN KEY (commit_author_id) REFERENCES commit_authors(id)" + } + ], + "Triggers": [] + }, { "Name": "own_background_jobs", "Comment": "", @@ -18727,103 +18824,6 @@ "Constraints": null, "Triggers": [] }, - { - "Name": "own_aggregate_recent_contribution", - "Comment": "", - "Columns": [ - { - "Name": "changed_file_path_id", - "Index": 3, - "TypeName": "integer", - "IsNullable": false, - "Default": "", - "CharacterMaximumLength": 0, - "IsIdentity": false, - "IdentityGeneration": "", - "IsGenerated": "NEVER", - "GenerationExpression": "", - "Comment": "" - }, - { - "Name": "commit_author_id", - "Index": 2, - "TypeName": "integer", - "IsNullable": false, - "Default": "", - "CharacterMaximumLength": 0, - "IsIdentity": false, - "IdentityGeneration": "", - "IsGenerated": "NEVER", - "GenerationExpression": "", - "Comment": "" - }, - { - "Name": "contributions_count", - "Index": 4, - "TypeName": "integer", - "IsNullable": true, - "Default": "0", - "CharacterMaximumLength": 0, - "IsIdentity": false, - "IdentityGeneration": "", - "IsGenerated": "NEVER", - "GenerationExpression": "", - "Comment": "" - }, - { - "Name": "id", - "Index": 1, - "TypeName": "integer", - "IsNullable": false, - "Default": "nextval('own_aggregate_recent_contribution_id_seq'::regclass)", - "CharacterMaximumLength": 0, - "IsIdentity": false, - "IdentityGeneration": "", - "IsGenerated": "NEVER", - "GenerationExpression": "", - "Comment": "" - } - ], - "Indexes": [ - { - "Name": "own_aggregate_recent_contribution_file_author", - "IsPrimaryKey": false, - "IsUnique": true, - "IsExclusion": false, - "IsDeferrable": false, - "IndexDefinition": "CREATE UNIQUE INDEX own_aggregate_recent_contribution_file_author ON own_aggregate_recent_contribution USING btree (changed_file_path_id, commit_author_id)", - "ConstraintType": "", - "ConstraintDefinition": "" - }, - { - "Name": "own_aggregate_recent_contribution_pkey", - "IsPrimaryKey": true, - "IsUnique": true, - "IsExclusion": false, - "IsDeferrable": false, - "IndexDefinition": "CREATE UNIQUE INDEX own_aggregate_recent_contribution_pkey ON own_aggregate_recent_contribution USING btree (id)", - "ConstraintType": "p", - "ConstraintDefinition": "PRIMARY KEY (id)" - } - ], - "Constraints": [ - { - "Name": "own_aggregate_recent_contribution_changed_file_path_id_fkey", - "ConstraintType": "f", - "RefTableName": "repo_paths", - "IsDeferrable": false, - "ConstraintDefinition": "FOREIGN KEY (changed_file_path_id) REFERENCES repo_paths(id)" - }, - { - "Name": "own_aggregate_recent_contribution_commit_author_id_fkey", - "ConstraintType": "f", - "RefTableName": "commit_authors", - "IsDeferrable": false, - "ConstraintDefinition": "FOREIGN KEY (commit_author_id) REFERENCES commit_authors(id)" - } - ], - "Triggers": [] - }, { "Name": "own_signal_recent_contribution", "Comment": "One entry per file changed in every commit that classifies as a contribution signal.", @@ -26104,4 +26104,4 @@ "Definition": " SELECT changeset_specs.id AS changeset_spec_id,\n COALESCE(changesets.id, (0)::bigint) AS changeset_id,\n changeset_specs.repo_id,\n changeset_specs.batch_spec_id,\n repo.name AS repo_name,\n COALESCE((changesets.metadata -\u003e\u003e 'Title'::text), (changesets.metadata -\u003e\u003e 'title'::text)) AS changeset_name,\n changesets.external_state,\n changesets.publication_state,\n changesets.reconciler_state,\n changesets.computed_state\n FROM ((changeset_specs\n LEFT JOIN changesets ON (((changesets.repo_id = changeset_specs.repo_id) AND (changesets.external_id = changeset_specs.external_id))))\n JOIN repo ON ((changeset_specs.repo_id = repo.id)))\n WHERE ((changeset_specs.external_id IS NOT NULL) AND (repo.deleted_at IS NULL));" } ] -} +} \ No newline at end of file diff --git a/internal/database/schema.md b/internal/database/schema.md index 76b234c951cfe..3e292d67f9316 100755 --- a/internal/database/schema.md +++ b/internal/database/schema.md @@ -2799,27 +2799,6 @@ Foreign-key constraints: ``` -# Table "public.own_signal_recent_contribution" -``` - Column | Type | Collation | Nullable | Default -----------------------+-----------------------------+-----------+----------+------------------------------------------------------------ - id | integer | | not null | nextval('own_signal_recent_contribution_id_seq'::regclass) - commit_author_id | integer | | not null | - changed_file_path_id | integer | | not null | - commit_timestamp | timestamp without time zone | | not null | - commit_id_hash | integer | | not null | -Indexes: - "own_signal_recent_contribution_pkey" PRIMARY KEY, btree (id) -Foreign-key constraints: - "own_signal_recent_contribution_changed_file_path_id_fkey" FOREIGN KEY (changed_file_path_id) REFERENCES repo_paths(id) - "own_signal_recent_contribution_commit_author_id_fkey" FOREIGN KEY (commit_author_id) REFERENCES commit_authors(id) -Triggers: - update_own_aggregate_recent_contribution AFTER INSERT ON own_signal_recent_contribution FOR EACH ROW EXECUTE FUNCTION update_own_aggregate_recent_contribution() - -``` - -One entry per file changed in every commit that classifies as a contribution signal. - # Table "public.own_background_jobs" ``` Column | Type | Collation | Nullable | Default @@ -2846,6 +2825,27 @@ Indexes: ``` +# Table "public.own_signal_recent_contribution" +``` + Column | Type | Collation | Nullable | Default +----------------------+-----------------------------+-----------+----------+------------------------------------------------------------ + id | integer | | not null | nextval('own_signal_recent_contribution_id_seq'::regclass) + commit_author_id | integer | | not null | + changed_file_path_id | integer | | not null | + commit_timestamp | timestamp without time zone | | not null | + commit_id_hash | integer | | not null | +Indexes: + "own_signal_recent_contribution_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + "own_signal_recent_contribution_changed_file_path_id_fkey" FOREIGN KEY (changed_file_path_id) REFERENCES repo_paths(id) + "own_signal_recent_contribution_commit_author_id_fkey" FOREIGN KEY (commit_author_id) REFERENCES commit_authors(id) +Triggers: + update_own_aggregate_recent_contribution AFTER INSERT ON own_signal_recent_contribution FOR EACH ROW EXECUTE FUNCTION update_own_aggregate_recent_contribution() + +``` + +One entry per file changed in every commit that classifies as a contribution signal. + # Table "public.package_repo_filters" ``` Column | Type | Collation | Nullable | Default diff --git a/migrations/frontend/squashed.sql b/migrations/frontend/squashed.sql index da97756efe6c7..eb27f606b7194 100755 --- a/migrations/frontend/squashed.sql +++ b/migrations/frontend/squashed.sql @@ -3538,26 +3538,6 @@ CREATE SEQUENCE own_aggregate_recent_contribution_id_seq ALTER SEQUENCE own_aggregate_recent_contribution_id_seq OWNED BY own_aggregate_recent_contribution.id; -CREATE TABLE own_signal_recent_contribution ( - id integer NOT NULL, - commit_author_id integer NOT NULL, - changed_file_path_id integer NOT NULL, - commit_timestamp timestamp without time zone NOT NULL, - commit_id_hash integer NOT NULL -); - -COMMENT ON TABLE own_signal_recent_contribution IS 'One entry per file changed in every commit that classifies as a contribution signal.'; - -CREATE SEQUENCE own_signal_recent_contribution_id_seq - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE own_signal_recent_contribution_id_seq OWNED BY own_signal_recent_contribution.id; - CREATE TABLE own_background_jobs ( id integer NOT NULL, state text DEFAULT 'queued'::text, @@ -3586,6 +3566,26 @@ CREATE SEQUENCE own_background_jobs_id_seq ALTER SEQUENCE own_background_jobs_id_seq OWNED BY own_background_jobs.id; +CREATE TABLE own_signal_recent_contribution ( + id integer NOT NULL, + commit_author_id integer NOT NULL, + changed_file_path_id integer NOT NULL, + commit_timestamp timestamp without time zone NOT NULL, + commit_id_hash integer NOT NULL +); + +COMMENT ON TABLE own_signal_recent_contribution IS 'One entry per file changed in every commit that classifies as a contribution signal.'; + +CREATE SEQUENCE own_signal_recent_contribution_id_seq + AS integer + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE own_signal_recent_contribution_id_seq OWNED BY own_signal_recent_contribution.id; + CREATE TABLE package_repo_filters ( id integer NOT NULL, behaviour text NOT NULL, @@ -4680,10 +4680,10 @@ ALTER TABLE ONLY outbound_webhooks ALTER COLUMN id SET DEFAULT nextval('outbound ALTER TABLE ONLY own_aggregate_recent_contribution ALTER COLUMN id SET DEFAULT nextval('own_aggregate_recent_contribution_id_seq'::regclass); -ALTER TABLE ONLY own_signal_recent_contribution ALTER COLUMN id SET DEFAULT nextval('own_signal_recent_contribution_id_seq'::regclass); - ALTER TABLE ONLY own_background_jobs ALTER COLUMN id SET DEFAULT nextval('own_background_jobs_id_seq'::regclass); +ALTER TABLE ONLY own_signal_recent_contribution ALTER COLUMN id SET DEFAULT nextval('own_signal_recent_contribution_id_seq'::regclass); + ALTER TABLE ONLY package_repo_filters ALTER COLUMN id SET DEFAULT nextval('package_repo_filters_id_seq'::regclass); ALTER TABLE ONLY package_repo_versions ALTER COLUMN id SET DEFAULT nextval('package_repo_versions_id_seq'::regclass); @@ -5069,12 +5069,12 @@ ALTER TABLE ONLY outbound_webhooks ALTER TABLE ONLY own_aggregate_recent_contribution ADD CONSTRAINT own_aggregate_recent_contribution_pkey PRIMARY KEY (id); -ALTER TABLE ONLY own_signal_recent_contribution - ADD CONSTRAINT own_signal_recent_contribution_pkey PRIMARY KEY (id); - ALTER TABLE ONLY own_background_jobs ADD CONSTRAINT own_background_jobs_pkey PRIMARY KEY (id); +ALTER TABLE ONLY own_signal_recent_contribution + ADD CONSTRAINT own_signal_recent_contribution_pkey PRIMARY KEY (id); + ALTER TABLE ONLY package_repo_filters ADD CONSTRAINT package_repo_filters_pkey PRIMARY KEY (id); @@ -6302,4 +6302,4 @@ SELECT pg_catalog.setval('lsif_configuration_policies_id_seq', 3, true); INSERT INTO roles VALUES (1, '2023-01-04 16:29:41.195966+00', true, 'USER'); INSERT INTO roles VALUES (2, '2023-01-04 16:29:41.195966+00', true, 'SITE_ADMINISTRATOR'); -SELECT pg_catalog.setval('roles_id_seq', 3, true); +SELECT pg_catalog.setval('roles_id_seq', 3, true); \ No newline at end of file From 11d0960b1bb97b0f792dcbf8e558a7296eddfd6e Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Wed, 26 Apr 2023 08:49:17 -0700 Subject: [PATCH 12/30] wip --- .../internal/own/background/background.go | 63 +++++++++++++++++-- internal/gitserver/client.go | 10 +++ internal/gitserver/commands.go | 58 +++++++++++++++++ 3 files changed, 125 insertions(+), 6 deletions(-) diff --git a/enterprise/internal/own/background/background.go b/enterprise/internal/own/background/background.go index 8766512fe7a30..7bb569abd547b 100644 --- a/enterprise/internal/own/background/background.go +++ b/enterprise/internal/own/background/background.go @@ -2,7 +2,6 @@ package background import ( "context" - "encoding/json" "fmt" "time" @@ -12,11 +11,13 @@ import ( "github.com/sourcegraph/log" "github.com/sourcegraph/sourcegraph/enterprise/internal/codeintel/shared/background" + "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/conf" "github.com/sourcegraph/sourcegraph/internal/database" "github.com/sourcegraph/sourcegraph/internal/database/basestore" "github.com/sourcegraph/sourcegraph/internal/database/dbutil" "github.com/sourcegraph/sourcegraph/internal/executor" + "github.com/sourcegraph/sourcegraph/internal/gitserver" "github.com/sourcegraph/sourcegraph/internal/goroutine" "github.com/sourcegraph/sourcegraph/internal/observation" "github.com/sourcegraph/sourcegraph/internal/ratelimit" @@ -28,18 +29,35 @@ import ( type IndexJobType struct { Name string - Id int + Id JobTypeId IndexInterval time.Duration RefreshInterval time.Duration } var IndexJobTypes = []IndexJobType{{ Name: "recent-contributors", - Id: 1, + Id: RecentContributors, IndexInterval: time.Hour * 24, RefreshInterval: time.Minute * 5, }} +var IndexJobById = mapIndexJobs(IndexJobTypes) + +func mapIndexJobs(types []IndexJobType) map[JobTypeId]IndexJobType { + mapped := make(map[JobTypeId]IndexJobType) + for _, jobType := range types { + mapped[jobType.Id] = jobType + } + return mapped +} + +type JobTypeId int + +const ( + _ JobTypeId = iota + RecentContributors +) + func featureFlagName(jobType IndexJobType) string { return fmt.Sprintf("own-background-index-repo-%s", jobType.Name) } @@ -173,13 +191,46 @@ type handler struct { limiter *ratelimit.InstrumentedLimiter } -func (h *handler) Handle(ctx context.Context, logger log.Logger, record *Job) error { +func (h *handler) Handle(ctx context.Context, lgr log.Logger, record *Job) error { err := h.limiter.Wait(ctx) if err != nil { return errors.Wrap(err, "limiter.Wait") } - jsonify, _ := json.Marshal(record) - logger.Info(fmt.Sprintf("Hello from the own background processor: %v", string(jsonify))) + // jsonify, _ := json.Marshal(record) + + switch JobTypeId(record.JobType) { + case RecentContributors: + repoStore := database.ReposWith(lgr, h.workerStore) + repoID := api.RepoID(record.RepoId) + repo, err := repoStore.Get(ctx, repoID) + if err != nil { + return errors.Wrap(err, "repoStore.Get") + } + commitLog, err := gitserver.NewClient().CommitLog(ctx, repo.Name, time.Now().AddDate(0, 0, -90)) + if err != nil { + return errors.Wrap(err, "CommitLog") + } + + count := 0 + contribStore := database.RecentContributionSignalStoreWith(h.workerStore) + for _, commit := range commitLog { + + err := contribStore.AddCommit(ctx, database.Commit{ + RepoID: repoID, + AuthorName: commit.AuthorName, + AuthorEmail: commit.AuthorEmail, + Timestamp: commit.Timestamp, + CommitSHA: commit.SHA, + FilesChanged: commit.ChangedFiles, + }) + if err != nil { + return errors.Wrap(err, "AddCommit") + } + count++ + } + lgr.Info("commits inserted", log.Int("count", count), log.Int("repo_id", record.RepoId)) + } + return nil } diff --git a/internal/gitserver/client.go b/internal/gitserver/client.go index 82dea600bc8af..fbe26af666d09 100644 --- a/internal/gitserver/client.go +++ b/internal/gitserver/client.go @@ -206,6 +206,14 @@ type HunkReader interface { Close() error } +type CommitLog struct { + AuthorEmail string + AuthorName string + Timestamp time.Time + SHA string + ChangedFiles []string +} + type Client interface { // AddrForRepo returns the gitserver address to use for the given repo name. AddrForRepo(api.RepoName) string @@ -381,6 +389,8 @@ type Client interface { // many commits will be returned. CommitGraph(ctx context.Context, repo api.RepoName, opts CommitGraphOptions) (_ *gitdomain.CommitGraph, err error) + CommitLog(ctx context.Context, repo api.RepoName, after time.Time) ([]CommitLog, error) + // CommitsUniqueToBranch returns a map from commits that exist on a particular // branch in the given repository to their committer date. This set of commits is // determined by listing `{branchName} ^HEAD`, which is interpreted as: all diff --git a/internal/gitserver/commands.go b/internal/gitserver/commands.go index 1a2f970fd0115..c69e743abadc3 100644 --- a/internal/gitserver/commands.go +++ b/internal/gitserver/commands.go @@ -324,6 +324,64 @@ func (c *clientImplementor) CommitGraph(ctx context.Context, repo api.RepoName, return gitdomain.ParseCommitGraph(strings.Split(string(out), "\n")), nil } +func (c *clientImplementor) CommitLog(ctx context.Context, repo api.RepoName, after time.Time) ([]CommitLog, error) { + args := []string{"log", "--pretty=format:'%H%ae%an%ad'", "--name-only", "--topo-order", "--no-merges"} + if !after.IsZero() { + args = append(args, fmt.Sprintf("--after=%s", after.Format(time.RFC3339))) + } + + cmd := c.gitCommand(repo, args...) + + out, err := cmd.CombinedOutput(ctx) + if err != nil { + return nil, err + } + var ls []CommitLog + lines := strings.Split(string(out), "\n\n") + + for _, logOutput := range lines { + partitions := strings.Split(logOutput, "\n") + if len(partitions) < 2 { + continue + } + metaLine := partitions[0] + changedFiles := partitions[1:] + + parts := strings.Split(metaLine, "") + if len(parts) != 4 { + continue + } + sha, authorEmail, authorName, timestamp := parts[0], parts[1], parts[2], parts[3] + // changedFiles := strings.Split(changed, "\n") + t, err := parseTimestamp(timestamp) + if err != nil { + fmt.Println(parts) + command := strings.Join(args, " ") + fmt.Println(command) + fmt.Println(fmt.Sprintf("repo_name: %s", repo)) + + return nil, errors.Wrapf(err, "parseTimestamp %s", timestamp) + } + ls = append(ls, CommitLog{ + SHA: sha, + AuthorEmail: authorEmail, + AuthorName: authorName, + Timestamp: t, + ChangedFiles: changedFiles, + }) + } + return ls, nil +} + +func parseTimestamp(timestamp string) (time.Time, error) { + layout := "Mon Jan 2 15:04:05 2006 -0700" + t, err := time.Parse(layout, timestamp) + if err != nil { + return time.Now(), err // Handle error and fallback to other timestamp format + } + return t, nil +} + // DevNullSHA 4b825dc642cb6eb9a060e54bf8d69288fbee4904 is `git hash-object -t // tree /dev/null`, which is used as the base when computing the `git diff` of // the root commit. From 1ee3bdc347f502fe80baeee86ebf7cd97cb00dd5 Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Wed, 26 Apr 2023 11:19:26 -0700 Subject: [PATCH 13/30] wip --- internal/gitserver/commands.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/gitserver/commands.go b/internal/gitserver/commands.go index c69e743abadc3..756ddef4d4163 100644 --- a/internal/gitserver/commands.go +++ b/internal/gitserver/commands.go @@ -374,6 +374,9 @@ func (c *clientImplementor) CommitLog(ctx context.Context, repo api.RepoName, af } func parseTimestamp(timestamp string) (time.Time, error) { + if strings.HasSuffix(timestamp, "'") { + timestamp = timestamp[:len(timestamp)-1] + } layout := "Mon Jan 2 15:04:05 2006 -0700" t, err := time.Parse(layout, timestamp) if err != nil { From ee4bf6bc6e9f771ae61ca2b315ee7409db6c8260 Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Wed, 26 Apr 2023 11:31:00 -0700 Subject: [PATCH 14/30] convert commit column to bytea --- internal/database/recent_contribution_signal.go | 9 +++------ .../database/recent_contribution_signal_test.go | 13 +++++++++++-- internal/database/schema.json | 4 ++-- internal/database/schema.md | 2 +- .../frontend/1681300431_ownership_signals/up.sql | 2 +- migrations/frontend/squashed.sql | 2 +- 6 files changed, 19 insertions(+), 13 deletions(-) diff --git a/internal/database/recent_contribution_signal.go b/internal/database/recent_contribution_signal.go index 70221b0900168..4f0a78f62d48b 100644 --- a/internal/database/recent_contribution_signal.go +++ b/internal/database/recent_contribution_signal.go @@ -3,7 +3,6 @@ package database import ( "context" "fmt" - "hash/fnv" "path" "time" @@ -186,13 +185,13 @@ const insertRecentContributorSignalFmtstr = ` commit_author_id, changed_file_path_id, commit_timestamp, - commit_id_hash + commit_id ) VALUES (%s, %s, %s, %s) ` // AddCommit inserts a recent contribution signal for each file changed by given commit. // -// As per schema, `commit_id_hash` is just a int hash of the git SHA. +// As per schema, `commit_id` is the git sha stored as bytea. // This is used for the purpose of removing old recent contributor signals. // The aggregate signals in `own_aggregate_recent_contribution` are updated atomically // for each new signal appearing in `own_signal_recent_contribution` by using @@ -217,13 +216,11 @@ func (s *recentContributionSignalStore) AddCommit(ctx context.Context, commit Co fmt.Println(fmt.Sprintf("commit: %s", commit.CommitSHA)) // Insert individual signals into own_signal_recent_contribution: for _, pathID := range pathIDs { - commitID := fnv.New32a() - commitID.Write([]byte(commit.CommitSHA)) q := sqlf.Sprintf(insertRecentContributorSignalFmtstr, authorID, pathID, commit.Timestamp, - commitID.Sum32(), + dbutil.CommitBytea(commit.CommitSHA), ) err = tx.Exec(ctx, q) if err != nil { diff --git a/internal/database/recent_contribution_signal_test.go b/internal/database/recent_contribution_signal_test.go index 093b269001e72..ddde9f80878ff 100644 --- a/internal/database/recent_contribution_signal_test.go +++ b/internal/database/recent_contribution_signal_test.go @@ -2,13 +2,16 @@ package database import ( "context" + "crypto/sha1" + "encoding/hex" "fmt" "testing" "time" - "github.com/sourcegraph/log/logtest" "github.com/stretchr/testify/assert" + "github.com/sourcegraph/log/logtest" + "github.com/sourcegraph/sourcegraph/internal/database/dbtest" "github.com/sourcegraph/sourcegraph/internal/types" ) @@ -53,7 +56,7 @@ func TestRecentContributionSignalStore(t *testing.T) { }, } { commit.Timestamp = time.Now() - commit.CommitSHA = fmt.Sprintf("sha%d", i) + commit.CommitSHA = gitSha(fmt.Sprintf("%d", i)) if err := store.AddCommit(ctx, commit); err != nil { t.Fatal(err) } @@ -103,3 +106,9 @@ func TestRecentContributionSignalStore(t *testing.T) { }) } } + +func gitSha(val string) string { + writer := sha1.New() + writer.Write([]byte(val)) + return hex.EncodeToString(writer.Sum(nil)) +} diff --git a/internal/database/schema.json b/internal/database/schema.json index d01c323fa3ce8..d57d2221902c3 100755 --- a/internal/database/schema.json +++ b/internal/database/schema.json @@ -18855,9 +18855,9 @@ "Comment": "" }, { - "Name": "commit_id_hash", + "Name": "commit_id", "Index": 5, - "TypeName": "integer", + "TypeName": "bytea", "IsNullable": false, "Default": "", "CharacterMaximumLength": 0, diff --git a/internal/database/schema.md b/internal/database/schema.md index 3e292d67f9316..000a65f2d5dce 100755 --- a/internal/database/schema.md +++ b/internal/database/schema.md @@ -2833,7 +2833,7 @@ Indexes: commit_author_id | integer | | not null | changed_file_path_id | integer | | not null | commit_timestamp | timestamp without time zone | | not null | - commit_id_hash | integer | | not null | + commit_id | bytea | | not null | Indexes: "own_signal_recent_contribution_pkey" PRIMARY KEY, btree (id) Foreign-key constraints: diff --git a/migrations/frontend/1681300431_ownership_signals/up.sql b/migrations/frontend/1681300431_ownership_signals/up.sql index bd1bb8b0e0702..b1420c3b45296 100644 --- a/migrations/frontend/1681300431_ownership_signals/up.sql +++ b/migrations/frontend/1681300431_ownership_signals/up.sql @@ -27,7 +27,7 @@ CREATE TABLE IF NOT EXISTS own_signal_recent_contribution ( commit_author_id INTEGER NOT NULL REFERENCES commit_authors(id), changed_file_path_id INTEGER NOT NULL REFERENCES repo_paths(id), commit_timestamp TIMESTAMP NOT NULL, - commit_id_hash INTEGER NOT NULL + commit_id bytea NOT NULL ); COMMENT ON TABLE own_signal_recent_contribution diff --git a/migrations/frontend/squashed.sql b/migrations/frontend/squashed.sql index eb27f606b7194..e0067656d0ab0 100755 --- a/migrations/frontend/squashed.sql +++ b/migrations/frontend/squashed.sql @@ -3571,7 +3571,7 @@ CREATE TABLE own_signal_recent_contribution ( commit_author_id integer NOT NULL, changed_file_path_id integer NOT NULL, commit_timestamp timestamp without time zone NOT NULL, - commit_id_hash integer NOT NULL + commit_id bytea NOT NULL ); COMMENT ON TABLE own_signal_recent_contribution IS 'One entry per file changed in every commit that classifies as a contribution signal.'; From f087f699fe1dfd39c4ff73ecd4e1af9f2de9b02b Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Wed, 26 Apr 2023 14:51:49 -0700 Subject: [PATCH 15/30] wip --- .../internal/own/background/background.go | 3 ++- internal/database/recent_contribution_signal.go | 17 ++++++++++++++++- internal/gitserver/commands.go | 6 +----- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/enterprise/internal/own/background/background.go b/enterprise/internal/own/background/background.go index 7bb569abd547b..934f543db21b0 100644 --- a/enterprise/internal/own/background/background.go +++ b/enterprise/internal/own/background/background.go @@ -213,6 +213,7 @@ func (h *handler) Handle(ctx context.Context, lgr log.Logger, record *Job) error count := 0 contribStore := database.RecentContributionSignalStoreWith(h.workerStore) + contribStore for _, commit := range commitLog { err := contribStore.AddCommit(ctx, database.Commit{ @@ -224,7 +225,7 @@ func (h *handler) Handle(ctx context.Context, lgr log.Logger, record *Job) error FilesChanged: commit.ChangedFiles, }) if err != nil { - return errors.Wrap(err, "AddCommit") + return errors.Wrapf(err, "AddCommit %v", commit) } count++ } diff --git a/internal/database/recent_contribution_signal.go b/internal/database/recent_contribution_signal.go index 4f0a78f62d48b..1faf0f27d0fdc 100644 --- a/internal/database/recent_contribution_signal.go +++ b/internal/database/recent_contribution_signal.go @@ -17,6 +17,9 @@ import ( type RecentContributionSignalStore interface { AddCommit(ctx context.Context, commit Commit) error FindRecentAuthors(ctx context.Context, repoID api.RepoID, path string) ([]RecentContributorSummary, error) + ClearSignals(ctx context.Context, repoID api.RepoID) error + Transact(ctx context.Context) (RecentContributionSignalStore, error) + Done(err error) error } func RecentContributionSignalStoreWith(other basestore.ShareableStore) RecentContributionSignalStore { @@ -46,7 +49,7 @@ func (s *recentContributionSignalStore) With(other basestore.ShareableStore) *re return &recentContributionSignalStore{Store: s.Store.With(other)} } -func (s *recentContributionSignalStore) Transact(ctx context.Context) (*recentContributionSignalStore, error) { +func (s *recentContributionSignalStore) Transact(ctx context.Context) (RecentContributionSignalStore, error) { txBase, err := s.Store.Transact(ctx) return &recentContributionSignalStore{Store: txBase}, err } @@ -189,6 +192,18 @@ const insertRecentContributorSignalFmtstr = ` ) VALUES (%s, %s, %s, %s) ` +func (s *recentContributionSignalStore) ClearSignals(ctx context.Context, repoID api.RepoID) error { + tables := []string{"own_signal_recent_contribution", "own_aggregate_recent_contribution"} + q := "WITH rps as (select id from repo_paths where repo_id = %s) delete from %s where changed_file_path_id in (select * from rps);" + + for _, table := range tables { + if err := s.Exec(ctx, sqlf.Sprintf(q, repoID, sqlf.Sprintf(table))); err != nil { + return errors.Wrapf(err, "table: %s", table) + } + } + return nil +} + // AddCommit inserts a recent contribution signal for each file changed by given commit. // // As per schema, `commit_id` is the git sha stored as bytea. diff --git a/internal/gitserver/commands.go b/internal/gitserver/commands.go index 756ddef4d4163..0b9086c74ce3a 100644 --- a/internal/gitserver/commands.go +++ b/internal/gitserver/commands.go @@ -325,7 +325,7 @@ func (c *clientImplementor) CommitGraph(ctx context.Context, repo api.RepoName, } func (c *clientImplementor) CommitLog(ctx context.Context, repo api.RepoName, after time.Time) ([]CommitLog, error) { - args := []string{"log", "--pretty=format:'%H%ae%an%ad'", "--name-only", "--topo-order", "--no-merges"} + args := []string{"log", "--pretty=format:%H%ae%an%ad", "--name-only", "--topo-order", "--no-merges"} if !after.IsZero() { args = append(args, fmt.Sprintf("--after=%s", after.Format(time.RFC3339))) } @@ -352,7 +352,6 @@ func (c *clientImplementor) CommitLog(ctx context.Context, repo api.RepoName, af continue } sha, authorEmail, authorName, timestamp := parts[0], parts[1], parts[2], parts[3] - // changedFiles := strings.Split(changed, "\n") t, err := parseTimestamp(timestamp) if err != nil { fmt.Println(parts) @@ -374,9 +373,6 @@ func (c *clientImplementor) CommitLog(ctx context.Context, repo api.RepoName, af } func parseTimestamp(timestamp string) (time.Time, error) { - if strings.HasSuffix(timestamp, "'") { - timestamp = timestamp[:len(timestamp)-1] - } layout := "Mon Jan 2 15:04:05 2006 -0700" t, err := time.Parse(layout, timestamp) if err != nil { From 3b2956f7c0034a36e4671f3c75df5a52698352ae Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Wed, 26 Apr 2023 16:01:19 -0700 Subject: [PATCH 16/30] wip --- .../internal/own/background/background.go | 52 +++++++++++++------ .../database/recent_contribution_signal.go | 13 +++-- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/enterprise/internal/own/background/background.go b/enterprise/internal/own/background/background.go index 934f543db21b0..e4a66c56feb3c 100644 --- a/enterprise/internal/own/background/background.go +++ b/enterprise/internal/own/background/background.go @@ -166,6 +166,7 @@ func makeWorker(ctx context.Context, db database.DB, observationCtx *observation task := handler{ workerStore: workerStore, limiter: limiter, + db: db, } worker := dbworker.NewWorker(ctx, workerStore, workerutil.Handler[*Job](&task), workerutil.WorkerOptions{ @@ -187,6 +188,7 @@ func makeWorker(ctx context.Context, db database.DB, observationCtx *observation } type handler struct { + db database.DB workerStore dbworkerstore.Store[*Job] limiter *ratelimit.InstrumentedLimiter } @@ -196,28 +198,41 @@ func (h *handler) Handle(ctx context.Context, lgr log.Logger, record *Job) error if err != nil { return errors.Wrap(err, "limiter.Wait") } - // jsonify, _ := json.Marshal(record) + var delegate signalIndexFunc switch JobTypeId(record.JobType) { case RecentContributors: - repoStore := database.ReposWith(lgr, h.workerStore) - repoID := api.RepoID(record.RepoId) - repo, err := repoStore.Get(ctx, repoID) - if err != nil { - return errors.Wrap(err, "repoStore.Get") - } - commitLog, err := gitserver.NewClient().CommitLog(ctx, repo.Name, time.Now().AddDate(0, 0, -90)) + delegate = handleRecentContributors + default: + return errors.New("unsupported own index job type") + } + + return delegate(ctx, lgr, api.RepoID(record.RepoId), h.db) +} + +type signalIndexFunc func(ctx context.Context, lgr log.Logger, repoId api.RepoID, db database.DB) error + +func handleRecentContributors(ctx context.Context, lgr log.Logger, repoId api.RepoID, db database.DB) error { + repoStore := db.Repos() + repo, err := repoStore.Get(ctx, repoId) + if err != nil { + return errors.Wrap(err, "repoStore.Get") + } + commitLog, err := gitserver.NewClient().CommitLog(ctx, repo.Name, time.Now().AddDate(0, 0, -90)) + if err != nil { + return errors.Wrap(err, "CommitLog") + } + + count := 0 + err = db.RecentContributionSignals().WithTransact(ctx, func(store database.RecentContributionSignalStore) error { + err := store.ClearSignals(ctx, repoId) if err != nil { - return errors.Wrap(err, "CommitLog") + return err } - count := 0 - contribStore := database.RecentContributionSignalStoreWith(h.workerStore) - contribStore for _, commit := range commitLog { - - err := contribStore.AddCommit(ctx, database.Commit{ - RepoID: repoID, + err := store.AddCommit(ctx, database.Commit{ + RepoID: repoId, AuthorName: commit.AuthorName, AuthorEmail: commit.AuthorEmail, Timestamp: commit.Timestamp, @@ -229,9 +244,12 @@ func (h *handler) Handle(ctx context.Context, lgr log.Logger, record *Job) error } count++ } - lgr.Info("commits inserted", log.Int("count", count), log.Int("repo_id", record.RepoId)) + lgr.Info("commits inserted", log.Int("count", count), log.Int("repo_id", int(repoId))) + return nil + }) + if err != nil { + return err } - return nil } diff --git a/internal/database/recent_contribution_signal.go b/internal/database/recent_contribution_signal.go index 1faf0f27d0fdc..518dfbebe61b2 100644 --- a/internal/database/recent_contribution_signal.go +++ b/internal/database/recent_contribution_signal.go @@ -18,8 +18,7 @@ type RecentContributionSignalStore interface { AddCommit(ctx context.Context, commit Commit) error FindRecentAuthors(ctx context.Context, repoID api.RepoID, path string) ([]RecentContributorSummary, error) ClearSignals(ctx context.Context, repoID api.RepoID) error - Transact(ctx context.Context) (RecentContributionSignalStore, error) - Done(err error) error + WithTransact(context.Context, func(store RecentContributionSignalStore) error) error } func RecentContributionSignalStoreWith(other basestore.ShareableStore) RecentContributionSignalStore { @@ -45,11 +44,17 @@ type recentContributionSignalStore struct { *basestore.Store } +func (s *recentContributionSignalStore) WithTransact(ctx context.Context, f func(store RecentContributionSignalStore) error) error { + return s.Store.WithTransact(ctx, func(tx *basestore.Store) error { + return f(RecentContributionSignalStoreWith(tx)) + }) +} + func (s *recentContributionSignalStore) With(other basestore.ShareableStore) *recentContributionSignalStore { return &recentContributionSignalStore{Store: s.Store.With(other)} } -func (s *recentContributionSignalStore) Transact(ctx context.Context) (RecentContributionSignalStore, error) { +func (s *recentContributionSignalStore) transact(ctx context.Context) (*recentContributionSignalStore, error) { txBase, err := s.Store.Transact(ctx) return &recentContributionSignalStore{Store: txBase}, err } @@ -212,7 +217,7 @@ func (s *recentContributionSignalStore) ClearSignals(ctx context.Context, repoID // for each new signal appearing in `own_signal_recent_contribution` by using // a trigger: `update_own_aggregate_recent_contribution`. func (s *recentContributionSignalStore) AddCommit(ctx context.Context, commit Commit) (err error) { - tx, err := s.Transact(ctx) + tx, err := s.transact(ctx) if err != nil { return err } From eee5961be63c87c88992ab0c70f83689f73997e0 Mon Sep 17 00:00:00 2001 From: Cezary Bartoszuk Date: Sun, 30 Apr 2023 23:04:14 +0200 Subject: [PATCH 17/30] Remove transactions from commit insertion --- .../internal/own/background/background.go | 40 +++++++++---------- .../database/recent_contribution_signal.go | 11 ++--- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/enterprise/internal/own/background/background.go b/enterprise/internal/own/background/background.go index e4a66c56feb3c..93cc28f05570a 100644 --- a/enterprise/internal/own/background/background.go +++ b/enterprise/internal/own/background/background.go @@ -10,6 +10,7 @@ import ( "golang.org/x/time/rate" "github.com/sourcegraph/log" + "github.com/sourcegraph/sourcegraph/enterprise/internal/codeintel/shared/background" "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/conf" @@ -224,29 +225,26 @@ func handleRecentContributors(ctx context.Context, lgr log.Logger, repoId api.Re } count := 0 - err = db.RecentContributionSignals().WithTransact(ctx, func(store database.RecentContributionSignalStore) error { - err := store.ClearSignals(ctx, repoId) - if err != nil { - return err - } + err = db.RecentContributionSignals().ClearSignals(ctx, repoId) + if err != nil { + return err + } - for _, commit := range commitLog { - err := store.AddCommit(ctx, database.Commit{ - RepoID: repoId, - AuthorName: commit.AuthorName, - AuthorEmail: commit.AuthorEmail, - Timestamp: commit.Timestamp, - CommitSHA: commit.SHA, - FilesChanged: commit.ChangedFiles, - }) - if err != nil { - return errors.Wrapf(err, "AddCommit %v", commit) - } - count++ + for _, commit := range commitLog { + err := db.RecentContributionSignals().AddCommit(ctx, database.Commit{ + RepoID: repoId, + AuthorName: commit.AuthorName, + AuthorEmail: commit.AuthorEmail, + Timestamp: commit.Timestamp, + CommitSHA: commit.SHA, + FilesChanged: commit.ChangedFiles, + }) + if err != nil { + return errors.Wrapf(err, "AddCommit %v", commit) } - lgr.Info("commits inserted", log.Int("count", count), log.Int("repo_id", int(repoId))) - return nil - }) + count++ + } + lgr.Info("commits inserted", log.Int("count", count), log.Int("repo_id", int(repoId))) if err != nil { return err } diff --git a/internal/database/recent_contribution_signal.go b/internal/database/recent_contribution_signal.go index 518dfbebe61b2..da3444ef36ae2 100644 --- a/internal/database/recent_contribution_signal.go +++ b/internal/database/recent_contribution_signal.go @@ -217,11 +217,12 @@ func (s *recentContributionSignalStore) ClearSignals(ctx context.Context, repoID // for each new signal appearing in `own_signal_recent_contribution` by using // a trigger: `update_own_aggregate_recent_contribution`. func (s *recentContributionSignalStore) AddCommit(ctx context.Context, commit Commit) (err error) { - tx, err := s.transact(ctx) - if err != nil { - return err - } - defer func() { err = tx.Done(err) }() + //tx, err := s.transact(ctx) + //if err != nil { + // return err + //} + //defer func() { err = tx.Done(err) }() + tx := s // Get or create commit author: authorID, err := tx.ensureAuthor(ctx, commit) From 1590f83297f72d82b7ceb11ea8112d01fd17801d Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Mon, 1 May 2023 11:20:25 -0700 Subject: [PATCH 18/30] remove comment --- internal/database/recent_contribution_signal.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/internal/database/recent_contribution_signal.go b/internal/database/recent_contribution_signal.go index da3444ef36ae2..a74e044ae6db3 100644 --- a/internal/database/recent_contribution_signal.go +++ b/internal/database/recent_contribution_signal.go @@ -217,20 +217,13 @@ func (s *recentContributionSignalStore) ClearSignals(ctx context.Context, repoID // for each new signal appearing in `own_signal_recent_contribution` by using // a trigger: `update_own_aggregate_recent_contribution`. func (s *recentContributionSignalStore) AddCommit(ctx context.Context, commit Commit) (err error) { - //tx, err := s.transact(ctx) - //if err != nil { - // return err - //} - //defer func() { err = tx.Done(err) }() - tx := s - // Get or create commit author: - authorID, err := tx.ensureAuthor(ctx, commit) + authorID, err := s.ensureAuthor(ctx, commit) if err != nil { return errors.Wrap(err, "cannot insert commit author") } // Get or create necessary repo paths: - pathIDs, err := tx.ensureRepoPaths(ctx, commit) + pathIDs, err := s.ensureRepoPaths(ctx, commit) if err != nil { return errors.Wrap(err, "cannot insert repo paths") } @@ -243,7 +236,7 @@ func (s *recentContributionSignalStore) AddCommit(ctx context.Context, commit Co commit.Timestamp, dbutil.CommitBytea(commit.CommitSHA), ) - err = tx.Exec(ctx, q) + err = s.Exec(ctx, q) if err != nil { return err } From e52ea1480f607c515ea9b00af6fdf2377a61891a Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Mon, 1 May 2023 11:31:55 -0700 Subject: [PATCH 19/30] refactor for testing --- .../internal/own/background/background.go | 40 ------------ .../own/background/recent_contributors.go | 65 +++++++++++++++++++ 2 files changed, 65 insertions(+), 40 deletions(-) create mode 100644 enterprise/internal/own/background/recent_contributors.go diff --git a/enterprise/internal/own/background/background.go b/enterprise/internal/own/background/background.go index 93cc28f05570a..be55424d38fbb 100644 --- a/enterprise/internal/own/background/background.go +++ b/enterprise/internal/own/background/background.go @@ -10,7 +10,6 @@ import ( "golang.org/x/time/rate" "github.com/sourcegraph/log" - "github.com/sourcegraph/sourcegraph/enterprise/internal/codeintel/shared/background" "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/conf" @@ -18,7 +17,6 @@ import ( "github.com/sourcegraph/sourcegraph/internal/database/basestore" "github.com/sourcegraph/sourcegraph/internal/database/dbutil" "github.com/sourcegraph/sourcegraph/internal/executor" - "github.com/sourcegraph/sourcegraph/internal/gitserver" "github.com/sourcegraph/sourcegraph/internal/goroutine" "github.com/sourcegraph/sourcegraph/internal/observation" "github.com/sourcegraph/sourcegraph/internal/ratelimit" @@ -213,44 +211,6 @@ func (h *handler) Handle(ctx context.Context, lgr log.Logger, record *Job) error type signalIndexFunc func(ctx context.Context, lgr log.Logger, repoId api.RepoID, db database.DB) error -func handleRecentContributors(ctx context.Context, lgr log.Logger, repoId api.RepoID, db database.DB) error { - repoStore := db.Repos() - repo, err := repoStore.Get(ctx, repoId) - if err != nil { - return errors.Wrap(err, "repoStore.Get") - } - commitLog, err := gitserver.NewClient().CommitLog(ctx, repo.Name, time.Now().AddDate(0, 0, -90)) - if err != nil { - return errors.Wrap(err, "CommitLog") - } - - count := 0 - err = db.RecentContributionSignals().ClearSignals(ctx, repoId) - if err != nil { - return err - } - - for _, commit := range commitLog { - err := db.RecentContributionSignals().AddCommit(ctx, database.Commit{ - RepoID: repoId, - AuthorName: commit.AuthorName, - AuthorEmail: commit.AuthorEmail, - Timestamp: commit.Timestamp, - CommitSHA: commit.SHA, - FilesChanged: commit.ChangedFiles, - }) - if err != nil { - return errors.Wrapf(err, "AddCommit %v", commit) - } - count++ - } - lgr.Info("commits inserted", log.Int("count", count), log.Int("repo_id", int(repoId))) - if err != nil { - return err - } - return nil -} - func janitorFunc(db database.DB, retention time.Duration) func(ctx context.Context) (numRecordsScanned, numRecordsAltered int, err error) { return func(ctx context.Context) (numRecordsScanned, numRecordsAltered int, err error) { ts := time.Now().Add(-1 * retention) diff --git a/enterprise/internal/own/background/recent_contributors.go b/enterprise/internal/own/background/recent_contributors.go new file mode 100644 index 0000000000000..9924821bb4a07 --- /dev/null +++ b/enterprise/internal/own/background/recent_contributors.go @@ -0,0 +1,65 @@ +package background + +import ( + "context" + "time" + + logger "github.com/sourcegraph/log" + "github.com/sourcegraph/sourcegraph/internal/api" + "github.com/sourcegraph/sourcegraph/internal/database" + "github.com/sourcegraph/sourcegraph/internal/gitserver" + "github.com/sourcegraph/sourcegraph/lib/errors" +) + +func handleRecentContributors(ctx context.Context, lgr logger.Logger, repoId api.RepoID, db database.DB) error { + indexer := newRecentContributorsIndexer(gitserver.NewClient(), db, lgr) + return indexer.indexRepo(ctx, repoId) +} + +type recentContributorsIndexer struct { + client gitserver.Client + db database.DB + logger logger.Logger +} + +func newRecentContributorsIndexer(client gitserver.Client, db database.DB, lgr logger.Logger) *recentContributorsIndexer { + return &recentContributorsIndexer{client: client, db: db, logger: lgr} +} + +func (r *recentContributorsIndexer) indexRepo(ctx context.Context, repoId api.RepoID) error { + repoStore := r.db.Repos() + repo, err := repoStore.Get(ctx, repoId) + if err != nil { + return errors.Wrap(err, "repoStore.Get") + } + commitLog, err := gitserver.NewClient().CommitLog(ctx, repo.Name, time.Now().AddDate(0, 0, -90)) + if err != nil { + return errors.Wrap(err, "CommitLog") + } + + store := r.db.RecentContributionSignals() + err = store.ClearSignals(ctx, repoId) + if err != nil { + return err + } + + for _, commit := range commitLog { + err := store.AddCommit(ctx, database.Commit{ + RepoID: repoId, + AuthorName: commit.AuthorName, + AuthorEmail: commit.AuthorEmail, + Timestamp: commit.Timestamp, + CommitSHA: commit.SHA, + FilesChanged: commit.ChangedFiles, + }) + if err != nil { + return errors.Wrapf(err, "AddCommit %v", commit) + } + } + r.logger.Info("commits inserted", logger.Int("count", len(commitLog)), logger.Int("repo_id", int(repoId))) + if err != nil { + return err + } + + return nil +} From 610e952e77e4d68be1e45b4c50d3567cabee4456 Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Mon, 1 May 2023 14:37:39 -0700 Subject: [PATCH 20/30] test for indexer --- .../own/background/recent_contributors.go | 2 +- .../background/recent_contributors_test.go | 132 ++++++++++++++++++ internal/gitserver/mocks_temp.go | 126 +++++++++++++++++ 3 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 enterprise/internal/own/background/recent_contributors_test.go diff --git a/enterprise/internal/own/background/recent_contributors.go b/enterprise/internal/own/background/recent_contributors.go index 9924821bb4a07..d13d30e69083f 100644 --- a/enterprise/internal/own/background/recent_contributors.go +++ b/enterprise/internal/own/background/recent_contributors.go @@ -32,7 +32,7 @@ func (r *recentContributorsIndexer) indexRepo(ctx context.Context, repoId api.Re if err != nil { return errors.Wrap(err, "repoStore.Get") } - commitLog, err := gitserver.NewClient().CommitLog(ctx, repo.Name, time.Now().AddDate(0, 0, -90)) + commitLog, err := r.client.CommitLog(ctx, repo.Name, time.Now().AddDate(0, 0, -90)) if err != nil { return errors.Wrap(err, "CommitLog") } diff --git a/enterprise/internal/own/background/recent_contributors_test.go b/enterprise/internal/own/background/recent_contributors_test.go new file mode 100644 index 0000000000000..6976669d4f775 --- /dev/null +++ b/enterprise/internal/own/background/recent_contributors_test.go @@ -0,0 +1,132 @@ +package background + +import ( + "context" + "crypto/sha1" + "encoding/hex" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/sourcegraph/log/logtest" + "github.com/sourcegraph/sourcegraph/internal/api" + "github.com/sourcegraph/sourcegraph/internal/database" + "github.com/sourcegraph/sourcegraph/internal/database/dbtest" + "github.com/sourcegraph/sourcegraph/internal/gitserver" + "github.com/sourcegraph/sourcegraph/internal/types" +) + +func Test_RecentContributorIndexFromGitserver(t *testing.T) { + logger := logtest.Scoped(t) + db := database.NewDB(logger, dbtest.NewDB(logger, t)) + + ctx := context.Background() + + err := db.Repos().Create(ctx, &types.Repo{ + ID: 1, + Name: "own/repo1", + }) + require.NoError(t, err) + + commits := []fakeCommit{ + { + name: "alice", + email: "alice@example.com", + changedFiles: []string{"file1.txt", "dir/file2.txt"}, + }, + { + name: "alice", + email: "alice@example.com", + changedFiles: []string{"file1.txt", "dir/file3.txt"}, + }, + { + name: "alice", + email: "alice@example.com", + changedFiles: []string{"file1.txt", "dir/file2.txt", "dir/subdir/file.txt"}, + }, + { + name: "bob", + email: "bob@example.com", + changedFiles: []string{"file1.txt", "dir2/file2.txt", "dir2/subdir/file.txt"}, + }, + } + + client := gitserver.NewMockClient() + client.CommitLogFunc.SetDefaultReturn(fakeCommitsToLog(commits), nil) + + indexer := newRecentContributorsIndexer(client, db, logger) + err = indexer.indexRepo(ctx, api.RepoID(1)) + require.NoError(t, err) + + for p, w := range map[string][]database.RecentContributorSummary{ + "dir": { + { + AuthorName: "alice", + AuthorEmail: "alice@example.com", + ContributionCount: 4, + }, + }, + "file1.txt": { + { + AuthorName: "alice", + AuthorEmail: "alice@example.com", + ContributionCount: 3, + }, + { + AuthorName: "bob", + AuthorEmail: "bob@example.com", + ContributionCount: 1, + }, + }, + "": { + { + AuthorName: "alice", + AuthorEmail: "alice@example.com", + ContributionCount: 7, + }, + { + AuthorName: "bob", + AuthorEmail: "bob@example.com", + ContributionCount: 3, + }, + }, + } { + path := p + want := w + t.Run(path, func(t *testing.T) { + got, err := db.RecentContributionSignals().FindRecentAuthors(ctx, 1, path) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, want, got) + }) + } +} + +func fakeCommitsToLog(commits []fakeCommit) (results []gitserver.CommitLog) { + for i, commit := range commits { + results = append(results, gitserver.CommitLog{ + AuthorEmail: commit.email, + AuthorName: commit.name, + Timestamp: time.Now(), + SHA: gitSha(fmt.Sprintf("%d", i)), + ChangedFiles: commit.changedFiles, + }) + } + return results +} + +type fakeCommit struct { + email string + name string + changedFiles []string +} + +func gitSha(val string) string { + writer := sha1.New() + writer.Write([]byte(val)) + return hex.EncodeToString(writer.Sum(nil)) +} diff --git a/internal/gitserver/mocks_temp.go b/internal/gitserver/mocks_temp.go index 8c186b9dd5dbf..081c0aaf1e8c2 100644 --- a/internal/gitserver/mocks_temp.go +++ b/internal/gitserver/mocks_temp.go @@ -52,6 +52,9 @@ type MockClient struct { // CommitGraphFunc is an instance of a mock function object controlling // the behavior of the method CommitGraph. CommitGraphFunc *ClientCommitGraphFunc + // CommitLogFunc is an instance of a mock function object controlling + // the behavior of the method CommitLog. + CommitLogFunc *ClientCommitLogFunc // CommitsFunc is an instance of a mock function object controlling the // behavior of the method Commits. CommitsFunc *ClientCommitsFunc @@ -229,6 +232,11 @@ func NewMockClient() *MockClient { return }, }, + CommitLogFunc: &ClientCommitLogFunc{ + defaultHook: func(context.Context, api.RepoName, time.Time) (r0 []CommitLog, r1 error) { + return + }, + }, CommitsFunc: &ClientCommitsFunc{ defaultHook: func(context.Context, authz.SubRepoPermissionChecker, api.RepoName, CommitsOptions) (r0 []*gitdomain.Commit, r1 error) { return @@ -491,6 +499,11 @@ func NewStrictMockClient() *MockClient { panic("unexpected invocation of MockClient.CommitGraph") }, }, + CommitLogFunc: &ClientCommitLogFunc{ + defaultHook: func(context.Context, api.RepoName, time.Time) ([]CommitLog, error) { + panic("unexpected invocation of MockClient.CommitLog") + }, + }, CommitsFunc: &ClientCommitsFunc{ defaultHook: func(context.Context, authz.SubRepoPermissionChecker, api.RepoName, CommitsOptions) ([]*gitdomain.Commit, error) { panic("unexpected invocation of MockClient.Commits") @@ -735,6 +748,9 @@ func NewMockClientFrom(i Client) *MockClient { CommitGraphFunc: &ClientCommitGraphFunc{ defaultHook: i.CommitGraph, }, + CommitLogFunc: &ClientCommitLogFunc{ + defaultHook: i.CommitLog, + }, CommitsFunc: &ClientCommitsFunc{ defaultHook: i.Commits, }, @@ -1855,6 +1871,116 @@ func (c ClientCommitGraphFuncCall) Results() []interface{} { return []interface{}{c.Result0, c.Result1} } +// ClientCommitLogFunc describes the behavior when the CommitLog method of +// the parent MockClient instance is invoked. +type ClientCommitLogFunc struct { + defaultHook func(context.Context, api.RepoName, time.Time) ([]CommitLog, error) + hooks []func(context.Context, api.RepoName, time.Time) ([]CommitLog, error) + history []ClientCommitLogFuncCall + mutex sync.Mutex +} + +// CommitLog delegates to the next hook function in the queue and stores the +// parameter and result values of this invocation. +func (m *MockClient) CommitLog(v0 context.Context, v1 api.RepoName, v2 time.Time) ([]CommitLog, error) { + r0, r1 := m.CommitLogFunc.nextHook()(v0, v1, v2) + m.CommitLogFunc.appendCall(ClientCommitLogFuncCall{v0, v1, v2, r0, r1}) + return r0, r1 +} + +// SetDefaultHook sets function that is called when the CommitLog method of +// the parent MockClient instance is invoked and the hook queue is empty. +func (f *ClientCommitLogFunc) SetDefaultHook(hook func(context.Context, api.RepoName, time.Time) ([]CommitLog, error)) { + f.defaultHook = hook +} + +// PushHook adds a function to the end of hook queue. Each invocation of the +// CommitLog method of the parent MockClient instance invokes the hook at +// the front of the queue and discards it. After the queue is empty, the +// default hook function is invoked for any future action. +func (f *ClientCommitLogFunc) PushHook(hook func(context.Context, api.RepoName, time.Time) ([]CommitLog, error)) { + f.mutex.Lock() + f.hooks = append(f.hooks, hook) + f.mutex.Unlock() +} + +// SetDefaultReturn calls SetDefaultHook with a function that returns the +// given values. +func (f *ClientCommitLogFunc) SetDefaultReturn(r0 []CommitLog, r1 error) { + f.SetDefaultHook(func(context.Context, api.RepoName, time.Time) ([]CommitLog, error) { + return r0, r1 + }) +} + +// PushReturn calls PushHook with a function that returns the given values. +func (f *ClientCommitLogFunc) PushReturn(r0 []CommitLog, r1 error) { + f.PushHook(func(context.Context, api.RepoName, time.Time) ([]CommitLog, error) { + return r0, r1 + }) +} + +func (f *ClientCommitLogFunc) nextHook() func(context.Context, api.RepoName, time.Time) ([]CommitLog, error) { + f.mutex.Lock() + defer f.mutex.Unlock() + + if len(f.hooks) == 0 { + return f.defaultHook + } + + hook := f.hooks[0] + f.hooks = f.hooks[1:] + return hook +} + +func (f *ClientCommitLogFunc) appendCall(r0 ClientCommitLogFuncCall) { + f.mutex.Lock() + f.history = append(f.history, r0) + f.mutex.Unlock() +} + +// History returns a sequence of ClientCommitLogFuncCall objects describing +// the invocations of this function. +func (f *ClientCommitLogFunc) History() []ClientCommitLogFuncCall { + f.mutex.Lock() + history := make([]ClientCommitLogFuncCall, len(f.history)) + copy(history, f.history) + f.mutex.Unlock() + + return history +} + +// ClientCommitLogFuncCall is an object that describes an invocation of +// method CommitLog on an instance of MockClient. +type ClientCommitLogFuncCall struct { + // Arg0 is the value of the 1st argument passed to this method + // invocation. + Arg0 context.Context + // Arg1 is the value of the 2nd argument passed to this method + // invocation. + Arg1 api.RepoName + // Arg2 is the value of the 3rd argument passed to this method + // invocation. + Arg2 time.Time + // Result0 is the value of the 1st result returned from this method + // invocation. + Result0 []CommitLog + // Result1 is the value of the 2nd result returned from this method + // invocation. + Result1 error +} + +// Args returns an interface slice containing the arguments of this +// invocation. +func (c ClientCommitLogFuncCall) Args() []interface{} { + return []interface{}{c.Arg0, c.Arg1, c.Arg2} +} + +// Results returns an interface slice containing the results of this +// invocation. +func (c ClientCommitLogFuncCall) Results() []interface{} { + return []interface{}{c.Result0, c.Result1} +} + // ClientCommitsFunc describes the behavior when the Commits method of the // parent MockClient instance is invoked. type ClientCommitsFunc struct { From bef3b2a57bdd0b5864a31f772b7425c0d8b1d73c Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Mon, 1 May 2023 14:43:36 -0700 Subject: [PATCH 21/30] remove print --- internal/database/recent_contribution_signal.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/database/recent_contribution_signal.go b/internal/database/recent_contribution_signal.go index a74e044ae6db3..8fe5516a1b409 100644 --- a/internal/database/recent_contribution_signal.go +++ b/internal/database/recent_contribution_signal.go @@ -2,7 +2,6 @@ package database import ( "context" - "fmt" "path" "time" @@ -227,7 +226,6 @@ func (s *recentContributionSignalStore) AddCommit(ctx context.Context, commit Co if err != nil { return errors.Wrap(err, "cannot insert repo paths") } - fmt.Println(fmt.Sprintf("commit: %s", commit.CommitSHA)) // Insert individual signals into own_signal_recent_contribution: for _, pathID := range pathIDs { q := sqlf.Sprintf(insertRecentContributorSignalFmtstr, From e43d44891625a05dd76b56518a64b789677baf14 Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Mon, 1 May 2023 14:45:17 -0700 Subject: [PATCH 22/30] wrap error --- enterprise/internal/own/background/recent_contributors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/internal/own/background/recent_contributors.go b/enterprise/internal/own/background/recent_contributors.go index d13d30e69083f..55670eb762ce5 100644 --- a/enterprise/internal/own/background/recent_contributors.go +++ b/enterprise/internal/own/background/recent_contributors.go @@ -40,7 +40,7 @@ func (r *recentContributorsIndexer) indexRepo(ctx context.Context, repoId api.Re store := r.db.RecentContributionSignals() err = store.ClearSignals(ctx, repoId) if err != nil { - return err + return errors.Wrap(err, "ClearSignals") } for _, commit := range commitLog { From 0839bc158944455224aff6d1df60328147c54893 Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Mon, 1 May 2023 15:17:39 -0700 Subject: [PATCH 23/30] add counter --- .../internal/own/background/recent_contributors.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/enterprise/internal/own/background/recent_contributors.go b/enterprise/internal/own/background/recent_contributors.go index 55670eb762ce5..9b142e232b415 100644 --- a/enterprise/internal/own/background/recent_contributors.go +++ b/enterprise/internal/own/background/recent_contributors.go @@ -4,6 +4,9 @@ import ( "context" "time" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + logger "github.com/sourcegraph/log" "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/database" @@ -26,6 +29,11 @@ func newRecentContributorsIndexer(client gitserver.Client, db database.DB, lgr l return &recentContributorsIndexer{client: client, db: db, logger: lgr} } +var commitCounter = promauto.NewCounter(prometheus.CounterOpts{ + Namespace: "src", + Name: "own_recent_contributors_commits_indexed_total", +}) + func (r *recentContributorsIndexer) indexRepo(ctx context.Context, repoId api.RepoID) error { repoStore := r.db.Repos() repo, err := repoStore.Get(ctx, repoId) @@ -57,6 +65,7 @@ func (r *recentContributorsIndexer) indexRepo(ctx context.Context, repoId api.Re } } r.logger.Info("commits inserted", logger.Int("count", len(commitLog)), logger.Int("repo_id", int(repoId))) + commitCounter.Add(float64(len(commitLog))) if err != nil { return err } From db4cdfc236cbbff1c27fc76c484541584a7168cc Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Mon, 1 May 2023 15:27:20 -0700 Subject: [PATCH 24/30] cleanup --- enterprise/internal/own/background/background.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/enterprise/internal/own/background/background.go b/enterprise/internal/own/background/background.go index be55424d38fbb..e3e9c54407868 100644 --- a/enterprise/internal/own/background/background.go +++ b/enterprise/internal/own/background/background.go @@ -40,16 +40,6 @@ var IndexJobTypes = []IndexJobType{{ RefreshInterval: time.Minute * 5, }} -var IndexJobById = mapIndexJobs(IndexJobTypes) - -func mapIndexJobs(types []IndexJobType) map[JobTypeId]IndexJobType { - mapped := make(map[JobTypeId]IndexJobType) - for _, jobType := range types { - mapped[jobType.Id] = jobType - } - return mapped -} - type JobTypeId int const ( @@ -190,6 +180,7 @@ type handler struct { db database.DB workerStore dbworkerstore.Store[*Job] limiter *ratelimit.InstrumentedLimiter + op *observation.Operation } func (h *handler) Handle(ctx context.Context, lgr log.Logger, record *Job) error { From 768f3628b51d62df9eb3131612fdba31368d915f Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Mon, 1 May 2023 16:25:14 -0700 Subject: [PATCH 25/30] bazel --- enterprise/internal/own/background/BUILD.bazel | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/enterprise/internal/own/background/BUILD.bazel b/enterprise/internal/own/background/BUILD.bazel index 5c8f664ff1f01..499e3b09f3737 100644 --- a/enterprise/internal/own/background/BUILD.bazel +++ b/enterprise/internal/own/background/BUILD.bazel @@ -4,17 +4,20 @@ go_library( name = "background", srcs = [ "background.go", + "recent_contributors.go", "scheduler.go", ], importpath = "github.com/sourcegraph/sourcegraph/enterprise/internal/own/background", visibility = ["//enterprise:__subpackages__"], deps = [ "//enterprise/internal/codeintel/shared/background", + "//internal/api", "//internal/conf", "//internal/database", "//internal/database/basestore", "//internal/database/dbutil", "//internal/executor", + "//internal/gitserver", "//internal/goroutine", "//internal/metrics", "//internal/observation", @@ -36,18 +39,26 @@ go_library( go_test( name = "background_test", timeout = "moderate", - srcs = ["scheduler_test.go"], + srcs = [ + "recent_contributors_test.go", + "scheduler_test.go", + ], embed = [":background"], tags = [ "requires-network", ], deps = [ + "//internal/api", "//internal/database", "//internal/database/basestore", "//internal/database/dbtest", + "//internal/gitserver", "//internal/observation", + "//internal/types", "@com_github_derision_test_glock//:glock", "@com_github_keegancsmith_sqlf//:sqlf", + "@com_github_sourcegraph_log//logtest", + "@com_github_stretchr_testify//assert", "@com_github_stretchr_testify//require", ], ) From f3b2a7424410e4338c1420ae0fb569b92d2a5c6e Mon Sep 17 00:00:00 2001 From: coury-clark Date: Tue, 2 May 2023 10:42:15 -0700 Subject: [PATCH 26/30] Apply suggestions from code review Co-authored-by: Alex Ostrikov --- enterprise/internal/own/background/background.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/internal/own/background/background.go b/enterprise/internal/own/background/background.go index e3e9c54407868..d896745007a02 100644 --- a/enterprise/internal/own/background/background.go +++ b/enterprise/internal/own/background/background.go @@ -40,7 +40,7 @@ var IndexJobTypes = []IndexJobType{{ RefreshInterval: time.Minute * 5, }} -type JobTypeId int +type JobTypeID int const ( _ JobTypeId = iota @@ -194,7 +194,7 @@ func (h *handler) Handle(ctx context.Context, lgr log.Logger, record *Job) error case RecentContributors: delegate = handleRecentContributors default: - return errors.New("unsupported own index job type") + return errcode.MakeNonRetryable(errors.New("unsupported own index job type")) } return delegate(ctx, lgr, api.RepoID(record.RepoId), h.db) From e33d68181d9f1f5034f4b2ef993ad419abc3cfba Mon Sep 17 00:00:00 2001 From: coury-clark Date: Tue, 2 May 2023 10:43:54 -0700 Subject: [PATCH 27/30] Apply suggestions from code review Co-authored-by: Alex Ostrikov --- internal/database/recent_contribution_signal.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/database/recent_contribution_signal.go b/internal/database/recent_contribution_signal.go index 8fe5516a1b409..269bd74933f04 100644 --- a/internal/database/recent_contribution_signal.go +++ b/internal/database/recent_contribution_signal.go @@ -196,12 +196,19 @@ const insertRecentContributorSignalFmtstr = ` ) VALUES (%s, %s, %s, %s) ` +const clearSignalsFmtstr = ` + WITH rps AS ( + SELECT id FROM repo_paths WHERE repo_id = %s + ) + DELETE FROM %s + WHERE changed_file_path_id IN (SELECT * FROM rps) +` + func (s *recentContributionSignalStore) ClearSignals(ctx context.Context, repoID api.RepoID) error { tables := []string{"own_signal_recent_contribution", "own_aggregate_recent_contribution"} - q := "WITH rps as (select id from repo_paths where repo_id = %s) delete from %s where changed_file_path_id in (select * from rps);" for _, table := range tables { - if err := s.Exec(ctx, sqlf.Sprintf(q, repoID, sqlf.Sprintf(table))); err != nil { + if err := s.Exec(ctx, sqlf.Sprintf(clearSignalsFmtstr, repoID, sqlf.Sprintf(table))); err != nil { return errors.Wrapf(err, "table: %s", table) } } From d2a2fd7816116be2301c0f66bc3f967e4b5c853f Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Tue, 2 May 2023 10:46:08 -0700 Subject: [PATCH 28/30] pr comments --- internal/gitserver/commands.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/internal/gitserver/commands.go b/internal/gitserver/commands.go index 0b9086c74ce3a..6adde6f70784b 100644 --- a/internal/gitserver/commands.go +++ b/internal/gitserver/commands.go @@ -324,6 +324,9 @@ func (c *clientImplementor) CommitGraph(ctx context.Context, repo api.RepoName, return gitdomain.ParseCommitGraph(strings.Split(string(out), "\n")), nil } +// CommitLog returns the repository commit log, including the file paths that were changed. The general approach to parsing +// is to separate the first line (the metadata line) from the remaining lines (the files), and then parse the metadata line +// into component parts separately. func (c *clientImplementor) CommitLog(ctx context.Context, repo api.RepoName, after time.Time) ([]CommitLog, error) { args := []string{"log", "--pretty=format:%H%ae%an%ad", "--name-only", "--topo-order", "--no-merges"} if !after.IsZero() { @@ -354,11 +357,6 @@ func (c *clientImplementor) CommitLog(ctx context.Context, repo api.RepoName, af sha, authorEmail, authorName, timestamp := parts[0], parts[1], parts[2], parts[3] t, err := parseTimestamp(timestamp) if err != nil { - fmt.Println(parts) - command := strings.Join(args, " ") - fmt.Println(command) - fmt.Println(fmt.Sprintf("repo_name: %s", repo)) - return nil, errors.Wrapf(err, "parseTimestamp %s", timestamp) } ls = append(ls, CommitLog{ @@ -376,7 +374,7 @@ func parseTimestamp(timestamp string) (time.Time, error) { layout := "Mon Jan 2 15:04:05 2006 -0700" t, err := time.Parse(layout, timestamp) if err != nil { - return time.Now(), err // Handle error and fallback to other timestamp format + return time.Time{}, err } return t, nil } From 7b6e8ce9fc3dd6a388a4bdaaf197b7a474f1edbb Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Tue, 2 May 2023 13:53:28 -0700 Subject: [PATCH 29/30] tests and fixes --- .../internal/own/background/background.go | 7 ++- internal/gitserver/commands.go | 11 +++- internal/gitserver/commands_test.go | 55 ++++++++++++++++++- 3 files changed, 66 insertions(+), 7 deletions(-) diff --git a/enterprise/internal/own/background/background.go b/enterprise/internal/own/background/background.go index d896745007a02..64f8a3eaf2d96 100644 --- a/enterprise/internal/own/background/background.go +++ b/enterprise/internal/own/background/background.go @@ -16,6 +16,7 @@ import ( "github.com/sourcegraph/sourcegraph/internal/database" "github.com/sourcegraph/sourcegraph/internal/database/basestore" "github.com/sourcegraph/sourcegraph/internal/database/dbutil" + "github.com/sourcegraph/sourcegraph/internal/errcode" "github.com/sourcegraph/sourcegraph/internal/executor" "github.com/sourcegraph/sourcegraph/internal/goroutine" "github.com/sourcegraph/sourcegraph/internal/observation" @@ -28,7 +29,7 @@ import ( type IndexJobType struct { Name string - Id JobTypeId + Id JobTypeID IndexInterval time.Duration RefreshInterval time.Duration } @@ -43,7 +44,7 @@ var IndexJobTypes = []IndexJobType{{ type JobTypeID int const ( - _ JobTypeId = iota + _ JobTypeID = iota RecentContributors ) @@ -190,7 +191,7 @@ func (h *handler) Handle(ctx context.Context, lgr log.Logger, record *Job) error } var delegate signalIndexFunc - switch JobTypeId(record.JobType) { + switch JobTypeID(record.JobType) { case RecentContributors: delegate = handleRecentContributors default: diff --git a/internal/gitserver/commands.go b/internal/gitserver/commands.go index 6adde6f70784b..bed8229f494da 100644 --- a/internal/gitserver/commands.go +++ b/internal/gitserver/commands.go @@ -334,11 +334,11 @@ func (c *clientImplementor) CommitLog(ctx context.Context, repo api.RepoName, af } cmd := c.gitCommand(repo, args...) - out, err := cmd.CombinedOutput(ctx) if err != nil { - return nil, err + return nil, errors.Wrap(err, "gitCommand") } + var ls []CommitLog lines := strings.Split(string(out), "\n\n") @@ -348,7 +348,12 @@ func (c *clientImplementor) CommitLog(ctx context.Context, repo api.RepoName, af continue } metaLine := partitions[0] - changedFiles := partitions[1:] + var changedFiles []string + for _, pt := range partitions[1:] { + if pt != "" { + changedFiles = append(changedFiles, pt) + } + } parts := strings.Split(metaLine, "") if len(parts) != 4 { diff --git a/internal/gitserver/commands_test.go b/internal/gitserver/commands_test.go index 07efaeffc7209..ec2da57cd03b4 100644 --- a/internal/gitserver/commands_test.go +++ b/internal/gitserver/commands_test.go @@ -15,6 +15,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + godiff "github.com/sourcegraph/go-diff/diff" "github.com/sourcegraph/sourcegraph/internal/gitserver/protocol" @@ -2016,7 +2018,7 @@ func TestParseCommitsUniqueToBranch(t *testing.T) { // KEEP } } -func TestParseBranchesContaining(t *testing.T) { //KEEP +func TestParseBranchesContaining(t *testing.T) { // KEEP names := parseBranchesContaining([]string{ "refs/tags/v0.7.0", "refs/tags/v0.5.1", @@ -3206,3 +3208,54 @@ func TestBlameHunkReader(t *testing.T) { } }) } + +func Test_CommitLog(t *testing.T) { + ClientMocks.LocalGitserver = true + defer ResetClientMocks() + + tests := map[string]struct { + extraGitCommands []string + wantFiles [][]string // put these in log reverse order + wantCommits int + wantErr string + }{ + "commit changes files": { + extraGitCommands: getGitCommandsWithFileLists([]string{"file1.txt", "file2.txt"}, []string{"file3.txt"}), + wantFiles: [][]string{{"file3.txt"}, {"file1.txt", "file2.txt"}}, + wantCommits: 2, + }, + "no commits": { + wantErr: "gitCommand: exit status 128", + }, + "one file two commits": { + extraGitCommands: getGitCommandsWithFileLists([]string{"file1.txt"}, []string{"file1.txt"}), + wantFiles: [][]string{{"file1.txt"}, {"file1.txt"}}, + wantCommits: 2, + }, + "one commit": { + extraGitCommands: getGitCommandsWithFileLists([]string{"file1.txt"}), + wantFiles: [][]string{{"file1.txt"}}, + wantCommits: 1, + }, + } + + for label, test := range tests { + t.Run(label, func(t *testing.T) { + repo := MakeGitRepository(t, test.extraGitCommands...) + logResults, err := NewClient().CommitLog(context.Background(), repo, time.Time{}) + if err != nil { + require.ErrorContains(t, err, test.wantErr) + } + + t.Log(test) + for i, result := range logResults { + t.Log(result) + assert.Equal(t, "a@a.com", result.AuthorEmail) + assert.Equal(t, "a", result.AuthorName) + assert.Equal(t, 40, len(result.SHA)) + assert.ElementsMatch(t, test.wantFiles[i], result.ChangedFiles) + } + assert.Equal(t, test.wantCommits, len(logResults)) + }) + } +} From afdd075f5e08c64252780a3ac67d32d23e8212c0 Mon Sep 17 00:00:00 2001 From: Coury Clark Date: Tue, 2 May 2023 13:56:02 -0700 Subject: [PATCH 30/30] bazel --- enterprise/internal/own/background/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/enterprise/internal/own/background/BUILD.bazel b/enterprise/internal/own/background/BUILD.bazel index 499e3b09f3737..0be38779ee144 100644 --- a/enterprise/internal/own/background/BUILD.bazel +++ b/enterprise/internal/own/background/BUILD.bazel @@ -16,6 +16,7 @@ go_library( "//internal/database", "//internal/database/basestore", "//internal/database/dbutil", + "//internal/errcode", "//internal/executor", "//internal/gitserver", "//internal/goroutine",