From 31ede573e7dde7552511331687533ac07314baf2 Mon Sep 17 00:00:00 2001 From: Jordan Krage Date: Sat, 23 Sep 2017 10:18:32 -0500 Subject: [PATCH] gps: source cache: add ok bool to singleSourceCache.getAllVersions() in order to avoid depending on len(0) vs nil distinction --- internal/gps/source.go | 6 ++++-- internal/gps/source_cache.go | 8 ++++---- internal/gps/source_cache_bolt.go | 8 ++++---- internal/gps/source_cache_bolt_test.go | 16 ++++++++-------- internal/gps/source_cache_multi.go | 16 ++++++++-------- internal/gps/source_cache_test.go | 8 ++++---- internal/gps/source_test.go | 4 ++-- 7 files changed, 34 insertions(+), 32 deletions(-) diff --git a/internal/gps/source.go b/internal/gps/source.go index 3ffab802..737676ad 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 cfc61083..0f285ec3 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) @@ -169,17 +169,17 @@ func (c *singleSourceCacheMemory) getVersionsFor(r Revision) ([]UnpairedVersion, return versionList, has } -func (c *singleSourceCacheMemory) getAllVersions() []PairedVersion { +func (c *singleSourceCacheMemory) getAllVersions() ([]PairedVersion, bool) { c.mut.Lock() vList := c.vList c.mut.Unlock() if vList == nil { - return nil + return nil, false } cp := make([]PairedVersion, len(vList)) copy(cp, vList) - return cp + 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 bf5c82d1..f2bae197 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 d12cbf96..d2d2d167 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 5ce9108d..01ade0ff 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 d9f2dd39..7331b439 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 66146128..639bec5d 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) }