diff options
-rw-r--r-- | changelogs/unreleased/nil-committer-author.yml | 5 | ||||
-rw-r--r-- | internal/service/ref/util.go | 45 | ||||
-rw-r--r-- | internal/service/ref/util_test.go | 120 |
3 files changed, 149 insertions, 21 deletions
diff --git a/changelogs/unreleased/nil-committer-author.yml b/changelogs/unreleased/nil-committer-author.yml new file mode 100644 index 000000000..9d84a049e --- /dev/null +++ b/changelogs/unreleased/nil-committer-author.yml @@ -0,0 +1,5 @@ +--- +title: Fix nil commit author dereference +merge_request: 800 +author: +type: fixed diff --git a/internal/service/ref/util.go b/internal/service/ref/util.go index 562d5b27c..6d328e615 100644 --- a/internal/service/ref/util.go +++ b/internal/service/ref/util.go @@ -21,29 +21,30 @@ func parseRef(ref []byte) ([][]byte, error) { return elements, nil } -func buildLocalBranch(c *catfile.Batch, elements [][]byte) (*pb.FindLocalBranchResponse, error) { - target, err := log.GetCommitCatfile(c, string(elements[1])) - if err != nil { - return nil, err +func buildLocalBranch(name []byte, target *pb.GitCommit) *pb.FindLocalBranchResponse { + response := &pb.FindLocalBranchResponse{ + Name: name, + CommitId: target.Id, + CommitSubject: target.Subject, } - author := pb.FindLocalBranchCommitAuthor{ - Name: target.Author.Name, - Email: target.Author.Email, - Date: target.Author.Date, + + if author := target.Author; author != nil { + response.CommitAuthor = &pb.FindLocalBranchCommitAuthor{ + Name: author.Name, + Email: author.Email, + Date: author.Date, + } } - committer := pb.FindLocalBranchCommitAuthor{ - Name: target.Committer.Name, - Email: target.Committer.Email, - Date: target.Committer.Date, + + if committer := target.Committer; committer != nil { + response.CommitCommitter = &pb.FindLocalBranchCommitAuthor{ + Name: committer.Name, + Email: committer.Email, + Date: committer.Date, + } } - return &pb.FindLocalBranchResponse{ - Name: elements[0], - CommitId: target.Id, - CommitSubject: target.Subject, - CommitAuthor: &author, - CommitCommitter: &committer, - }, nil + return response } func buildBranch(c *catfile.Batch, elements [][]byte) (*pb.FindAllBranchesResponse_Branch, error) { @@ -79,11 +80,13 @@ func newFindLocalBranchesWriter(stream pb.Ref_FindLocalBranchesServer, c *catfil if err != nil { return err } - branch, err := buildLocalBranch(c, elements) + + target, err := log.GetCommitCatfile(c, string(elements[1])) if err != nil { return err } - branches = append(branches, branch) + + branches = append(branches, buildLocalBranch(elements[0], target)) } return stream.Send(&pb.FindLocalBranchesResponse{Branches: branches}) } diff --git a/internal/service/ref/util_test.go b/internal/service/ref/util_test.go new file mode 100644 index 000000000..f155ee7cc --- /dev/null +++ b/internal/service/ref/util_test.go @@ -0,0 +1,120 @@ +package ref + +import ( + "testing" + + "github.com/golang/protobuf/ptypes/timestamp" + "github.com/stretchr/testify/require" + pb "gitlab.com/gitlab-org/gitaly-proto/go" +) + +func TestBuildLocalBranch(t *testing.T) { + testCases := []struct { + desc string + in *pb.GitCommit + out *pb.FindLocalBranchResponse + }{ + { + desc: "all required fields present", + in: &pb.GitCommit{ + Id: "b83d6e391c22777fca1ed3012fce84f633d7fed0", + Subject: []byte("Merge branch 'branch-merged' into 'master'"), + Body: []byte("Merge branch 'branch-merged' into 'master'\r\n\r\nadds bar folder and branch-test text file to check Repository merged_to_root_ref method\r\n\r\n\r\n\r\nSee merge request !12"), + Author: &pb.CommitAuthor{ + Name: []byte("Job van der Voort"), + Email: []byte("job@gitlab.com"), + Date: ×tamp.Timestamp{Seconds: 1474987066}, + }, + Committer: &pb.CommitAuthor{ + Name: []byte("Job van der Voort"), + Email: []byte("job@gitlab.com"), + Date: ×tamp.Timestamp{Seconds: 1474987066}, + }, + ParentIds: []string{ + "1b12f15a11fc6e62177bef08f47bc7b5ce50b141", + "498214de67004b1da3d820901307bed2a68a8ef6", + }, + BodySize: 162, + }, + out: &pb.FindLocalBranchResponse{ + Name: []byte("my-branch"), + CommitId: "b83d6e391c22777fca1ed3012fce84f633d7fed0", + CommitSubject: []byte("Merge branch 'branch-merged' into 'master'"), + CommitAuthor: &pb.FindLocalBranchCommitAuthor{ + Name: []byte("Job van der Voort"), + Email: []byte("job@gitlab.com"), + Date: ×tamp.Timestamp{Seconds: 1474987066}, + }, + CommitCommitter: &pb.FindLocalBranchCommitAuthor{ + Name: []byte("Job van der Voort"), + Email: []byte("job@gitlab.com"), + Date: ×tamp.Timestamp{Seconds: 1474987066}, + }, + }, + }, + { + desc: "missing author", + in: &pb.GitCommit{ + Id: "b83d6e391c22777fca1ed3012fce84f633d7fed0", + Subject: []byte("Merge branch 'branch-merged' into 'master'"), + Body: []byte("Merge branch 'branch-merged' into 'master'\r\n\r\nadds bar folder and branch-test text file to check Repository merged_to_root_ref method\r\n\r\n\r\n\r\nSee merge request !12"), + Committer: &pb.CommitAuthor{ + Name: []byte("Job van der Voort"), + Email: []byte("job@gitlab.com"), + Date: ×tamp.Timestamp{Seconds: 1474987066}, + }, + ParentIds: []string{ + "1b12f15a11fc6e62177bef08f47bc7b5ce50b141", + "498214de67004b1da3d820901307bed2a68a8ef6", + }, + BodySize: 162, + }, + out: &pb.FindLocalBranchResponse{ + Name: []byte("my-branch"), + CommitId: "b83d6e391c22777fca1ed3012fce84f633d7fed0", + CommitSubject: []byte("Merge branch 'branch-merged' into 'master'"), + CommitAuthor: nil, + CommitCommitter: &pb.FindLocalBranchCommitAuthor{ + Name: []byte("Job van der Voort"), + Email: []byte("job@gitlab.com"), + Date: ×tamp.Timestamp{Seconds: 1474987066}, + }, + }, + }, + { + desc: "missing committer", + in: &pb.GitCommit{ + Id: "b83d6e391c22777fca1ed3012fce84f633d7fed0", + Subject: []byte("Merge branch 'branch-merged' into 'master'"), + Body: []byte("Merge branch 'branch-merged' into 'master'\r\n\r\nadds bar folder and branch-test text file to check Repository merged_to_root_ref method\r\n\r\n\r\n\r\nSee merge request !12"), + Author: &pb.CommitAuthor{ + Name: []byte("Job van der Voort"), + Email: []byte("job@gitlab.com"), + Date: ×tamp.Timestamp{Seconds: 1474987066}, + }, + ParentIds: []string{ + "1b12f15a11fc6e62177bef08f47bc7b5ce50b141", + "498214de67004b1da3d820901307bed2a68a8ef6", + }, + BodySize: 162, + }, + out: &pb.FindLocalBranchResponse{ + Name: []byte("my-branch"), + CommitId: "b83d6e391c22777fca1ed3012fce84f633d7fed0", + CommitSubject: []byte("Merge branch 'branch-merged' into 'master'"), + CommitAuthor: &pb.FindLocalBranchCommitAuthor{ + Name: []byte("Job van der Voort"), + Email: []byte("job@gitlab.com"), + Date: ×tamp.Timestamp{Seconds: 1474987066}, + }, + CommitCommitter: nil, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + require.Equal(t, *tc.out, *buildLocalBranch([]byte("my-branch"), tc.in)) + }) + } +} |