repo: split the Repo interface to avoid abstraction leak in RepoCache

Michael Muré created 7 years ago

Change summary

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(-)

Detailed changes

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")
 	}

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() {

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

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 {

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

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 {

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.")

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

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{

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)
 }

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(),
 	}

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)
 

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 {

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),

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
 

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

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