cache: some refactoring

Michael Muré created 7 years ago

Change summary

cache/cache.go                | 153 +++++++++++++++---------------------
graphql/resolvers/mutation.go |  37 +++++++-
termui/error_popup.go         |  15 +-
termui/termui.go              |   2 
4 files changed, 103 insertions(+), 104 deletions(-)

Detailed changes

cache/cache.go

@@ -27,23 +27,23 @@ type RepoCacher interface {
 	ResolveBugPrefix(prefix string) (BugCacher, error)
 	AllBugIds() ([]string, error)
 	ClearAllBugs()
-	Commit(bug BugCacher) error
 
 	// Mutations
-
 	NewBug(title string, message string) (BugCacher, error)
-
-	AddComment(repoRef *string, prefix string, message string) (BugCacher, error)
-	ChangeLabels(repoRef *string, prefix string, added []string, removed []string) (BugCacher, error)
-	Open(repoRef *string, prefix string) (BugCacher, error)
-	Close(repoRef *string, prefix string) (BugCacher, error)
-	SetTitle(repoRef *string, prefix string, title string) (BugCacher, error)
 }
 
 type BugCacher interface {
 	Snapshot() *bug.Snapshot
 	ClearSnapshot()
-	bug() *bug.Bug
+
+	// Mutations
+	AddComment(message string) (BugCacher, error)
+	ChangeLabels(added []string, removed []string) (BugCacher, error)
+	Open() (BugCacher, error)
+	Close() (BugCacher, error)
+	SetTitle(title string) (BugCacher, error)
+
+	Commit() (BugCacher, error)
 }
 
 // Cacher ------------------------
@@ -135,7 +135,7 @@ func (c *RepoCache) ResolveBug(id string) (BugCacher, error) {
 		return nil, err
 	}
 
-	cached = NewBugCache(b)
+	cached = NewBugCache(c.repo, b)
 	c.bugs[id] = cached
 
 	return cached, nil
@@ -168,7 +168,7 @@ func (c *RepoCache) ResolveBugPrefix(prefix string) (BugCacher, error) {
 		return nil, err
 	}
 
-	cached := NewBugCache(b)
+	cached := NewBugCache(c.repo, b)
 	c.bugs[b.Id()] = cached
 
 	return cached, nil
@@ -182,14 +182,6 @@ func (c *RepoCache) ClearAllBugs() {
 	c.bugs = make(map[string]BugCacher)
 }
 
-func (c *RepoCache) Commit(bug BugCacher) error {
-	err := bug.bug().Commit(c.repo)
-	if err != nil {
-		return err
-	}
-	return nil
-}
-
 func (c *RepoCache) NewBug(title string, message string) (BugCacher, error) {
 	author, err := bug.GetUser(c.repo)
 	if err != nil {
@@ -206,135 +198,116 @@ func (c *RepoCache) NewBug(title string, message string) (BugCacher, error) {
 		return nil, err
 	}
 
-	cached := NewBugCache(b)
+	cached := NewBugCache(c.repo, b)
 	c.bugs[b.Id()] = cached
 
 	return cached, nil
 }
 
-func (c *RepoCache) AddComment(repoRef *string, prefix string, message string) (BugCacher, error) {
-	author, err := bug.GetUser(c.repo)
-	if err != nil {
-		return nil, err
+// Bug ------------------------
+
+type BugCache struct {
+	repo repository.Repo
+	bug  *bug.Bug
+	snap *bug.Snapshot
+}
+
+func NewBugCache(repo repository.Repo, b *bug.Bug) BugCacher {
+	return &BugCache{
+		repo: repo,
+		bug:  b,
+	}
+}
+
+func (c *BugCache) Snapshot() *bug.Snapshot {
+	if c.snap == nil {
+		snap := c.bug.Compile()
+		c.snap = &snap
 	}
+	return c.snap
+}
 
-	cached, err := c.ResolveBugPrefix(prefix)
+func (c *BugCache) ClearSnapshot() {
+	c.snap = nil
+}
+
+func (c *BugCache) AddComment(message string) (BugCacher, error) {
+	author, err := bug.GetUser(c.repo)
 	if err != nil {
 		return nil, err
 	}
 
-	operations.Comment(cached.bug(), author, message)
+	operations.Comment(c.bug, author, message)
 
 	// TODO: perf --> the snapshot could simply be updated with the new op
-	cached.ClearSnapshot()
+	c.ClearSnapshot()
 
-	return cached, nil
+	return c, nil
 }
 
-func (c *RepoCache) ChangeLabels(repoRef *string, prefix string, added []string, removed []string) (BugCacher, error) {
+func (c *BugCache) ChangeLabels(added []string, removed []string) (BugCacher, error) {
 	author, err := bug.GetUser(c.repo)
 	if err != nil {
 		return nil, err
 	}
 
-	cached, err := c.ResolveBugPrefix(prefix)
-	if err != nil {
-		return nil, err
-	}
-
-	err = operations.ChangeLabels(nil, cached.bug(), author, added, removed)
+	err = operations.ChangeLabels(nil, c.bug, author, added, removed)
 	if err != nil {
 		return nil, err
 	}
 
 	// TODO: perf --> the snapshot could simply be updated with the new op
-	cached.ClearSnapshot()
+	c.ClearSnapshot()
 
-	return cached, nil
+	return c, nil
 }
 
-func (c *RepoCache) Open(repoRef *string, prefix string) (BugCacher, error) {
+func (c *BugCache) Open() (BugCacher, error) {
 	author, err := bug.GetUser(c.repo)
 	if err != nil {
 		return nil, err
 	}
 
-	cached, err := c.ResolveBugPrefix(prefix)
-	if err != nil {
-		return nil, err
-	}
-
-	operations.Open(cached.bug(), author)
+	operations.Open(c.bug, author)
 
 	// TODO: perf --> the snapshot could simply be updated with the new op
-	cached.ClearSnapshot()
+	c.ClearSnapshot()
 
-	return cached, nil
+	return c, nil
 }
 
-func (c *RepoCache) Close(repoRef *string, prefix string) (BugCacher, error) {
+func (c *BugCache) Close() (BugCacher, error) {
 	author, err := bug.GetUser(c.repo)
 	if err != nil {
 		return nil, err
 	}
 
-	cached, err := c.ResolveBugPrefix(prefix)
-	if err != nil {
-		return nil, err
-	}
-
-	operations.Close(cached.bug(), author)
+	operations.Close(c.bug, author)
 
 	// TODO: perf --> the snapshot could simply be updated with the new op
-	cached.ClearSnapshot()
+	c.ClearSnapshot()
 
-	return cached, nil
+	return c, nil
 }
 
-func (c *RepoCache) SetTitle(repoRef *string, prefix string, title string) (BugCacher, error) {
+func (c *BugCache) SetTitle(title string) (BugCacher, error) {
 	author, err := bug.GetUser(c.repo)
 	if err != nil {
 		return nil, err
 	}
 
-	cached, err := c.ResolveBugPrefix(prefix)
-	if err != nil {
-		return nil, err
-	}
-
-	operations.SetTitle(cached.bug(), author, title)
+	operations.SetTitle(c.bug, author, title)
 
 	// TODO: perf --> the snapshot could simply be updated with the new op
-	cached.ClearSnapshot()
-
-	return cached, nil
-}
-
-// Bug ------------------------
-
-type BugCache struct {
-	b    *bug.Bug
-	snap *bug.Snapshot
-}
+	c.ClearSnapshot()
 
-func NewBugCache(b *bug.Bug) BugCacher {
-	return &BugCache{
-		b: b,
-	}
+	return c, nil
 }
 
-func (c *BugCache) Snapshot() *bug.Snapshot {
-	if c.snap == nil {
-		snap := c.b.Compile()
-		c.snap = &snap
+func (c *BugCache) Commit() (BugCacher, error) {
+	err := c.bug.Commit(c.repo)
+	if err != nil {
+		return nil, err
 	}
-	return c.snap
-}
-
-func (c *BugCache) ClearSnapshot() {
-	c.snap = nil
-}
-
-func (c *BugCache) bug() *bug.Bug {
-	return c.b
+	return c, nil
 }

graphql/resolvers/mutation.go

@@ -46,7 +46,7 @@ func (r mutationResolver) Commit(ctx context.Context, repoRef *string, prefix st
 		return bug.Snapshot{}, err
 	}
 
-	err = repo.Commit(b)
+	b, err = b.Commit()
 	if err != nil {
 		return bug.Snapshot{}, err
 	}
@@ -62,7 +62,12 @@ func (r mutationResolver) AddComment(ctx context.Context, repoRef *string, prefi
 		return bug.Snapshot{}, err
 	}
 
-	b, err := repo.AddComment(repoRef, prefix, message)
+	b, err := repo.ResolveBugPrefix(prefix)
+	if err != nil {
+		return bug.Snapshot{}, err
+	}
+
+	b, err = b.AddComment(message)
 	if err != nil {
 		return bug.Snapshot{}, err
 	}
@@ -78,7 +83,12 @@ func (r mutationResolver) ChangeLabels(ctx context.Context, repoRef *string, pre
 		return bug.Snapshot{}, err
 	}
 
-	b, err := repo.ChangeLabels(repoRef, prefix, added, removed)
+	b, err := repo.ResolveBugPrefix(prefix)
+	if err != nil {
+		return bug.Snapshot{}, err
+	}
+
+	b, err = b.ChangeLabels(added, removed)
 	if err != nil {
 		return bug.Snapshot{}, err
 	}
@@ -94,7 +104,12 @@ func (r mutationResolver) Open(ctx context.Context, repoRef *string, prefix stri
 		return bug.Snapshot{}, err
 	}
 
-	b, err := repo.Open(repoRef, prefix)
+	b, err := repo.ResolveBugPrefix(prefix)
+	if err != nil {
+		return bug.Snapshot{}, err
+	}
+
+	b, err = b.Open()
 	if err != nil {
 		return bug.Snapshot{}, err
 	}
@@ -110,7 +125,12 @@ func (r mutationResolver) Close(ctx context.Context, repoRef *string, prefix str
 		return bug.Snapshot{}, err
 	}
 
-	b, err := repo.Close(repoRef, prefix)
+	b, err := repo.ResolveBugPrefix(prefix)
+	if err != nil {
+		return bug.Snapshot{}, err
+	}
+
+	b, err = b.Close()
 	if err != nil {
 		return bug.Snapshot{}, err
 	}
@@ -126,7 +146,12 @@ func (r mutationResolver) SetTitle(ctx context.Context, repoRef *string, prefix
 		return bug.Snapshot{}, err
 	}
 
-	b, err := repo.SetTitle(repoRef, prefix, title)
+	b, err := repo.ResolveBugPrefix(prefix)
+	if err != nil {
+		return bug.Snapshot{}, err
+	}
+
+	b, err = b.SetTitle(title)
 	if err != nil {
 		return bug.Snapshot{}, err
 	}

termui/error_popup.go

@@ -9,12 +9,12 @@ import (
 const errorPopupView = "errorPopupView"
 
 type errorPopup struct {
-	err string
+	message string
 }
 
 func newErrorPopup() *errorPopup {
 	return &errorPopup{
-		err: "",
+		message: "",
 	}
 }
 
@@ -30,14 +30,14 @@ func (ep *errorPopup) keybindings(g *gocui.Gui) error {
 }
 
 func (ep *errorPopup) layout(g *gocui.Gui) error {
-	if ep.err == "" {
+	if ep.message == "" {
 		return nil
 	}
 
 	maxX, maxY := g.Size()
 
 	width := minInt(30, maxX)
-	wrapped, nblines := word_wrap(ep.err, width-2)
+	wrapped, nblines := word_wrap(ep.message, width-2)
 	height := minInt(nblines+2, maxY)
 	x0 := (maxX - width) / 2
 	y0 := (maxY - height) / 2
@@ -49,6 +49,7 @@ func (ep *errorPopup) layout(g *gocui.Gui) error {
 		}
 
 		v.Frame = true
+		v.Title = "Error"
 
 		fmt.Fprintf(v, wrapped)
 	}
@@ -61,12 +62,12 @@ func (ep *errorPopup) layout(g *gocui.Gui) error {
 }
 
 func (ep *errorPopup) close(g *gocui.Gui, v *gocui.View) error {
-	ep.err = ""
+	ep.message = ""
 	return g.DeleteView(errorPopupView)
 }
 
-func (ep *errorPopup) isActive() bool {
-	return ep.err != ""
+func (ep *errorPopup) activate(message string) {
+	ep.message = message
 }
 
 func word_wrap(text string, lineWidth int) (string, int) {

termui/termui.go

@@ -156,7 +156,7 @@ func newBugWithEditor(g *gocui.Gui, v *gocui.View) error {
 	}
 
 	if err == input.ErrEmptyTitle {
-		ui.errorPopup.err = err.Error()
+		ui.errorPopup.activate("Empty title, aborting.")
 	} else {
 		_, err = ui.cache.NewBug(title, message)
 		if err != nil {