dmitri.shuralyov.com/service/change/...

gerritapi: determine merged commit safely

Use the CurrentRevision of the CL to determine the merge commit,
rather than unsafely parsing it out of the human-readable message.

Background

Up until recently, all "autogenerated:gerrit:merged"-tagged messages
that I've encountered on the Gerrit instance at https://go-review.googlesource.com
have started with the 46-byte-long "Change has been successfully
cherry-picked as " string, followed by a 40-byte-long commit hash.

I didn't originally see a way to retrieve the merged commit from the
API response directly, so I implemented an unsafe string slice
operation that assumed the familiar message format.

A CL that used merge submit type rather than cherry-pick was submitted
at https://go-review.googlesource.com/c/go/+/164238/1#message-e75ad5d4bec9069a005bdc9efad6fbe8af8eb935
and generated a "autogenerated:gerrit:merged"-tagged message with
a different format, causing a panic.

By now, I've found a way to use the Gerrit API to determine the merge
commit. When a CL has an "autogenerated:gerrit:merged"-tagged messaged,
the CurrentRevision of the CL is the merge commit. Use it, since it's
more robust and safe.
dmitshur committed 6 months ago commit c25fb47d71b3e3fdf73798c58fffb56fa8be2921
gerritapi/gerritapi.go
@@ -255,11 +255,13 @@ func (s service) GetDiff(ctx context.Context, repo string, id uint64, opt *chang
 
 func (s service) ListTimeline(ctx context.Context, repo string, id uint64, opt *change.ListTimelineOptions) ([]interface{}, error) {
 	// TODO: Pagination. Respect opt.Start and opt.Length, if given.
 
 	project := project(repo)
-	chg, resp, err := s.cl.Changes.GetChangeDetail(fmt.Sprintf("%s~%d", project, id), nil)
+	chg, resp, err := s.cl.Changes.GetChangeDetail(fmt.Sprintf("%s~%d", project, id), &gerrit.ChangeOptions{
+		AdditionalFields: []string{"CURRENT_REVISION"},
+	})
 	if err != nil {
 		if resp != nil && resp.StatusCode == http.StatusNotFound {
 			return nil, os.ErrNotExist
 		}
 		return nil, err
@@ -282,11 +284,11 @@ func (s service) ListTimeline(ctx context.Context, repo string, id uint64, opt *
 			case "gerrit:merged":
 				timeline = append(timeline, change.TimelineItem{
 					Actor:     s.gerritUser(message.Author),
 					CreatedAt: message.Date.Time,
 					Payload: change.MergedEvent{
-						CommitID: message.Message[46:86], // TODO: Make safer.
+						CommitID: chg.CurrentRevision,
 						RefName:  chg.Branch,
 					},
 				})
 			case "gerrit:newPatchSet":
 				// Parse a new patchset message, check if it has comments.