Avoid unnecessary rescans

commands/scanandfixrepos.go

@@ -1,12 +1,15 @@
 package commands
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"github.com/jfrog/frogbot/commands/utils"
 	"github.com/jfrog/froggit-go/vcsclient"
+	"github.com/jfrog/jfrog-client-go/utils/log"
 	"path/filepath"
 	"strings"
+	"time"
 )
 
 type ScanAndFixRepositories struct {
@@ -33,12 +36,26 @@ func (saf *ScanAndFixRepositories) Run(configAggregator utils.FrogbotConfigAggre
 
 func (saf *ScanAndFixRepositories) scanAndFixSingleRepository(repoConfig *utils.FrogbotRepoConfig, client vcsclient.VcsClient) error {
 	for _, branch := range repoConfig.Branches {
-		err := saf.downloadAndRunScanAndFix(repoConfig, branch, client)
+		shouldScan, checkedCommit, err := saf.shouldScanRepositoryByFrogBotCommitStatus(context.Background(), repoConfig, client, branch)
+		if err != nil {
+			return err
+		}
+		if !shouldScan {
+			log.Info(fmt.Sprintf("Commit %s has already been scanned by FrogBot", checkedCommit))
+			continue
+		}
+		err = saf.downloadAndRunScanAndFix(repoConfig, branch, client)
+		if err != nil {
+			errorDescription := fmt.Sprintf("FrogBot error %s", err)
+			log.Error(errorDescription)
+			err = saf.setCommitBuildStatus(client, repoConfig, vcsclient.Fail, checkedCommit, errorDescription)
+			return err
+		}
+		err = saf.setCommitBuildStatus(client, repoConfig, vcsclient.Pass, checkedCommit, "FrogBot scanned")
 		if err != nil {
 			return err
 		}
 	}
-
 	return nil
 }
 
@@ -68,3 +85,63 @@ func (saf *ScanAndFixRepositories) downloadAndRunScanAndFix(repository *utils.Fr
 	cfp := CreateFixPullRequestsCmd{dryRun: saf.dryRun, dryRunRepoPath: filepath.Join(saf.dryRunRepoPath, repository.RepoName)}
 	return cfp.scanAndFixRepository(repository, branch, client)
 }
+
+func (saf ScanAndFixRepositories) setCommitBuildStatus(client vcsclient.VcsClient, repoConfig *utils.FrogbotRepoConfig, state vcsclient.CommitStatus, commitHash string, description string) error {
+	background := context.Background()
+	err := client.SetCommitStatus(background, state, repoConfig.RepoOwner, repoConfig.RepoName, commitHash, utils.ProductId, description, utils.FrogbotReadMeUrl)
+	if err != nil {
+		log.Error("Failed to mark last commit as checked")
+		return err
+	}
+	log.Info(fmt.Sprintf("Successfully marked commit %s as checked by FrogBot", commitHash))
+	return nil
+}
+
+// Checking last FrogBot commit status that indicates whether FrogBot has already scanned this branch or not
+func (saf ScanAndFixRepositories) shouldScanRepositoryByFrogBotCommitStatus(ctx context.Context, repoConfig *utils.FrogbotRepoConfig, client vcsclient.VcsClient, branch string) (shouldScan bool, commitHash string, err error) {
+	owner := repoConfig.RepoOwner
+	repo := repoConfig.RepoName
+	latestCommit, err := client.GetLatestCommit(ctx, owner, repo, branch)
+	if latestCommit.Hash == "" {
+		// No commits at all
+		return true, "", nil
+	}
+	if err != nil {
+		return false, "", err
+	}
+	ref := latestCommit.Hash
+	statuses, err := client.GetCommitStatuses(ctx, owner, repo, ref)
+	if err != nil {
+		return false, "", err
+	}
+	return shouldScanBranchByStatus(statuses), latestCommit.Hash, err
+}
+
+// Returns true if the latest commit status by FrogBot is not successful
+// OR it's older than DefaultAmountOfDaysToRescanRepo.
+func shouldScanBranchByStatus(statuses []vcsclient.CommitStatusInfo) bool {
+	length := len(statuses)
+	if length == 0 {
+		return true
+	}
+	latestStatus := statuses[length-1]
+	if !strings.Contains(latestStatus.DetailsUrl, utils.FrogbotReadMeUrl) {
+		return shouldScanBranchByStatus(statuses[0 : length-1])
+	}
+	return isStatusOldAndNeedScan(latestStatus) || latestStatus.State != vcsclient.Pass
+}
+
+// Checks if status need rescan because it is older than DefaultAmountOfDaysToRescanRepo
+func isStatusOldAndNeedScan(latestStatus vcsclient.CommitStatusInfo) bool {
+	statusLastUpdatedTime := time.Time{}
+	if !latestStatus.CreatedAt.IsZero() {
+		statusLastUpdatedTime = latestStatus.CreatedAt
+	}
+	if !latestStatus.LastUpdatedAt.IsZero() {
+		statusLastUpdatedTime = latestStatus.LastUpdatedAt
+	}
+	if statusLastUpdatedTime.IsZero() {
+		return true
+	}
+	return statusLastUpdatedTime.Before(time.Now().UTC().AddDate(0, 0, -utils.DefaultAmountOfDaysToRescanRepo))
+}

commands/scanandfixrepos_test.go

@@ -62,6 +62,161 @@ func TestScanAndFixRepos(t *testing.T) {
 	assert.NoError(t, cmd.Run(configAggregator, client))
 }
 
+func TestShouldScanBranchByStatus(t *testing.T) {
+	commitStatusTestCases := []struct {
+		statuses    []vcsclient.CommitStatusInfo
+		description string
+		expected    bool
+	}{
+		{
+			statuses: []vcsclient.CommitStatusInfo{
+				{
+					State:         vcsclient.Pass,
+					Description:   "description",
+					DetailsUrl:    utils.FrogbotReadMeUrl,
+					Creator:       "test",
+					LastUpdatedAt: time.Now().UTC(),
+				}, {
+					State:         vcsclient.InProgress,
+					Description:   "this is the latest commit",
+					DetailsUrl:    utils.FrogbotReadMeUrl,
+					Creator:       "test",
+					LastUpdatedAt: time.Now().UTC(),
+				},
+			},
+			description: "Latest status is not successful",
+			expected:    true,
+		},
+		{
+			statuses: []vcsclient.CommitStatusInfo{
+				{
+					State:         vcsclient.InProgress,
+					Description:   "description",
+					DetailsUrl:    utils.FrogbotReadMeUrl,
+					Creator:       "test",
+					LastUpdatedAt: time.Now().UTC(),
+				}, {
+					State:         vcsclient.Pass,
+					Description:   "this is the latest commit",
+					DetailsUrl:    utils.FrogbotReadMeUrl,
+					Creator:       "test",
+					LastUpdatedAt: time.Now().UTC(),
+				},
+			},
+			description: "Latest status is successful",
+			expected:    false,
+		},
+		{
+			statuses:    []vcsclient.CommitStatusInfo{},
+			description: "Empty statuses",
+			expected:    true,
+		},
+		{
+			statuses: []vcsclient.CommitStatusInfo{
+				{
+					State:         vcsclient.Fail,
+					Description:   "description",
+					DetailsUrl:    utils.FrogbotReadMeUrl,
+					Creator:       "test",
+					LastUpdatedAt: time.Now().UTC(),
+				}, {
+					State:         vcsclient.InProgress,
+					Description:   "this is the latest commit",
+					DetailsUrl:    utils.FrogbotReadMeUrl,
+					Creator:       "test",
+					LastUpdatedAt: time.Now().UTC(),
+				},
+			},
+			description: "FrogBot failed statues should scan",
+			expected:    true,
+		},
+		{
+			statuses: []vcsclient.CommitStatusInfo{
+				{
+					State:         vcsclient.Fail,
+					Description:   "description",
+					DetailsUrl:    "some other url",
+					Creator:       "test",
+					LastUpdatedAt: time.Now().UTC(),
+				}, {
+					State:         vcsclient.InProgress,
+					Description:   "this is the latest commit",
+					DetailsUrl:    "some other url",
+					Creator:       "test",
+					LastUpdatedAt: time.Now().UTC(),
+				},
+				{
+					State:         vcsclient.Pass,
+					Description:   "this is the latest commit",
+					DetailsUrl:    "some other url",
+					Creator:       "test",
+					LastUpdatedAt: time.Now().UTC(),
+				},
+			},
+			description: "Non FrogBot statues",
+			expected:    true,
+		},
+	}
+	for _, tt := range commitStatusTestCases {
+		t.Run(tt.description, func(t *testing.T) {
+			shouldScan := shouldScanBranchByStatus(tt.statuses)
+			assert.Equal(t, tt.expected, shouldScan)
+		})
+	}
+}
+
+func TestIsStatusOldAndNeedScan(t *testing.T) {
+	testCases := []struct {
+		commitStatusInfo vcsclient.CommitStatusInfo
+		description      string
+		expected         bool
+	}{
+		{
+			commitStatusInfo: vcsclient.CommitStatusInfo{
+				State:         0,
+				Description:   "",
+				DetailsUrl:    "",
+				Creator:       "",
+				CreatedAt:     time.Now().UTC().AddDate(0, -3, 0),
+				LastUpdatedAt: time.Now().UTC().AddDate(0, 0, -utils.DefaultAmountOfDaysToRescanRepo-1),
+			},
+			expected:    true,
+			description: "Last Update time is priority",
+		},
+		{
+			commitStatusInfo: vcsclient.CommitStatusInfo{
+				State:         0,
+				Description:   "",
+				DetailsUrl:    "",
+				Creator:       "",
+				CreatedAt:     time.Now(),
+				LastUpdatedAt: time.Now(),
+			},
+			expected:    false,
+			description: "No scan needed ",
+		},
+		{
+			commitStatusInfo: vcsclient.CommitStatusInfo{
+				State:         0,
+				Description:   "",
+				DetailsUrl:    "",
+				Creator:       "",
+				CreatedAt:     time.Now().UTC(),
+				LastUpdatedAt: time.Time{},
+			},
+			expected:    false,
+			description: "Creation time fallback",
+		},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.description, func(t *testing.T) {
+			needScan := isStatusOldAndNeedScan(tt.commitStatusInfo)
+			assert.Equal(t, tt.expected, needScan)
+		})
+	}
+}
+
 func createReposGitEnvironment(t *testing.T, wd, port string, repositories ...string) {
 	for _, repository := range repositories {
 		fullWdPath := filepath.Join(wd, repository)

commands/utils/consts.go

@@ -68,8 +68,11 @@ const (
 	WhatIsFrogbotMd       = "\n\n[What is Frogbot?](https://github.com/jfrog/frogbot#readme)\n"
 
 	// Product ID for usage reporting
-	productId = "frogbot"
+	ProductId = "frogbot"
 
 	// The 'GITHUB_ACTIONS' environment variable exists when the CI is GitHub Actions
 	GitHubActionsEnv = "GITHUB_ACTIONS"
+	FrogbotReadMeUrl = "https://github.com/jfrog/frogbot#readme"
+
+	DefaultAmountOfDaysToRescanRepo = 4
 )

commands/utils/utils.go

@@ -90,7 +90,7 @@ func ReportUsage(commandName string, serverDetails *config.ServerDetails, usageR
 		log.Debug(usage.ReportUsagePrefix + err.Error())
 		return
 	}
-	err = usage.SendReportUsage(productId, commandName, serviceManager)
+	err = usage.SendReportUsage(ProductId, commandName, serviceManager)
 	if err != nil {
 		log.Debug(err.Error())
 		return

commands/utils/utils_test.go

@@ -78,7 +78,7 @@ func createUsageHandler(t *testing.T, commandName string) http.HandlerFunc {
 			buf := new(bytes.Buffer)
 			_, err := buf.ReadFrom(r.Body)
 			assert.NoError(t, err)
-			assert.Equal(t, fmt.Sprintf(`{"productId":"%s","features":[{"featureId":"%s"}]}`, productId, commandName), buf.String())
+			assert.Equal(t, fmt.Sprintf(`{"productId":"%s","features":[{"featureId":"%s"}]}`, ProductId, commandName), buf.String())
 
 			// Send response OK
 			w.WriteHeader(http.StatusOK)

go.mod

@@ -110,3 +110,5 @@ require (
 	gopkg.in/ini.v1 v1.67.0 // indirect
 	gopkg.in/warnings.v0 v0.1.2 // indirect
 )
+
+replace github.com/jfrog/froggit-go v1.6.3 => github.com/eyaldelarea/froggit-go v1.6.1-0.20230326093136-095fcd577f99

go.sum

@@ -98,6 +98,8 @@ github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1m
 github.com/envoyproxy/go-control-plane v0.9.7/go.mod h1:cwu0lG7PUMfa9snN8LXBig5ynNVH9qI8YYLbd1fK2po=
 github.com/envoyproxy/go-control-plane v0.9.9-0.20201210154907-fd9021fe5dad/go.mod h1:cXg6YxExXjJnVBQHBLXeUAgxn2UodCpnH306RInaBQk=
 github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
+github.com/eyaldelarea/froggit-go v1.6.1-0.20230326093136-095fcd577f99 h1:MCb/ddiLudW1SEw0pR173WYpqGN1eo1GIOPwKkhXSgM=
+github.com/eyaldelarea/froggit-go v1.6.1-0.20230326093136-095fcd577f99/go.mod h1:xfsfQXzSaAM04RV9IyU5heBiRrsm2oS6rFCfEofQr6U=
 github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w=
 github.com/forPelevin/gomoji v1.1.8 h1:JElzDdt0TyiUlecy6PfITDL6eGvIaxqYH1V52zrd0qQ=
 github.com/forPelevin/gomoji v1.1.8/go.mod h1:8+Z3KNGkdslmeGZBC3tCrwMrcPy5GRzAD+gL9NAwMXg=
@@ -223,8 +225,6 @@ github.com/jedib0t/go-pretty/v6 v6.4.6/go.mod h1:Ndk3ase2CkQbXLLNf5QDHoYb6J9WtVf
 github.com/jessevdk/go-flags v1.5.0/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4=
 github.com/jfrog/build-info-go v1.8.9 h1:2zShj2ATlNznyNd2F1R8vPJiYxnUEUoc98G/Chd5+S8=
 github.com/jfrog/build-info-go v1.8.9/go.mod h1:dQ8OKddrbgtO3jK9uLYoqmRGNEjuDuNXV0bSRdpeTCI=
-github.com/jfrog/froggit-go v1.6.3 h1:IExHc616HEQdk3Uk0rS3acbKJgJCrp9vYg5ZGim7C/o=
-github.com/jfrog/froggit-go v1.6.3/go.mod h1:xfsfQXzSaAM04RV9IyU5heBiRrsm2oS6rFCfEofQr6U=
 github.com/jfrog/gofrog v1.2.5 h1:jCgJC0iGQ8bU7jCC+YEFJTNINyngApIrhd8BjZAVRIE=
 github.com/jfrog/gofrog v1.2.5/go.mod h1:o00tSRff6IapTgaCMuX1Cs9MH08Y1JqnsKgRtx91Gc4=
 github.com/jfrog/jfrog-cli-core/v2 v2.29.9 h1:Z04r+KkRDf+BqWnDX9vbiUeD/7nBzHQGYld2xjdjIgo=