chore: code-refactoring (#89)

* adding git commit message truncation logic for large messages

* first commit draft

* cleaning dead code

* added commit count

* fix git format in cmd

* added interfaces

* comments

* removing output

* minor restructuring

* removing deuplicacy for OpenNewRepo

* changes

* fixes

* wire

* old story refactor

* fixes for IT tests

* cleaning

* refactorings

* minor

* test fixes

* test fixes

* test fixes

* PR comments

* removing old changes

* cleaning up

* wire

* minor changes

* added comments on interface methods

* next error handling

* added timeout and context in one flow

* added timeouts

* remaning flows

* minor changes

* renaming to gitCtx

* added command timeout

* added command timeout

* moving cancel defer in poller

* renaming

* added command level timeouts

* fix

* minor

* removing process timeout

* refactoring

* cli for analytics

* changes

* changes

* minor

* adding gogit timeout

* debugging

* testing fix

* fixes

* fixes

* minor refactor

* pr comments

* fixed it tests

* common-lib upgrade

* common lib from main

* fixes-be

* adding support for last seen hash

* code refactoring

* internal name changed to internals

* code made common

* code refactoring in GitBaseManager.go

* code refactoring in GitBaseManager.go

* code refactoring in GitBaseManager.go

* code refactoring

* code refactoring

* code refactoring

* code refactoring

* code refactoring

* code refactoring as suggested in code review

* commented code removed

* commented code retrieved

* GitManagerImpl retrieved

* GitManagerImpl - wire file corrected

* code review comments incorporated

* code review comments incorporated

* app directory created

* code review comments incorporated

* code review comments incorporated

* code review comments incorporated

* wireSet generated

* wireSet and test file changed

* fix for history being overwritten

* fixing for empty history

* changes

---------

Co-authored-by: Subhashish <subhashish@devtron.ai>

api/GrpcHandler.go

@@ -2,7 +2,7 @@ package api
 
 import (
 	"context"
-	"github.com/devtron-labs/git-sensor/internal/sql"
+	"github.com/devtron-labs/git-sensor/internals/sql"
 	"github.com/devtron-labs/git-sensor/pkg"
 	"github.com/devtron-labs/git-sensor/pkg/git"
 	pb "github.com/devtron-labs/protos/gitSensor"
@@ -355,6 +355,7 @@ func (impl *GrpcHandlerImpl) GetCommitMetadataForPipelineMaterial(ctx context.Co
 
 		return nil, err
 	}
+
 	if res == nil {
 		res1 := &pb.GitCommit{}
 		return res1, nil

api/GrpcHandler_test.go

@@ -3,7 +3,7 @@ package api
 import (
 	"context"
 	"encoding/json"
-	"github.com/devtron-labs/git-sensor/internal/sql"
+	"github.com/devtron-labs/git-sensor/internals/sql"
 	"github.com/devtron-labs/git-sensor/pkg"
 	"github.com/devtron-labs/git-sensor/pkg/git"
 	"github.com/devtron-labs/git-sensor/pkg/mocks"

api/RestHandler.go

@@ -18,7 +18,7 @@ package api
 
 import (
 	"encoding/json"
-	"github.com/devtron-labs/git-sensor/internal/sql"
+	"github.com/devtron-labs/git-sensor/internals/sql"
 	"github.com/devtron-labs/git-sensor/pkg"
 	"github.com/devtron-labs/git-sensor/pkg/git"
 	"github.com/gorilla/mux"

App.go → app/App.go

@@ -14,7 +14,7 @@
  *  limitations under the License.
  */
 
-package main
+package app
 
 import (
 	"context"
@@ -24,7 +24,7 @@ import (
 	pubsub "github.com/devtron-labs/common-lib/pubsub-lib"
 	"github.com/devtron-labs/git-sensor/api"
 	"github.com/devtron-labs/git-sensor/bean"
-	"github.com/devtron-labs/git-sensor/internal/middleware"
+	"github.com/devtron-labs/git-sensor/internals/middleware"
 	"github.com/devtron-labs/git-sensor/pkg/git"
 	pb "github.com/devtron-labs/protos/gitSensor"
 	"github.com/go-pg/pg"

internal/Configuration.go → internals/Configuration.go

@@ -1,13 +1,12 @@
-package internal
+package internals
 
-import (
-	"github.com/caarlos0/env"
-)
+import "github.com/caarlos0/env"
 
 type Configuration struct {
 	CommitStatsTimeoutInSec int    `env:"COMMIT_STATS_TIMEOUT_IN_SEC" envDefault:"2"`
 	EnableFileStats         bool   `env:"ENABLE_FILE_STATS" envDefault:"false"`
 	GitHistoryCount         int    `env:"GIT_HISTORY_COUNT" envDefault:"15"`
+	CloningMode             string `env:"CLONING_MODE" envDefault:"FULL"`
 	MinLimit                int    `env:"MIN_LIMIT_FOR_PVC" envDefault:"1"` // in MB
 	UseGitCli               bool   `env:"USE_GIT_CLI" envDefault:"false"`
 	AnalyticsDebug          bool   `env:"ANALYTICS_DEBUG" envDefault:"false"`

internal/Lockutil.go → internals/Lockutil.go

@@ -14,7 +14,7 @@
  *  limitations under the License.
  */
 
-package internal
+package internals
 
 import (
 	"go.uber.org/zap"

internal/util/HelperUtil.go → internals/util/HelperUtil.go

@@ -1,7 +1,7 @@
 package util
 
 import (
-	"github.com/devtron-labs/git-sensor/internal/sql"
+	"github.com/devtron-labs/git-sensor/internals/sql"
 	"strings"
 )
 

pkg/RepoManages.go

@@ -20,9 +20,9 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"github.com/devtron-labs/git-sensor/internal"
-	"github.com/devtron-labs/git-sensor/internal/sql"
-	"github.com/devtron-labs/git-sensor/internal/util"
+	"github.com/devtron-labs/git-sensor/internals"
+	"github.com/devtron-labs/git-sensor/internals/sql"
+	"github.com/devtron-labs/git-sensor/internals/util"
 	"github.com/devtron-labs/git-sensor/pkg/git"
 	_ "github.com/robfig/cron/v3"
 	"go.uber.org/zap"
@@ -58,15 +58,15 @@ type RepoManagerImpl struct {
 	repositoryManagerAnalytics                    git.RepositoryManagerAnalytics
 	gitProviderRepository                         sql.GitProviderRepository
 	ciPipelineMaterialRepository                  sql.CiPipelineMaterialRepository
-	locker                                        *internal.RepositoryLocker
+	locker                                        *internals.RepositoryLocker
 	gitWatcher                                    git.GitWatcher
 	webhookEventRepository                        sql.WebhookEventRepository
 	webhookEventParsedDataRepository              sql.WebhookEventParsedDataRepository
 	webhookEventDataMappingRepository             sql.WebhookEventDataMappingRepository
 	webhookEventDataMappingFilterResultRepository sql.WebhookEventDataMappingFilterResultRepository
 	webhookEventBeanConverter                     git.WebhookEventBeanConverter
-	configuration                                 *internal.Configuration
-	gitManager                                    git.GitManagerImpl
+	configuration                                 *internals.Configuration
+	gitManager                                    git.GitManager
 }
 
 func NewRepoManagerImpl(
@@ -76,14 +76,14 @@ func NewRepoManagerImpl(
 	repositoryManagerAnalytics git.RepositoryManagerAnalytics,
 	gitProviderRepository sql.GitProviderRepository,
 	ciPipelineMaterialRepository sql.CiPipelineMaterialRepository,
-	locker *internal.RepositoryLocker,
+	locker *internals.RepositoryLocker,
 	gitWatcher git.GitWatcher, webhookEventRepository sql.WebhookEventRepository,
 	webhookEventParsedDataRepository sql.WebhookEventParsedDataRepository,
 	webhookEventDataMappingRepository sql.WebhookEventDataMappingRepository,
 	webhookEventDataMappingFilterResultRepository sql.WebhookEventDataMappingFilterResultRepository,
 	webhookEventBeanConverter git.WebhookEventBeanConverter,
-	configuration *internal.Configuration,
-	gitManager git.GitManagerImpl,
+	configuration *internals.Configuration,
+	gitManager git.GitManager,
 ) *RepoManagerImpl {
 	return &RepoManagerImpl{
 		logger:                            logger,
@@ -194,6 +194,10 @@ func (impl RepoManagerImpl) updatePipelineMaterialCommit(gitCtx git.GitContext,
 			impl.logger.Errorw("error in fetching material", "err", err)
 			continue
 		}
+
+		gitCtx = gitCtx.WithCredentials(material.GitProvider.UserName, material.GitProvider.Password).
+			WithCloningMode(impl.configuration.CloningMode)
+
 		fetchCount := impl.configuration.GitHistoryCount
 		commits, err := impl.repositoryManager.ChangesSince(gitCtx, material.CheckoutLocation, pipelineMaterial.Value, "", "", fetchCount)
 		//commits, err := impl.FetchChanges(pipelineMaterial.Id, "", "", 0)
@@ -349,15 +353,18 @@ func (impl RepoManagerImpl) checkoutMaterial(gitCtx git.GitContext, material *sq
 	if err != nil {
 		return material, nil
 	}
-	checkoutPath, err := git.GetLocationForMaterial(material)
+
+	gitCtx = gitCtx.WithCredentials(userName, password).
+		WithCloningMode(impl.configuration.CloningMode)
+
+	checkoutPath, checkoutLocationForFetching, err := impl.repositoryManager.GetCheckoutPathAndLocation(gitCtx, material, gitProvider.Url)
 	if err != nil {
 		return material, err
 	}
-	gitCtx = gitCtx.WithCredentials(userName, password)
 
 	err = impl.repositoryManager.Add(gitCtx, material.GitProviderId, checkoutPath, material.Url, gitProvider.AuthMode, gitProvider.SshPrivateKey)
 	if err == nil {
-		material.CheckoutLocation = checkoutPath
+		material.CheckoutLocation = checkoutLocationForFetching
 		material.CheckoutStatus = true
 	} else {
 		material.CheckoutStatus = false
@@ -631,7 +638,10 @@ func (impl RepoManagerImpl) GetLatestCommitForBranch(gitCtx git.GitContext, pipe
 	}()
 
 	userName, password, err := git.GetUserNamePassword(gitMaterial.GitProvider)
-	gitCtx = gitCtx.WithCredentials(userName, password)
+
+	gitCtx = gitCtx.WithCredentials(userName, password).
+		WithCloningMode(impl.configuration.CloningMode)
+
 	updated, repo, err := impl.repositoryManager.Fetch(gitCtx, gitMaterial.Url, gitMaterial.CheckoutLocation)
 	if !updated {
 		impl.logger.Warn("repository is up to date")
@@ -663,7 +673,7 @@ func (impl RepoManagerImpl) GetLatestCommitForBranch(gitCtx git.GitContext, pipe
 		return nil, err
 	}
 
-	commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repo, branchName, "", "", 1)
+	commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repo, branchName, "", "", 1, gitMaterial.CheckoutLocation)
 
 	if commits == nil {
 		return nil, err
@@ -698,6 +708,8 @@ func (impl RepoManagerImpl) GetCommitMetadataForPipelineMaterial(gitCtx git.GitC
 		return nil, err
 	}
 
+	gitCtx = gitCtx.WithCredentials(gitMaterial.GitProvider.UserName, gitMaterial.GitProvider.Password).
+		WithCloningMode(impl.configuration.CloningMode)
 	// validate checkout status of gitMaterial
 	if !gitMaterial.CheckoutStatus {
 		impl.logger.Errorw("checkout not success", "gitMaterialId", gitMaterialId)
@@ -711,7 +723,6 @@ func (impl RepoManagerImpl) GetCommitMetadataForPipelineMaterial(gitCtx git.GitC
 		repoLock.Mutex.Unlock()
 		impl.locker.ReturnLocker(gitMaterial.Id)
 	}()
-
 	commits, err := impl.repositoryManager.ChangesSince(gitCtx, gitMaterial.CheckoutLocation, branchName, "", gitHash, 1)
 	if err != nil {
 		impl.logger.Errorw("error while fetching commit info", "pipelineMaterialId", pipelineMaterialId, "gitHash", gitHash, "err", err)
@@ -751,6 +762,10 @@ func (impl RepoManagerImpl) GetReleaseChanges(gitCtx git.GitContext, request *Re
 		repoLock.Mutex.Unlock()
 		impl.locker.ReturnLocker(gitMaterial.Id)
 	}()
+
+	gitCtx = gitCtx.WithCredentials(gitMaterial.GitProvider.UserName, gitMaterial.GitProvider.Password).
+		WithCloningMode(impl.configuration.CloningMode)
+
 	gitChanges, err := impl.repositoryManagerAnalytics.ChangesSinceByRepositoryForAnalytics(gitCtx, gitMaterial.CheckoutLocation, request.OldCommit, request.NewCommit)
 	if err != nil {
 		impl.logger.Errorw("error in computing changes", "req", request, "err", err)

pkg/git/Bean.go

@@ -19,7 +19,7 @@ package git
 import (
 	"encoding/json"
 	"fmt"
-	"github.com/devtron-labs/git-sensor/internal/sql"
+	"github.com/devtron-labs/git-sensor/internals/sql"
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing/object"
 	"io"

pkg/git/GitBaseManager.go

@@ -5,8 +5,8 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"github.com/devtron-labs/git-sensor/internal"
-	"github.com/devtron-labs/git-sensor/internal/sql"
+	"github.com/devtron-labs/git-sensor/internals"
+	"github.com/devtron-labs/git-sensor/internals/sql"
 	"github.com/devtron-labs/git-sensor/util"
 	"go.uber.org/zap"
 	"os"
@@ -29,8 +29,6 @@ type GitManager interface {
 	OpenRepoPlain(checkoutPath string) (*GitRepository, error)
 	// Init initializes a git repo
 	Init(gitCtx GitContext, rootDir string, remoteUrl string, isBare bool) error
-	// FetchDiffStatBetweenCommits returns the file stats reponse on executing git action
-	FetchDiffStatBetweenCommits(gitCtx GitContext, oldHash string, newHash string, rootDir string) (response, errMsg string, err error)
 }
 
 // GitManagerBase Base methods which will be available to all implementation of the parent interface
@@ -43,16 +41,19 @@ type GitManagerBase interface {
 	Checkout(gitCtx GitContext, rootDir, branch string) (response, errMsg string, err error)
 	// ConfigureSshCommand configures ssh in git repo
 	ConfigureSshCommand(gitCtx GitContext, rootDir string, sshPrivateKeyPath string) (response, errMsg string, err error)
+	//  FetchDiffStatBetweenCommits returns the file stats reponse on executing git action
+	FetchDiffStatBetweenCommits(gitCtx GitContext, oldHash string, newHash string, rootDir string) (response, errMsg string, err error)
 	// LogMergeBase get the commit diff between using a merge base strategy
 	LogMergeBase(gitCtx GitContext, rootDir, from string, to string) ([]*Commit, error)
+	ExecuteCustomCommand(gitContext GitContext, name string, arg ...string) (response, errMsg string, err error)
 }
 type GitManagerBaseImpl struct {
 	logger            *zap.SugaredLogger
-	conf              *internal.Configuration
+	conf              *internals.Configuration
 	commandTimeoutMap map[string]int
 }
 
-func NewGitManagerBaseImpl(logger *zap.SugaredLogger, config *internal.Configuration) *GitManagerBaseImpl {
+func NewGitManagerBaseImpl(logger *zap.SugaredLogger, config *internals.Configuration) *GitManagerBaseImpl {
 
 	commandTimeoutMap, err := parseCmdTimeoutJson(config)
 	if err != nil {
@@ -62,52 +63,36 @@ func NewGitManagerBaseImpl(logger *zap.SugaredLogger, config *internal.Configura
 	return &GitManagerBaseImpl{logger: logger, conf: config, commandTimeoutMap: commandTimeoutMap}
 }
 
-func parseCmdTimeoutJson(config *internal.Configuration) (map[string]int, error) {
-	commandTimeoutMap := make(map[string]int)
-	var err error
-	if config.CliCmdTimeoutJson != "" {
-		err = json.Unmarshal([]byte(config.CliCmdTimeoutJson), &commandTimeoutMap)
-	}
-	return commandTimeoutMap, err
-}
-
 type GitManagerImpl struct {
 	GitManager
 }
 
-func NewGitManagerImpl(configuration *internal.Configuration,
-	cliGitManager GitCliManager,
-	goGitManager GoGitSDKManager) GitManagerImpl {
+func NewGitManagerImpl(logger *zap.SugaredLogger, configuration *internals.Configuration) *GitManagerImpl {
 
+	baseImpl := NewGitManagerBaseImpl(logger, configuration)
 	if configuration.UseGitCli {
-		return GitManagerImpl{cliGitManager}
+		return &GitManagerImpl{
+			GitManager: NewGitCliManagerImpl(baseImpl, logger),
+		}
+
+	}
+	return &GitManagerImpl{
+		GitManager: NewGoGitSDKManagerImpl(baseImpl, logger),
 	}
-	return GitManagerImpl{goGitManager}
 }
 
-func (impl *GitManagerImpl) OpenNewRepo(gitCtx GitContext, location string, url string) (*GitRepository, error) {
-
-	r, err := impl.OpenRepoPlain(location)
-	if err != nil {
-		err = os.RemoveAll(location)
-		if err != nil {
-			return r, fmt.Errorf("error in cleaning checkout path: %s", err)
-		}
-		err = impl.Init(gitCtx, location, url, true)
-		if err != nil {
-			return r, fmt.Errorf("err in git init: %s", err)
-		}
-		r, err = impl.OpenRepoPlain(location)
-		if err != nil {
-			return r, fmt.Errorf("err in git init: %s", err)
-		}
+func parseCmdTimeoutJson(config *internals.Configuration) (map[string]int, error) {
+	commandTimeoutMap := make(map[string]int)
+	var err error
+	if config.CliCmdTimeoutJson != "" {
+		err = json.Unmarshal([]byte(config.CliCmdTimeoutJson), &commandTimeoutMap)
 	}
-	return r, nil
+	return commandTimeoutMap, err
 }
 
 func (impl *GitManagerBaseImpl) Fetch(gitCtx GitContext, rootDir string) (response, errMsg string, err error) {
 	impl.logger.Debugw("git fetch ", "location", rootDir)
-	cmd, cancel := impl.CreateCmdWithContext(gitCtx, "git", "-C", rootDir, "fetch", "origin", "--tags", "--force")
+	cmd, cancel := impl.createCmdWithContext(gitCtx, "git", "-C", rootDir, "fetch", "origin", "--tags", "--force")
 	defer cancel()
 	output, errMsg, err := impl.runCommandWithCred(cmd, gitCtx.Username, gitCtx.Password)
 	impl.logger.Debugw("fetch output", "root", rootDir, "opt", output, "errMsg", errMsg, "error", err)
@@ -116,7 +101,7 @@ func (impl *GitManagerBaseImpl) Fetch(gitCtx GitContext, rootDir string) (respon
 
 func (impl *GitManagerBaseImpl) Checkout(gitCtx GitContext, rootDir, branch string) (response, errMsg string, err error) {
 	impl.logger.Debugw("git checkout ", "location", rootDir)
-	cmd, cancel := impl.CreateCmdWithContext(gitCtx, "git", "-C", rootDir, "checkout", branch, "--force")
+	cmd, cancel := impl.createCmdWithContext(gitCtx, "git", "-C", rootDir, "checkout", branch, "--force")
 	defer cancel()
 	output, errMsg, err := impl.runCommand(cmd)
 	impl.logger.Debugw("checkout output", "root", rootDir, "opt", output, "errMsg", errMsg, "error", err)
@@ -133,7 +118,7 @@ func (impl *GitManagerBaseImpl) LogMergeBase(gitCtx GitContext, rootDir, from st
 	}
 	cmdArgs := []string{"-C", rootDir, "log", from + "..." + toCommitHash, "--date=iso-strict", GITFORMAT}
 	impl.logger.Debugw("git", cmdArgs)
-	cmd, cancel := impl.CreateCmdWithContext(gitCtx, "git", cmdArgs...)
+	cmd, cancel := impl.createCmdWithContext(gitCtx, "git", cmdArgs...)
 	defer cancel()
 	output, errMsg, err := impl.runCommand(cmd)
 	impl.logger.Debugw("root", rootDir, "opt", output, "errMsg", errMsg, "error", err)
@@ -181,7 +166,7 @@ func (impl *GitManagerBaseImpl) runCommand(cmd *exec.Cmd) (response, errMsg stri
 func (impl *GitManagerBaseImpl) ConfigureSshCommand(gitCtx GitContext, rootDir string, sshPrivateKeyPath string) (response, errMsg string, err error) {
 	impl.logger.Debugw("configuring ssh command on ", "location", rootDir)
 	coreSshCommand := fmt.Sprintf("ssh -i %s -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no", sshPrivateKeyPath)
-	cmd, cancel := impl.CreateCmdWithContext(gitCtx, "git", "-C", rootDir, "config", "core.sshCommand", coreSshCommand)
+	cmd, cancel := impl.createCmdWithContext(gitCtx, "git", "-C", rootDir, "config", "core.sshCommand", coreSshCommand)
 	defer cancel()
 	output, errMsg, err := impl.runCommand(cmd)
 	impl.logger.Debugw("configure ssh command output ", "root", rootDir, "opt", output, "errMsg", errMsg, "error", err)
@@ -277,15 +262,15 @@ func (impl *GitManagerBaseImpl) FetchDiffStatBetweenCommits(gitCtx GitContext, o
 		newHash = oldHash
 		oldHash = oldHash + "^"
 	}
-	cmd, cancel := impl.CreateCmdWithContext(gitCtx, "git", "-C", rootDir, "diff", "--numstat", oldHash, newHash)
+	cmd, cancel := impl.createCmdWithContext(gitCtx, "git", "-C", rootDir, "diff", "--numstat", oldHash, newHash)
 	defer cancel()
 
 	output, errMsg, err := impl.runCommandWithCred(cmd, gitCtx.Username, gitCtx.Password)
 	impl.logger.Debugw("root", rootDir, "opt", output, "errMsg", errMsg, "error", err)
 	return output, errMsg, err
 }
 
-func (impl *GitManagerBaseImpl) CreateCmdWithContext(ctx GitContext, name string, arg ...string) (*exec.Cmd, context.CancelFunc) {
+func (impl *GitManagerBaseImpl) createCmdWithContext(ctx GitContext, name string, arg ...string) (*exec.Cmd, context.CancelFunc) {
 	newCtx := ctx
 	cancel := func() {}
 
@@ -308,3 +293,10 @@ func (impl *GitManagerBaseImpl) getCommandTimeout(command string) int {
 	}
 	return timeout
 }
+
+func (impl *GitManagerBaseImpl) ExecuteCustomCommand(gitContext GitContext, name string, arg ...string) (response, errMsg string, err error) {
+	cmd, cancel := impl.createCmdWithContext(gitContext, name, arg...)
+	defer cancel()
+	output, errMsg, err := impl.runCommandWithCred(cmd, gitContext.Username, gitContext.Password)
+	return output, errMsg, err
+}