Fix githubMgr.Find: hit cache when find the specified file (anz-bank#63)

* only return mod when the specified file is found
* show instruction when the importing repo is private
* only re-download file when there are new commits pushed to ref @ver
* refine tests

go.mod

@@ -3,11 +3,11 @@ module github.com/anz-bank/pkg
 go 1.13
 
 require (
+	bou.ke/monkey v1.0.2
 	contrib.go.opencensus.io/exporter/prometheus v0.2.0
 	github.com/alecthomas/assert v0.0.0-20170929043011-405dbfeb8e38
 	github.com/alecthomas/colour v0.1.0 // indirect
 	github.com/alecthomas/repr v0.0.0-20181024024818-d37bc2a10ba1 // indirect
-	github.com/anz-bank/sysl-examples v0.0.1 // indirect
 	github.com/arr-ai/frozen v0.11.1
 	github.com/golang/protobuf v1.4.0
 	github.com/google/go-github/v32 v32.1.0

mod/github.go

@@ -3,13 +3,15 @@ package mod
 import (
 	"context"
 	"fmt"
+	"net/http"
 	"os"
 	"path"
 	"path/filepath"
 	"strings"
 
 	"github.com/google/go-github/v32/github"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 	"golang.org/x/oauth2"
 )
 
@@ -38,22 +40,36 @@ func (d *githubMgr) Init(cacheDir, accessToken *string) error {
 	return nil
 }
 
+type NotFoundError struct {
+	Message string
+}
+
+func (e *NotFoundError) Error() string {
+	return e.Message
+}
+
+type RateLimitError = github.RateLimitError
+
 func (d *githubMgr) Get(filename, ver string, m *Modules) (*Module, error) {
 	repoPath, err := getGitHubRepoPath(filename)
 	if err != nil {
 		return nil, err
 	}
 	ctx := context.Background()
-	var refOps *github.RepositoryContentGetOptions
-	if ver != "" {
-		refOps = &github.RepositoryContentGetOptions{Ref: ver}
+
+	if ver == "" {
+		ver = MasterBranch
 	}
+	refOps := &github.RepositoryContentGetOptions{Ref: ver}
 
 	fileContent, _, _, err := d.client.Repositories.GetContents(ctx, repoPath.owner, repoPath.repo, repoPath.path, refOps)
 	if err != nil {
-		if _, ok := err.(*github.RateLimitError); ok {
-			return nil, errors.Wrap(err,
-				"\033[1;36mplease setup your GitHub access token\033[0m")
+		if err, ok := err.(*github.RateLimitError); ok {
+			e := RateLimitError(*err)
+			return nil, &e
+		}
+		if err, ok := err.(*github.ErrorResponse); ok && err.Response.StatusCode == http.StatusNotFound {
+			return nil, &NotFoundError{Message: err.Error()}
 		}
 		return nil, err
 	}
@@ -62,25 +78,23 @@ func (d *githubMgr) Get(filename, ver string, m *Modules) (*Module, error) {
 	if err != nil {
 		return nil, err
 	}
-	if ver == "" || ver == MasterBranch {
-		ref, _, err := d.client.Git.GetRef(ctx, repoPath.owner, repoPath.repo, "heads/"+MasterBranch)
-		if err != nil {
-			return nil, err
-		}
-		ver = "v0.0.0-" + ref.GetObject().GetSHA()[:12]
+
+	ref, err := d.GetCacheRef(repoPath, ver)
+	if err != nil {
+		return nil, err
 	}
 
 	name := strings.Join([]string{"github.com", repoPath.owner, repoPath.repo}, "/")
 	dir := filepath.Join(d.cacheDir, "github.com", repoPath.owner, repoPath.repo)
-	dir = AppendVersion(dir, ver)
+	dir = AppendVersion(dir, ref)
 	new := &Module{
 		Name:    name,
 		Dir:     dir,
-		Version: ver,
+		Version: ref,
 	}
 
 	fname := filepath.Join(dir, repoPath.path)
-	if !fileExists(fname, false) {
+	if !FileExists(fname, false) {
 		err = writeFile(fname, []byte(content))
 		if err != nil {
 			return nil, err
@@ -91,15 +105,30 @@ func (d *githubMgr) Get(filename, ver string, m *Modules) (*Module, error) {
 	return new, nil
 }
 
-func (*githubMgr) Find(filename, ver string, m *Modules) *Module {
-	if ver == "" || ver == MasterBranch {
+func (d *githubMgr) Find(filename, ver string, m *Modules) *Module {
+	if ver == "" {
+		ver = MasterBranch
+	}
+
+	repoPath, err := getGitHubRepoPath(filename)
+	if err != nil {
+		logrus.Debug("get github repository path error:", err)
+		return nil
+	}
+
+	ref, err := d.GetCacheRef(repoPath, ver)
+	if err != nil {
+		logrus.Debug("get github repository ref error:", err)
 		return nil
 	}
 
 	for _, mod := range *m {
 		if hasPathPrefix(mod.Name, filename) {
-			if mod.Version == ver {
-				return mod
+			if mod.Version == ref {
+				relpath, err := filepath.Rel(mod.Name, filename)
+				if err == nil && FileExists(filepath.Join(d.cacheDir, mod.Dir, relpath), false) {
+					return mod
+				}
 			}
 		}
 	}
@@ -109,7 +138,7 @@ func (*githubMgr) Find(filename, ver string, m *Modules) *Module {
 
 func (d *githubMgr) Load(m *Modules) error {
 	githubPath := filepath.Join(d.cacheDir, "github.com")
-	if !fileExists(githubPath, true) {
+	if !FileExists(githubPath, true) {
 		if err := os.MkdirAll(githubPath, 0770); err != nil {
 			return err
 		}
@@ -190,3 +219,19 @@ func writeFile(filename string, content []byte) error {
 	}
 	return nil
 }
+
+const SHA_LENGTH = 12
+
+func (d *githubMgr) GetCacheRef(repoPath *githubRepoPath, ref string) (string, error) {
+	ctx := context.Background()
+	_, _, err := d.client.Git.GetRef(ctx, repoPath.owner, repoPath.repo, "tags/"+ref)
+	if err == nil { // `ver` is a tag
+		return ref, nil
+	}
+
+	branch, _, err := d.client.Git.GetRef(ctx, repoPath.owner, repoPath.repo, "heads/"+ref)
+	if err == nil { // `ver` is a branch
+		return "v0.0.0-" + branch.GetObject().GetSHA()[:SHA_LENGTH], nil
+	}
+	return "", err
+}

mod/github_test.go

@@ -1,8 +1,14 @@
 package mod
 
 import (
+	"fmt"
+	"os"
+	"path"
+	"path/filepath"
+	"reflect"
 	"testing"
 
+	"bou.ke/monkey"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -12,14 +18,19 @@ func TestGitHubMgrInit(t *testing.T) {
 	err := githubmod.Init(&dir, nil)
 	assert.NoError(t, err)
 
+	err = githubmod.Init(&dir, accessTokenForTest())
+	assert.NoError(t, err)
+
 	err = githubmod.Init(nil, nil)
 	assert.Error(t, err)
+	err = githubmod.Init(nil, accessTokenForTest())
+	assert.Error(t, err)
 }
 
 func TestGitHubMgrGet(t *testing.T) {
 	githubmod := &githubMgr{}
 	dir := ".pkgcache"
-	err := githubmod.Init(&dir, nil)
+	err := githubmod.Init(&dir, accessTokenForTest())
 	assert.NoError(t, err)
 	testMods := Modules{}
 
@@ -42,25 +53,66 @@ func TestGitHubMgrGet(t *testing.T) {
 }
 
 func TestGitHubMgrFind(t *testing.T) {
-	githubmod := &githubMgr{}
+	cacheDir := ".pkgcache"
+	repo := "github.com/foo/bar"
+	filea := "filea"
+	fileb := "fileb"
+	tagRef := "v0.2.0"
+	masterRef := "v0.0.0-41f04d3bba15"
+	tagRepoDir := "github.com/foo/bar@v0.2.0"
+	masterRepoDir := "github.com/foo/bar@v0.0.0-41f04d3bba15"
+
+	githubmod := &githubMgr{cacheDir: cacheDir}
 	testMods := Modules{}
-	local := &Module{Name: "local", Version: "v0.0.1"}
-	mod1 := &Module{Name: "remote", Version: "v1.0.0-41f04d3bba15"}
-	mod2 := &Module{Name: "remote", Version: "v0.2.0"}
-	testMods.Add(local)
-	testMods.Add(mod1)
-	testMods.Add(mod2)
-
-	assert.Equal(t, local, githubmod.Find("local/filename", "v0.0.1", &testMods))
-	assert.Equal(t, local, githubmod.Find("local/filename2", "v0.0.1", &testMods))
-	assert.Equal(t, local, githubmod.Find(".//local/filename", "v0.0.1", &testMods))
-	assert.Equal(t, local, githubmod.Find("local", "v0.0.1", &testMods))
-	assert.Nil(t, githubmod.Find("local2/filename", "v0.0.1", &testMods))
-
-	assert.Nil(t, githubmod.Find("local/filename", MasterBranch, &testMods))
-
-	assert.Equal(t, mod2, githubmod.Find("remote/filename", "v0.2.0", &testMods))
-	assert.Nil(t, githubmod.Find("remote/filename", "v1.0.0", &testMods))
+	tagMod := &Module{
+		Name:    repo,
+		Version: tagRef,
+		Dir:     tagRepoDir,
+	}
+	masterMod := &Module{
+		Name:    repo,
+		Version: masterRef,
+		Dir:     masterRepoDir,
+	}
+	testMods.Add(tagMod)
+	testMods.Add(masterMod)
+
+	monkey.Patch(FileExists, func(filename string, _ bool) bool {
+		files := []string{
+			filepath.Join(cacheDir, tagRepoDir, filea),
+			filepath.Join(cacheDir, tagRepoDir, fileb),
+			filepath.Join(cacheDir, masterRepoDir, filea),
+		}
+		for _, f := range files {
+			if filename == f {
+				return true
+			}
+		}
+		return false
+	})
+
+	monkey.PatchInstanceMethod(reflect.TypeOf(githubmod), "GetCacheRef", func(_ *githubMgr, _ *githubRepoPath, ref string) (string, error) {
+		switch ref {
+		case tagRef:
+			return tagRef, nil
+		case MasterBranch:
+			return masterRef, nil
+		}
+		return "", fmt.Errorf("ref not found")
+	})
+	defer monkey.UnpatchAll()
+
+	assert.Equal(t, tagMod, githubmod.Find(path.Join(repo, filea), tagRef, &testMods))
+	assert.Equal(t, tagMod, githubmod.Find(path.Join(repo, fileb), tagRef, &testMods))
+	assert.Nil(t, githubmod.Find(repo, tagRef, &testMods))
+	assert.Nil(t, githubmod.Find(path.Join(repo, "wrong"), tagRef, &testMods))
+
+	assert.Equal(t, masterMod, githubmod.Find(path.Join(repo, filea), MasterBranch, &testMods))
+	assert.Equal(t, masterMod, githubmod.Find(path.Join(repo, filea), "", &testMods))
+	assert.Nil(t, githubmod.Find(repo, MasterBranch, &testMods))
+	assert.Nil(t, githubmod.Find(path.Join(repo, fileb), MasterBranch, &testMods))
+
+	assert.Nil(t, githubmod.Find("github.com/foo/wrongrepo/files", tagRef, &testMods))
 }
 
 func TestGetGitHubRepoPath(t *testing.T) {
@@ -88,3 +140,30 @@ func TestGetGitHubRepoPath(t *testing.T) {
 		})
 	}
 }
+
+func TestGetCacheRef(t *testing.T) {
+	githubmod := &githubMgr{}
+	dir := ".pkgcache"
+	err := githubmod.Init(&dir, accessTokenForTest())
+	assert.NoError(t, err)
+	repoPath := &githubRepoPath{
+		owner: "anz-bank",
+		repo:  "pkg",
+	}
+	ref, err := githubmod.GetCacheRef(repoPath, "v0.0.7")
+	assert.NoError(t, err)
+	assert.Equal(t, "v0.0.7", ref)
+
+	ref, err = githubmod.GetCacheRef(repoPath, MasterBranch)
+	assert.NoError(t, err)
+	assert.Equal(t, "v0.0.0-", ref[:7])
+}
+
+func accessTokenForTest() *string {
+	var accessToken *string
+	rawToken := os.Getenv("GITHUB_ACCESS_TOKEN")
+	if rawToken != "" {
+		accessToken = &rawToken
+	}
+	return accessToken
+}

mod/gomodules_test.go

@@ -12,32 +12,28 @@ func TestModInit(t *testing.T) {
 	gomod := &goModules{}
 
 	// assumes the test folder (cwd) is not a go module folder
-	removeFile(t, fs, "go.sum")
-	removeFile(t, fs, "go.mod")
+	removeGomodFile(t, fs)
 
 	err := gomod.Init("test")
 	assert.NoError(t, err)
 
-	removeFile(t, fs, "go.sum")
-	removeFile(t, fs, "go.mod")
+	removeGomodFile(t, fs)
 }
 
 func TestModInitAlreadyExists(t *testing.T) {
 	fs := afero.NewOsFs()
 	gomod := &goModules{}
 
 	// assumes the test folder (cwd) is not a go module folder
-	removeFile(t, fs, "go.sum")
-	removeFile(t, fs, "go.mod")
+	removeGomodFile(t, fs)
 
 	err := gomod.Init("test")
 	assert.NoError(t, err)
 
 	err = gomod.Init("test")
 	assert.Error(t, err)
 
-	removeFile(t, fs, "go.sum")
-	removeFile(t, fs, "go.mod")
+	removeGomodFile(t, fs)
 }
 
 func TestGoModulesGet(t *testing.T) {

mod/module.go

@@ -59,7 +59,7 @@ func Config(m ModeType, gomodName, cacheDir, accessToken *string) error {
 		manager = gh
 	case GoModulesMode:
 		gm := &goModules{}
-		if !fileExists("go.mod", false) {
+		if !FileExists("go.mod", false) {
 			var modName string
 			if gomodName != nil {
 				modName = *gomodName
@@ -103,7 +103,7 @@ func hasPathPrefix(prefix, s string) bool {
 	return s == prefix
 }
 
-func fileExists(filename string, isDir bool) bool {
+func FileExists(filename string, isDir bool) bool {
 	info, err := os.Stat(filename)
 	if os.IsNotExist(err) {
 		return false