bug: refactor to limit abstraction leak and to have a more reusable code for the UIs

15 files changed, 235 insertions, 113 deletions

diff --git a/bug/bug.go b/bug/bug.go
index 4df6996c..d698a055 100644
--- a/bug/bug.go
+++ b/bug/bug.go
@@ -8,16 +8,16 @@ import (
 	"strings"
 )
 
-const BugsRefPattern = "refs/bugs/"
-const BugsRemoteRefPattern = "refs/remote/%s/bugs/"
-const OpsEntryName = "ops"
-const RootEntryName = "root"
+const bugsRefPattern = "refs/bugs/"
+const bugsRemoteRefPattern = "refs/remote/%s/bugs/"
+const opsEntryName = "ops"
+const rootEntryName = "root"
 
-const IdLength = 40
-const HumanIdLength = 7
+const idLength = 40
+const humanIdLength = 7
 
 // Bug hold the data of a bug thread, organized in a way close to
-// how it will be persisted inside Git. This is the datastructure
+// how it will be persisted inside Git. This is the data structure
 // used for merge of two different version.
 type Bug struct {
 	// Id used as unique identifier
@@ -40,8 +40,8 @@ func NewBug() *Bug {
 }
 
 // Find an existing Bug matching a prefix
-func FindBug(repo repository.Repo, prefix string) (*Bug, error) {
-	ids, err := repo.ListRefs(BugsRefPattern)
+func FindLocalBug(repo repository.Repo, prefix string) (*Bug, error) {
+	ids, err := repo.ListRefs(bugsRefPattern)
 
 	if err != nil {
 		return nil, err
@@ -64,11 +64,21 @@ func FindBug(repo repository.Repo, prefix string) (*Bug, error) {
 		return nil, fmt.Errorf("Multiple matching bug found:\n%s", strings.Join(matching, "\n"))
 	}
 
-	return ReadBug(repo, BugsRefPattern+matching[0])
+	return ReadLocalBug(repo, matching[0])
+}
+
+func ReadLocalBug(repo repository.Repo, id string) (*Bug, error) {
+	ref := bugsRefPattern + id
+	return readBug(repo, ref)
+}
+
+func ReadRemoteBug(repo repository.Repo, remote string, id string) (*Bug, error) {
+	ref := fmt.Sprintf(bugsRemoteRefPattern, remote) + id
+	return readBug(repo, ref)
 }
 
 // Read and parse a Bug from git
-func ReadBug(repo repository.Repo, ref string) (*Bug, error) {
+func readBug(repo repository.Repo, ref string) (*Bug, error) {
 	hashes, err := repo.ListCommits(ref)
 
 	if err != nil {
@@ -78,7 +88,7 @@ func ReadBug(repo repository.Repo, ref string) (*Bug, error) {
 	refSplitted := strings.Split(ref, "/")
 	id := refSplitted[len(refSplitted)-1]
 
-	if len(id) != IdLength {
+	if len(id) != idLength {
 		return nil, fmt.Errorf("Invalid ref length")
 	}
 
@@ -102,12 +112,12 @@ func ReadBug(repo repository.Repo, ref string) (*Bug, error) {
 		rootFound := false
 
 		for _, entry := range entries {
-			if entry.Name == OpsEntryName {
+			if entry.Name == opsEntryName {
 				opsEntry = entry
 				opsFound = true
 				continue
 			}
-			if entry.Name == RootEntryName {
+			if entry.Name == rootEntryName {
 				rootEntry = entry
 				rootFound = true
 			}
@@ -150,6 +160,50 @@ func ReadBug(repo repository.Repo, ref string) (*Bug, error) {
 	return &bug, nil
 }
 
+type StreamedBug struct {
+	Bug *Bug
+	Err error
+}
+
+// Read and parse all local bugs
+func ReadAllLocalBugs(repo repository.Repo) <-chan StreamedBug {
+	return readAllBugs(repo, bugsRefPattern)
+}
+
+// Read and parse all remote bugs for a given remote
+func ReadAllRemoteBugs(repo repository.Repo, 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 {
+	out := make(chan StreamedBug)
+
+	go func() {
+		defer close(out)
+
+		refs, err := repo.ListRefs(refPrefix)
+		if err != nil {
+			out <- StreamedBug{Err: err}
+			return
+		}
+
+		for _, ref := range refs {
+			b, err := readBug(repo, ref)
+
+			if err != nil {
+				out <- StreamedBug{Err: err}
+				return
+			}
+
+			out <- StreamedBug{Bug: b}
+		}
+	}()
+
+	return out
+}
+
 // IsValid check if the Bug data is valid
 func (bug *Bug) IsValid() bool {
 	// non-empty
@@ -216,9 +270,9 @@ func (bug *Bug) Commit(repo repository.Repo) error {
 	// Write a Git tree referencing this blob
 	hash, err = repo.StoreTree([]repository.TreeEntry{
 		// the last pack of ops
-		{ObjectType: repository.Blob, Hash: hash, Name: OpsEntryName},
+		{ObjectType: repository.Blob, Hash: hash, Name: opsEntryName},
 		// always the first pack of ops (might be the same)
-		{ObjectType: repository.Blob, Hash: bug.rootPack, Name: RootEntryName},
+		{ObjectType: repository.Blob, Hash: bug.rootPack, Name: rootEntryName},
 	})
 
 	if err != nil {
@@ -244,7 +298,7 @@ func (bug *Bug) Commit(repo repository.Repo) error {
 	}
 
 	// Create or update the Git reference for this bug
-	ref := fmt.Sprintf("%s%s", BugsRefPattern, bug.id)
+	ref := fmt.Sprintf("%s%s", bugsRefPattern, bug.id)
 	err = repo.UpdateRef(ref, hash)
 
 	if err != nil {
@@ -326,7 +380,7 @@ func (bug *Bug) Merge(repo repository.Repo, other *Bug) (bool, error) {
 
 	// Update the git ref
 	if updated {
-		err := repo.UpdateRef(BugsRefPattern+bug.id, bug.lastCommit)
+		err := repo.UpdateRef(bugsRefPattern+bug.id, bug.lastCommit)
 		if err != nil {
 			return false, err
 		}
@@ -347,8 +401,12 @@ func (bug *Bug) Id() string {
 
 // Return the Bug identifier truncated for human consumption
 func (bug *Bug) HumanId() string {
-	format := fmt.Sprintf("%%.%ds", HumanIdLength)
-	return fmt.Sprintf(format, bug.Id())
+	return formatHumanId(bug.Id())
+}
+
+func formatHumanId(id string) string {
+	format := fmt.Sprintf("%%.%ds", humanIdLength)
+	return fmt.Sprintf(format, id)
 }
 
 // Lookup for the very first operation of the bug.
diff --git a/bug/remote_actions.go b/bug/remote_actions.go
new file mode 100644
index 00000000..2070ba21
--- /dev/null
+++ b/bug/remote_actions.go
@@ -0,0 +1,119 @@
+package bug
+
+import (
+	"fmt"
+	"github.com/MichaelMure/git-bug/repository"
+	"strings"
+)
+
+const MsgNew = "new"
+const MsgInvalid = "invalid data"
+const MsgUpdated = "updated"
+const MsgNothing = "nothing to do"
+
+func Fetch(repo repository.Repo, remote string) error {
+	remoteRefSpec := fmt.Sprintf(bugsRemoteRefPattern, remote)
+	fetchRefSpec := fmt.Sprintf("%s*:%s*", bugsRefPattern, remoteRefSpec)
+
+	return repo.FetchRefs(remote, fetchRefSpec)
+}
+
+func Push(repo repository.Repo, remote string) error {
+	return repo.PushRefs(remote, bugsRefPattern+"*")
+}
+
+type MergeResult struct {
+	Err error
+
+	Id      string
+	HumanId string
+	Status  string
+}
+
+func newMergeError(id string, err error) MergeResult {
+	return MergeResult{
+		Id:      id,
+		HumanId: formatHumanId(id),
+		Status:  err.Error(),
+	}
+}
+
+func newMergeStatus(id string, status string) MergeResult {
+	return MergeResult{
+		Id:      id,
+		HumanId: formatHumanId(id),
+		Status:  status,
+	}
+}
+
+func MergeAll(repo repository.Repo, remote string) <-chan MergeResult {
+	out := make(chan MergeResult)
+
+	go func() {
+		defer close(out)
+
+		remoteRefSpec := fmt.Sprintf(bugsRemoteRefPattern, remote)
+		remoteRefs, err := repo.ListRefs(remoteRefSpec)
+
+		if err != nil {
+			out <- MergeResult{Err: err}
+			return
+		}
+
+		for _, remoteRef := range remoteRefs {
+			refSplitted := strings.Split(remoteRef, "/")
+			id := refSplitted[len(refSplitted)-1]
+
+			remoteBug, err := readBug(repo, remoteRef)
+
+			if err != nil {
+				out <- newMergeError(id, err)
+				continue
+			}
+
+			// Check for error in remote data
+			if !remoteBug.IsValid() {
+				out <- newMergeStatus(id, MsgInvalid)
+				continue
+			}
+
+			localRef := bugsRefPattern + remoteBug.Id()
+			localExist, err := repo.RefExist(localRef)
+
+			// the bug is not local yet, simply create the reference
+			if !localExist {
+				err := repo.CopyRef(remoteRef, localRef)
+
+				if err != nil {
+					out <- newMergeError(id, err)
+					return
+				}
+
+				out <- newMergeStatus(id, MsgNew)
+				continue
+			}
+
+			localBug, err := readBug(repo, localRef)
+
+			if err != nil {
+				out <- newMergeError(id, err)
+				return
+			}
+
+			updated, err := localBug.Merge(repo, remoteBug)
+
+			if err != nil {
+				out <- newMergeError(id, err)
+				return
+			}
+
+			if updated {
+				out <- newMergeStatus(id, MsgUpdated)
+			} else {
+				out <- newMergeStatus(id, MsgNothing)
+			}
+		}
+	}()
+
+	return out
+}
diff --git a/commands/close.go b/commands/close.go
index be0ff6e4..58446d71 100644
--- a/commands/close.go
+++ b/commands/close.go
@@ -18,7 +18,7 @@ func runCloseBug(cmd *cobra.Command, args []string) error {
 
 	prefix := args[0]
 
-	b, err := bug.FindBug(repo, prefix)
+	b, err := bug.FindLocalBug(repo, prefix)
 	if err != nil {
 		return err
 	}
diff --git a/commands/comment.go b/commands/comment.go
index cbd34ab3..252fb7e4 100644
--- a/commands/comment.go
+++ b/commands/comment.go
@@ -44,7 +44,7 @@ func runComment(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	b, err := bug.FindBug(repo, prefix)
+	b, err := bug.FindLocalBug(repo, prefix)
 	if err != nil {
 		return err
 	}
diff --git a/commands/label.go b/commands/label.go
index 08d7a78a..e1679972 100644
--- a/commands/label.go
+++ b/commands/label.go
@@ -21,7 +21,7 @@ func runLabel(cmd *cobra.Command, args []string) error {
 
 	prefix := args[0]
 
-	b, err := bug.FindBug(repo, prefix)
+	b, err := bug.FindLocalBug(repo, prefix)
 	if err != nil {
 		return err
 	}
diff --git a/commands/ls.go b/commands/ls.go
index 1bc2afff..5a5d96c6 100644
--- a/commands/ls.go
+++ b/commands/ls.go
@@ -2,28 +2,22 @@ package commands
 
 import (
 	"fmt"
-	b "github.com/MichaelMure/git-bug/bug"
+	"github.com/MichaelMure/git-bug/bug"
 	"github.com/MichaelMure/git-bug/util"
 	"github.com/spf13/cobra"
 )
 
 func runLsBug(cmd *cobra.Command, args []string) error {
-	ids, err := repo.ListRefs(b.BugsRefPattern)
+	bugs := bug.ReadAllLocalBugs(repo)
 
-	if err != nil {
-		return err
-	}
-
-	for _, ref := range ids {
-		bug, err := b.ReadBug(repo, b.BugsRefPattern+ref)
-
-		if err != nil {
-			return err
+	for b := range bugs {
+		if b.Err != nil {
+			return b.Err
 		}
 
-		snapshot := bug.Compile()
+		snapshot := b.Bug.Compile()
 
-		var author b.Person
+		var author bug.Person
 
 		if len(snapshot.Comments) > 0 {
 			create := snapshot.Comments[0]
@@ -35,7 +29,7 @@ func runLsBug(cmd *cobra.Command, args []string) error {
 		authorFmt := fmt.Sprintf("%-15.15s", author.Name)
 
 		fmt.Printf("%s %s\t%s\t%s\t%s\n",
-			util.Cyan(bug.HumanId()),
+			util.Cyan(b.Bug.HumanId()),
 			util.Yellow(snapshot.Status),
 			titleFmt,
 			util.Magenta(authorFmt),
diff --git a/commands/open.go b/commands/open.go
index 6fe1e9d3..7fa59b49 100644
--- a/commands/open.go
+++ b/commands/open.go
@@ -18,7 +18,7 @@ func runOpenBug(cmd *cobra.Command, args []string) error {
 
 	prefix := args[0]
 
-	b, err := bug.FindBug(repo, prefix)
+	b, err := bug.FindLocalBug(repo, prefix)
 	if err != nil {
 		return err
 	}
diff --git a/commands/pull.go b/commands/pull.go
index 83c2101a..ac6a3732 100644
--- a/commands/pull.go
+++ b/commands/pull.go
@@ -19,62 +19,19 @@ func runPull(cmd *cobra.Command, args []string) error {
 
 	fmt.Printf("Fetching remote ...\n\n")
 
-	if err := repo.FetchRefs(remote, bug.BugsRefPattern+"*", bug.BugsRemoteRefPattern+"*"); err != nil {
+	if err := bug.Fetch(repo, remote); err != nil {
 		return err
 	}
 
 	fmt.Printf("\nMerging data ...\n\n")
 
-	remoteRefSpec := fmt.Sprintf(bug.BugsRemoteRefPattern, remote)
-	remoteRefs, err := repo.ListRefs(remoteRefSpec)
-
-	if err != nil {
-		return err
-	}
-
-	for _, ref := range remoteRefs {
-		remoteRef := fmt.Sprintf(bug.BugsRemoteRefPattern, remote) + ref
-		remoteBug, err := bug.ReadBug(repo, remoteRef)
-
-		if err != nil {
-			return err
-		}
-
-		// Check for error in remote data
-		if !remoteBug.IsValid() {
-			fmt.Printf("%s: %s\n", remoteBug.HumanId(), "invalid remote data")
-			continue
-		}
-
-		localRef := bug.BugsRefPattern + remoteBug.Id()
-		localExist, err := repo.RefExist(localRef)
-
-		// the bug is not local yet, simply create the reference
-		if !localExist {
-			err := repo.CopyRef(remoteRef, localRef)
-
-			if err != nil {
-				return err
-			}
-
-			fmt.Printf("%s: %s\n", remoteBug.HumanId(), "new")
-			continue
-		}
-
-		localBug, err := bug.ReadBug(repo, localRef)
-
-		if err != nil {
-			return err
-		}
-
-		updated, err := localBug.Merge(repo, remoteBug)
-
-		if err != nil {
-			return err
+	for merge := range bug.MergeAll(repo, remote) {
+		if merge.Err != nil {
+			return merge.Err
 		}
 
-		if updated {
-			fmt.Printf("%s: %s\n", remoteBug.HumanId(), "updated")
+		if merge.Status != bug.MsgNothing {
+			fmt.Printf("%s: %s\n", merge.HumanId, merge.Status)
 		}
 	}
 
diff --git a/commands/push.go b/commands/push.go
index 37479bfd..e2c29c68 100644
--- a/commands/push.go
+++ b/commands/push.go
@@ -16,9 +16,10 @@ func runPush(cmd *cobra.Command, args []string) error {
 		remote = args[0]
 	}
 
-	if err := repo.PushRefs(remote, bug.BugsRefPattern+"*"); err != nil {
+	if err := bug.Push(repo, remote); err != nil {
 		return err
 	}
+
 	return nil
 }
 
diff --git a/commands/show.go b/commands/show.go
index 9d330aab..8cf3e34a 100644
--- a/commands/show.go
+++ b/commands/show.go
@@ -20,7 +20,7 @@ func runShowBug(cmd *cobra.Command, args []string) error {
 
 	prefix := args[0]
 
-	b, err := bug.FindBug(repo, prefix)
+	b, err := bug.FindLocalBug(repo, prefix)
 	if err != nil {
 		return err
 	}
diff --git a/commands/webui.go b/commands/webui.go
index c4db2ae6..802fc5df 100644
--- a/commands/webui.go
+++ b/commands/webui.go
@@ -19,7 +19,7 @@ func runWebUI(cmd *cobra.Command, args []string) error {
 		var err error
 		port, err = freeport.GetFreePort()
 		if err != nil {
-			log.Fatal(err)
+			return err
 		}
 	}
 
diff --git a/graphql/schema.go b/graphql/schema.go
index 7419e98a..8312933d 100644
--- a/graphql/schema.go
+++ b/graphql/schema.go
@@ -18,12 +18,12 @@ func graphqlSchema() (graphql.Schema, error) {
 			Resolve: func(p graphql.ResolveParams) (interface{}, error) {
 				repo := p.Context.Value("repo").(repository.Repo)
 				id, _ := p.Args["id"].(string)
-				bug, err := bug.FindBug(repo, id)
+				b, err := bug.FindLocalBug(repo, id)
 				if err != nil {
 					return nil, err
 				}
 
-				snapshot := bug.Compile()
+				snapshot := b.Compile()
 
 				return snapshot, nil
 			},
@@ -33,22 +33,15 @@ func graphqlSchema() (graphql.Schema, error) {
 			Type: graphql.NewList(bugType),
 			Resolve: func(p graphql.ResolveParams) (interface{}, error) {
 				repo := p.Context.Value("repo").(repository.Repo)
-				ids, err := repo.ListRefs(bug.BugsRefPattern)
-
-				if err != nil {
-					return nil, err
-				}
 
 				var snapshots []bug.Snapshot
 
-				for _, ref := range ids {
-					bug, err := bug.ReadBug(repo, bug.BugsRefPattern+ref)
-
-					if err != nil {
-						return nil, err
+				for sb := range bug.ReadAllLocalBugs(repo) {
+					if sb.Err != nil {
+						return nil, sb.Err
 					}
 
-					snapshots = append(snapshots, bug.Compile())
+					snapshots = append(snapshots, sb.Bug.Compile())
 				}
 
 				return snapshots, nil
diff --git a/repository/git.go b/repository/git.go
index 8d265325..648f56c1 100644
--- a/repository/git.go
+++ b/repository/git.go
@@ -19,6 +19,8 @@ type GitRepo struct {
 
 // Run the given git command with the given I/O reader/writers, returning an error if it fails.
 func (repo *GitRepo) runGitCommandWithIO(stdin io.Reader, stdout, stderr io.Writer, args ...string) error {
+	fmt.Println("Running git", strings.Join(args, " "))
+
 	cmd := exec.Command("git", args...)
 	cmd.Dir = repo.Path
 	cmd.Stdin = stdin
@@ -99,10 +101,8 @@ func (repo *GitRepo) GetCoreEditor() (string, error) {
 }
 
 // FetchRefs fetch git refs from a remote
-func (repo *GitRepo) FetchRefs(remote, refPattern, remoteRefPattern string) error {
-	remoteRefSpec := fmt.Sprintf(remoteRefPattern, remote)
-	fetchRefSpec := fmt.Sprintf("%s:%s", refPattern, remoteRefSpec)
-	err := repo.runGitCommandInline("fetch", remote, fetchRefSpec)
+func (repo *GitRepo) FetchRefs(remote, refSpec string) error {
+	err := repo.runGitCommandInline("fetch", remote, "\""+refSpec+"\"")
 
 	if err != nil {
 		return fmt.Errorf("failed to fetch from the remote '%s': %v", remote, err)
@@ -112,8 +112,8 @@ func (repo *GitRepo) FetchRefs(remote, refPattern, remoteRefPattern string) erro
 }
 
 // PushRefs push git refs to a remote
-func (repo *GitRepo) PushRefs(remote string, refPattern string) error {
-	err := repo.runGitCommandInline("push", remote, refPattern)
+func (repo *GitRepo) PushRefs(remote string, refSpec string) error {
+	err := repo.runGitCommandInline("push", remote, refSpec)
 
 	if err != nil {
 		return fmt.Errorf("failed to push to the remote '%s': %v", remote, err)
diff --git a/repository/mock_repo.go b/repository/mock_repo.go
index c5ac53a4..4bd25738 100644
--- a/repository/mock_repo.go
+++ b/repository/mock_repo.go
@@ -48,11 +48,11 @@ func (r *mockRepoForTest) GetCoreEditor() (string, error) {
 }
 
 // PushRefs push git refs to a remote
-func (r *mockRepoForTest) PushRefs(remote string, refPattern string) error {
+func (r *mockRepoForTest) PushRefs(remote string, refSpec string) error {
 	return nil
 }
 
-func (r *mockRepoForTest) FetchRefs(remote string, refPattern string, remoteRefPattern string) error {
+func (r *mockRepoForTest) FetchRefs(remote string, refSpec string) error {
 	return nil
 }
 
diff --git a/repository/repo.go b/repository/repo.go
index 7ba8a8b5..6ab7be91 100644
--- a/repository/repo.go
+++ b/repository/repo.go
@@ -22,10 +22,10 @@ type Repo interface {
 	GetCoreEditor() (string, error)
 
 	// FetchRefs fetch git refs from a remote
-	FetchRefs(remote string, refPattern string, remoteRefPattern string) error
+	FetchRefs(remote string, refSpec string) error
 
 	// PushRefs push git refs to a remote
-	PushRefs(remote string, refPattern string) error
+	PushRefs(remote string, refSpec string) error
 
 	// StoreData will store arbitrary data and return the corresponding hash
 	StoreData(data []byte) (util.Hash, error)