From 82eaceffc1d750832a2a66f206749d2dca968cce Mon Sep 17 00:00:00 2001 From: Michael Muré Date: Fri, 21 Sep 2018 18:18:51 +0200 Subject: repo: split the Repo interface to avoid abstraction leak in RepoCache --- bug/bug.go | 16 ++++++++-------- bug/bug_actions.go | 4 ++-- bug/interface.go | 2 +- bug/with_snapshot.go | 2 +- cache/multi_repo_cache.go | 4 ++-- cache/repo_cache.go | 26 ++++++++++++++++++++------ commands/add.go | 2 +- commands/comment_add.go | 2 +- commands/root.go | 2 +- commands/select/select.go | 8 ++++---- graphql/handler.go | 2 +- input/input.go | 12 ++++++------ misc/random_bugs/create_random_bugs.go | 4 ++-- repository/mock_repo.go | 2 +- repository/repo.go | 12 ++++++++++-- termui/termui.go | 8 ++++---- tests/read_bugs_test.go | 2 +- 17 files changed, 66 insertions(+), 44 deletions(-) diff --git a/bug/bug.go b/bug/bug.go index 203d5a1a..993d3d7c 100644 --- a/bug/bug.go +++ b/bug/bug.go @@ -64,7 +64,7 @@ func NewBug() *Bug { } // FindLocalBug find an existing Bug matching a prefix -func FindLocalBug(repo repository.Repo, prefix string) (*Bug, error) { +func FindLocalBug(repo repository.ClockedRepo, prefix string) (*Bug, error) { ids, err := ListLocalIds(repo) if err != nil { @@ -92,19 +92,19 @@ func FindLocalBug(repo repository.Repo, prefix string) (*Bug, error) { } // ReadLocalBug will read a local bug from its hash -func ReadLocalBug(repo repository.Repo, id string) (*Bug, error) { +func ReadLocalBug(repo repository.ClockedRepo, id string) (*Bug, error) { ref := bugsRefPattern + id return readBug(repo, ref) } // ReadRemoteBug will read a remote bug from its hash -func ReadRemoteBug(repo repository.Repo, remote string, id string) (*Bug, error) { +func ReadRemoteBug(repo repository.ClockedRepo, remote string, id string) (*Bug, error) { ref := fmt.Sprintf(bugsRemoteRefPattern, remote) + id return readBug(repo, ref) } // readBug will read and parse a Bug from git -func readBug(repo repository.Repo, ref string) (*Bug, error) { +func readBug(repo repository.ClockedRepo, ref string) (*Bug, error) { hashes, err := repo.ListCommits(ref) // TODO: this is not perfect, it might be a command invoke error @@ -218,18 +218,18 @@ type StreamedBug struct { } // ReadAllLocalBugs read and parse all local bugs -func ReadAllLocalBugs(repo repository.Repo) <-chan StreamedBug { +func ReadAllLocalBugs(repo repository.ClockedRepo) <-chan StreamedBug { return readAllBugs(repo, bugsRefPattern) } // ReadAllRemoteBugs read and parse all remote bugs for a given remote -func ReadAllRemoteBugs(repo repository.Repo, remote string) <-chan StreamedBug { +func ReadAllRemoteBugs(repo repository.ClockedRepo, remote string) <-chan StreamedBug { refPrefix := fmt.Sprintf(bugsRemoteRefPattern, remote) return readAllBugs(repo, refPrefix) } // Read and parse all available bug with a given ref prefix -func readAllBugs(repo repository.Repo, refPrefix string) <-chan StreamedBug { +func readAllBugs(repo repository.ClockedRepo, refPrefix string) <-chan StreamedBug { out := make(chan StreamedBug) go func() { @@ -331,7 +331,7 @@ func (bug *Bug) HasPendingOp() bool { } // Commit write the staging area in Git and move the operations to the packs -func (bug *Bug) Commit(repo repository.Repo) error { +func (bug *Bug) Commit(repo repository.ClockedRepo) error { if bug.staging.IsEmpty() { return fmt.Errorf("can't commit a bug with no pending operation") } diff --git a/bug/bug_actions.go b/bug/bug_actions.go index 8be4420f..487ba25e 100644 --- a/bug/bug_actions.go +++ b/bug/bug_actions.go @@ -25,7 +25,7 @@ func Push(repo repository.Repo, remote string) (string, error) { // Pull will do a Fetch + MergeAll // This function won't give details on the underlying process. If you need more // use Fetch and MergeAll separately. -func Pull(repo repository.Repo, remote string) error { +func Pull(repo repository.ClockedRepo, remote string) error { _, err := Fetch(repo, remote) if err != nil { return err @@ -41,7 +41,7 @@ func Pull(repo repository.Repo, remote string) error { } // MergeAll will merge all the available remote bug -func MergeAll(repo repository.Repo, remote string) <-chan MergeResult { +func MergeAll(repo repository.ClockedRepo, remote string) <-chan MergeResult { out := make(chan MergeResult) go func() { diff --git a/bug/interface.go b/bug/interface.go index 72dee61c..186c26fc 100644 --- a/bug/interface.go +++ b/bug/interface.go @@ -22,7 +22,7 @@ type Interface interface { HasPendingOp() bool // Commit write the staging area in Git and move the operations to the packs - Commit(repo repository.Repo) error + Commit(repo repository.ClockedRepo) error // Merge a different version of the same bug by rebasing operations of this bug // that are not present in the other on top of the chain of operations of the diff --git a/bug/with_snapshot.go b/bug/with_snapshot.go index a004324b..48274ed5 100644 --- a/bug/with_snapshot.go +++ b/bug/with_snapshot.go @@ -34,7 +34,7 @@ func (b *WithSnapshot) Append(op Operation) { } // Commit intercept Bug.Commit() to update the snapshot efficiently -func (b *WithSnapshot) Commit(repo repository.Repo) error { +func (b *WithSnapshot) Commit(repo repository.ClockedRepo) error { err := b.Bug.Commit(repo) if err != nil { diff --git a/cache/multi_repo_cache.go b/cache/multi_repo_cache.go index c5328b7e..ec435ff2 100644 --- a/cache/multi_repo_cache.go +++ b/cache/multi_repo_cache.go @@ -19,7 +19,7 @@ func NewMultiRepoCache() MultiRepoCache { } // RegisterRepository register a named repository. Use this for multi-repo setup -func (c *MultiRepoCache) RegisterRepository(ref string, repo repository.Repo) error { +func (c *MultiRepoCache) RegisterRepository(ref string, repo repository.ClockedRepo) error { r, err := NewRepoCache(repo) if err != nil { return err @@ -30,7 +30,7 @@ func (c *MultiRepoCache) RegisterRepository(ref string, repo repository.Repo) er } // RegisterDefaultRepository register a unnamed repository. Use this for mono-repo setup -func (c *MultiRepoCache) RegisterDefaultRepository(repo repository.Repo) error { +func (c *MultiRepoCache) RegisterDefaultRepository(repo repository.ClockedRepo) error { r, err := NewRepoCache(repo) if err != nil { return err diff --git a/cache/repo_cache.go b/cache/repo_cache.go index 26f2855f..a43c6684 100644 --- a/cache/repo_cache.go +++ b/cache/repo_cache.go @@ -24,14 +24,14 @@ const formatVersion = 1 type RepoCache struct { // the underlying repo - repo repository.Repo + repo repository.ClockedRepo // excerpt of bugs data for all bugs excerpts map[string]*BugExcerpt // bug loaded in memory bugs map[string]*BugCache } -func NewRepoCache(r repository.Repo) (*RepoCache, error) { +func NewRepoCache(r repository.ClockedRepo) (*RepoCache, error) { c := &RepoCache{ repo: r, bugs: make(map[string]*BugCache), @@ -55,10 +55,24 @@ func NewRepoCache(r repository.Repo) (*RepoCache, error) { return c, c.write() } -// Repository return the underlying repository. -// If you use this, make sure to never change the repo state. -func (c *RepoCache) Repository() repository.Repo { - return c.repo +// GetPath returns the path to the repo. +func (c *RepoCache) GetPath() string { + return c.repo.GetPath() +} + +// GetPath returns the path to the repo. +func (c *RepoCache) GetCoreEditor() (string, error) { + return c.repo.GetCoreEditor() +} + +// GetUserName returns the name the the user has used to configure git +func (c *RepoCache) GetUserName() (string, error) { + return c.repo.GetUserName() +} + +// GetUserEmail returns the email address that the user has used to configure git. +func (c *RepoCache) GetUserEmail() (string, error) { + return c.repo.GetUserEmail() } func (c *RepoCache) lock() error { diff --git a/commands/add.go b/commands/add.go index 3fa75f26..944f0a99 100644 --- a/commands/add.go +++ b/commands/add.go @@ -31,7 +31,7 @@ func runAddBug(cmd *cobra.Command, args []string) error { defer backend.Close() if addMessage == "" || addTitle == "" { - addTitle, addMessage, err = input.BugCreateEditorInput(backend.Repository(), addTitle, addMessage) + addTitle, addMessage, err = input.BugCreateEditorInput(backend, addTitle, addMessage) if err == input.ErrEmptyTitle { fmt.Println("Empty title, aborting.") diff --git a/commands/comment_add.go b/commands/comment_add.go index 8898ea45..0a83eb65 100644 --- a/commands/comment_add.go +++ b/commands/comment_add.go @@ -29,7 +29,7 @@ func runCommentAdd(cmd *cobra.Command, args []string) error { } if commentAddMessage == "" { - commentAddMessage, err = input.BugCommentEditorInput(backend.Repository()) + commentAddMessage, err = input.BugCommentEditorInput(backend) if err == input.ErrEmptyMessage { fmt.Println("Empty message, aborting.") return nil diff --git a/commands/root.go b/commands/root.go index 298fb427..ebf688dc 100644 --- a/commands/root.go +++ b/commands/root.go @@ -13,7 +13,7 @@ import ( const rootCommandName = "git-bug" // package scoped var to hold the repo after the PreRun execution -var repo repository.Repo +var repo repository.ClockedRepo // RootCmd represents the base command when called without any subcommands var RootCmd = &cobra.Command{ diff --git a/commands/select/select.go b/commands/select/select.go index 5d6cee7f..203a2f1e 100644 --- a/commands/select/select.go +++ b/commands/select/select.go @@ -55,7 +55,7 @@ func ResolveBug(repo *cache.RepoCache, args []string) (*cache.BugCache, []string // Select will select a bug for future use func Select(repo *cache.RepoCache, id string) error { - selectPath := selectFilePath(repo.Repository()) + selectPath := selectFilePath(repo) f, err := os.OpenFile(selectPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666) if err != nil { @@ -72,13 +72,13 @@ func Select(repo *cache.RepoCache, id string) error { // Clear will clear the selected bug, if any func Clear(repo *cache.RepoCache) error { - selectPath := selectFilePath(repo.Repository()) + selectPath := selectFilePath(repo) return os.Remove(selectPath) } func selected(repo *cache.RepoCache) (*cache.BugCache, error) { - selectPath := selectFilePath(repo.Repository()) + selectPath := selectFilePath(repo) f, err := os.Open(selectPath) if err != nil { @@ -120,6 +120,6 @@ func selected(repo *cache.RepoCache) (*cache.BugCache, error) { return b, nil } -func selectFilePath(repo repository.Repo) string { +func selectFilePath(repo repository.RepoCommon) string { return path.Join(repo.GetPath(), ".git", "git-bug", selectFile) } diff --git a/graphql/handler.go b/graphql/handler.go index ed5047c4..1e8498c8 100644 --- a/graphql/handler.go +++ b/graphql/handler.go @@ -17,7 +17,7 @@ type Handler struct { *resolvers.RootResolver } -func NewHandler(repo repository.Repo) (Handler, error) { +func NewHandler(repo repository.ClockedRepo) (Handler, error) { h := Handler{ RootResolver: resolvers.NewRootResolver(), } diff --git a/input/input.go b/input/input.go index 4b70bbd4..6a7c8c7c 100644 --- a/input/input.go +++ b/input/input.go @@ -35,7 +35,7 @@ const bugTitleCommentTemplate = `%s%s // BugCreateEditorInput will open the default editor in the terminal with a // template for the user to fill. The file is then processed to extract title // and message. -func BugCreateEditorInput(repo repository.Repo, preTitle string, preMessage string) (string, string, error) { +func BugCreateEditorInput(repo repository.RepoCommon, preTitle string, preMessage string) (string, string, error) { if preMessage != "" { preMessage = "\n\n" + preMessage } @@ -86,7 +86,7 @@ const bugCommentTemplate = ` // BugCommentEditorInput will open the default editor in the terminal with a // template for the user to fill. The file is then processed to extract a comment. -func BugCommentEditorInput(repo repository.Repo) (string, error) { +func BugCommentEditorInput(repo repository.RepoCommon) (string, error) { raw, err := launchEditorWithTemplate(repo, messageFilename, bugCommentTemplate) if err != nil { @@ -121,7 +121,7 @@ const bugTitleTemplate = `%s // BugTitleEditorInput will open the default editor in the terminal with a // template for the user to fill. The file is then processed to extract a title. -func BugTitleEditorInput(repo repository.Repo, preTitle string) (string, error) { +func BugTitleEditorInput(repo repository.RepoCommon, preTitle string) (string, error) { template := fmt.Sprintf(bugTitleTemplate, preTitle) raw, err := launchEditorWithTemplate(repo, messageFilename, template) @@ -180,7 +180,7 @@ const queryTemplate = `%s // QueryEditorInput will open the default editor in the terminal with a // template for the user to fill. The file is then processed to extract a query. -func QueryEditorInput(repo repository.Repo, preQuery string) (string, error) { +func QueryEditorInput(repo repository.RepoCommon, preQuery string) (string, error) { template := fmt.Sprintf(queryTemplate, preQuery) raw, err := launchEditorWithTemplate(repo, messageFilename, template) @@ -206,7 +206,7 @@ func QueryEditorInput(repo repository.Repo, preQuery string) (string, error) { // launchEditorWithTemplate will launch an editor as launchEditor do, but with a // provided template. -func launchEditorWithTemplate(repo repository.Repo, fileName string, template string) (string, error) { +func launchEditorWithTemplate(repo repository.RepoCommon, fileName string, template string) (string, error) { path := fmt.Sprintf("%s/.git/%s", repo.GetPath(), fileName) err := ioutil.WriteFile(path, []byte(template), 0644) @@ -227,7 +227,7 @@ func launchEditorWithTemplate(repo repository.Repo, fileName string, template st // // This method returns the text that was read from the temporary file, or // an error if any step in the process failed. -func launchEditor(repo repository.Repo, fileName string) (string, error) { +func launchEditor(repo repository.RepoCommon, fileName string) (string, error) { path := fmt.Sprintf("%s/.git/%s", repo.GetPath(), fileName) defer os.Remove(path) diff --git a/misc/random_bugs/create_random_bugs.go b/misc/random_bugs/create_random_bugs.go index 75ce5f82..dfef4e07 100644 --- a/misc/random_bugs/create_random_bugs.go +++ b/misc/random_bugs/create_random_bugs.go @@ -29,11 +29,11 @@ func DefaultOptions() Options { } } -func CommitRandomBugs(repo repository.Repo, opts Options) { +func CommitRandomBugs(repo repository.ClockedRepo, opts Options) { CommitRandomBugsWithSeed(repo, opts, time.Now().UnixNano()) } -func CommitRandomBugsWithSeed(repo repository.Repo, opts Options, seed int64) { +func CommitRandomBugsWithSeed(repo repository.ClockedRepo, opts Options, seed int64) { bugs := GenerateRandomBugsWithSeed(opts, seed) for _, b := range bugs { diff --git a/repository/mock_repo.go b/repository/mock_repo.go index 23d3ef7d..2b911783 100644 --- a/repository/mock_repo.go +++ b/repository/mock_repo.go @@ -23,7 +23,7 @@ type commit struct { parent git.Hash } -func NewMockRepoForTest() Repo { +func NewMockRepoForTest() *mockRepoForTest { return &mockRepoForTest{ blobs: make(map[git.Hash][]byte), trees: make(map[git.Hash]string), diff --git a/repository/repo.go b/repository/repo.go index 3d18431d..053837db 100644 --- a/repository/repo.go +++ b/repository/repo.go @@ -9,8 +9,7 @@ import ( "github.com/MichaelMure/git-bug/util/lamport" ) -// Repo represents a source code repository. -type Repo interface { +type RepoCommon interface { // GetPath returns the path to the repo. GetPath() string @@ -22,6 +21,11 @@ type Repo interface { // GetCoreEditor returns the name of the editor that the user has used to configure git. GetCoreEditor() (string, error) +} + +// Repo represents a source code repository. +type Repo interface { + RepoCommon // FetchRefs fetch git refs from a remote FetchRefs(remote string, refSpec string) (string, error) @@ -67,6 +71,10 @@ type Repo interface { // GetTreeHash return the git tree hash referenced in a commit GetTreeHash(commit git.Hash) (git.Hash, error) +} + +type ClockedRepo interface { + Repo LoadClocks() error diff --git a/termui/termui.go b/termui/termui.go index 403fcc97..80c23844 100644 --- a/termui/termui.go +++ b/termui/termui.go @@ -170,7 +170,7 @@ func newBugWithEditor(repo *cache.RepoCache) error { ui.g.Close() ui.g = nil - title, message, err := input.BugCreateEditorInput(ui.cache.Repository(), "", "") + title, message, err := input.BugCreateEditorInput(ui.cache, "", "") if err != nil && err != input.ErrEmptyTitle { return err @@ -210,7 +210,7 @@ func addCommentWithEditor(bug *cache.BugCache) error { ui.g.Close() ui.g = nil - message, err := input.BugCommentEditorInput(ui.cache.Repository()) + message, err := input.BugCommentEditorInput(ui.cache) if err != nil && err != input.ErrEmptyMessage { return err @@ -243,7 +243,7 @@ func setTitleWithEditor(bug *cache.BugCache) error { ui.g.Close() ui.g = nil - title, err := input.BugTitleEditorInput(ui.cache.Repository(), bug.Snapshot().Title) + title, err := input.BugTitleEditorInput(ui.cache, bug.Snapshot().Title) if err != nil && err != input.ErrEmptyTitle { return err @@ -276,7 +276,7 @@ func editQueryWithEditor(bt *bugTable) error { ui.g.Close() ui.g = nil - queryStr, err := input.QueryEditorInput(bt.repo.Repository(), bt.queryStr) + queryStr, err := input.QueryEditorInput(bt.repo, bt.queryStr) if err != nil { return err diff --git a/tests/read_bugs_test.go b/tests/read_bugs_test.go index b4513bb4..c3c7d9ea 100644 --- a/tests/read_bugs_test.go +++ b/tests/read_bugs_test.go @@ -8,7 +8,7 @@ import ( "github.com/MichaelMure/git-bug/repository" ) -func createFilledRepo(bugNumber int) repository.Repo { +func createFilledRepo(bugNumber int) repository.ClockedRepo { repo := createRepo(false) var seed int64 = 42 -- cgit