From b42fae382af0a43b0d51fb312abd4ff55df43877 Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Wed, 25 May 2022 07:55:28 -0400 Subject: [PATCH 01/12] feat: make local storage configurable --- commands/env.go | 4 +++- entity/dag/example_test.go | 5 +++-- misc/random_bugs/cmd/main.go | 4 +++- repository/gogit.go | 40 ++++++++++++++++++++++++------------ repository/gogit_test.go | 6 +++--- repository/gogit_testing.go | 6 ++++-- 6 files changed, 43 insertions(+), 22 deletions(-) diff --git a/commands/env.go b/commands/env.go index ac7b789a4be0e1008e0d40a05db7e18e81eff627..52be4225b21b44f2e0244d33694efa2466128eb2 100644 --- a/commands/env.go +++ b/commands/env.go @@ -14,6 +14,8 @@ import ( "github.com/MichaelMure/git-bug/util/interrupt" ) +const gitBugApplicationName = "git-bug" + // Env is the environment of a command type Env struct { repo repository.ClockedRepo @@ -54,7 +56,7 @@ func loadRepo(env *Env) func(*cobra.Command, []string) error { return fmt.Errorf("unable to get the current working directory: %q", err) } - env.repo, err = repository.OpenGoGitRepo(cwd, []repository.ClockLoader{bug.ClockLoader}) + env.repo, err = repository.OpenGoGitRepo(cwd, gitBugApplicationName, []repository.ClockLoader{bug.ClockLoader}) if err == repository.ErrNotARepo { return fmt.Errorf("%s must be run from within a git repo", rootCommandName) } diff --git a/entity/dag/example_test.go b/entity/dag/example_test.go index d034e59db935e0da91fd1ea24b011383fd995100..7290b4d46b0e01420a61fe0be59651f8d4cd3e53 100644 --- a/entity/dag/example_test.go +++ b/entity/dag/example_test.go @@ -336,14 +336,15 @@ func Read(repo repository.ClockedRepo, id entity.Id) (*ProjectConfig, error) { } func Example_entity() { + const gitBugApplicationName = "git-bug" // Note: this example ignore errors for readability // Note: variable names get a little confusing as we are simulating both side in the same function // Let's start by defining two git repository and connecting them as remote repoRenePath, _ := os.MkdirTemp("", "") repoIsaacPath, _ := os.MkdirTemp("", "") - repoRene, _ := repository.InitGoGitRepo(repoRenePath) - repoIsaac, _ := repository.InitGoGitRepo(repoIsaacPath) + repoRene, _ := repository.InitGoGitRepo(repoRenePath, gitBugApplicationName) + repoIsaac, _ := repository.InitGoGitRepo(repoIsaacPath, gitBugApplicationName) _ = repoRene.AddRemote("origin", repoIsaacPath) _ = repoIsaac.AddRemote("origin", repoRenePath) diff --git a/misc/random_bugs/cmd/main.go b/misc/random_bugs/cmd/main.go index 010ae6d1bb4606455c11fb3197eb2448e5004bfd..b017439e099c1bcce1ba73ed905682d80fc13e16 100644 --- a/misc/random_bugs/cmd/main.go +++ b/misc/random_bugs/cmd/main.go @@ -11,6 +11,8 @@ import ( // This program will randomly generate a collection of bugs in the repository // of the current path func main() { + const gitBugApplicationName = "git-bug" + dir, err := os.Getwd() if err != nil { panic(err) @@ -20,7 +22,7 @@ func main() { bug.ClockLoader, } - repo, err := repository.OpenGoGitRepo(dir, loaders) + repo, err := repository.OpenGoGitRepo(dir, gitBugApplicationName, loaders) if err != nil { panic(err) } diff --git a/repository/gogit.go b/repository/gogit.go index 54677902d1efc2c0acba6b593afb6df3c8f97ca9..af1b9fa46a068d27e520daa6b372366fd4c56915 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -26,6 +26,7 @@ import ( ) const clockPath = "clocks" +const indexPath = "indexes" var _ ClockedRepo = &GoGitRepo{} var _ TestedRepo = &GoGitRepo{} @@ -49,8 +50,11 @@ type GoGitRepo struct { localStorage billy.Filesystem } -// OpenGoGitRepo open an already existing repo at the given path -func OpenGoGitRepo(path string, clockLoaders []ClockLoader) (*GoGitRepo, error) { +// OpenGoGitRepo opens an already existing repo at the given path and with +// the specified application name. Given a repository path of "~/myrepo" +// and an application name of "git-bug", local storage for the application +// will be configured at "~/myrepo/.git/git-bug". +func OpenGoGitRepo(path, application string, clockLoaders []ClockLoader) (*GoGitRepo, error) { path, err := detectGitPath(path) if err != nil { return nil, err @@ -72,7 +76,7 @@ func OpenGoGitRepo(path string, clockLoaders []ClockLoader) (*GoGitRepo, error) clocks: make(map[string]lamport.Clock), indexes: make(map[string]bleve.Index), keyring: k, - localStorage: osfs.New(filepath.Join(path, "git-bug")), + localStorage: osfs.New(filepath.Join(path, application)), } for _, loader := range clockLoaders { @@ -94,8 +98,11 @@ func OpenGoGitRepo(path string, clockLoaders []ClockLoader) (*GoGitRepo, error) return repo, nil } -// InitGoGitRepo create a new empty git repo at the given path -func InitGoGitRepo(path string) (*GoGitRepo, error) { +// InitGoGitRepo creates a new empty git repo at the given path and with +// the specified application name. Given a repository path of "~/myrepo" +// and an application name of "git-bug", local storage for the application +// will be configured at "~/myrepo/.git/git-bug". +func InitGoGitRepo(path, application string) (*GoGitRepo, error) { r, err := gogit.PlainInit(path, false) if err != nil { return nil, err @@ -112,12 +119,15 @@ func InitGoGitRepo(path string) (*GoGitRepo, error) { clocks: make(map[string]lamport.Clock), indexes: make(map[string]bleve.Index), keyring: k, - localStorage: osfs.New(filepath.Join(path, ".git", "git-bug")), + localStorage: osfs.New(filepath.Join(path, ".git", application)), }, nil } -// InitBareGoGitRepo create a new --bare empty git repo at the given path -func InitBareGoGitRepo(path string) (*GoGitRepo, error) { +// InitBareGoGitRepo creates a new --bare empty git repo at the given path +// and with the specified application name. Given a repository path of +// "~/myrepo" and an application name of "git-bug", local storage for the +// application will be configured at "~/myrepo/.git/git-bug". +func InitBareGoGitRepo(path, application string) (*GoGitRepo, error) { r, err := gogit.PlainInit(path, true) if err != nil { return nil, err @@ -134,7 +144,7 @@ func InitBareGoGitRepo(path string) (*GoGitRepo, error) { clocks: make(map[string]lamport.Clock), indexes: make(map[string]bleve.Index), keyring: k, - localStorage: osfs.New(filepath.Join(path, "git-bug")), + localStorage: osfs.New(filepath.Join(path, application)), }, nil } @@ -295,7 +305,8 @@ func (repo *GoGitRepo) GetRemotes() (map[string]string, error) { return result, nil } -// LocalStorage return a billy.Filesystem giving access to $RepoPath/.git/git-bug +// LocalStorage returns a billy.Filesystem giving access to +// $RepoPath/.git/$ApplicationName. func (repo *GoGitRepo) LocalStorage() billy.Filesystem { return repo.localStorage } @@ -309,7 +320,8 @@ func (repo *GoGitRepo) GetBleveIndex(name string) (bleve.Index, error) { return index, nil } - path := filepath.Join(repo.path, "git-bug", "indexes", name) + // path := filepath.Join(repo.path, "git-bug", "indexes", name) + path := filepath.Join(repo.localStorage.Root(), indexPath, name) index, err := bleve.Open(path) if err == nil { @@ -340,7 +352,8 @@ func (repo *GoGitRepo) ClearBleveIndex(name string) error { repo.indexesMutex.Lock() defer repo.indexesMutex.Unlock() - path := filepath.Join(repo.path, "git-bug", "indexes", name) + // path := filepath.Join(repo.path, "git-bug", "indexes", name) + path := filepath.Join(repo.localStorage.Root(), indexPath, name) err := os.RemoveAll(path) if err != nil { @@ -781,7 +794,8 @@ func (repo *GoGitRepo) AllClocks() (map[string]lamport.Clock, error) { result := make(map[string]lamport.Clock) - files, err := ioutil.ReadDir(filepath.Join(repo.path, "git-bug", clockPath)) + // files, err := ioutil.ReadDir(filepath.Join(repo.path, "git-bug", clockPath)) + files, err := ioutil.ReadDir(filepath.Join(repo.localStorage.Root(), clockPath)) if os.IsNotExist(err) { return nil, nil } diff --git a/repository/gogit_test.go b/repository/gogit_test.go index a2bb49b9c0dcb745551b4b9e7bec3ce0c7e12cf6..941b563a407131734441c8add433b80d56af0837 100644 --- a/repository/gogit_test.go +++ b/repository/gogit_test.go @@ -17,7 +17,7 @@ func TestNewGoGitRepo(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(plainRoot) - _, err = InitGoGitRepo(plainRoot) + _, err = InitGoGitRepo(plainRoot, testApplicationName) require.NoError(t, err) plainGitDir := filepath.Join(plainRoot, ".git") @@ -26,7 +26,7 @@ func TestNewGoGitRepo(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(bareRoot) - _, err = InitBareGoGitRepo(bareRoot) + _, err = InitBareGoGitRepo(bareRoot, testApplicationName) require.NoError(t, err) bareGitDir := bareRoot @@ -52,7 +52,7 @@ func TestNewGoGitRepo(t *testing.T) { } for i, tc := range tests { - r, err := OpenGoGitRepo(tc.inPath, nil) + r, err := OpenGoGitRepo(tc.inPath, testApplicationName, nil) if tc.err { require.Error(t, err, i) diff --git a/repository/gogit_testing.go b/repository/gogit_testing.go index cad776b383535e8d9f1972730e56952cb3c182f9..f80f62c0d3abc3d10362cf4c7f990d96b912e603 100644 --- a/repository/gogit_testing.go +++ b/repository/gogit_testing.go @@ -7,6 +7,8 @@ import ( "github.com/99designs/keyring" ) +const testApplicationName = "git-bug" + // This is intended for testing only func CreateGoGitTestRepo(bare bool) TestedRepo { @@ -15,7 +17,7 @@ func CreateGoGitTestRepo(bare bool) TestedRepo { log.Fatal(err) } - var creator func(string) (*GoGitRepo, error) + var creator func(string, string) (*GoGitRepo, error) if bare { creator = InitBareGoGitRepo @@ -23,7 +25,7 @@ func CreateGoGitTestRepo(bare bool) TestedRepo { creator = InitGoGitRepo } - repo, err := creator(dir) + repo, err := creator(dir, testApplicationName) if err != nil { log.Fatal(err) } From e29f58bf853d0cd4825cb590ba973a9d9ab7ea36 Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Wed, 25 May 2022 07:59:56 -0400 Subject: [PATCH 02/12] chore: clean-up commented code --- repository/gogit.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/repository/gogit.go b/repository/gogit.go index af1b9fa46a068d27e520daa6b372366fd4c56915..e876629a0253cbf4edec47c1a6627ed2788d67ea 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -320,7 +320,6 @@ func (repo *GoGitRepo) GetBleveIndex(name string) (bleve.Index, error) { return index, nil } - // path := filepath.Join(repo.path, "git-bug", "indexes", name) path := filepath.Join(repo.localStorage.Root(), indexPath, name) index, err := bleve.Open(path) @@ -352,7 +351,6 @@ func (repo *GoGitRepo) ClearBleveIndex(name string) error { repo.indexesMutex.Lock() defer repo.indexesMutex.Unlock() - // path := filepath.Join(repo.path, "git-bug", "indexes", name) path := filepath.Join(repo.localStorage.Root(), indexPath, name) err := os.RemoveAll(path) @@ -794,7 +792,6 @@ func (repo *GoGitRepo) AllClocks() (map[string]lamport.Clock, error) { result := make(map[string]lamport.Clock) - // files, err := ioutil.ReadDir(filepath.Join(repo.path, "git-bug", clockPath)) files, err := ioutil.ReadDir(filepath.Join(repo.localStorage.Root(), clockPath)) if os.IsNotExist(err) { return nil, nil From e120fdb97e3af76198eadc8a44e6feb732b4dd83 Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Thu, 26 May 2022 13:39:13 -0400 Subject: [PATCH 03/12] refactor: use namespace instead of application of applicationName --- commands/env.go | 4 ++-- entity/dag/example_test.go | 6 +++--- misc/random_bugs/cmd/main.go | 4 ++-- repository/gogit.go | 38 ++++++++++++++++++------------------ repository/gogit_testing.go | 4 ++-- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/commands/env.go b/commands/env.go index 52be4225b21b44f2e0244d33694efa2466128eb2..a6bca7e4ccc8fdef6b58b122e866e8b45248410c 100644 --- a/commands/env.go +++ b/commands/env.go @@ -14,7 +14,7 @@ import ( "github.com/MichaelMure/git-bug/util/interrupt" ) -const gitBugApplicationName = "git-bug" +const gitBugNamespace = "git-bug" // Env is the environment of a command type Env struct { @@ -56,7 +56,7 @@ func loadRepo(env *Env) func(*cobra.Command, []string) error { return fmt.Errorf("unable to get the current working directory: %q", err) } - env.repo, err = repository.OpenGoGitRepo(cwd, gitBugApplicationName, []repository.ClockLoader{bug.ClockLoader}) + env.repo, err = repository.OpenGoGitRepo(cwd, gitBugNamespace, []repository.ClockLoader{bug.ClockLoader}) if err == repository.ErrNotARepo { return fmt.Errorf("%s must be run from within a git repo", rootCommandName) } diff --git a/entity/dag/example_test.go b/entity/dag/example_test.go index 7290b4d46b0e01420a61fe0be59651f8d4cd3e53..106e359c1bed417f68577786ce7821a4fd7f4f68 100644 --- a/entity/dag/example_test.go +++ b/entity/dag/example_test.go @@ -336,15 +336,15 @@ func Read(repo repository.ClockedRepo, id entity.Id) (*ProjectConfig, error) { } func Example_entity() { - const gitBugApplicationName = "git-bug" + const gitBugNamespace = "git-bug" // Note: this example ignore errors for readability // Note: variable names get a little confusing as we are simulating both side in the same function // Let's start by defining two git repository and connecting them as remote repoRenePath, _ := os.MkdirTemp("", "") repoIsaacPath, _ := os.MkdirTemp("", "") - repoRene, _ := repository.InitGoGitRepo(repoRenePath, gitBugApplicationName) - repoIsaac, _ := repository.InitGoGitRepo(repoIsaacPath, gitBugApplicationName) + repoRene, _ := repository.InitGoGitRepo(repoRenePath, gitBugNamespace) + repoIsaac, _ := repository.InitGoGitRepo(repoIsaacPath, gitBugNamespace) _ = repoRene.AddRemote("origin", repoIsaacPath) _ = repoIsaac.AddRemote("origin", repoRenePath) diff --git a/misc/random_bugs/cmd/main.go b/misc/random_bugs/cmd/main.go index b017439e099c1bcce1ba73ed905682d80fc13e16..c3506efb326b8d5b8f347f78acc42b5477ff7937 100644 --- a/misc/random_bugs/cmd/main.go +++ b/misc/random_bugs/cmd/main.go @@ -11,7 +11,7 @@ import ( // This program will randomly generate a collection of bugs in the repository // of the current path func main() { - const gitBugApplicationName = "git-bug" + const gitBugNamespace = "git-bug" dir, err := os.Getwd() if err != nil { @@ -22,7 +22,7 @@ func main() { bug.ClockLoader, } - repo, err := repository.OpenGoGitRepo(dir, gitBugApplicationName, loaders) + repo, err := repository.OpenGoGitRepo(dir, gitBugNamespace, loaders) if err != nil { panic(err) } diff --git a/repository/gogit.go b/repository/gogit.go index e876629a0253cbf4edec47c1a6627ed2788d67ea..46a2cb0a733314f3bcba4d242e3b18dd8574f796 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -50,11 +50,11 @@ type GoGitRepo struct { localStorage billy.Filesystem } -// OpenGoGitRepo opens an already existing repo at the given path and with -// the specified application name. Given a repository path of "~/myrepo" -// and an application name of "git-bug", local storage for the application -// will be configured at "~/myrepo/.git/git-bug". -func OpenGoGitRepo(path, application string, clockLoaders []ClockLoader) (*GoGitRepo, error) { +// OpenGoGitRepo opens an already existing repo at the given path and +// with the specified LocalStorage namespace. Given a repository path +// of "~/myrepo" and a namespace of "git-bug", local storage for the +// GoGitRepo will be configured at "~/myrepo/.git/git-bug". +func OpenGoGitRepo(path, namespace string, clockLoaders []ClockLoader) (*GoGitRepo, error) { path, err := detectGitPath(path) if err != nil { return nil, err @@ -76,7 +76,7 @@ func OpenGoGitRepo(path, application string, clockLoaders []ClockLoader) (*GoGit clocks: make(map[string]lamport.Clock), indexes: make(map[string]bleve.Index), keyring: k, - localStorage: osfs.New(filepath.Join(path, application)), + localStorage: osfs.New(filepath.Join(path, namespace)), } for _, loader := range clockLoaders { @@ -98,11 +98,11 @@ func OpenGoGitRepo(path, application string, clockLoaders []ClockLoader) (*GoGit return repo, nil } -// InitGoGitRepo creates a new empty git repo at the given path and with -// the specified application name. Given a repository path of "~/myrepo" -// and an application name of "git-bug", local storage for the application -// will be configured at "~/myrepo/.git/git-bug". -func InitGoGitRepo(path, application string) (*GoGitRepo, error) { +// InitGoGitRepo creates a new empty git repo at the given path and +// with the specified LocalStorage namespace. Given a repository path +// of "~/myrepo" and a namespace of "git-bug", local storage for the +// GoGitRepo will be configured at "~/myrepo/.git/git-bug". +func InitGoGitRepo(path, namespace string) (*GoGitRepo, error) { r, err := gogit.PlainInit(path, false) if err != nil { return nil, err @@ -119,15 +119,15 @@ func InitGoGitRepo(path, application string) (*GoGitRepo, error) { clocks: make(map[string]lamport.Clock), indexes: make(map[string]bleve.Index), keyring: k, - localStorage: osfs.New(filepath.Join(path, ".git", application)), + localStorage: osfs.New(filepath.Join(path, ".git", namespace)), }, nil } -// InitBareGoGitRepo creates a new --bare empty git repo at the given path -// and with the specified application name. Given a repository path of -// "~/myrepo" and an application name of "git-bug", local storage for the -// application will be configured at "~/myrepo/.git/git-bug". -func InitBareGoGitRepo(path, application string) (*GoGitRepo, error) { +// InitBareGoGitRepo creates a new --bare empty git repo at the given +// path and with the specified LocalStorage namespace. Given a repository +// path of "~/myrepo" and a namespace of "git-bug", local storage for the +// GoGitRepo will be configured at "~/myrepo/.git/git-bug". +func InitBareGoGitRepo(path, namespace string) (*GoGitRepo, error) { r, err := gogit.PlainInit(path, true) if err != nil { return nil, err @@ -144,7 +144,7 @@ func InitBareGoGitRepo(path, application string) (*GoGitRepo, error) { clocks: make(map[string]lamport.Clock), indexes: make(map[string]bleve.Index), keyring: k, - localStorage: osfs.New(filepath.Join(path, application)), + localStorage: osfs.New(filepath.Join(path, namespace)), }, nil } @@ -306,7 +306,7 @@ func (repo *GoGitRepo) GetRemotes() (map[string]string, error) { } // LocalStorage returns a billy.Filesystem giving access to -// $RepoPath/.git/$ApplicationName. +// $RepoPath/.git/$Namespace. func (repo *GoGitRepo) LocalStorage() billy.Filesystem { return repo.localStorage } diff --git a/repository/gogit_testing.go b/repository/gogit_testing.go index f80f62c0d3abc3d10362cf4c7f990d96b912e603..7647c71162edc61221f7a28b17c1e05dc8d37f28 100644 --- a/repository/gogit_testing.go +++ b/repository/gogit_testing.go @@ -7,7 +7,7 @@ import ( "github.com/99designs/keyring" ) -const testApplicationName = "git-bug" +const namespace = "git-bug" // This is intended for testing only @@ -25,7 +25,7 @@ func CreateGoGitTestRepo(bare bool) TestedRepo { creator = InitGoGitRepo } - repo, err := creator(dir, testApplicationName) + repo, err := creator(dir, namespace) if err != nil { log.Fatal(err) } From 8821b67d1bd0809d3fd3e87baf391e07aa54722a Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Thu, 26 May 2022 13:40:52 -0400 Subject: [PATCH 04/12] test: add verification that localStorage.Root() resolves to the correct absolute filepath --- repository/gogit_test.go | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/repository/gogit_test.go b/repository/gogit_test.go index 941b563a407131734441c8add433b80d56af0837..d7b919b9a72f8d7b35247aa4255641d38f182dde 100644 --- a/repository/gogit_test.go +++ b/repository/gogit_test.go @@ -17,7 +17,7 @@ func TestNewGoGitRepo(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(plainRoot) - _, err = InitGoGitRepo(plainRoot, testApplicationName) + _, err = InitGoGitRepo(plainRoot, namespace) require.NoError(t, err) plainGitDir := filepath.Join(plainRoot, ".git") @@ -26,7 +26,7 @@ func TestNewGoGitRepo(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(bareRoot) - _, err = InitBareGoGitRepo(bareRoot, testApplicationName) + _, err = InitBareGoGitRepo(bareRoot, namespace) require.NoError(t, err) bareGitDir := bareRoot @@ -52,7 +52,7 @@ func TestNewGoGitRepo(t *testing.T) { } for i, tc := range tests { - r, err := OpenGoGitRepo(tc.inPath, testApplicationName, nil) + r, err := OpenGoGitRepo(tc.inPath, namespace, nil) if tc.err { require.Error(t, err, i) @@ -66,3 +66,36 @@ func TestNewGoGitRepo(t *testing.T) { func TestGoGitRepo(t *testing.T) { RepoTest(t, CreateGoGitTestRepo, CleanupTestRepos) } + +func TestGoGitRepo_Indexes(t *testing.T) { + t.Parallel() + + plainRoot, err := ioutil.TempDir("", "") + require.NoError(t, err) + // defer os.RemoveAll(plainRoot) + + repo, err := InitGoGitRepo(plainRoot, namespace) + require.NoError(t, err) + + // Can create indices + indexA, err := repo.GetBleveIndex("a") + require.NoError(t, err) + require.NotZero(t, indexA) + require.FileExists(t, filepath.Join(plainRoot, ".git", namespace, "indexes", "a", "index_meta.json")) + require.FileExists(t, filepath.Join(plainRoot, ".git", namespace, "indexes", "a", "store")) + + indexB, err := repo.GetBleveIndex("b") + require.NoError(t, err) + require.NotZero(t, indexB) + require.DirExists(t, filepath.Join(plainRoot, ".git", namespace, "indexes", "b")) + + // Can get an existing index + indexA, err = repo.GetBleveIndex("a") + require.NoError(t, err) + require.NotZero(t, indexA) + + // Can delete an index + err = repo.ClearBleveIndex("a") + require.NoError(t, err) + require.NoDirExists(t, filepath.Join(plainRoot, ".git", namespace, "indexes", "a")) +} From 86dd450aaf592aa065a17d49d70d9d0352fa5ca3 Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Sat, 28 May 2022 08:07:34 -0400 Subject: [PATCH 05/12] test: clean up temp dir and repo correctly --- entity/dag/example_test.go | 2 ++ repository/gogit_test.go | 22 +++++++++++++++++----- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/entity/dag/example_test.go b/entity/dag/example_test.go index 106e359c1bed417f68577786ce7821a4fd7f4f68..948d6aeb5fc9695a9e3253f4a627ca80049931df 100644 --- a/entity/dag/example_test.go +++ b/entity/dag/example_test.go @@ -344,7 +344,9 @@ func Example_entity() { repoRenePath, _ := os.MkdirTemp("", "") repoIsaacPath, _ := os.MkdirTemp("", "") repoRene, _ := repository.InitGoGitRepo(repoRenePath, gitBugNamespace) + defer repoRene.Close() repoIsaac, _ := repository.InitGoGitRepo(repoIsaacPath, gitBugNamespace) + defer repoIsaac.Close() _ = repoRene.AddRemote("origin", repoIsaacPath) _ = repoIsaac.AddRemote("origin", repoRenePath) diff --git a/repository/gogit_test.go b/repository/gogit_test.go index d7b919b9a72f8d7b35247aa4255641d38f182dde..49eae30931dacb0d39494ae6139835722c432d10 100644 --- a/repository/gogit_test.go +++ b/repository/gogit_test.go @@ -15,19 +15,25 @@ func TestNewGoGitRepo(t *testing.T) { // Plain plainRoot, err := ioutil.TempDir("", "") require.NoError(t, err) - defer os.RemoveAll(plainRoot) + t.Cleanup(func() { + require.NoError(t, os.RemoveAll(plainRoot)) + }) - _, err = InitGoGitRepo(plainRoot, namespace) + plainRepo, err := InitGoGitRepo(plainRoot, namespace) require.NoError(t, err) + require.NoError(t, plainRepo.Close()) plainGitDir := filepath.Join(plainRoot, ".git") // Bare bareRoot, err := ioutil.TempDir("", "") require.NoError(t, err) - defer os.RemoveAll(bareRoot) + t.Cleanup(func() { + require.NoError(t, os.RemoveAll(bareRoot)) + }) - _, err = InitBareGoGitRepo(bareRoot, namespace) + bareRepo, err := InitBareGoGitRepo(bareRoot, namespace) require.NoError(t, err) + require.NoError(t, bareRepo.Close()) bareGitDir := bareRoot tests := []struct { @@ -59,6 +65,7 @@ func TestNewGoGitRepo(t *testing.T) { } else { require.NoError(t, err, i) assert.Equal(t, filepath.ToSlash(tc.outPath), filepath.ToSlash(r.path), i) + require.NoError(t, r.Close()) } } } @@ -72,10 +79,15 @@ func TestGoGitRepo_Indexes(t *testing.T) { plainRoot, err := ioutil.TempDir("", "") require.NoError(t, err) - // defer os.RemoveAll(plainRoot) + t.Cleanup(func() { + require.NoError(t, os.RemoveAll(plainRoot)) + }) repo, err := InitGoGitRepo(plainRoot, namespace) require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, repo.Close()) + }) // Can create indices indexA, err := repo.GetBleveIndex("a") From 50de0306dff5aa83981471f1245363aa40a268fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Tue, 31 May 2022 12:24:58 +0200 Subject: [PATCH 06/12] gogit: close index before deleting it on disk --- repository/gogit.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/repository/gogit.go b/repository/gogit.go index 46a2cb0a733314f3bcba4d242e3b18dd8574f796..71cddfb2c44fead02fccf984442a46161861db46 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -351,21 +351,20 @@ func (repo *GoGitRepo) ClearBleveIndex(name string) error { repo.indexesMutex.Lock() defer repo.indexesMutex.Unlock() - path := filepath.Join(repo.localStorage.Root(), indexPath, name) - - err := os.RemoveAll(path) - if err != nil { - return err - } - if index, ok := repo.indexes[name]; ok { - err = index.Close() + err := index.Close() if err != nil { return err } delete(repo.indexes, name) } + path := filepath.Join(repo.localStorage.Root(), indexPath, name) + err := os.RemoveAll(path) + if err != nil { + return err + } + return nil } @@ -580,7 +579,7 @@ func (repo *GoGitRepo) StoreCommit(treeHash Hash, parents ...Hash) (Hash, error) return repo.StoreSignedCommit(treeHash, nil, parents...) } -// StoreCommit will store a Git commit with the given Git tree. If signKey is not nil, the commit +// StoreSignedCommit will store a Git commit with the given Git tree. If signKey is not nil, the commit // will be signed accordingly. func (repo *GoGitRepo) StoreSignedCommit(treeHash Hash, signKey *openpgp.Entity, parents ...Hash) (Hash, error) { cfg, err := repo.r.Config() From da9f95e495476679866863007f44e611627ee2b3 Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Tue, 31 May 2022 07:22:55 -0400 Subject: [PATCH 07/12] fix: remove only t.Parallel() --- repository/gogit_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/repository/gogit_test.go b/repository/gogit_test.go index 49eae30931dacb0d39494ae6139835722c432d10..7fee91fbc64c85a52de289a71cb3ac6ca102fdb9 100644 --- a/repository/gogit_test.go +++ b/repository/gogit_test.go @@ -75,8 +75,6 @@ func TestGoGitRepo(t *testing.T) { } func TestGoGitRepo_Indexes(t *testing.T) { - t.Parallel() - plainRoot, err := ioutil.TempDir("", "") require.NoError(t, err) t.Cleanup(func() { From ccc342e81482e5fe6144fdb6c547331eab2cb98a Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Tue, 31 May 2022 07:47:56 -0400 Subject: [PATCH 08/12] refactor: simplify creation of temp dir - after 1.15 --- repository/gogit_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/repository/gogit_test.go b/repository/gogit_test.go index 7fee91fbc64c85a52de289a71cb3ac6ca102fdb9..c376de221dc305ad76ea87936393bc719cabf3dd 100644 --- a/repository/gogit_test.go +++ b/repository/gogit_test.go @@ -75,11 +75,7 @@ func TestGoGitRepo(t *testing.T) { } func TestGoGitRepo_Indexes(t *testing.T) { - plainRoot, err := ioutil.TempDir("", "") - require.NoError(t, err) - t.Cleanup(func() { - require.NoError(t, os.RemoveAll(plainRoot)) - }) + plainRoot := t.TempDir() repo, err := InitGoGitRepo(plainRoot, namespace) require.NoError(t, err) From c732a18ac2b6ea3d09e5c72660ffcf9687ebb9db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Tue, 31 May 2022 19:44:53 +0200 Subject: [PATCH 09/12] misc: move all completions in a dedicated folder --- .gitignore | 8 ++++---- README.md | 4 +--- git-bug.go | 2 +- misc/{bash_completion => completion/bash}/git-bug | 2 ++ misc/{fish_completion => completion/fish}/git-bug | 0 misc/{ => completion}/gen_completion.go | 10 ++++++---- .../powershell}/git-bug | 0 misc/{zsh_completion => completion/zsh}/git-bug | 0 8 files changed, 14 insertions(+), 12 deletions(-) rename misc/{bash_completion => completion/bash}/git-bug (99%) rename misc/{fish_completion => completion/fish}/git-bug (100%) rename misc/{ => completion}/gen_completion.go (87%) rename misc/{powershell_completion => completion/powershell}/git-bug (100%) rename misc/{zsh_completion => completion/zsh}/git-bug (100%) diff --git a/.gitignore b/.gitignore index be1fc3f1c79365fda767169a31550b13d22bf273..c61d1233896a273a3cdfcc20a8af53b2d14b964c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,8 +1,8 @@ git-bug -!/misc/bash_completion/git-bug -!/misc/fish_completion/git-bug -!/misc/powershell_completion/git-bug -!/misc/zsh_completion/git-bug +!/misc/completion/bash/git-bug +!/misc/completion/fish/git-bug +!/misc/completion/powershell/git-bug +!/misc/completion/zsh/git-bug .gitkeep dist coverage.txt diff --git a/README.md b/README.md index 8c51d6d584e324baab784d05dadbfe6dcb3d9b0a..323e4391f6a9a5d7e2d490671f4be9ea1e4d582e 100644 --- a/README.md +++ b/README.md @@ -272,9 +272,7 @@ Interested by how it works ? Have a look at the [data model](doc/model.md) and t ## Misc -- [Bash completion](misc/bash_completion) -- [Zsh completion](misc/zsh_completion) -- [PowerShell completion](misc/powershell_completion) +- [Bash, Zsh, fish, powershell completion](misc/completion) - [ManPages](doc/man) ## Planned features diff --git a/git-bug.go b/git-bug.go index 995fc559da14b53db37962fd40f5c66b17171caf..99fe81c5a20ec5ddc206567c03f219e8f2a93fe7 100644 --- a/git-bug.go +++ b/git-bug.go @@ -1,5 +1,5 @@ //go:generate go run doc/gen_docs.go -//go:generate go run misc/gen_completion.go +//go:generate go run misc/completion/gen_completion.go package main diff --git a/misc/bash_completion/git-bug b/misc/completion/bash/git-bug similarity index 99% rename from misc/bash_completion/git-bug rename to misc/completion/bash/git-bug index 85d2a15b7a471710879968c8f0a40d3ba8f02a42..92b90d3cc2ef4486ba3f3e48091e98ccb9c5a02d 100644 --- a/misc/bash_completion/git-bug +++ b/misc/completion/bash/git-bug @@ -287,6 +287,8 @@ fi # ex: ts=4 sw=4 et filetype=sh +# Custom bash code to connect the git completion for "git bug" to the +# git-bug completion for "git-bug" _git_bug() { local cur prev words cword split diff --git a/misc/fish_completion/git-bug b/misc/completion/fish/git-bug similarity index 100% rename from misc/fish_completion/git-bug rename to misc/completion/fish/git-bug diff --git a/misc/gen_completion.go b/misc/completion/gen_completion.go similarity index 87% rename from misc/gen_completion.go rename to misc/completion/gen_completion.go index 1f86124df9fc262f3ca5255b61ef2da3f70af9d1..5f64383246b0e63a070f17432132e2ad09cea208 100644 --- a/misc/gen_completion.go +++ b/misc/completion/gen_completion.go @@ -44,13 +44,15 @@ func genBash(root *cobra.Command) error { if err != nil { return err } - f, err := os.Create(filepath.Join(cwd, "misc", "bash_completion", "git-bug")) + f, err := os.Create(filepath.Join(cwd, "misc", "completion", "bash", "git-bug")) if err != nil { return err } defer f.Close() const patch = ` +# Custom bash code to connect the git completion for "git bug" to the +# git-bug completion for "git-bug" _git_bug() { local cur prev words cword split @@ -102,7 +104,7 @@ func genFish(root *cobra.Command) error { if err != nil { return err } - dir := filepath.Join(cwd, "misc", "fish_completion", "git-bug") + dir := filepath.Join(cwd, "misc", "completion", "fish", "git-bug") return root.GenFishCompletionFile(dir, true) } @@ -111,7 +113,7 @@ func genPowerShell(root *cobra.Command) error { if err != nil { return err } - path := filepath.Join(cwd, "misc", "powershell_completion", "git-bug") + path := filepath.Join(cwd, "misc", "completion", "powershell", "git-bug") return root.GenPowerShellCompletionFile(path) } @@ -120,6 +122,6 @@ func genZsh(root *cobra.Command) error { if err != nil { return err } - path := filepath.Join(cwd, "misc", "zsh_completion", "git-bug") + path := filepath.Join(cwd, "misc", "completion", "zsh", "git-bug") return root.GenZshCompletionFile(path) } diff --git a/misc/powershell_completion/git-bug b/misc/completion/powershell/git-bug similarity index 100% rename from misc/powershell_completion/git-bug rename to misc/completion/powershell/git-bug diff --git a/misc/zsh_completion/git-bug b/misc/completion/zsh/git-bug similarity index 100% rename from misc/zsh_completion/git-bug rename to misc/completion/zsh/git-bug From 1c219f67698771ae3b0d1d08205263e5ba280a71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Tue, 31 May 2022 20:16:22 +0200 Subject: [PATCH 10/12] doc: more discoverable docs --- README.md | 8 ++++++-- doc/README.md | 15 +++++++++++++++ doc/model.md | 2 ++ 3 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 doc/README.md diff --git a/README.md b/README.md index 323e4391f6a9a5d7e2d490671f4be9ea1e4d582e..ad7e62098d583b1d557d8115de5417d509126e33 100644 --- a/README.md +++ b/README.md @@ -268,7 +268,11 @@ git bug bridge rm [] ## Internals -Interested by how it works ? Have a look at the [data model](doc/model.md) and the [internal bird-view](doc/architecture.md). +Interested in how it works ? Have a look at the [data model](doc/model.md) and the [internal bird-view](doc/architecture.md). + +Or maybe you want to [make your own distributed data-structure in git](entity/dag/example_test.go) ? + +See also all the [docs](doc). ## Misc @@ -279,7 +283,7 @@ Interested by how it works ? Have a look at the [data model](doc/model.md) and t - media embedding - more bridges -- extendable data model to support arbitrary bug tracker +- webUI that can be used as a public portal to accept user's input - inflatable raptor ## Contribute diff --git a/doc/README.md b/doc/README.md new file mode 100644 index 0000000000000000000000000000000000000000..cf9fd845eae5f9391dfd97b879f8528d695522c7 --- /dev/null +++ b/doc/README.md @@ -0,0 +1,15 @@ +# Documentation + +## For users + +- [data model](model.md) describe how the data model works and why. +- [query language](queries.md) describe git-bug's query language. +- [How-to: Read and edit offline your Github/Gitlab/Jira issues with git-bug](howto-github.md) + +## For developers + +- :exclamation: [data model](model.md) describe how the data model works and why. +- :exclamation: [internal bird-view](architecture.md) gives an overview of the project architecture. +- :exclamation: [Entity/DAG](../entity/dag/example_test.go) explain how to easily make your own distributed entity in git. +- [query language](queries.md) describe git-bug's query language. +- [JIRA bridge de v notes](jira_bridge.md) \ No newline at end of file diff --git a/doc/model.md b/doc/model.md index de0de42adab36e0032146ff6395519c703f62399..0aad478966aef5d33dfceacc00bb02cd0fc675f7 100644 --- a/doc/model.md +++ b/doc/model.md @@ -3,6 +3,8 @@ Entities data model If you are not familiar with [git internals](https://git-scm.com/book/en/v1/Git-Internals), you might first want to read about them, as the `git-bug` data model is built on top of them. +In a different format, see how you can easily make your own [distributed data structure](../entity/dag/example_test.go). + ## Entities (bug, author, ...) are a series of edit operations As entities are stored and edited in multiple processes at the same time, it's not possible to store the current state like it would be done in a normal application. If two processes change the same entity and later try to merge the states, we wouldn't know which change takes precedence or how to merge those states. From fe231231c68e1f117b6018e008d6d09812604043 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Sun, 5 Jun 2022 13:23:02 +0200 Subject: [PATCH 11/12] graphql: fix two invalid mutex lock leading to data races --- api/graphql/models/lazy_bug.go | 6 +++--- api/graphql/models/lazy_identity.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/graphql/models/lazy_bug.go b/api/graphql/models/lazy_bug.go index a7840df283b4f319f3f055f22cf3186e988cde52..bc26aaca387852845060d469e18deb46ad9f896f 100644 --- a/api/graphql/models/lazy_bug.go +++ b/api/graphql/models/lazy_bug.go @@ -49,13 +49,13 @@ func NewLazyBug(cache *cache.RepoCache, excerpt *cache.BugExcerpt) *lazyBug { } func (lb *lazyBug) load() error { + lb.mu.Lock() + defer lb.mu.Unlock() + if lb.snap != nil { return nil } - lb.mu.Lock() - defer lb.mu.Unlock() - b, err := lb.cache.ResolveBug(lb.excerpt.Id) if err != nil { return err diff --git a/api/graphql/models/lazy_identity.go b/api/graphql/models/lazy_identity.go index 002c38e4c4dc8a28b1001a51e4527d2fd9d9deae..451bdd544b998dd8b407deef67d81a30221432e4 100644 --- a/api/graphql/models/lazy_identity.go +++ b/api/graphql/models/lazy_identity.go @@ -41,13 +41,13 @@ func NewLazyIdentity(cache *cache.RepoCache, excerpt *cache.IdentityExcerpt) *la } func (li *lazyIdentity) load() (*cache.IdentityCache, error) { + li.mu.Lock() + defer li.mu.Unlock() + if li.id != nil { return li.id, nil } - li.mu.Lock() - defer li.mu.Unlock() - id, err := li.cache.ResolveIdentity(li.excerpt.Id) if err != nil { return nil, fmt.Errorf("cache: missing identity %v", li.excerpt.Id) From 7348fb9edb68ca9142f5d87673da48cef733b3d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Sun, 5 Jun 2022 15:01:08 +0200 Subject: [PATCH 12/12] github: fix data race when closing event channel I believe the issue was twofold: When done importing, the calling context is likely still valid, so if the output channel is not read enough and reach capacity, some event producer down the line can be blocked trying to send in that channel. When closing it, this send is still trying to proceed, which is illegal in go. In rateLimitHandlerClient, there was a need to 2 different type of output channel: core.ExportResult and ImportEvent. To do so, the previous code was using a single channel type RateLimitingEvent and a series of goroutines to read/cast/send to the final channel. This could result in more async goroutine being stuck trying to send in an at-capacity channel. Instead, the code now use a simple synchronous callback to directly push to the final output channel. No concurrency needed anymore and the code is simpler. Any of those fixes could have resolved the data race, but both fixes is more correct. --- bridge/github/client.go | 106 +++++++++++------------- bridge/github/export.go | 15 +--- bridge/github/import_events.go | 40 +++++++++ bridge/github/import_mediator.go | 134 +++++++++++++------------------ 4 files changed, 144 insertions(+), 151 deletions(-) create mode 100644 bridge/github/import_events.go diff --git a/bridge/github/client.go b/bridge/github/client.go index 10f0a03c33882084732a36ab3e0e35f282893dc4..00981a569789d2d18f8f2ef29973fd6025b2a520 100644 --- a/bridge/github/client.go +++ b/bridge/github/client.go @@ -7,8 +7,9 @@ import ( "strings" "time" - "github.com/MichaelMure/git-bug/bridge/core" "github.com/shurcooL/githubv4" + + "github.com/MichaelMure/git-bug/bridge/core" ) var _ Client = &githubv4.Client{} @@ -29,79 +30,69 @@ func newRateLimitHandlerClient(httpClient *http.Client) *rateLimitHandlerClient return &rateLimitHandlerClient{sc: githubv4.NewClient(httpClient)} } -type RateLimitingEvent struct { - msg string -} - -// mutate calls the github api with a graphql mutation and for each rate limiting event it sends an -// export result. +// mutate calls the github api with a graphql mutation and sends a core.ExportResult for each rate limiting event func (c *rateLimitHandlerClient) mutate(ctx context.Context, m interface{}, input githubv4.Input, vars map[string]interface{}, out chan<- core.ExportResult) error { // prepare a closure for the mutation mutFun := func(ctx context.Context) error { return c.sc.Mutate(ctx, m, input, vars) } - limitEvents := make(chan RateLimitingEvent) - defer close(limitEvents) - go func() { - for e := range limitEvents { - select { - case <-ctx.Done(): - return - case out <- core.NewExportRateLimiting(e.msg): - } + callback := func(msg string) { + select { + case <-ctx.Done(): + case out <- core.NewExportRateLimiting(msg): } - }() - return c.callAPIAndRetry(mutFun, ctx, limitEvents) + } + return c.callAPIAndRetry(ctx, mutFun, callback) } -// queryWithLimitEvents calls the github api with a graphql query and it sends rate limiting events -// to a given channel of type RateLimitingEvent. -func (c *rateLimitHandlerClient) queryWithLimitEvents(ctx context.Context, query interface{}, vars map[string]interface{}, limitEvents chan<- RateLimitingEvent) error { - // prepare a closure fot the query +// queryImport calls the github api with a graphql query, and sends an ImportEvent for each rate limiting event +func (c *rateLimitHandlerClient) queryImport(ctx context.Context, query interface{}, vars map[string]interface{}, importEvents chan<- ImportEvent) error { + // prepare a closure for the query queryFun := func(ctx context.Context) error { return c.sc.Query(ctx, query, vars) } - return c.callAPIAndRetry(queryFun, ctx, limitEvents) + callback := func(msg string) { + select { + case <-ctx.Done(): + case importEvents <- RateLimitingEvent{msg}: + } + } + return c.callAPIAndRetry(ctx, queryFun, callback) } -// queryWithImportEvents calls the github api with a graphql query and it sends rate limiting events -// to a given channel of type ImportEvent. -func (c *rateLimitHandlerClient) queryWithImportEvents(ctx context.Context, query interface{}, vars map[string]interface{}, importEvents chan<- ImportEvent) error { - // forward rate limiting events to channel of import events - limitEvents := make(chan RateLimitingEvent) - defer close(limitEvents) - go func() { - for e := range limitEvents { - select { - case <-ctx.Done(): - return - case importEvents <- e: - } +// queryImport calls the github api with a graphql query, and sends a core.ExportResult for each rate limiting event +func (c *rateLimitHandlerClient) queryExport(ctx context.Context, query interface{}, vars map[string]interface{}, out chan<- core.ExportResult) error { + // prepare a closure for the query + queryFun := func(ctx context.Context) error { + return c.sc.Query(ctx, query, vars) + } + callback := func(msg string) { + select { + case <-ctx.Done(): + case out <- core.NewExportRateLimiting(msg): } - }() - return c.queryWithLimitEvents(ctx, query, vars, limitEvents) + } + return c.callAPIAndRetry(ctx, queryFun, callback) } -// queryPrintMsgs calls the github api with a graphql query and it prints for ever rate limiting -// event a message to stdout. +// queryPrintMsgs calls the github api with a graphql query, and prints a message to stdout for every rate limiting event . func (c *rateLimitHandlerClient) queryPrintMsgs(ctx context.Context, query interface{}, vars map[string]interface{}) error { - // print rate limiting events directly to stdout. - limitEvents := make(chan RateLimitingEvent) - defer close(limitEvents) - go func() { - for e := range limitEvents { - fmt.Println(e.msg) - } - }() - return c.queryWithLimitEvents(ctx, query, vars, limitEvents) + // prepare a closure for the query + queryFun := func(ctx context.Context) error { + return c.sc.Query(ctx, query, vars) + } + callback := func(msg string) { + fmt.Println(msg) + } + return c.callAPIAndRetry(ctx, queryFun, callback) } // callAPIAndRetry calls the Github GraphQL API (inderectely through callAPIDealWithLimit) and in // case of error it repeats the request to the Github API. The parameter `apiCall` is intended to be // a closure containing a query or a mutation to the Github GraphQL API. -func (c *rateLimitHandlerClient) callAPIAndRetry(apiCall func(context.Context) error, ctx context.Context, events chan<- RateLimitingEvent) error { +func (c *rateLimitHandlerClient) callAPIAndRetry(ctx context.Context, apiCall func(context.Context) error, rateLimitEvent func(msg string)) error { var err error - if err = c.callAPIDealWithLimit(apiCall, ctx, events); err == nil { + if err = c.callAPIDealWithLimit(ctx, apiCall, rateLimitEvent); err == nil { return nil } // failure; the reason may be temporary network problems or internal errors @@ -117,7 +108,7 @@ func (c *rateLimitHandlerClient) callAPIAndRetry(apiCall func(context.Context) e stop(timer) return ctx.Err() case <-timer.C: - err = c.callAPIDealWithLimit(apiCall, ctx, events) + err = c.callAPIDealWithLimit(ctx, apiCall, rateLimitEvent) if err == nil { return nil } @@ -127,10 +118,10 @@ func (c *rateLimitHandlerClient) callAPIAndRetry(apiCall func(context.Context) e } // callAPIDealWithLimit calls the Github GraphQL API and if the Github API returns a rate limiting -// error, then it waits until the rate limit is reset and it repeats the request to the API. The +// error, then it waits until the rate limit is reset, and it repeats the request to the API. The // parameter `apiCall` is intended to be a closure containing a query or a mutation to the Github // GraphQL API. -func (c *rateLimitHandlerClient) callAPIDealWithLimit(apiCall func(context.Context) error, ctx context.Context, events chan<- RateLimitingEvent) error { +func (c *rateLimitHandlerClient) callAPIDealWithLimit(ctx context.Context, apiCall func(context.Context) error, rateLimitCallback func(msg string)) error { qctx, cancel := context.WithTimeout(ctx, defaultTimeout) defer cancel() // call the function fun() @@ -155,11 +146,8 @@ func (c *rateLimitHandlerClient) callAPIDealWithLimit(apiCall func(context.Conte resetTime.String(), ) // Send message about rate limiting event. - select { - case <-ctx.Done(): - return ctx.Err() - case events <- RateLimitingEvent{msg}: - } + rateLimitCallback(msg) + // Pause current goroutine timer := time.NewTimer(time.Until(resetTime)) select { diff --git a/bridge/github/export.go b/bridge/github/export.go index 8c40eb74d9692fef5fbe0a0a3ac7d70d4ba9ba1b..35d456c202fa5f13ba6f05c073021564d299f582 100644 --- a/bridge/github/export.go +++ b/bridge/github/export.go @@ -486,23 +486,10 @@ func (ge *githubExporter) cacheGithubLabels(ctx context.Context, gc *rateLimitHa } q := labelsQuery{} - // When performing the queries we have to forward rate limiting events to the - // current channel of export results. - events := make(chan RateLimitingEvent) - defer close(events) - go func() { - for e := range events { - select { - case <-ctx.Done(): - return - case ge.out <- core.NewExportRateLimiting(e.msg): - } - } - }() hasNextPage := true for hasNextPage { - if err := gc.queryWithLimitEvents(ctx, &q, variables, events); err != nil { + if err := gc.queryExport(ctx, &q, variables, ge.out); err != nil { return err } diff --git a/bridge/github/import_events.go b/bridge/github/import_events.go new file mode 100644 index 0000000000000000000000000000000000000000..7ae86d75cdd667ba0b9417042934078451e2a082 --- /dev/null +++ b/bridge/github/import_events.go @@ -0,0 +1,40 @@ +package github + +import "github.com/shurcooL/githubv4" + +type ImportEvent interface { + isImportEvent() +} + +type RateLimitingEvent struct { + msg string +} + +func (RateLimitingEvent) isImportEvent() {} + +type IssueEvent struct { + issue +} + +func (IssueEvent) isImportEvent() {} + +type IssueEditEvent struct { + issueId githubv4.ID + userContentEdit +} + +func (IssueEditEvent) isImportEvent() {} + +type TimelineEvent struct { + issueId githubv4.ID + timelineItem +} + +func (TimelineEvent) isImportEvent() {} + +type CommentEditEvent struct { + commentId githubv4.ID + userContentEdit +} + +func (CommentEditEvent) isImportEvent() {} diff --git a/bridge/github/import_mediator.go b/bridge/github/import_mediator.go index db9f877cd816efdb298b14be444e7f464ee2369b..be4e388009cb03c764f3521a18380dd754c29b80 100644 --- a/bridge/github/import_mediator.go +++ b/bridge/github/import_mediator.go @@ -9,6 +9,7 @@ import ( const ( // These values influence how fast the github graphql rate limit is exhausted. + NumIssues = 40 NumIssueEdits = 100 NumTimelineItems = 100 @@ -41,43 +42,6 @@ type importMediator struct { err error } -type ImportEvent interface { - isImportEvent() -} - -func (RateLimitingEvent) isImportEvent() {} - -type IssueEvent struct { - issue -} - -func (IssueEvent) isImportEvent() {} - -type IssueEditEvent struct { - issueId githubv4.ID - userContentEdit -} - -func (IssueEditEvent) isImportEvent() {} - -type TimelineEvent struct { - issueId githubv4.ID - timelineItem -} - -func (TimelineEvent) isImportEvent() {} - -type CommentEditEvent struct { - commentId githubv4.ID - userContentEdit -} - -func (CommentEditEvent) isImportEvent() {} - -func (mm *importMediator) NextImportEvent() ImportEvent { - return <-mm.importEvents -} - func NewImportMediator(ctx context.Context, client *rateLimitHandlerClient, owner, project string, since time.Time) *importMediator { mm := importMediator{ gh: client, @@ -87,48 +51,24 @@ func NewImportMediator(ctx context.Context, client *rateLimitHandlerClient, owne importEvents: make(chan ImportEvent, ChanCapacity), err: nil, } - go func() { - mm.fillImportEvents(ctx) - close(mm.importEvents) - }() - return &mm -} -type varmap map[string]interface{} + go mm.start(ctx) -func newIssueVars(owner, project string, since time.Time) varmap { - return varmap{ - "owner": githubv4.String(owner), - "name": githubv4.String(project), - "issueSince": githubv4.DateTime{Time: since}, - "issueFirst": githubv4.Int(NumIssues), - "issueEditLast": githubv4.Int(NumIssueEdits), - "issueEditBefore": (*githubv4.String)(nil), - "timelineFirst": githubv4.Int(NumTimelineItems), - "timelineAfter": (*githubv4.String)(nil), - "commentEditLast": githubv4.Int(NumCommentEdits), - "commentEditBefore": (*githubv4.String)(nil), - } -} - -func newIssueEditVars() varmap { - return varmap{ - "issueEditLast": githubv4.Int(NumIssueEdits), - } + return &mm } -func newTimelineVars() varmap { - return varmap{ - "timelineFirst": githubv4.Int(NumTimelineItems), - "commentEditLast": githubv4.Int(NumCommentEdits), - "commentEditBefore": (*githubv4.String)(nil), - } +func (mm *importMediator) start(ctx context.Context) { + ctx, cancel := context.WithCancel(ctx) + mm.fillImportEvents(ctx) + // Make sure we cancel everything when we are done, instead of relying on the parent context + // This should unblock pending send to the channel if the capacity was reached and avoid a panic/race when closing. + cancel() + close(mm.importEvents) } -func newCommentEditVars() varmap { - return varmap{ - "commentEditLast": githubv4.Int(NumCommentEdits), - } +// NextImportEvent returns the next ImportEvent, or nil if done. +func (mm *importMediator) NextImportEvent() ImportEvent { + return <-mm.importEvents } func (mm *importMediator) Error() error { @@ -138,7 +78,7 @@ func (mm *importMediator) Error() error { func (mm *importMediator) User(ctx context.Context, loginName string) (*user, error) { query := userQuery{} vars := varmap{"login": githubv4.String(loginName)} - if err := mm.gh.queryWithImportEvents(ctx, &query, vars, mm.importEvents); err != nil { + if err := mm.gh.queryImport(ctx, &query, vars, mm.importEvents); err != nil { return nil, err } return &query.User, nil @@ -200,7 +140,7 @@ func (mm *importMediator) queryIssueEdits(ctx context.Context, nid githubv4.ID, vars["issueEditBefore"] = cursor } query := issueEditQuery{} - if err := mm.gh.queryWithImportEvents(ctx, &query, vars, mm.importEvents); err != nil { + if err := mm.gh.queryImport(ctx, &query, vars, mm.importEvents); err != nil { mm.err = err return nil, false } @@ -244,7 +184,7 @@ func (mm *importMediator) queryTimeline(ctx context.Context, nid githubv4.ID, cu vars["timelineAfter"] = cursor } query := timelineQuery{} - if err := mm.gh.queryWithImportEvents(ctx, &query, vars, mm.importEvents); err != nil { + if err := mm.gh.queryImport(ctx, &query, vars, mm.importEvents); err != nil { mm.err = err return nil, false } @@ -294,7 +234,7 @@ func (mm *importMediator) queryCommentEdits(ctx context.Context, nid githubv4.ID vars["commentEditBefore"] = cursor } query := commentEditQuery{} - if err := mm.gh.queryWithImportEvents(ctx, &query, vars, mm.importEvents); err != nil { + if err := mm.gh.queryImport(ctx, &query, vars, mm.importEvents); err != nil { mm.err = err return nil, false } @@ -313,7 +253,7 @@ func (mm *importMediator) queryIssue(ctx context.Context, cursor githubv4.String vars["issueAfter"] = cursor } query := issueQuery{} - if err := mm.gh.queryWithImportEvents(ctx, &query, vars, mm.importEvents); err != nil { + if err := mm.gh.queryImport(ctx, &query, vars, mm.importEvents); err != nil { mm.err = err return nil, false } @@ -334,3 +274,41 @@ func reverse(eds []userContentEdit) chan userContentEdit { }() return ret } + +// varmap is a container for Github API's pagination variables +type varmap map[string]interface{} + +func newIssueVars(owner, project string, since time.Time) varmap { + return varmap{ + "owner": githubv4.String(owner), + "name": githubv4.String(project), + "issueSince": githubv4.DateTime{Time: since}, + "issueFirst": githubv4.Int(NumIssues), + "issueEditLast": githubv4.Int(NumIssueEdits), + "issueEditBefore": (*githubv4.String)(nil), + "timelineFirst": githubv4.Int(NumTimelineItems), + "timelineAfter": (*githubv4.String)(nil), + "commentEditLast": githubv4.Int(NumCommentEdits), + "commentEditBefore": (*githubv4.String)(nil), + } +} + +func newIssueEditVars() varmap { + return varmap{ + "issueEditLast": githubv4.Int(NumIssueEdits), + } +} + +func newTimelineVars() varmap { + return varmap{ + "timelineFirst": githubv4.Int(NumTimelineItems), + "commentEditLast": githubv4.Int(NumCommentEdits), + "commentEditBefore": (*githubv4.String)(nil), + } +} + +func newCommentEditVars() varmap { + return varmap{ + "commentEditLast": githubv4.Int(NumCommentEdits), + } +}