diff --git a/internal/gps/source.go b/internal/gps/source.go index 3ffab80217..737676adf4 100644 --- a/internal/gps/source.go +++ b/internal/gps/source.go @@ -494,8 +494,10 @@ func (sg *sourceGateway) listVersions(ctx context.Context) ([]PairedVersion, err if err != nil { return nil, err } - - return sg.cache.getAllVersions(), nil + if pvs, ok := sg.cache.getAllVersions(); ok { + return pvs, nil + } + return nil, nil } func (sg *sourceGateway) revisionPresentIn(ctx context.Context, r Revision) (bool, error) { diff --git a/internal/gps/source_cache.go b/internal/gps/source_cache.go index 5724c613fa..0f285ec341 100644 --- a/internal/gps/source_cache.go +++ b/internal/gps/source_cache.go @@ -43,7 +43,7 @@ type singleSourceCache interface { getVersionsFor(Revision) ([]UnpairedVersion, bool) // Gets all the version pairs currently known to the cache. - getAllVersions() []PairedVersion + getAllVersions() ([]PairedVersion, bool) // Get the revision corresponding to the given unpaired version. getRevisionFor(UnpairedVersion) (Revision, bool) @@ -61,9 +61,10 @@ type singleSourceCache interface { } type singleSourceCacheMemory struct { - mut sync.RWMutex // protects all maps + mut sync.RWMutex // protects all fields infos map[ProjectAnalyzerInfo]map[Revision]projectInfo ptrees map[Revision]pkgtree.PackageTree + vList []PairedVersion // replaced, never modified vMap map[UnpairedVersion]Revision rMap map[Revision][]UnpairedVersion } @@ -136,13 +137,14 @@ func (c *singleSourceCacheMemory) getPackageTree(r Revision) (pkgtree.PackageTre func (c *singleSourceCacheMemory) setVersionMap(versionList []PairedVersion) { c.mut.Lock() + c.vList = versionList // TODO(sdboyer) how do we handle cache consistency here - revs that may // be out of date vis-a-vis the ptrees or infos maps? for r := range c.rMap { c.rMap[r] = nil } - c.vMap = make(map[UnpairedVersion]Revision) + c.vMap = make(map[UnpairedVersion]Revision, len(versionList)) for _, pv := range versionList { u, r := pv.Unpair(), pv.Revision() @@ -167,15 +169,17 @@ func (c *singleSourceCacheMemory) getVersionsFor(r Revision) ([]UnpairedVersion, return versionList, has } -func (c *singleSourceCacheMemory) getAllVersions() []PairedVersion { - if len(c.vMap) == 0 { - return nil - } - vlist := make([]PairedVersion, 0, len(c.vMap)) - for v, r := range c.vMap { - vlist = append(vlist, v.Pair(r)) +func (c *singleSourceCacheMemory) getAllVersions() ([]PairedVersion, bool) { + c.mut.Lock() + vList := c.vList + c.mut.Unlock() + + if vList == nil { + return nil, false } - return vlist + cp := make([]PairedVersion, len(vList)) + copy(cp, vList) + return cp, true } func (c *singleSourceCacheMemory) getRevisionFor(uv UnpairedVersion) (Revision, bool) { diff --git a/internal/gps/source_cache_bolt.go b/internal/gps/source_cache_bolt.go index bf5c82d1dc..f2bae19738 100644 --- a/internal/gps/source_cache_bolt.go +++ b/internal/gps/source_cache_bolt.go @@ -351,8 +351,7 @@ func (s *singleSourceCacheBolt) getVersionsFor(rev Revision) (uvs []UnpairedVers return } -func (s *singleSourceCacheBolt) getAllVersions() []PairedVersion { - var pvs []PairedVersion +func (s *singleSourceCacheBolt) getAllVersions() (pvs []PairedVersion, ok bool) { err := s.viewSourceBucket(func(src *bolt.Bucket) error { versions := cacheFindLatestValid(src, cacheVersion, s.epoch) if versions == nil { @@ -369,14 +368,15 @@ func (s *singleSourceCacheBolt) getAllVersions() []PairedVersion { return err } pvs = append(pvs, uv.Pair(Revision(v))) + ok = true return nil }) }) if err != nil { s.logger.Println(errors.Wrap(err, "failed to get all cached versions")) - return nil + return nil, false } - return pvs + return } func (s *singleSourceCacheBolt) getRevisionFor(uv UnpairedVersion) (rev Revision, ok bool) { diff --git a/internal/gps/source_cache_bolt_test.go b/internal/gps/source_cache_bolt_test.go index d12cbf960e..d2d2d16710 100644 --- a/internal/gps/source_cache_bolt_test.go +++ b/internal/gps/source_cache_bolt_test.go @@ -119,8 +119,8 @@ func TestBoltCacheTimeout(t *testing.T) { } comparePackageTree(t, ptree, got) - gotV := c.getAllVersions() - if len(gotV) != len(pvs) { + gotV, ok := c.getAllVersions() + if !ok || len(gotV) != len(pvs) { t.Errorf("unexpected versions:\n\t(GOT): %#v\n\t(WNT): %#v", gotV, pvs) } else { SortPairedForDowngrade(gotV) @@ -161,8 +161,8 @@ func TestBoltCacheTimeout(t *testing.T) { } comparePackageTree(t, ptree, gotPtree) - pvs := c.getAllVersions() - if len(pvs) > 0 { + pvs, ok := c.getAllVersions() + if ok || len(pvs) > 0 { t.Errorf("expected no cached versions, but got:\n\t%#v", pvs) } } @@ -194,8 +194,8 @@ func TestBoltCacheTimeout(t *testing.T) { } comparePackageTree(t, ptree, got) - gotV := c.getAllVersions() - if len(gotV) != len(pvs) { + gotV, ok := c.getAllVersions() + if !ok || len(gotV) != len(pvs) { t.Errorf("unexpected versions:\n\t(GOT): %#v\n\t(WNT): %#v", gotV, pvs) } else { SortPairedForDowngrade(gotV) @@ -282,8 +282,8 @@ func TestBoltCacheTimeout(t *testing.T) { } comparePackageTree(t, newPtree, got) - gotV := c.getAllVersions() - if len(gotV) != len(newPVS) { + gotV, ok := c.getAllVersions() + if !ok || len(gotV) != len(newPVS) { t.Errorf("unexpected versions:\n\t(GOT): %#v\n\t(WNT): %#v", gotV, newPVS) } else { SortPairedForDowngrade(gotV) diff --git a/internal/gps/source_cache_multi.go b/internal/gps/source_cache_multi.go index 5ce9108d0f..01ade0ff8a 100644 --- a/internal/gps/source_cache_multi.go +++ b/internal/gps/source_cache_multi.go @@ -77,19 +77,19 @@ func (c *multiCache) getVersionsFor(rev Revision) ([]UnpairedVersion, bool) { return c.disk.getVersionsFor(rev) } -func (c *multiCache) getAllVersions() []PairedVersion { - pvs := c.mem.getAllVersions() - if pvs != nil { - return pvs +func (c *multiCache) getAllVersions() ([]PairedVersion, bool) { + pvs, ok := c.mem.getAllVersions() + if ok { + return pvs, true } - pvs = c.disk.getAllVersions() - if pvs != nil { + pvs, ok = c.disk.getAllVersions() + if ok { c.mem.setVersionMap(pvs) - return pvs + return pvs, true } - return nil + return nil, false } func (c *multiCache) getRevisionFor(uv UnpairedVersion) (Revision, bool) { diff --git a/internal/gps/source_cache_test.go b/internal/gps/source_cache_test.go index d9f2dd3982..7331b4396a 100644 --- a/internal/gps/source_cache_test.go +++ b/internal/gps/source_cache_test.go @@ -305,8 +305,8 @@ func (test singleSourceCacheTest) run(t *testing.T) { } t.Run("getAllVersions", func(t *testing.T) { - got := c.getAllVersions() - if len(got) != len(versions) { + got, ok := c.getAllVersions() + if !ok || len(got) != len(versions) { t.Errorf("unexpected versions:\n\t(GOT): %#v\n\t(WNT): %#v", got, versions) } else { SortPairedForDowngrade(got) @@ -566,8 +566,8 @@ func (discardCache) getVersionsFor(Revision) ([]UnpairedVersion, bool) { return nil, false } -func (discardCache) getAllVersions() []PairedVersion { - return nil +func (discardCache) getAllVersions() ([]PairedVersion, bool) { + return nil, false } func (discardCache) getRevisionFor(UnpairedVersion) (Revision, bool) { diff --git a/internal/gps/source_test.go b/internal/gps/source_test.go index 6614612840..639bec5dba 100644 --- a/internal/gps/source_test.go +++ b/internal/gps/source_test.go @@ -60,8 +60,8 @@ func testSourceGateway(t *testing.T) { t.Fatalf("error on cloning git repo: %s", err) } - cvlist := sg.cache.getAllVersions() - if len(cvlist) != 4 { + cvlist, ok := sg.cache.getAllVersions() + if !ok || len(cvlist) != 4 { t.Fatalf("repo setup should've cached four versions, got %v: %s", len(cvlist), cvlist) }