[fix] diff-stats api returns total commits and files changed in compare branches (#323)

gitrpc/diff.go

@@ -12,6 +12,8 @@ import (
 
 	"github.com/harness/gitness/gitrpc/internal/streamio"
 	"github.com/harness/gitness/gitrpc/rpc"
+
+	"golang.org/x/sync/errgroup"
 )
 
 type DiffParams struct {
@@ -85,3 +87,68 @@ func (c *Client) DiffShortStat(ctx context.Context, params *DiffParams) (DiffSho
 		Deletions: int(stat.GetDeletions()),
 	}, nil
 }
+
+type DiffStatsOutput struct {
+	Commits      int
+	FilesChanged int
+}
+
+func (c *Client) DiffStats(ctx context.Context, params *DiffParams) (DiffStatsOutput, error) {
+	// declare variables which will be used in go routines,
+	// no need for atomic operations because writing and reading variable
+	// doesn't happen at the same time
+	var (
+		totalCommits int
+		totalFiles   int
+	)
+
+	errGroup, groupCtx := errgroup.WithContext(ctx)
+
+	errGroup.Go(func() error {
+		// read total commits
+
+		options := &GetCommitDivergencesParams{
+			ReadParams: params.ReadParams,
+			Requests: []CommitDivergenceRequest{
+				{
+					From: params.HeadRef,
+					To:   params.BaseRef,
+				},
+			},
+		}
+
+		rpcOutput, err := c.GetCommitDivergences(groupCtx, options)
+		if err != nil {
+			return fmt.Errorf("failed to count pull request commits: %w", err)
+		}
+		if len(rpcOutput.Divergences) > 0 {
+			totalCommits = int(rpcOutput.Divergences[0].Ahead)
+		}
+		return nil
+	})
+
+	errGroup.Go(func() error {
+		// read short stat
+		stat, err := c.DiffShortStat(groupCtx, &DiffParams{
+			ReadParams: params.ReadParams,
+			BaseRef:    params.BaseRef,
+			HeadRef:    params.HeadRef,
+			MergeBase:  true, // must be true, because commitDivergences use tripple dot notation
+		})
+		if err != nil {
+			return fmt.Errorf("failed to count pull request file changes: %w", err)
+		}
+		totalFiles = stat.Files
+		return nil
+	})
+
+	err := errGroup.Wait()
+	if err != nil {
+		return DiffStatsOutput{}, err
+	}
+
+	return DiffStatsOutput{
+		Commits:      totalCommits,
+		FilesChanged: totalFiles,
+	}, nil
+}

gitrpc/interface.go

@@ -47,6 +47,7 @@ type Interface interface {
 	 */
 	RawDiff(ctx context.Context, in *DiffParams, w io.Writer) error
 	DiffShortStat(ctx context.Context, params *DiffParams) (DiffShortStatOutput, error)
+	DiffStats(ctx context.Context, params *DiffParams) (DiffStatsOutput, error)
 
 	/*
 	 * Merge services

gitrpc/rpc/http.pb.go

@@ -151,6 +151,7 @@ type ServicePackRequest struct {
 	// Depending on the service the matching base type has to be passed
 	//
 	// Types that are assignable to Base:
+	//
 	//	*ServicePackRequest_ReadBase
 	//	*ServicePackRequest_WriteBase
 	Base isServicePackRequest_Base `protobuf_oneof:"base"`

gitrpc/rpc/operations.pb.go

@@ -247,6 +247,7 @@ type CommitFilesAction struct {
 	unknownFields protoimpl.UnknownFields
 
 	// Types that are assignable to Payload:
+	//
 	//	*CommitFilesAction_Header
 	//	*CommitFilesAction_Content
 	Payload isCommitFilesAction_Payload `protobuf_oneof:"payload"`
@@ -330,6 +331,7 @@ type CommitFilesRequest struct {
 	unknownFields protoimpl.UnknownFields
 
 	// Types that are assignable to Payload:
+	//
 	//	*CommitFilesRequest_Header
 	//	*CommitFilesRequest_Action
 	Payload isCommitFilesRequest_Payload `protobuf_oneof:"payload"`

gitrpc/rpc/repo.pb.go

@@ -130,6 +130,7 @@ type CreateRepositoryRequest struct {
 	unknownFields protoimpl.UnknownFields
 
 	// Types that are assignable to Data:
+	//
 	//	*CreateRepositoryRequest_Header
 	//	*CreateRepositoryRequest_File
 	Data isCreateRepositoryRequest_Data `protobuf_oneof:"data"`

gitrpc/rpc/shared.pb.go

@@ -298,6 +298,7 @@ type FileUpload struct {
 	unknownFields protoimpl.UnknownFields
 
 	// Types that are assignable to Data:
+	//
 	//	*FileUpload_Header
 	//	*FileUpload_Chunk
 	Data isFileUpload_Data `protobuf_oneof:"data"`

internal/api/controller/pullreq/pr_find.go

@@ -14,8 +14,6 @@ import (
 	"github.com/harness/gitness/internal/auth"
 	"github.com/harness/gitness/types"
 	"github.com/harness/gitness/types/enum"
-
-	"golang.org/x/sync/errgroup"
 )
 
 // Find returns a pull request from the provided repository.
@@ -39,83 +37,32 @@ func (c *Controller) Find(
 		return nil, err
 	}
 
-	pr.Stats.Commits, pr.Stats.FilesChanged, err = c.getStats(ctx, repo, pr)
+	baseRef := pr.TargetBranch
+	headRef := pr.SourceSHA
+	if pr.MergeBaseSHA != nil {
+		baseRef = *pr.MergeBaseSHA
+	}
+	if pr.MergeHeadSHA != nil {
+		headRef = *pr.MergeHeadSHA
+	}
+
+	output, err := c.gitRPCClient.DiffStats(ctx, &gitrpc.DiffParams{
+		ReadParams: gitrpc.CreateRPCReadParams(repo),
+		BaseRef:    baseRef,
+		HeadRef:    headRef,
+	})
 	if err != nil {
 		return nil, err
 	}
 
+	pr.Stats.DiffStats.Commits = output.Commits
+	pr.Stats.DiffStats.FilesChanged = output.FilesChanged
+
 	updateMergeStatus(pr)
 
 	return pr, nil
 }
 
-func (c *Controller) getStats(
-	ctx context.Context,
-	repo *types.Repository,
-	pr *types.PullReq,
-) (int, int, error) {
-	// declare variables which will be used in go routines,
-	// no need for atomic operations because writing and reading variable
-	// doesn't happen at the same time
-	var (
-		totalCommits int
-		totalFiles   int
-	)
-
-	gitRef := pr.SourceBranch
-	afterRef := pr.TargetBranch
-	if pr.State == enum.PullReqStateMerged {
-		gitRef = *pr.MergeHeadSHA
-		afterRef = *pr.MergeBaseSHA
-	}
-
-	errGroup, groupCtx := errgroup.WithContext(ctx)
-
-	errGroup.Go(func() error {
-		// read total commits
-		options := &gitrpc.GetCommitDivergencesParams{
-			ReadParams: gitrpc.CreateRPCReadParams(repo),
-			Requests: []gitrpc.CommitDivergenceRequest{
-				{
-					From: gitRef,
-					To:   afterRef,
-				},
-			},
-		}
-
-		rpcOutput, err := c.gitRPCClient.GetCommitDivergences(groupCtx, options)
-		if err != nil {
-			return fmt.Errorf("failed to count pull request commits: %w", err)
-		}
-		if len(rpcOutput.Divergences) > 0 {
-			totalCommits = int(rpcOutput.Divergences[0].Ahead)
-		}
-		return nil
-	})
-
-	errGroup.Go(func() error {
-		// read short stat
-		stat, err := c.gitRPCClient.DiffShortStat(groupCtx, &gitrpc.DiffParams{
-			ReadParams: gitrpc.CreateRPCReadParams(repo),
-			BaseRef:    afterRef,
-			HeadRef:    gitRef,
-			MergeBase:  true,
-		})
-		if err != nil {
-			return fmt.Errorf("failed to count pull request file changes: %w", err)
-		}
-		totalFiles = stat.Files
-		return nil
-	})
-
-	err := errGroup.Wait()
-	if err != nil {
-		return 0, 0, err
-	}
-
-	return totalCommits, totalFiles, nil
-}
-
 func updateMergeStatus(pr *types.PullReq) {
 	mc := ""
 	if pr.MergeConflicts != nil {

internal/api/controller/repo/diff.go

@@ -13,6 +13,7 @@ import (
 	apiauth "github.com/harness/gitness/internal/api/auth"
 	"github.com/harness/gitness/internal/api/usererror"
 	"github.com/harness/gitness/internal/auth"
+	"github.com/harness/gitness/types"
 	"github.com/harness/gitness/types/enum"
 )
 
@@ -65,3 +66,38 @@ func parseDiffPath(path string) (CompareInfo, error) {
 		MergeBase: strings.Contains(path, "..."),
 	}, nil
 }
+
+func (c *Controller) DiffStats(
+	ctx context.Context,
+	session *auth.Session,
+	repoRef string,
+	path string,
+) (types.DiffStats, error) {
+	repo, err := c.repoStore.FindByRef(ctx, repoRef)
+	if err != nil {
+		return types.DiffStats{}, err
+	}
+
+	if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, enum.PermissionRepoView, false); err != nil {
+		return types.DiffStats{}, err
+	}
+
+	info, err := parseDiffPath(path)
+	if err != nil {
+		return types.DiffStats{}, err
+	}
+
+	output, err := c.gitRPCClient.DiffStats(ctx, &gitrpc.DiffParams{
+		ReadParams: gitrpc.CreateRPCReadParams(repo),
+		BaseRef:    info.BaseRef,
+		HeadRef:    info.HeadRef,
+	})
+	if err != nil {
+		return types.DiffStats{}, err
+	}
+
+	return types.DiffStats{
+		Commits:      output.Commits,
+		FilesChanged: output.FilesChanged,
+	}, nil
+}

internal/api/handler/repo/diff.go

@@ -31,3 +31,26 @@ func HandleRawDiff(repoCtrl *repo.Controller) http.HandlerFunc {
 		}
 	}
 }
+
+// HandleDiffStats how diff statistics of two commits, branches or tags.
+func HandleDiffStats(repoCtrl *repo.Controller) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		ctx := r.Context()
+		session, _ := request.AuthSessionFrom(ctx)
+		repoRef, err := request.GetRepoRefFromPath(r)
+		if err != nil {
+			render.TranslatedUserError(w, err)
+			return
+		}
+
+		path := request.GetOptionalRemainderFromPath(r)
+
+		output, err := repoCtrl.DiffStats(ctx, session, repoRef, path)
+		if err != nil {
+			render.TranslatedUserError(w, err)
+			return
+		}
+
+		render.JSON(w, http.StatusOK, output)
+	}
+}

internal/api/openapi/repo.go

@@ -506,4 +506,14 @@ func repoOperations(reflector *openapi3.Reflector) {
 	_ = reflector.SetJSONResponse(&opMergeCheck, new(usererror.Error), http.StatusUnauthorized)
 	_ = reflector.SetJSONResponse(&opMergeCheck, new(usererror.Error), http.StatusForbidden)
 	_ = reflector.Spec.AddOperation(http.MethodPost, "/repos/{repo_ref}/merge-check/{range}", opMergeCheck)
+
+	opDiffStats := openapi3.Operation{}
+	opDiffStats.WithTags("repository")
+	opDiffStats.WithMapOfAnything(map[string]interface{}{"operationId": "diffStats"})
+	_ = reflector.SetRequest(&opDiffStats, new(getRawDiffRequest), http.MethodGet)
+	_ = reflector.SetJSONResponse(&opDiffStats, new(types.DiffStats), http.StatusOK)
+	_ = reflector.SetJSONResponse(&opDiffStats, new(usererror.Error), http.StatusInternalServerError)
+	_ = reflector.SetJSONResponse(&opDiffStats, new(usererror.Error), http.StatusUnauthorized)
+	_ = reflector.SetJSONResponse(&opDiffStats, new(usererror.Error), http.StatusForbidden)
+	_ = reflector.Spec.AddOperation(http.MethodGet, "/repos/{repo_ref}/diff-stats/{range}", opDiffStats)
 }

internal/router/api.go

@@ -218,6 +218,9 @@ func setupRepos(r chi.Router, repoCtrl *repo.Controller, pullreqCtrl *pullreq.Co
 			r.Route("/merge-check", func(r chi.Router) {
 				r.Post("/*", handlerrepo.HandleMergeCheck(repoCtrl))
 			})
+			r.Route("/diff-stats", func(r chi.Router) {
+				r.Get("/*", handlerrepo.HandleDiffStats(repoCtrl))
+			})
 
 			SetupPullReq(r, pullreqCtrl)
 

internal/store/database/pullreq.go

@@ -477,8 +477,10 @@ func mapPullReq(pr *pullReq) *types.PullReq {
 		Merger:         nil,
 		Stats: types.PullReqStats{
 			Conversations: pr.CommentCount,
-			Commits:       0,
-			FilesChanged:  0,
+			DiffStats: types.DiffStats{
+				Commits:      0,
+				FilesChanged: 0,
+			},
 		},
 	}
 }

types/pullreq.go

@@ -50,11 +50,16 @@ type PullReq struct {
 	Stats  PullReqStats   `json:"stats"`
 }
 
-// PullReqStats shows total number of conversations, commits and modified files.
+// DiffStats shows total number of commits and modified files.
+type DiffStats struct {
+	Commits      int `json:"commits"`
+	FilesChanged int `json:"files_changed"`
+}
+
+// PullReqStats shows Diff statistics and number of conversations.
 type PullReqStats struct {
-	Conversations int `json:"conversations"`
-	Commits       int `json:"commits"`
-	FilesChanged  int `json:"files_changed"`
+	DiffStats
+	Conversations int `json:"conversations,omitempty"`
 }
 
 // PullReqFilter stores pull request query parameters.