Repository: jenkins-x/lighthouse
Pull request #1337 (2021-07-01 13:12:07)

fix: avoid unnecessary fetch if we have a sha locally already


Patch:
diff --git a/pkg/filebrowser/git_file_browser.go b/pkg/filebrowser/git_file_browser.go
index f0d2baa4..17df05c6 100644
--- a/pkg/filebrowser/git_file_browser.go
+++ b/pkg/filebrowser/git_file_browser.go
@@ -4,6 +4,7 @@ import (
 	"io/ioutil"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"sync"
 
@@ -23,6 +24,10 @@ type gitFileBrowser struct {
 
 const headBranchPrefix = "HEAD branch:"
 
+var (
+	shaRegex = regexp.MustCompile("\\b[0-9a-f]{7,40}\\b")
+)
+
 // NewFileBrowserFromGitClient creates a new file browser from an Scm client
 func NewFileBrowserFromGitClient(clientFactory git.ClientFactory) Interface {
 	return &gitFileBrowser{
@@ -169,6 +174,18 @@ func (c *repoClientFacade) UseRef(ref string, fc FetchCache) error {
 	}
 
 	shouldFetch := fc.ShouldFetch(c.fullName, ref)
+	if shouldFetch && IsSHA(ref) {
+		// lets check if we've already fetched this sha
+		sha, err := c.repoClient.HasSHA(ref)
+		if err == nil && sha != "" {
+			shouldFetch = false
+			logrus.StandardLogger().WithFields(map[string]interface{}{
+				"Name": c.fullName,
+				"Ref":  ref,
+				"File": "git_file_browser",
+			}).Info("not fetching ref as we already have it")
+		}
+	}
 	if shouldFetch {
 		logrus.StandardLogger().WithFields(map[string]interface{}{
 			"Name": c.fullName,
@@ -246,3 +263,8 @@ func runCmd(dir, cmd string, arg ...string) (string, error) {
 	}
 	return text, nil
 }
+
+// IsSHA returns true if the given ref is a git sha
+func IsSHA(ref string) bool {
+	return shaRegex.MatchString(ref)
+}
diff --git a/pkg/git/v2/interactor.go b/pkg/git/v2/interactor.go
index 71b3f915..5ce23901 100644
--- a/pkg/git/v2/interactor.go
+++ b/pkg/git/v2/interactor.go
@@ -66,6 +66,8 @@ type Interactor interface {
 	MergeCommitsExistBetween(target, head string) (bool, error)
 	// ShowRef returns the commit for a commitlike. Unlike rev-parse it does not require a checkout.
 	ShowRef(commitlike string) (string, error)
+	// HasSHA checks if a ref is a sha we have fetched locally
+	HasSHA(ref string) (string, error)
 }
 
 // cacher knows how to cache and update repositories in a central cache
@@ -375,3 +377,12 @@ func (i *interactor) ShowRef(commitlike string) (string, error) {
 	}
 	return strings.TrimSpace(string(out)), nil
 }
+
+func (i *interactor) HasSHA(ref string) (string, error) {
+	i.logger.Infof("Checking if %s is a fetched SHA", ref)
+	out, err := i.executor.Run("cat-file", "commit", ref)
+	if err != nil {
+		return "", fmt.Errorf("failed to verify ref is a sha: %s: %v", ref, err)
+	}
+	return strings.TrimSpace(string(out)), nil
+}


Test patch:
diff --git a/pkg/filebrowser/git_file_browser_test.go b/pkg/filebrowser/git_file_browser_test.go
index 599ca03d..590cd8ad 100644
--- a/pkg/filebrowser/git_file_browser_test.go
+++ b/pkg/filebrowser/git_file_browser_test.go
@@ -1,7 +1,8 @@
-package filebrowser
+package filebrowser_test
 
 import (
 	"fmt"
+	"github.com/jenkins-x/lighthouse/pkg/filebrowser"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -19,9 +20,9 @@ func TestGitFileBrowser(t *testing.T) {
 
 	cf, err := git.NewClientFactory()
 	require.NoError(t, err, "failed to create git client factory")
-	fb := NewFileBrowserFromGitClient(cf)
+	fb := filebrowser.NewFileBrowserFromGitClient(cf)
 
-	fc := NewFetchCache()
+	fc := filebrowser.NewFetchCache()
 
 	owner := "jenkins-x-quickstarts"
 	repo := "node-http"
@@ -107,7 +108,7 @@ func TestGitFileBrowser_Clone_CreateTag_FetchRef(t *testing.T) {
 	require.NoError(t, err, "failed to find git binary")
 	fmt.Println(baseDir)
 
-	fc := NewFetchCache()
+	fc := filebrowser.NewFetchCache()
 
 	defer os.RemoveAll(baseDir)
 
@@ -138,7 +139,7 @@ func TestGitFileBrowser_Clone_CreateTag_FetchRef(t *testing.T) {
 
 	cf, err := git.NewLocalClientFactory(baseDir, userGetter, censor)
 	require.NoError(t, err, "failed to create git client factory")
-	fb := NewFileBrowserFromGitClient(cf)
+	fb := filebrowser.NewFileBrowserFromGitClient(cf)
 
 	files, err := fb.ListFiles("org", "repo", "", "master", fc)
 	require.NoError(t, err, "failed to list files")
@@ -167,3 +168,17 @@ func TestGitFileBrowser_Clone_CreateTag_FetchRef(t *testing.T) {
 	require.NoError(t, err, "failed to lst files in v0.0.1 tag")
 	require.Equal(t, string(data), "README-update-1")
 }
+
+func TestIsSHA(t *testing.T) {
+	testCases := map[string]bool{
+		"de6cc99": true,
+		"de6cc99a6de8ca34b8884fcc05945bd30033f330": true,
+		"main":    false,
+		"123_567": false,
+	}
+
+	for ref, expected := range testCases {
+		got := filebrowser.IsSHA(ref)
+		assert.Equal(t, expected, got, "for ref: %s", ref)
+	}
+}