cache: generic withSnapshot, some cleanup

Michael Muré created

Change summary

api/graphql/graphql_test.go        |  5 ++
cache/bug_cache.go                 |  2 
cache/cached.go                    | 47 --------------------------
cache/repo_cache_test.go           |  2 
cache/subcache.go                  |  7 ++-
cache/with_snapshot.go             | 56 ++++++++++++++++++++++++++++++++
commands/execenv/env.go            |  8 ++--
commands/webui.go                  | 16 ++++++++
entities/bug/snapshot.go           |  4 ++
entities/bug/with_snapshot.go      | 53 ------------------------------
entity/dag/op_set_metadata_test.go |  6 +++
entity/dag/operation.go            |  2 +
12 files changed, 98 insertions(+), 110 deletions(-)

Detailed changes

api/graphql/graphql_test.go 🔗

@@ -19,8 +19,11 @@ func TestQueries(t *testing.T) {
 	random_bugs.FillRepoWithSeed(repo, 10, 42)
 
 	mrc := cache.NewMultiRepoCache()
-	_, _, err := mrc.RegisterDefaultRepository(repo)
+	_, events, err := mrc.RegisterDefaultRepository(repo)
 	require.NoError(t, err)
+	for event := range events {
+		require.NoError(t, event.Err)
+	}
 
 	handler := NewHandler(mrc, nil)
 

cache/bug_cache.go 🔗

@@ -28,7 +28,7 @@ func NewBugCache(b *bug.Bug, repo repository.ClockedRepo, getUserIdentity getUse
 			repo:            repo,
 			entityUpdated:   entityUpdated,
 			getUserIdentity: getUserIdentity,
-			entity:          &bug.WithSnapshot{Bug: b},
+			entity:          &withSnapshot[*bug.Snapshot, bug.Operation]{Interface: b},
 		},
 	}
 }

cache/cached.go 🔗

@@ -9,52 +9,7 @@ import (
 	"github.com/MichaelMure/git-bug/util/lamport"
 )
 
-// type withSnapshot[SnapT dag.Snapshot, OpT dag.OperationWithApply[SnapT]] struct {
-// 	dag.Interface[SnapT, OpT]
-// 	snap dag.Snapshot
-// }
-//
-//
-// func (ws *withSnapshot[SnapT, OpT]) Compile() dag.Snapshot {
-// 	if ws.snap == nil {
-// 		snap := ws.Interface.Compile()
-// 		ws.snap = snap
-// 	}
-// 	return ws.snap
-// }
-//
-// // Append intercept Bug.Append() to update the snapshot efficiently
-// func (ws *withSnapshot[SnapT, OpT]) Append(op OpT) {
-// 	ws.Interface.Append(op)
-//
-// 	if ws.snap == nil {
-// 		return
-// 	}
-//
-// 	op.Apply(ws.snap)
-// 	ws.snap. = append(ws.snap.Operations, op)
-// }
-//
-// // Commit intercept Bug.Commit() to update the snapshot efficiently
-// func (ws *withSnapshot[SnapT, OpT]) Commit(repo repository.ClockedRepo) error {
-// 	err := ws.Interface.Commit(repo)
-//
-// 	if err != nil {
-// 		ws.snap = nil
-// 		return err
-// 	}
-//
-// 	// Commit() shouldn't change anything of the bug state apart from the
-// 	// initial ID set
-//
-// 	if ws.snap == nil {
-// 		return nil
-// 	}
-//
-// 	ws.snap.id = ws.Interface.Id()
-// 	return nil
-// }
-
+// CachedEntityBase provide the base function of an entity managed by the cache.
 type CachedEntityBase[SnapT dag.Snapshot, OpT dag.Operation] struct {
 	repo            repository.ClockedRepo
 	entityUpdated   func(id entity.Id) error

cache/repo_cache_test.go 🔗

@@ -98,7 +98,7 @@ func TestCache(t *testing.T) {
 	cache, events, err = NewRepoCache(repo)
 	noBuildEventErrors(t, events)
 	require.NoError(t, err)
-	require.Empty(t, cache.bugs.cached)
+	require.Len(t, cache.bugs.cached, 0)
 	require.Len(t, cache.bugs.excerpts, 2)
 	require.Len(t, cache.identities.cached, 0)
 	require.Len(t, cache.identities.excerpts, 2)

cache/subcache.go 🔗

@@ -198,9 +198,10 @@ func (sc *SubCache[EntityT, ExcerptT, CacheT]) Build() error {
 			return e.Err
 		}
 
-		// TODO: doesn't actually record in cache, should we?
 		cached := sc.makeCached(e.Entity, sc.entityUpdated)
 		sc.excerpts[e.Entity.Id()] = sc.makeExcerpt(cached)
+		// might as well keep them in memory
+		sc.cached[e.Entity.Id()] = cached
 
 		indexData := sc.makeIndexData(cached)
 		if err := indexer(e.Entity.Id().String(), indexData); err != nil {
@@ -417,12 +418,12 @@ func (sc *SubCache[EntityT, ExcerptT, CacheT]) MergeAll(remote string) <-chan en
 			switch result.Status {
 			case entity.MergeStatusNew, entity.MergeStatusUpdated:
 				e := result.Entity.(EntityT)
-
-				// TODO: doesn't actually record in cache, should we?
 				cached := sc.makeCached(e, sc.entityUpdated)
 
 				sc.mu.Lock()
 				sc.excerpts[result.Id] = sc.makeExcerpt(cached)
+				// might as well keep them in memory
+				sc.cached[result.Id] = cached
 				sc.mu.Unlock()
 			}
 		}

cache/with_snapshot.go 🔗

@@ -0,0 +1,56 @@
+package cache
+
+import (
+	"sync"
+
+	"github.com/MichaelMure/git-bug/entity/dag"
+	"github.com/MichaelMure/git-bug/repository"
+)
+
+var _ dag.Interface[dag.Snapshot, dag.OperationWithApply[dag.Snapshot]] = &withSnapshot[dag.Snapshot, dag.OperationWithApply[dag.Snapshot]]{}
+
+// withSnapshot encapsulate an entity and maintain a snapshot efficiently.
+type withSnapshot[SnapT dag.Snapshot, OpT dag.OperationWithApply[SnapT]] struct {
+	dag.Interface[SnapT, OpT]
+	mu   sync.Mutex
+	snap *SnapT
+}
+
+func (ws *withSnapshot[SnapT, OpT]) Compile() SnapT {
+	ws.mu.Lock()
+	defer ws.mu.Unlock()
+	if ws.snap == nil {
+		snap := ws.Interface.Compile()
+		ws.snap = &snap
+	}
+	return *ws.snap
+}
+
+// Append intercept Bug.Append() to update the snapshot efficiently
+func (ws *withSnapshot[SnapT, OpT]) Append(op OpT) {
+	ws.mu.Lock()
+	defer ws.mu.Unlock()
+
+	ws.Interface.Append(op)
+
+	if ws.snap == nil {
+		return
+	}
+
+	op.Apply(*ws.snap)
+	(*ws.snap).AppendOperation(op)
+}
+
+// Commit intercept Bug.Commit() to update the snapshot efficiently
+func (ws *withSnapshot[SnapT, OpT]) Commit(repo repository.ClockedRepo) error {
+	ws.mu.Lock()
+	defer ws.mu.Unlock()
+
+	err := ws.Interface.Commit(repo)
+	if err != nil {
+		ws.snap = nil
+		return err
+	}
+
+	return nil
+}

commands/execenv/env.go 🔗

@@ -135,20 +135,20 @@ func LoadBackend(env *Env) func(*cobra.Command, []string) error {
 		}
 
 		if events != nil {
-			_, _ = fmt.Fprintln(os.Stderr, "Building cache... ")
+			env.Err.Println("Building cache... ")
 		}
 
 		for event := range events {
 			if event.Err != nil {
-				_, _ = fmt.Fprintf(os.Stderr, "Cache building error [%s]: %v\n", event.Typename, event.Err)
+				env.Err.Printf("Cache building error [%s]: %v\n", event.Typename, event.Err)
 				continue
 			}
 
 			switch event.Event {
 			case cache.BuildEventStarted:
-				_, _ = fmt.Fprintf(os.Stderr, "[%s] started\n", event.Typename)
+				env.Err.Printf("[%s] started\n", event.Typename)
 			case cache.BuildEventFinished:
-				_, _ = fmt.Fprintf(os.Stderr, "[%s] done\n", event.Typename)
+				env.Err.Printf("[%s] done\n", event.Typename)
 			}
 		}
 

commands/webui.go 🔗

@@ -105,11 +105,25 @@ func runWebUI(env *execenv.Env, opts webUIOptions) error {
 	}
 
 	mrc := cache.NewMultiRepoCache()
-	_, _, err := mrc.RegisterDefaultRepository(env.Repo)
+	_, events, err := mrc.RegisterDefaultRepository(env.Repo)
 	if err != nil {
 		return err
 	}
 
+	for event := range events {
+		if event.Err != nil {
+			env.Err.Printf("Cache building error [%s]: %v\n", event.Typename, event.Err)
+			continue
+		}
+
+		switch event.Event {
+		case cache.BuildEventStarted:
+			env.Err.Printf("[%s] started\n", event.Typename)
+		case cache.BuildEventFinished:
+			env.Err.Printf("[%s] done\n", event.Typename)
+		}
+	}
+
 	var errOut io.Writer
 	if opts.logErrors {
 		errOut = env.Err

entities/bug/snapshot.go 🔗

@@ -43,6 +43,10 @@ func (snap *Snapshot) AllOperations() []dag.Operation {
 	return snap.Operations
 }
 
+func (snap *Snapshot) AppendOperation(op dag.Operation) {
+	snap.Operations = append(snap.Operations, op)
+}
+
 // EditTime returns the last time a bug was modified
 func (snap *Snapshot) EditTime() time.Time {
 	if len(snap.Operations) == 0 {

entities/bug/with_snapshot.go 🔗

@@ -1,53 +0,0 @@
-package bug
-
-import (
-	"github.com/MichaelMure/git-bug/repository"
-)
-
-var _ Interface = &WithSnapshot{}
-
-// WithSnapshot encapsulate a Bug and maintain the corresponding Snapshot efficiently
-type WithSnapshot struct {
-	*Bug
-	snap *Snapshot
-}
-
-func (b *WithSnapshot) Compile() *Snapshot {
-	if b.snap == nil {
-		snap := b.Bug.Compile()
-		b.snap = snap
-	}
-	return b.snap
-}
-
-// Append intercept Bug.Append() to update the snapshot efficiently
-func (b *WithSnapshot) Append(op Operation) {
-	b.Bug.Append(op)
-
-	if b.snap == nil {
-		return
-	}
-
-	op.Apply(b.snap)
-	b.snap.Operations = append(b.snap.Operations, op)
-}
-
-// Commit intercept Bug.Commit() to update the snapshot efficiently
-func (b *WithSnapshot) Commit(repo repository.ClockedRepo) error {
-	err := b.Bug.Commit(repo)
-
-	if err != nil {
-		b.snap = nil
-		return err
-	}
-
-	// Commit() shouldn't change anything of the bug state apart from the
-	// initial ID set
-
-	if b.snap == nil {
-		return nil
-	}
-
-	b.snap.id = b.Bug.Id()
-	return nil
-}

entity/dag/op_set_metadata_test.go 🔗

@@ -12,6 +12,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+var _ Snapshot = &snapshotMock{}
+
 type snapshotMock struct {
 	ops []Operation
 }
@@ -20,6 +22,10 @@ func (s *snapshotMock) AllOperations() []Operation {
 	return s.ops
 }
 
+func (s *snapshotMock) AppendOperation(op Operation) {
+	s.ops = append(s.ops, op)
+}
+
 func TestSetMetadata(t *testing.T) {
 	snap := &snapshotMock{}
 

entity/dag/operation.go 🔗

@@ -90,6 +90,8 @@ type OperationDoesntChangeSnapshot interface {
 type Snapshot interface {
 	// AllOperations returns all the operations that have been applied to that snapshot, in order
 	AllOperations() []Operation
+	// AppendOperation add an operation in the list
+	AppendOperation(op Operation)
 }
 
 // OpBase implement the common feature that every Operation should support.