Merge pull request #961 from MichaelMure/cache-events

Michael Muré created 2 years ago

cache: simplify cache building events handling

Change summary

api/graphql/graphql_test.go        |  5 +--
api/http/git_file_handlers_test.go |  3 -
cache/multi_repo_cache.go          | 37 ++++++++++++++----------
cache/repo_cache.go                | 47 ++++++++++++++++---------------
commands/execenv/env.go            |  8 +----
commands/webui.go                  |  5 --
6 files changed, 51 insertions(+), 54 deletions(-)

Detailed changes

api/graphql/graphql_test.go

@@ -19,8 +19,7 @@ func TestQueries(t *testing.T) {
 	random_bugs.FillRepoWithSeed(repo, 10, 42)
 
 	mrc := cache.NewMultiRepoCache()
-	_, events, err := mrc.RegisterDefaultRepository(repo)
-	require.NoError(t, err)
+	_, events := mrc.RegisterDefaultRepository(repo)
 	for event := range events {
 		require.NoError(t, event.Err)
 	}
@@ -217,6 +216,6 @@ func TestQueries(t *testing.T) {
 		}
 	}
 
-	err = c.Post(query, &resp)
+	err := c.Post(query, &resp)
 	assert.NoError(t, err)
 }

api/http/git_file_handlers_test.go

@@ -22,8 +22,7 @@ func TestGitFileHandlers(t *testing.T) {
 	repo := repository.CreateGoGitTestRepo(t, false)
 
 	mrc := cache.NewMultiRepoCache()
-	repoCache, events, err := mrc.RegisterDefaultRepository(repo)
-	require.NoError(t, err)
+	repoCache, events := mrc.RegisterDefaultRepository(repo)
 	for event := range events {
 		require.NoError(t, event.Err)
 	}

cache/multi_repo_cache.go

@@ -21,25 +21,30 @@ func NewMultiRepoCache() *MultiRepoCache {
 }
 
 // RegisterRepository register a named repository. Use this for multi-repo setup
-func (c *MultiRepoCache) RegisterRepository(name string, repo repository.ClockedRepo) (*RepoCache, chan BuildEvent, error) {
-	r, events, err := NewNamedRepoCache(repo, name)
-	if err != nil {
-		return nil, nil, err
-	}
+func (c *MultiRepoCache) RegisterRepository(repo repository.ClockedRepo, name string) (*RepoCache, chan BuildEvent) {
+	r, events := NewNamedRepoCache(repo, name)
 
-	c.repos[name] = r
-	return r, events, nil
-}
+	// intercept events to make sure the cache building process succeed properly
+	out := make(chan BuildEvent)
+	go func() {
+		defer close(out)
 
-// RegisterDefaultRepository register an unnamed repository. Use this for mono-repo setup
-func (c *MultiRepoCache) RegisterDefaultRepository(repo repository.ClockedRepo) (*RepoCache, chan BuildEvent, error) {
-	r, events, err := NewRepoCache(repo)
-	if err != nil {
-		return nil, nil, err
-	}
+		for event := range events {
+			out <- event
+			if event.Err != nil {
+				return
+			}
+		}
+
+		c.repos[name] = r
+	}()
+
+	return r, out
+}
 
-	c.repos[defaultRepoName] = r
-	return r, events, nil
+// RegisterDefaultRepository register an unnamed repository. Use this for single-repo setup
+func (c *MultiRepoCache) RegisterDefaultRepository(repo repository.ClockedRepo) (*RepoCache, chan BuildEvent) {
+	return c.RegisterRepository(repo, defaultRepoName)
 }
 
 // DefaultRepo retrieve the default repository

cache/repo_cache.go

@@ -72,17 +72,17 @@ type RepoCache struct {
 	userIdentityId entity.Id
 }
 
-// NewRepoCache create or open an unnamed (aka default) cache on top of a raw repository.
-// If the returned BuildEvent channel is not nil, the caller is expected to read all events before the cache is considered
+// NewRepoCache create or open a cache on top of a raw repository.
+// The caller is expected to read all returned events before the cache is considered
 // ready to use.
-func NewRepoCache(r repository.ClockedRepo) (*RepoCache, chan BuildEvent, error) {
+func NewRepoCache(r repository.ClockedRepo) (*RepoCache, chan BuildEvent) {
 	return NewNamedRepoCache(r, defaultRepoName)
 }
 
 // NewNamedRepoCache create or open a named cache on top of a raw repository.
-// If the returned BuildEvent channel is not nil, the caller is expected to read all events before the cache is considered
+// The caller is expected to read all returned events before the cache is considered
 // ready to use.
-func NewNamedRepoCache(r repository.ClockedRepo, name string) (*RepoCache, chan BuildEvent, error) {
+func NewNamedRepoCache(r repository.ClockedRepo, name string) (*RepoCache, chan BuildEvent) {
 	c := &RepoCache{
 		repo: r,
 		name: name,
@@ -102,35 +102,36 @@ func NewNamedRepoCache(r repository.ClockedRepo, name string) (*RepoCache, chan
 	}
 
 	// small buffer so that below functions can emit an event without blocking
-	events := make(chan BuildEvent, 10)
-	defer close(events)
+	events := make(chan BuildEvent)
 
-	err := c.lock(events)
-	if err != nil {
-		return &RepoCache{}, events, err
-	}
+	go func() {
+		defer close(events)
 
-	err = c.load()
-	if err == nil {
-		return c, events, nil
-	}
+		err := c.lock(events)
+		if err != nil {
+			events <- BuildEvent{Err: err}
+			return
+		}
+
+		err = c.load()
+		if err == nil {
+			return
+		}
 
-	// Cache is either missing, broken or outdated. Rebuilding.
-	c.buildCache(events)
+		// Cache is either missing, broken or outdated. Rebuilding.
+		c.buildCache(events)
+	}()
 
-	return c, events, nil
+	return c, events
 }
 
 func NewRepoCacheNoEvents(r repository.ClockedRepo) (*RepoCache, error) {
-	cache, events, err := NewRepoCache(r)
-	if err != nil {
-		return nil, err
-	}
+	cache, events := NewRepoCache(r)
 	for event := range events {
 		if event.Err != nil {
 			for range events {
 			}
-			return nil, err
+			return nil, event.Err
 		}
 	}
 	return cache, nil

commands/execenv/env.go

@@ -129,15 +129,11 @@ func LoadBackend(env *Env) func(*cobra.Command, []string) error {
 		}
 
 		var events chan cache.BuildEvent
-		env.Backend, events, err = cache.NewRepoCache(env.Repo)
-		if err != nil {
-			return err
-		}
+		env.Backend, events = cache.NewRepoCache(env.Repo)
 
 		for event := range events {
 			if event.Err != nil {
-				env.Err.Printf("Cache building error [%s]: %v\n", event.Typename, event.Err)
-				continue
+				return event.Err
 			}
 			switch event.Event {
 			case cache.BuildEventCacheIsBuilt:

commands/webui.go

@@ -106,11 +106,8 @@ func runWebUI(env *execenv.Env, opts webUIOptions) error {
 	}
 
 	mrc := cache.NewMultiRepoCache()
-	_, events, err := mrc.RegisterDefaultRepository(env.Repo)
-	if err != nil {
-		return err
-	}
 
+	_, events := mrc.RegisterDefaultRepository(env.Repo)
 	for event := range events {
 		if event.Err != nil {
 			env.Err.Printf("Cache building error [%s]: %v\n", event.Typename, event.Err)