[scm-63] fix for empty commits and diff in PR (#228)

gitrpc/interface.go

@@ -43,5 +43,5 @@ type Interface interface {
 	/*
 	 * Merge services
 	 */
-	MergeBranch(ctx context.Context, in *MergeBranchParams) (string, error)
+	MergeBranch(ctx context.Context, in *MergeBranchParams) (MergeBranchOutput, error)
 }

gitrpc/internal/gitea/merge.go

@@ -273,3 +273,23 @@ func (g Adapter) GetDiffTree(ctx context.Context, repoPath, baseBranch, headBran
 
 	return out.String(), nil
 }
+
+// GetMergeBase checks and returns merge base of two branches and the reference used as base.
+func (g Adapter) GetMergeBase(ctx context.Context, repoPath, remote, base, head string) (string, string, error) {
+	if remote == "" {
+		remote = "origin"
+	}
+
+	if remote != "origin" {
+		tmpBaseName := git.RemotePrefix + remote + "/tmp_" + base
+		// Fetch commit into a temporary branch in order to be able to handle commits and tags
+		_, _, err := git.NewCommand(ctx, "fetch", "--no-tags", remote, "--",
+			base+":"+tmpBaseName).RunStdString(&git.RunOpts{Dir: repoPath})
+		if err == nil {
+			base = tmpBaseName
+		}
+	}
+
+	stdout, _, err := git.NewCommand(ctx, "merge-base", "--", base, head).RunStdString(&git.RunOpts{Dir: repoPath})
+	return strings.TrimSpace(stdout), base, err
+}

gitrpc/internal/service/interface.go

@@ -43,6 +43,7 @@ type GitAdapter interface {
 	CreateTemporaryRepoForPR(ctx context.Context, reposTempPath string, pr *types.PullRequest) (string, error)
 	Merge(ctx context.Context, pr *types.PullRequest, mergeMethod string, trackingBranch string,
 		tmpBasePath string, mergeMsg string, env []string) error
+	GetMergeBase(ctx context.Context, repoPath, remote, base, head string) (string, string, error)
 	GetDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string) (string, error)
 	RawDiff(ctx context.Context, repoPath, base, head string, w io.Writer, args ...string) error
 }

gitrpc/internal/service/merge.go

@@ -132,11 +132,16 @@ func (s MergeService) MergeBranch(
 		mergeMsg += "\n\n" + strings.TrimSpace(request.GetMessage())
 	}
 
+	// no error check needed, all branches are created on fly as ref to remote ones
+	baseCommitSHA, _, _ := s.adapter.GetMergeBase(ctx, tmpBasePath, "origin", baseBranch, trackingBranch)
+	headCommit, _ := s.adapter.GetCommit(ctx, tmpBasePath, trackingBranch)
+	headCommitSHA := headCommit.SHA
+
 	if err = s.adapter.Merge(ctx, pr, "merge", trackingBranch, tmpBasePath, mergeMsg, env); err != nil {
 		return nil, err
 	}
 
-	mergeCommitID, err := s.adapter.GetFullCommitID(ctx, tmpBasePath, baseBranch)
+	mergeCommitSHA, err := s.adapter.GetFullCommitID(ctx, tmpBasePath, baseBranch)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get full commit id for the new merge: %w", err)
 	}
@@ -151,7 +156,9 @@ func (s MergeService) MergeBranch(
 	}
 
 	return &rpc.MergeBranchResponse{
-		CommitId: mergeCommitID,
+		MergeSha: mergeCommitSHA,
+		BaseSha:  baseCommitSHA,
+		HeadSha:  headCommitSHA,
 	}, nil
 }
 

gitrpc/merge.go

@@ -10,6 +10,7 @@ import (
 	"github.com/harness/gitness/gitrpc/rpc"
 )
 
+// MergeBranchParams is input structure object for merging operation.
 type MergeBranchParams struct {
 	WriteParams
 	BaseBranch       string
@@ -21,9 +22,18 @@ type MergeBranchParams struct {
 	DeleteHeadBranch bool
 }
 
-func (c *Client) MergeBranch(ctx context.Context, params *MergeBranchParams) (string, error) {
+// MergeBranchResult is result object from merging and returns
+// base, head and commit sha.
+type MergeBranchOutput struct {
+	MergedSHA string
+	BaseSHA   string
+	HeadSHA   string
+}
+
+// MergeBranch merge head branch to base branch.
+func (c *Client) MergeBranch(ctx context.Context, params *MergeBranchParams) (MergeBranchOutput, error) {
 	if params == nil {
-		return "", ErrNoParamsProvided
+		return MergeBranchOutput{}, ErrNoParamsProvided
 	}
 
 	resp, err := c.mergeService.MergeBranch(ctx, &rpc.MergeBranchRequest{
@@ -36,7 +46,11 @@ func (c *Client) MergeBranch(ctx context.Context, params *MergeBranchParams) (st
 		DeleteHeadBranch: params.DeleteHeadBranch,
 	})
 	if err != nil {
-		return "", err
+		return MergeBranchOutput{}, err
 	}
-	return resp.CommitId, nil
+	return MergeBranchOutput{
+		MergedSHA: resp.GetMergeSha(),
+		BaseSHA:   resp.GetBaseSha(),
+		HeadSHA:   resp.GetHeadSha(),
+	}, nil
 }

gitrpc/params.go

@@ -0,0 +1,17 @@
+// Copyright 2022 Harness Inc. All rights reserved.
+// Use of this source code is governed by the Polyform Free Trial License
+// that can be found in the LICENSE.md file for this repository.
+
+package gitrpc
+
+type Repository interface {
+	GetGitUID() string
+}
+
+// CreateRPCReadParams creates base read parameters for gitrpc read operations.
+// IMPORTANT: repo is assumed to be not nil!
+func CreateRPCReadParams(repo Repository) ReadParams {
+	return ReadParams{
+		RepoUID: repo.GetGitUID(),
+	}
+}

gitrpc/proto/merge.proto

@@ -29,8 +29,9 @@ message MergeBranchRequest {
   bool delete_head_branch = 7;
 }
 
-// This comment is left unintentionally blank.
 message MergeBranchResponse {
-  // The merge commit the branch will be updated to. The caller can still abort the merge.
+  // The merge_sha is merge commit between head_sha and base_sha
-  string commit_id = 1;
+  string merge_sha = 1;
+  string base_sha  = 2;
+  string head_sha  = 3;
 }

gitrpc/rpc/diff.pb.go

@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go. DO NOT EDIT.
 // versions:
 // 	protoc-gen-go v1.28.1
-// 	protoc        v3.21.11
+// 	protoc        v3.21.9
 // source: diff.proto
 
 package rpc

gitrpc/rpc/diff_grpc.pb.go

@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go-grpc. DO NOT EDIT.
 // versions:
 // - protoc-gen-go-grpc v1.2.0
-// - protoc             v3.21.11
+// - protoc             v3.21.9
 // source: diff.proto
 
 package rpc

gitrpc/rpc/http.pb.go

@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go. DO NOT EDIT.
 // versions:
 // 	protoc-gen-go v1.28.1
-// 	protoc        v3.21.11
+// 	protoc        v3.21.9
 // source: http.proto
 
 package rpc
@@ -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/http_grpc.pb.go

@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go-grpc. DO NOT EDIT.
 // versions:
 // - protoc-gen-go-grpc v1.2.0
-// - protoc             v3.21.11
+// - protoc             v3.21.9
 // source: http.proto
 
 package rpc

gitrpc/rpc/merge.pb.go

@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go. DO NOT EDIT.
 // versions:
 // 	protoc-gen-go v1.28.1
-// 	protoc        v3.21.11
+// 	protoc        v3.21.9
 // source: merge.proto
 
 package rpc
@@ -122,14 +122,15 @@ func (x *MergeBranchRequest) GetDeleteHeadBranch() bool {
 	return false
 }
 
-// This comment is left unintentionally blank.
 type MergeBranchResponse struct {
 	state         protoimpl.MessageState
 	sizeCache     protoimpl.SizeCache
 	unknownFields protoimpl.UnknownFields
 
-	// The merge commit the branch will be updated to. The caller can still abort the merge.
+	// The merge_sha is merge commit between head_sha and base_sha
-	CommitId string `protobuf:"bytes,1,opt,name=commit_id,json=commitId,proto3" json:"commit_id,omitempty"`
+	MergeSha string `protobuf:"bytes,1,opt,name=merge_sha,json=mergeSha,proto3" json:"merge_sha,omitempty"`
+	BaseSha  string `protobuf:"bytes,2,opt,name=base_sha,json=baseSha,proto3" json:"base_sha,omitempty"`
+	HeadSha  string `protobuf:"bytes,3,opt,name=head_sha,json=headSha,proto3" json:"head_sha,omitempty"`
 }
 
 func (x *MergeBranchResponse) Reset() {
@@ -164,9 +165,23 @@ func (*MergeBranchResponse) Descriptor() ([]byte, []int) {
 	return file_merge_proto_rawDescGZIP(), []int{1}
 }
 
-func (x *MergeBranchResponse) GetCommitId() string {
+func (x *MergeBranchResponse) GetMergeSha() string {
 	if x != nil {
-		return x.CommitId
+		return x.MergeSha
+	}
+	return ""
+}
+
+func (x *MergeBranchResponse) GetBaseSha() string {
+	if x != nil {
+		return x.BaseSha
+	}
+	return ""
+}
+
+func (x *MergeBranchResponse) GetHeadSha() string {
+	if x != nil {
+		return x.HeadSha
 	}
 	return ""
 }
@@ -191,18 +206,22 @@ var file_merge_proto_rawDesc = []byte{
 	0x05, 0x66, 0x6f, 0x72, 0x63, 0x65, 0x12, 0x2c, 0x0a, 0x12, 0x64, 0x65, 0x6c, 0x65, 0x74, 0x65,
 	0x5f, 0x68, 0x65, 0x61, 0x64, 0x5f, 0x62, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x18, 0x07, 0x20, 0x01,
 	0x28, 0x08, 0x52, 0x10, 0x64, 0x65, 0x6c, 0x65, 0x74, 0x65, 0x48, 0x65, 0x61, 0x64, 0x42, 0x72,
-	0x61, 0x6e, 0x63, 0x68, 0x22, 0x32, 0x0a, 0x13, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x42, 0x72, 0x61,
+	0x61, 0x6e, 0x63, 0x68, 0x22, 0x68, 0x0a, 0x13, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x42, 0x72, 0x61,
-	0x6e, 0x63, 0x68, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x1b, 0x0a, 0x09, 0x63,
+	0x6e, 0x63, 0x68, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x1b, 0x0a, 0x09, 0x6d,
-	0x6f, 0x6d, 0x6d, 0x69, 0x74, 0x5f, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08,
+	0x65, 0x72, 0x67, 0x65, 0x5f, 0x73, 0x68, 0x61, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08,
-	0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74, 0x49, 0x64, 0x32, 0x52, 0x0a, 0x0c, 0x4d, 0x65, 0x72, 0x67,
+	0x6d, 0x65, 0x72, 0x67, 0x65, 0x53, 0x68, 0x61, 0x12, 0x19, 0x0a, 0x08, 0x62, 0x61, 0x73, 0x65,
-	0x65, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x12, 0x42, 0x0a, 0x0b, 0x4d, 0x65, 0x72, 0x67,
+	0x5f, 0x73, 0x68, 0x61, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x62, 0x61, 0x73, 0x65,
-	0x65, 0x42, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x12, 0x17, 0x2e, 0x72, 0x70, 0x63, 0x2e, 0x4d, 0x65,
+	0x53, 0x68, 0x61, 0x12, 0x19, 0x0a, 0x08, 0x68, 0x65, 0x61, 0x64, 0x5f, 0x73, 0x68, 0x61, 0x18,
-	0x72, 0x67, 0x65, 0x42, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74,
+	0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x68, 0x65, 0x61, 0x64, 0x53, 0x68, 0x61, 0x32, 0x52,
-	0x1a, 0x18, 0x2e, 0x72, 0x70, 0x63, 0x2e, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x42, 0x72, 0x61, 0x6e,
+	0x0a, 0x0c, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x12, 0x42,
-	0x63, 0x68, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, 0x42, 0x27, 0x5a, 0x25,
+	0x0a, 0x0b, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x42, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x12, 0x17, 0x2e,
-	0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x72, 0x6e, 0x65,
+	0x72, 0x70, 0x63, 0x2e, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x42, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x52,
-	0x73, 0x73, 0x2f, 0x67, 0x69, 0x74, 0x6e, 0x65, 0x73, 0x73, 0x2f, 0x67, 0x69, 0x74, 0x72, 0x70,
+	0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x18, 0x2e, 0x72, 0x70, 0x63, 0x2e, 0x4d, 0x65, 0x72,
-	0x63, 0x2f, 0x72, 0x70, 0x63, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33,
+	0x67, 0x65, 0x42, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65,
+	0x22, 0x00, 0x42, 0x27, 0x5a, 0x25, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d,
+	0x2f, 0x68, 0x61, 0x72, 0x6e, 0x65, 0x73, 0x73, 0x2f, 0x67, 0x69, 0x74, 0x6e, 0x65, 0x73, 0x73,
+	0x2f, 0x67, 0x69, 0x74, 0x72, 0x70, 0x63, 0x2f, 0x72, 0x70, 0x63, 0x62, 0x06, 0x70, 0x72, 0x6f,
+	0x74, 0x6f, 0x33,
 }
 
 var (

gitrpc/rpc/merge_grpc.pb.go

@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go-grpc. DO NOT EDIT.
 // versions:
 // - protoc-gen-go-grpc v1.2.0
-// - protoc             v3.21.11
+// - protoc             v3.21.9
 // source: merge.proto
 
 package rpc

gitrpc/rpc/operations.pb.go

@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go. DO NOT EDIT.
 // versions:
 // 	protoc-gen-go v1.28.1
-// 	protoc        v3.21.11
+// 	protoc        v3.21.9
 // source: operations.proto
 
 package rpc
@@ -239,6 +239,7 @@ type CommitFilesAction struct {
 	unknownFields protoimpl.UnknownFields
 
 	// Types that are assignable to Payload:
+	//
 	//	*CommitFilesAction_Header
 	//	*CommitFilesAction_Content
 	Payload isCommitFilesAction_Payload `protobuf_oneof:"payload"`
@@ -322,6 +323,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/operations_grpc.pb.go

@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go-grpc. DO NOT EDIT.
 // versions:
 // - protoc-gen-go-grpc v1.2.0
-// - protoc             v3.21.11
+// - protoc             v3.21.9
 // source: operations.proto
 
 package rpc

gitrpc/rpc/ref.pb.go

@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go. DO NOT EDIT.
 // versions:
 // 	protoc-gen-go v1.28.1
-// 	protoc        v3.21.11
+// 	protoc        v3.21.9
 // source: ref.proto
 
 package rpc

gitrpc/rpc/ref_grpc.pb.go

@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go-grpc. DO NOT EDIT.
 // versions:
 // - protoc-gen-go-grpc v1.2.0
-// - protoc             v3.21.11
+// - protoc             v3.21.9
 // source: ref.proto
 
 package rpc

gitrpc/rpc/repo.pb.go

@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go. DO NOT EDIT.
 // versions:
 // 	protoc-gen-go v1.28.1
-// 	protoc        v3.21.11
+// 	protoc        v3.21.9
 // source: repo.proto
 
 package rpc
@@ -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/repo_grpc.pb.go

@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go-grpc. DO NOT EDIT.
 // versions:
 // - protoc-gen-go-grpc v1.2.0
-// - protoc             v3.21.11
+// - protoc             v3.21.9
 // source: repo.proto
 
 package rpc

gitrpc/rpc/shared.pb.go

@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go. DO NOT EDIT.
 // versions:
 // 	protoc-gen-go v1.28.1
-// 	protoc        v3.21.11
+// 	protoc        v3.21.9
 // source: shared.proto
 
 package rpc
@@ -240,6 +240,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/merge.go

@@ -92,8 +92,8 @@ func (c *Controller) Merge(
 		// TODO: for forking merge title might be different?
 		mergeTitle := fmt.Sprintf("Merge branch '%s' of %s (#%d)", pr.SourceBranch, sourceRepo.Path, pr.Number)
 
-		// TODO: do we really want to do this in the DB transaction?
+		var mergeOutput gitrpc.MergeBranchOutput
-		sha, err = c.gitRPCClient.MergeBranch(ctx, &gitrpc.MergeBranchParams{
+		mergeOutput, err = c.gitRPCClient.MergeBranch(ctx, &gitrpc.MergeBranchParams{
 			WriteParams:      writeParams,
 			BaseBranch:       pr.TargetBranch,
 			HeadRepoUID:      sourceRepo.GitUID,
@@ -114,6 +114,9 @@ func (c *Controller) Merge(
 		pr.MergedBy = &session.Principal.ID
 		pr.State = enum.PullReqStateMerged
 
+		pr.MergeBaseSHA = &mergeOutput.BaseSHA
+		pr.MergeHeadSHA = &mergeOutput.HeadSHA
+
 		err = c.pullreqStore.Update(ctx, pr)
 		if err != nil {
 			return fmt.Errorf("failed to update pull request: %w", err)

internal/api/controller/pullreq/pr_commits.go

@@ -0,0 +1,65 @@
+// Copyright 2022 Harness Inc. All rights reserved.
+// Use of this source code is governed by the Polyform Free Trial License
+// that can be found in the LICENSE.md file for this repository.
+
+package pullreq
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/harness/gitness/gitrpc"
+	"github.com/harness/gitness/internal/api/controller"
+	"github.com/harness/gitness/internal/auth"
+	"github.com/harness/gitness/types"
+	"github.com/harness/gitness/types/enum"
+)
+
+// Commits lists all commits from pr head branch.
+func (c *Controller) Commits(
+	ctx context.Context,
+	session *auth.Session,
+	repoRef string,
+	pullreqNum int64,
+	filter *types.PaginationFilter,
+) ([]types.Commit, error) {
+	repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoEdit)
+	if err != nil {
+		return nil, fmt.Errorf("failed to acquire access to repo: %w", err)
+	}
+
+	pr, err := c.pullreqStore.FindByNumber(ctx, repo.ID, pullreqNum)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get pull request by number: %w", err)
+	}
+
+	gitRef := pr.SourceBranch
+	afterRef := pr.TargetBranch
+	if pr.State == enum.PullReqStateMerged {
+		gitRef = *pr.MergeHeadSHA
+		afterRef = *pr.MergeBaseSHA
+	}
+
+	rpcOut, err := c.gitRPCClient.ListCommits(ctx, &gitrpc.ListCommitsParams{
+		ReadParams: gitrpc.CreateRPCReadParams(repo),
+		GitREF:     gitRef,
+		After:      afterRef,
+		Page:       int32(filter.Page),
+		Limit:      int32(filter.Limit),
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	commits := make([]types.Commit, len(rpcOut.Commits))
+	for i := range rpcOut.Commits {
+		var commit *types.Commit
+		commit, err = controller.MapCommit(&rpcOut.Commits[i])
+		if err != nil {
+			return nil, fmt.Errorf("failed to map commit: %w", err)
+		}
+		commits[i] = *commit
+	}
+
+	return commits, nil
+}

internal/api/controller/pullreq/pr_diff.go

@@ -0,0 +1,52 @@
+// Copyright 2022 Harness Inc. All rights reserved.
+// Use of this source code is governed by the Polyform Free Trial License
+// that can be found in the LICENSE.md file for this repository.
+
+package pullreq
+
+import (
+	"context"
+	"fmt"
+	"io"
+
+	"github.com/harness/gitness/gitrpc"
+	"github.com/harness/gitness/internal/auth"
+	"github.com/harness/gitness/types/enum"
+)
+
+// RawDiff writes raw git diff to writer w.
+func (c *Controller) RawDiff(
+	ctx context.Context,
+	session *auth.Session,
+	repoRef string,
+	pullreqNum int64,
+	w io.Writer,
+) error {
+	repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoEdit)
+	if err != nil {
+		return fmt.Errorf("failed to acquire access to target repo: %w", err)
+	}
+
+	pr, err := c.pullreqStore.FindByNumber(ctx, repo.ID, pullreqNum)
+	if err != nil {
+		return fmt.Errorf("failed to get pull request by number: %w", err)
+	}
+
+	headRef := pr.TargetBranch
+	baseRef := pr.SourceBranch
+	if pr.State == enum.PullReqStateMerged {
+		if pr.MergeBaseSHA != nil {
+			baseRef = *pr.MergeBaseSHA
+		}
+		if pr.MergeHeadSHA != nil {
+			headRef = *pr.MergeHeadSHA
+		}
+	}
+
+	return c.gitRPCClient.RawDiff(ctx, &gitrpc.RawDiffParams{
+		ReadParams: gitrpc.CreateRPCReadParams(repo),
+		BaseRef:    baseRef,
+		HeadRef:    headRef,
+		MergeBase:  true,
+	}, w)
+}

internal/api/controller/repo/get_content.go

@@ -8,11 +8,12 @@ import (
 	"context"
 	"encoding/base64"
 	"fmt"
-	"time"
 
 	"github.com/harness/gitness/gitrpc"
 	apiauth "github.com/harness/gitness/internal/api/auth"
+	"github.com/harness/gitness/internal/api/controller"
 	"github.com/harness/gitness/internal/auth"
+	"github.com/harness/gitness/types"
 	"github.com/harness/gitness/types/enum"
 )
 
@@ -32,29 +33,11 @@ const (
 )
 
 type ContentInfo struct {
-	Type         ContentType `json:"type"`
+	Type         ContentType   `json:"type"`
-	SHA          string      `json:"sha"`
+	SHA          string        `json:"sha"`
-	Name         string      `json:"name"`
+	Name         string        `json:"name"`
-	Path         string      `json:"path"`
+	Path         string        `json:"path"`
-	LatestCommit *Commit     `json:"latest_commit,omitempty"`
+	LatestCommit *types.Commit `json:"latest_commit,omitempty"`
-}
-
-type Commit struct {
-	SHA       string    `json:"sha"`
-	Title     string    `json:"title"`
-	Message   string    `json:"message"`
-	Author    Signature `json:"author"`
-	Committer Signature `json:"committer"`
-}
-
-type Signature struct {
-	Identity Identity  `json:"identity"`
-	When     time.Time `json:"when"`
-}
-
-type Identity struct {
-	Name  string `json:"name"`
-	Email string `json:"email"`
 }
 
 type GetContentOutput struct {
@@ -274,7 +257,7 @@ func mapToContentInfo(node *gitrpc.TreeNode, commit *gitrpc.Commit) (*ContentInf
 
 	// parse commit only if available
 	if commit != nil {
-		res.LatestCommit, err = mapCommit(commit)
+		res.LatestCommit, err = controller.MapCommit(commit)
 		if err != nil {
 			return nil, err
 		}
@@ -283,44 +266,6 @@ func mapToContentInfo(node *gitrpc.TreeNode, commit *gitrpc.Commit) (*ContentInf
 	return res, nil
 }
 
-func mapCommit(c *gitrpc.Commit) (*Commit, error) {
-	if c == nil {
-		return nil, fmt.Errorf("commit is nil")
-	}
-
-	author, err := mapSignature(&c.Author)
-	if err != nil {
-		return nil, fmt.Errorf("failed to map author: %w", err)
-	}
-
-	committer, err := mapSignature(&c.Committer)
-	if err != nil {
-		return nil, fmt.Errorf("failed to map committer: %w", err)
-	}
-
-	return &Commit{
-		SHA:       c.SHA,
-		Title:     c.Title,
-		Message:   c.Message,
-		Author:    *author,
-		Committer: *committer,
-	}, nil
-}
-
-func mapSignature(s *gitrpc.Signature) (*Signature, error) {
-	if s == nil {
-		return nil, fmt.Errorf("signature is nil")
-	}
-
-	return &Signature{
-		Identity: Identity{
-			Name:  s.Identity.Name,
-			Email: s.Identity.Email,
-		},
-		When: s.When,
-	}, nil
-}
-
 func mapNodeModeToContentType(m gitrpc.TreeNodeMode) (ContentType, error) {
 	switch m {
 	case gitrpc.TreeNodeModeFile, gitrpc.TreeNodeModeExec:

internal/api/controller/repo/list_branches.go

@@ -10,15 +10,16 @@ import (
 
 	"github.com/harness/gitness/gitrpc"
 	apiauth "github.com/harness/gitness/internal/api/auth"
+	"github.com/harness/gitness/internal/api/controller"
 	"github.com/harness/gitness/internal/auth"
 	"github.com/harness/gitness/types"
 	"github.com/harness/gitness/types/enum"
 )
 
 type Branch struct {
-	Name   string  `json:"name"`
+	Name   string        `json:"name"`
-	SHA    string  `json:"sha"`
+	SHA    string        `json:"sha"`
-	Commit *Commit `json:"commit,omitempty"`
+	Commit *types.Commit `json:"commit,omitempty"`
 }
 
 /*
@@ -88,10 +89,10 @@ func mapToRPCSortOrder(o enum.Order) gitrpc.SortOrder {
 }
 
 func mapBranch(b gitrpc.Branch) (Branch, error) {
-	var commit *Commit
+	var commit *types.Commit
 	if b.Commit != nil {
 		var err error
-		commit, err = mapCommit(b.Commit)
+		commit, err = controller.MapCommit(b.Commit)
 		if err != nil {
 			return Branch{}, err
 		}

internal/api/controller/repo/list_commit_tags.go

@@ -10,19 +10,20 @@ import (
 
 	"github.com/harness/gitness/gitrpc"
 	apiauth "github.com/harness/gitness/internal/api/auth"
+	"github.com/harness/gitness/internal/api/controller"
 	"github.com/harness/gitness/internal/auth"
 	"github.com/harness/gitness/types"
 	"github.com/harness/gitness/types/enum"
 )
 
 type CommitTag struct {
-	Name        string     `json:"name"`
+	Name        string           `json:"name"`
-	SHA         string     `json:"sha"`
+	SHA         string           `json:"sha"`
-	IsAnnotated bool       `json:"is_annotated"`
+	IsAnnotated bool             `json:"is_annotated"`
-	Title       string     `json:"title,omitempty"`
+	Title       string           `json:"title,omitempty"`
-	Message     string     `json:"message,omitempty"`
+	Message     string           `json:"message,omitempty"`
-	Tagger      *Signature `json:"tagger,omitempty"`
+	Tagger      *types.Signature `json:"tagger,omitempty"`
-	Commit      *Commit    `json:"commit,omitempty"`
+	Commit      *types.Commit    `json:"commit,omitempty"`
 }
 
 /*
@@ -78,19 +79,19 @@ func mapToRPCTagSortOption(o enum.TagSortOption) gitrpc.TagSortOption {
 }
 
 func mapCommitTag(t gitrpc.CommitTag) (CommitTag, error) {
-	var commit *Commit
+	var commit *types.Commit
 	if t.Commit != nil {
 		var err error
-		commit, err = mapCommit(t.Commit)
+		commit, err = controller.MapCommit(t.Commit)
 		if err != nil {
 			return CommitTag{}, err
 		}
 	}
 
-	var tagger *Signature
+	var tagger *types.Signature
 	if t.Tagger != nil {
 		var err error
-		tagger, err = mapSignature(t.Tagger)
+		tagger, err = controller.MapSignature(t.Tagger)
 		if err != nil {
 			return CommitTag{}, err
 		}

internal/api/controller/repo/list_commits.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/harness/gitness/gitrpc"
 	apiauth "github.com/harness/gitness/internal/api/auth"
+	"github.com/harness/gitness/internal/api/controller"
 	"github.com/harness/gitness/internal/auth"
 	"github.com/harness/gitness/types"
 	"github.com/harness/gitness/types/enum"
@@ -19,7 +20,7 @@ import (
 * ListCommits lists the commits of a repo.
  */
 func (c *Controller) ListCommits(ctx context.Context, session *auth.Session,
-	repoRef string, gitRef string, filter *types.CommitFilter) ([]Commit, error) {
+	repoRef string, gitRef string, filter *types.CommitFilter) ([]types.Commit, error) {
 	repo, err := c.repoStore.FindByRef(ctx, repoRef)
 	if err != nil {
 		return nil, err
@@ -45,10 +46,10 @@ func (c *Controller) ListCommits(ctx context.Context, session *auth.Session,
 		return nil, err
 	}
 
-	commits := make([]Commit, len(rpcOut.Commits))
+	commits := make([]types.Commit, len(rpcOut.Commits))
 	for i := range rpcOut.Commits {
-		var commit *Commit
+		var commit *types.Commit
-		commit, err = mapCommit(&rpcOut.Commits[i])
+		commit, err = controller.MapCommit(&rpcOut.Commits[i])
 		if err != nil {
 			return nil, fmt.Errorf("failed to map commit: %w", err)
 		}

internal/api/controller/util.go

@@ -18,6 +18,9 @@ import (
 	"github.com/rs/zerolog/log"
 )
 
+// TODO: this file should be in gitrpc package and should accept
+// params as interface (contract)
+
 // CreateRPCWriteParams creates base write parameters for gitrpc write operations.
 // IMPORTANT: session & repo are assumed to be not nil!
 // TODO: this is duplicate function from repo controller, we need to see where this
@@ -50,3 +53,41 @@ func CreateRPCWriteParams(ctx context.Context, urlProvider *url.Provider,
 		EnvVars: envVars,
 	}, nil
 }
+
+func MapCommit(c *gitrpc.Commit) (*types.Commit, error) {
+	if c == nil {
+		return nil, fmt.Errorf("commit is nil")
+	}
+
+	author, err := MapSignature(&c.Author)
+	if err != nil {
+		return nil, fmt.Errorf("failed to map author: %w", err)
+	}
+
+	committer, err := MapSignature(&c.Committer)
+	if err != nil {
+		return nil, fmt.Errorf("failed to map committer: %w", err)
+	}
+
+	return &types.Commit{
+		SHA:       c.SHA,
+		Title:     c.Title,
+		Message:   c.Message,
+		Author:    *author,
+		Committer: *committer,
+	}, nil
+}
+
+func MapSignature(s *gitrpc.Signature) (*types.Signature, error) {
+	if s == nil {
+		return nil, fmt.Errorf("signature is nil")
+	}
+
+	return &types.Signature{
+		Identity: types.Identity{
+			Name:  s.Identity.Name,
+			Email: s.Identity.Email,
+		},
+		When: s.When,
+	}, nil
+}

internal/api/handler/pullreq/pr_commits.go

@@ -0,0 +1,51 @@
+// Copyright 2022 Harness Inc. All rights reserved.
+// Use of this source code is governed by the Polyform Free Trial License
+// that can be found in the LICENSE.md file for this repository.
+
+package pullreq
+
+import (
+	"net/http"
+
+	"github.com/harness/gitness/internal/api/controller/pullreq"
+	"github.com/harness/gitness/internal/api/render"
+	"github.com/harness/gitness/internal/api/request"
+	"github.com/harness/gitness/types"
+)
+
+// HandleCommits returns commits for PR.
+func HandleCommits(pullreqCtrl *pullreq.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
+		}
+
+		pullreqNumber, err := request.GetPullReqNumberFromPath(r)
+		if err != nil {
+			render.TranslatedUserError(w, err)
+			return
+		}
+
+		filter := &types.PaginationFilter{
+			Page:  request.ParsePage(r),
+			Limit: request.ParseLimit(r),
+		}
+
+		// gitref is Head branch in this case
+		commits, err := pullreqCtrl.Commits(ctx, session, repoRef, pullreqNumber, filter)
+		if err != nil {
+			render.TranslatedUserError(w, err)
+			return
+		}
+
+		// TODO: get last page indicator explicitly - current check is wrong in case len % limit == 0
+		isLastPage := len(commits) < filter.Limit
+		render.PaginationNoTotal(r, w, filter.Page, filter.Limit, isLastPage)
+		render.JSON(w, http.StatusOK, commits)
+	}
+}

internal/api/handler/pullreq/pr_diff.go

@@ -0,0 +1,40 @@
+// Copyright 2022 Harness Inc. All rights reserved.
+// Use of this source code is governed by the Polyform Free Trial License
+// that can be found in the LICENSE.md file for this repository.
+
+package pullreq
+
+import (
+	"net/http"
+
+	"github.com/harness/gitness/internal/api/controller/pullreq"
+	"github.com/harness/gitness/internal/api/render"
+	"github.com/harness/gitness/internal/api/request"
+)
+
+// HandleRawDiff returns raw git diff for PR.
+func HandleRawDiff(pullreqCtrl *pullreq.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
+		}
+
+		pullreqNumber, err := request.GetPullReqNumberFromPath(r)
+		if err != nil {
+			render.TranslatedUserError(w, err)
+			return
+		}
+
+		if err = pullreqCtrl.RawDiff(ctx, session, repoRef, pullreqNumber, w); err != nil {
+			render.TranslatedUserError(w, err)
+			return
+		}
+
+		w.WriteHeader(http.StatusOK)
+	}
+}

internal/api/openapi/pullreq.go

@@ -413,4 +413,27 @@ func pullReqOperations(reflector *openapi3.Reflector) {
 	_ = reflector.SetJSONResponse(&mergePullReqOp, new(usererror.Error), http.StatusUnprocessableEntity)
 	_ = reflector.Spec.AddOperation(http.MethodPost,
 		"/repos/{repo_ref}/pullreq/{pullreq_number}/merge", mergePullReqOp)
+
+	opListCommits := openapi3.Operation{}
+	opListCommits.WithTags("pullreq")
+	opListCommits.WithMapOfAnything(map[string]interface{}{"operationId": "listCommits"})
+	opListCommits.WithParameters(queryParameterPage, queryParameterLimit)
+	_ = reflector.SetRequest(&opListCommits, new(pullReqRequest), http.MethodGet)
+	_ = reflector.SetJSONResponse(&opListCommits, []types.Commit{}, http.StatusOK)
+	_ = reflector.SetJSONResponse(&opListCommits, new(usererror.Error), http.StatusInternalServerError)
+	_ = reflector.SetJSONResponse(&opListCommits, new(usererror.Error), http.StatusUnauthorized)
+	_ = reflector.SetJSONResponse(&opListCommits, new(usererror.Error), http.StatusForbidden)
+	_ = reflector.SetJSONResponse(&opListCommits, new(usererror.Error), http.StatusNotFound)
+	_ = reflector.Spec.AddOperation(http.MethodGet, "/repos/{repo_ref}/pullreq/{pullreq_number}/commits", opListCommits)
+
+	opRawDiff := openapi3.Operation{}
+	opRawDiff.WithTags("pullreq")
+	opRawDiff.WithMapOfAnything(map[string]interface{}{"operationId": "rawDiff"})
+	_ = reflector.SetRequest(&opRawDiff, new(pullReqRequest), http.MethodGet)
+	_ = reflector.SetStringResponse(&opRawDiff, http.StatusOK, "text/plain")
+	_ = reflector.SetJSONResponse(&opRawDiff, new(usererror.Error), http.StatusInternalServerError)
+	_ = reflector.SetJSONResponse(&opRawDiff, new(usererror.Error), http.StatusUnauthorized)
+	_ = reflector.SetJSONResponse(&opRawDiff, new(usererror.Error), http.StatusForbidden)
+	_ = reflector.SetJSONResponse(&opRawDiff, new(usererror.Error), http.StatusNotFound)
+	_ = reflector.Spec.AddOperation(http.MethodGet, "/repos/{repo_ref}/pullreq/{pullreq_number}/diff", opRawDiff)
 }

internal/api/openapi/repo.go

@@ -374,7 +374,7 @@ func repoOperations(reflector *openapi3.Reflector) {
 	opListCommits.WithParameters(queryParameterGitRef, queryParameterAfterCommits,
 		queryParameterPage, queryParameterLimit)
 	_ = reflector.SetRequest(&opListCommits, new(listCommitsRequest), http.MethodGet)
-	_ = reflector.SetJSONResponse(&opListCommits, []repo.Commit{}, http.StatusOK)
+	_ = reflector.SetJSONResponse(&opListCommits, []types.Commit{}, http.StatusOK)
 	_ = reflector.SetJSONResponse(&opListCommits, new(usererror.Error), http.StatusInternalServerError)
 	_ = reflector.SetJSONResponse(&opListCommits, new(usererror.Error), http.StatusUnauthorized)
 	_ = reflector.SetJSONResponse(&opListCommits, new(usererror.Error), http.StatusForbidden)

internal/api/request/git.go

@@ -64,7 +64,9 @@ func ParseTagFilter(r *http.Request) *types.TagFilter {
 func ParseCommitFilter(r *http.Request) *types.CommitFilter {
 	return &types.CommitFilter{
 		After: QueryParamOrDefault(r, QueryParamAfter, ""),
-		Page:  ParsePage(r),
+		PaginationFilter: types.PaginationFilter{
-		Limit: ParseLimit(r),
+			Page:  ParsePage(r),
+			Limit: ParseLimit(r),
+		},
 	}
 }

internal/router/api.go

@@ -249,6 +249,8 @@ func SetupPullReq(r chi.Router, pullreqCtrl *pullreq.Controller) {
 				r.Post("/", handlerpullreq.HandleReviewSubmit(pullreqCtrl))
 			})
 			r.Post("/merge", handlerpullreq.HandleMerge(pullreqCtrl))
+			r.Get("/diff", handlerpullreq.HandleRawDiff(pullreqCtrl))
+			r.Get("/commits", handlerpullreq.HandleCommits(pullreqCtrl))
 		})
 	})
 }

internal/store/database/pullreq.go

@@ -226,6 +226,8 @@ func (s *PullReqStore) Update(ctx context.Context, pr *types.PullReq) error {
 		,pullreq_merged_by = :pullreq_merged_by
 		,pullreq_merged = :pullreq_merged
 		,pullreq_merge_strategy = :pullreq_merge_strategy
+		,pullreq_merge_head_sha = :pullreq_merge_head_sha
+		,pullreq_merge_base_sha = :pullreq_merge_base_sha
 	WHERE pullreq_id = :pullreq_id AND pullreq_version = :pullreq_version - 1`
 
 	db := dbtx.GetAccessor(ctx, s.db)

mocks/mock_client.go

@@ -8,10 +8,9 @@ import (
 	context "context"
 	reflect "reflect"
 
+	gomock "github.com/golang/mock/gomock"
 	user "github.com/harness/gitness/internal/api/controller/user"
 	types "github.com/harness/gitness/types"
-
-	gomock "github.com/golang/mock/gomock"
 )
 
 // MockClient is a mock of Client interface.

types/git.go

@@ -4,15 +4,24 @@
 
 package types
 
-import "github.com/harness/gitness/types/enum"
+import (
+	"time"
+
+	"github.com/harness/gitness/types/enum"
+)
 
 const NilSHA = "0000000000000000000000000000000000000000"
 
+// PaginationFilter stores pagination query parameters.
+type PaginationFilter struct {
+	Page  int `json:"page"`
+	Limit int `json:"limit"`
+}
+
 // CommitFilter stores commit query parameters.
 type CommitFilter struct {
+	PaginationFilter
 	After string `json:"after"`
-	Page  int    `json:"page"`
-	Limit int    `json:"limit"`
 }
 
 // BranchFilter stores branch query parameters.
@@ -32,3 +41,21 @@ type TagFilter struct {
 	Page  int                `json:"page"`
 	Size  int                `json:"size"`
 }
+
+type Commit struct {
+	SHA       string    `json:"sha"`
+	Title     string    `json:"title"`
+	Message   string    `json:"message"`
+	Author    Signature `json:"author"`
+	Committer Signature `json:"committer"`
+}
+
+type Signature struct {
+	Identity Identity  `json:"identity"`
+	When     time.Time `json:"when"`
+}
+
+type Identity struct {
+	Name  string `json:"name"`
+	Email string `json:"email"`
+}

types/repo.go

@@ -38,6 +38,10 @@ type Repository struct {
 	GitURL string `db:"-" json:"git_url"`
 }
 
+func (r Repository) GetGitUID() string {
+	return r.GitUID
+}
+
 // RepoFilter stores repo query parameters.
 type RepoFilter struct {
 	Page  int           `json:"page"`