Merge pull request #628 from MichaelMure/entity-fix

Michael Muré created 4 years ago

cache: many fixes following the dag entity migration

Change summary

bug/operation.go         |  2 ++
cache/repo_cache.go      |  2 +-
cache/repo_cache_test.go |  5 +++--
cache/resolvers.go       | 29 +++++++++++++++++++++++++++++
entity/dag/clock.go      |  7 ++++---
identity/resolver.go     | 35 +++++++++++++++++++++++++++++++++++
repository/gogit.go      |  2 +-
7 files changed, 75 insertions(+), 7 deletions(-)

Detailed changes

bug/operation.go

@@ -113,6 +113,8 @@ func operationUnmarshaller(author identity.Interface, raw json.RawMessage) (dag.
 		op.Author_ = author
 	case *CreateOperation:
 		op.Author_ = author
+	case *EditCommentOperation:
+		op.Author_ = author
 	case *LabelChangeOperation:
 		op.Author_ = author
 	case *NoOpOperation:

cache/repo_cache.go

@@ -195,7 +195,7 @@ func (c *RepoCache) buildCache() error {
 
 	c.bugExcerpts = make(map[entity.Id]*BugExcerpt)
 
-	allBugs := bug.ReadAll(c.repo)
+	allBugs := bug.ReadAllWithResolver(c.repo, newIdentityCacheResolverNoLock(c))
 
 	// wipe the index just to be sure
 	err := c.repo.ClearBleveIndex("bug")

cache/repo_cache_test.go

@@ -86,11 +86,12 @@ func TestCache(t *testing.T) {
 	require.Empty(t, cache.identities)
 	require.Empty(t, cache.identitiesExcerpts)
 
-	// Reload, only excerpt are loaded
+	// Reload, only excerpt are loaded, but as we need to load the identities used in the bugs
+	// to check the signatures, we also load the identity used above
 	cache, err = NewRepoCache(repo)
 	require.NoError(t, err)
 	require.Empty(t, cache.bugs)
-	require.Empty(t, cache.identities)
+	require.Len(t, cache.identities, 1)
 	require.Len(t, cache.bugExcerpts, 2)
 	require.Len(t, cache.identitiesExcerpts, 2)
 

cache/resolvers.go

@@ -20,3 +20,32 @@ func newIdentityCacheResolver(cache *RepoCache) *identityCacheResolver {
 func (i *identityCacheResolver) ResolveIdentity(id entity.Id) (identity.Interface, error) {
 	return i.cache.ResolveIdentity(id)
 }
+
+var _ identity.Resolver = &identityCacheResolverNoLock{}
+
+// identityCacheResolverNoLock is an identity Resolver that retrieve identities from
+// the cache, without locking it.
+type identityCacheResolverNoLock struct {
+	cache *RepoCache
+}
+
+func newIdentityCacheResolverNoLock(cache *RepoCache) *identityCacheResolverNoLock {
+	return &identityCacheResolverNoLock{cache: cache}
+}
+
+func (ir *identityCacheResolverNoLock) ResolveIdentity(id entity.Id) (identity.Interface, error) {
+	cached, ok := ir.cache.identities[id]
+	if ok {
+		return cached, nil
+	}
+
+	i, err := identity.ReadLocal(ir.cache.repo, id)
+	if err != nil {
+		return nil, err
+	}
+
+	cached = NewIdentityCache(ir.cache, i)
+	ir.cache.identities[id] = cached
+
+	return cached, nil
+}

entity/dag/clock.go

@@ -9,7 +9,7 @@ import (
 
 // ClockLoader is the repository.ClockLoader for Entity
 func ClockLoader(defs ...Definition) repository.ClockLoader {
-	clocks := make([]string, len(defs)*2)
+	clocks := make([]string, 0, len(defs)*2)
 	for _, def := range defs {
 		clocks = append(clocks, fmt.Sprintf(creationClockPattern, def.Namespace))
 		clocks = append(clocks, fmt.Sprintf(editClockPattern, def.Namespace))
@@ -18,8 +18,9 @@ func ClockLoader(defs ...Definition) repository.ClockLoader {
 	return repository.ClockLoader{
 		Clocks: clocks,
 		Witnesser: func(repo repository.ClockedRepo) error {
-			// We don't care about the actual identity so an IdentityStub will do
-			resolver := identity.NewStubResolver()
+			// we need to actually load the identities because of the commit signature check when reading,
+			// which require the full identities with crypto keys
+			resolver := identity.NewCachedResolver(identity.NewSimpleResolver(repo))
 
 			for _, def := range defs {
 				// we actually just need to read all entities,

identity/resolver.go

@@ -1,6 +1,8 @@
 package identity
 
 import (
+	"sync"
+
 	"github.com/MichaelMure/git-bug/entity"
 	"github.com/MichaelMure/git-bug/repository"
 )
@@ -34,3 +36,36 @@ func NewStubResolver() *StubResolver {
 func (s *StubResolver) ResolveIdentity(id entity.Id) (Interface, error) {
 	return &IdentityStub{id: id}, nil
 }
+
+// CachedResolver is a resolver ensuring that loading is done only once through another Resolver.
+type CachedResolver struct {
+	mu         sync.RWMutex
+	resolver   Resolver
+	identities map[entity.Id]Interface
+}
+
+func NewCachedResolver(resolver Resolver) *CachedResolver {
+	return &CachedResolver{
+		resolver:   resolver,
+		identities: make(map[entity.Id]Interface),
+	}
+}
+
+func (c *CachedResolver) ResolveIdentity(id entity.Id) (Interface, error) {
+	c.mu.RLock()
+	if i, ok := c.identities[id]; ok {
+		c.mu.RUnlock()
+		return i, nil
+	}
+	c.mu.RUnlock()
+
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	i, err := c.resolver.ResolveIdentity(id)
+	if err != nil {
+		return nil, err
+	}
+	c.identities[id] = i
+	return i, nil
+}

repository/gogit.go

@@ -335,7 +335,7 @@ func (repo *GoGitRepo) ClearBleveIndex(name string) error {
 	repo.indexesMutex.Lock()
 	defer repo.indexesMutex.Unlock()
 
-	path := filepath.Join(repo.path, "indexes", name)
+	path := filepath.Join(repo.path, "git-bug", "indexes", name)
 
 	err := os.RemoveAll(path)
 	if err != nil {