Merge pull request #93 from devtron-labs/changes-since-refactoring

chore: code refactoring

pkg/RepoManages.go

@@ -199,7 +199,8 @@ func (impl RepoManagerImpl) updatePipelineMaterialCommit(gitCtx git.GitContext,
 			WithCloningMode(impl.configuration.CloningMode)
 
 		fetchCount := impl.configuration.GitHistoryCount
-		commits, err := impl.repositoryManager.ChangesSince(gitCtx, material.CheckoutLocation, pipelineMaterial.Value, "", "", fetchCount)
+		var repository *git.GitRepository
+		commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repository, pipelineMaterial.Value, "", "", fetchCount, material.CheckoutLocation, true)
 		//commits, err := impl.FetchChanges(pipelineMaterial.Id, "", "", 0)
 		if err == nil {
 			impl.logger.Infow("commits found", "commit", commits)
@@ -353,11 +354,13 @@ func (impl RepoManagerImpl) checkoutMaterial(gitCtx git.GitContext, material *sq
 	gitCtx = gitCtx.WithCredentials(userName, password).
 		WithCloningMode(impl.configuration.CloningMode)
 
-	checkoutPath, checkoutLocationForFetching, err := impl.repositoryManager.GetCheckoutPathAndLocation(gitCtx, material, gitProvider.Url)
+	checkoutPath, _, _, err := impl.repositoryManager.GetCheckoutLocationFromGitUrl(material, gitCtx.CloningMode)
 	if err != nil {
 		return material, err
 	}
 
+	checkoutLocationForFetching := impl.repositoryManager.GetCheckoutLocation(gitCtx, material, gitProvider.Url, checkoutPath)
+
 	err = impl.repositoryManager.Add(gitCtx, material.GitProviderId, checkoutPath, material.Url, gitProvider.AuthMode, gitProvider.SshPrivateKey)
 	if err == nil {
 		material.CheckoutLocation = checkoutLocationForFetching
@@ -669,7 +672,7 @@ func (impl RepoManagerImpl) GetLatestCommitForBranch(gitCtx git.GitContext, pipe
 		return nil, err
 	}
 
-	commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repo, branchName, "", "", 1, gitMaterial.CheckoutLocation)
+	commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repo, branchName, "", "", 1, gitMaterial.CheckoutLocation, false)
 
 	if commits == nil {
 		return nil, err
@@ -719,7 +722,8 @@ 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)
+	var repository *git.GitRepository
+	commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repository, branchName, "", gitHash, 1, gitMaterial.CheckoutLocation, true)
 	if err != nil {
 		impl.logger.Errorw("error while fetching commit info", "pipelineMaterialId", pipelineMaterialId, "gitHash", gitHash, "err", err)
 		return nil, err

pkg/git/RepositoryManager.go

@@ -43,15 +43,13 @@ type RepositoryManager interface {
 	Add(gitCtx GitContext, gitProviderId int, location, url string, authMode sql.AuthMode, sshPrivateKeyContent string) error
 	GetSshPrivateKeyPath(gitCtx GitContext, gitProviderId int, location, url string, authMode sql.AuthMode, sshPrivateKeyContent string) (string, error)
 	FetchRepo(gitCtx GitContext, location string) error
-	GetLocationForMaterial(material *sql.GitMaterial, cloningMode string) (location string, httpMatched bool, shMatched bool, err error)
-	GetCheckoutPathAndLocation(gitCtx GitContext, material *sql.GitMaterial, url string) (string, string, error)
+	GetCheckoutLocationFromGitUrl(material *sql.GitMaterial, cloningMode string) (location string, httpMatched bool, shMatched bool, err error)
+	GetCheckoutLocation(gitCtx GitContext, material *sql.GitMaterial, url, checkoutPath string) string
 	TrimLastGitCommit(gitCommits []*GitCommitBase, count int) []*GitCommitBase
 	// Clean cleans a directory
 	Clean(cloneDir string) error
-	// ChangesSince given the checkput path, retrieves the latest commits for the gt repo existing on the path
-	ChangesSince(gitCtx GitContext, checkoutPath string, branch string, from string, to string, count int) ([]*GitCommitBase, error)
 	// ChangesSinceByRepository returns the latest commits list for the given range and count for an existing repo
-	ChangesSinceByRepository(gitCtx GitContext, repository *GitRepository, branch string, from string, to string, count int, checkoutPath string) ([]*GitCommitBase, error)
+	ChangesSinceByRepository(gitCtx GitContext, repository *GitRepository, branch string, from string, to string, count int, checkoutPath string, openNewGitRepo bool) ([]*GitCommitBase, error)
 	// GetCommitMetadata retrieves the commit metadata for given hash
 	GetCommitMetadata(gitCtx GitContext, checkoutPath, commitHash string) (*GitCommitBase, error)
 	// GetCommitForTag retrieves the commit metadata for given tag
@@ -84,7 +82,7 @@ func (impl *RepositoryManagerImpl) IsSpaceAvailableOnDisk() bool {
 	return availableSpace > int64(impl.configuration.MinLimit)*1024*1024
 }
 
-func (impl *RepositoryManagerImpl) GetLocationForMaterial(material *sql.GitMaterial, cloningMode string) (location string, httpMatched bool, shMatched bool, err error) {
+func (impl *RepositoryManagerImpl) GetCheckoutLocationFromGitUrl(material *sql.GitMaterial, cloningMode string) (location string, httpMatched bool, shMatched bool, err error) {
 	//gitRegex := `/(?:git|ssh|https?|git@[-\w.]+):(\/\/)?(.*?)(\.git)(\/?|\#[-\d\w._]+?)$/`
 	httpsRegex := `^https.*`
 	httpsMatched, err := regexp.MatchString(httpsRegex, material.Url)
@@ -104,15 +102,8 @@ func (impl *RepositoryManagerImpl) GetLocationForMaterial(material *sql.GitMater
 	return "", httpsMatched, sshMatched, fmt.Errorf("unsupported format url %s", material.Url)
 }
 
-func (impl *RepositoryManagerImpl) GetCheckoutPathAndLocation(gitCtx GitContext, material *sql.GitMaterial, url string) (string, string, error) {
-	var checkoutPath string
-	var checkoutLocationForFetching string
-	checkoutPath, _, _, err := impl.GetLocationForMaterial(material, gitCtx.CloningMode)
-	if err != nil {
-		return checkoutPath, checkoutLocationForFetching, err
-	}
-	checkoutLocationForFetching = checkoutPath
-	return checkoutPath, checkoutLocationForFetching, nil
+func (impl *RepositoryManagerImpl) GetCheckoutLocation(gitCtx GitContext, material *sql.GitMaterial, url, checkoutPath string) string {
+	return checkoutPath
 }
 
 func (impl *RepositoryManagerImpl) Add(gitCtx GitContext, gitProviderId int, location, url string, authMode sql.AuthMode, sshPrivateKeyContent string) error {
@@ -239,12 +230,23 @@ func (impl *RepositoryManagerImpl) GetCommitMetadata(gitCtx GitContext, checkout
 
 // from -> old commit
 // to -> new commit
-func (impl *RepositoryManagerImpl) ChangesSinceByRepository(gitCtx GitContext, repository *GitRepository, branch string, from string, to string, count int, checkoutPath string) ([]*GitCommitBase, error) {
+func (impl *RepositoryManagerImpl) ChangesSinceByRepository(gitCtx GitContext, repository *GitRepository, branch string, from string, to string, count int, checkoutPath string, openNewGitRepo bool) ([]*GitCommitBase, error) {
 	// fix for azure devops (manual trigger webhook bases pipeline) :
 	// branch name comes as 'refs/heads/master', we need to extract actual branch name out of it.
 	// https://stackoverflow.com/questions/59956206/how-to-get-a-branch-name-with-a-slash-in-azure-devops
 
 	var err error
+	if count == 0 {
+		count = impl.configuration.GitHistoryCount
+	}
+
+	if openNewGitRepo {
+		repository, err = impl.gitManager.OpenRepoPlain(checkoutPath)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	start := time.Now()
 	defer func() {
 		util.TriggerGitOperationMetrics("changesSinceByRepository", start, err)
@@ -332,25 +334,6 @@ func (impl *RepositoryManagerImpl) TrimLastGitCommit(gitCommits []*GitCommitBase
 	return gitCommits
 }
 
-func (impl *RepositoryManagerImpl) ChangesSince(gitCtx GitContext, checkoutPath string, branch string, from string, to string, count int) ([]*GitCommitBase, error) {
-	var err error
-	start := time.Now()
-	defer func() {
-		util.TriggerGitOperationMetrics("changesSince", start, err)
-	}()
-	if count == 0 {
-		count = impl.configuration.GitHistoryCount
-	}
-	r, err := impl.gitManager.OpenRepoPlain(checkoutPath)
-	if err != nil {
-		return nil, err
-	}
-	///---------------------
-	return impl.ChangesSinceByRepository(gitCtx, r, branch, from, to, count, checkoutPath)
-	///----------------------
-
-}
-
 func (impl *RepositoryManagerImpl) CreateSshFileIfNotExistsAndConfigureSshCommand(gitCtx GitContext, location string, gitProviderId int, sshPrivateKeyContent string) (string, error) {
 	// add private key
 	var err error

pkg/git/Util.go

@@ -46,9 +46,8 @@ const (
 
 func GetProjectName(url string) string {
 	//if url = https://github.com/devtron-labs/git-sensor.git then it will return git-sensor
-	projName := strings.Split(url, ".")[1]
-	projectName := projName[strings.LastIndex(projName, "/")+1:]
-	return projectName
+	url = url[strings.LastIndex(url, "/")+1:]
+	return strings.TrimSuffix(url, ".git")
 }
 func GetCheckoutPath(url string, cloneLocation string) string {
 	//url= https://github.com/devtron-labs/git-sensor.git cloneLocation= git-base/1/github.com/prakash100198

pkg/git/Watcher.go

@@ -199,7 +199,7 @@ func (impl GitWatcherImpl) pollGitMaterialAndNotify(material *sql.GitMaterial) e
 		// there might be the case if ssh private key gets flush from disk, so creating and single retrying in this case
 		if gitProvider.AuthMode == sql.AUTH_MODE_SSH {
 			if strings.Contains(material.CheckoutLocation, "/.git") {
-				location, _, _, err = impl.repositoryManager.GetLocationForMaterial(material, gitCtx.CloningMode)
+				location, _, _, err = impl.repositoryManager.GetCheckoutLocationFromGitUrl(material, gitCtx.CloningMode)
 				if err != nil {
 					impl.logger.Errorw("error in getting clone location ", "material", material, "err", err)
 					return err
@@ -246,7 +246,7 @@ func (impl GitWatcherImpl) pollGitMaterialAndNotify(material *sql.GitMaterial) e
 			lastSeenHash = material.LastSeenHash
 		}
 		fetchCount := impl.configuration.GitHistoryCount
-		commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repo, material.Value, lastSeenHash, "", fetchCount, checkoutLocation)
+		commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repo, material.Value, lastSeenHash, "", fetchCount, checkoutLocation, false)
 		if err != nil {
 			material.Errored = true
 			material.ErrorMsg = err.Error()