Repository: google/go-github
Pull request #3392 (2024-12-17 10:57:47)

fix: DownloadReleaseAsset handles renamed repository
This fixes issue #3081, such that `Repositories.DownloadReleaseAsset` correctly handles scenarios where a non-`nil` `followRedirectsClient` argument has been passed, and the specified repository has been renamed to a new name.

Previously, in such scenarios...

1. The initial `GET /repos/<OWNER>/<REPO>/releases/assets/<ASSET ID>` responds 301 w/ a `Location: <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>`.
2. The `followRedirectsClient` issues a `GET <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>` with a `Accept: */*` header.
3. `GET <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>` responds 200 with a JSON response body (rather than 302 w/ a `Location: <MEDIA SERVER ASSET URL>` header) due to the presence of the `Accept: */*` header (and the absence of a `Accept: application/octet-stream` header) from step 2.

The following `curl` offers an example of correct redirect follows & correct `Accept: application/octet-stream`-preservation in this scenario:

```
curl \
  --verbose \
  --location \
  --header "Accept: application/octet-stream" \
  https://api.github.com/repos/mterwill/go-github-issue-demo/releases/assets/151970555
```

Now, as per this change:

1. The initial `GET /repos/<OWNER>/<REPO>/releases/assets/<ASSET ID>` responds 301 w/ a `Location: <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>`.
2. The `followRedirectsClient` issues a `GET <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>` with a `Accept: application/octet-stream` header [as required by the GH API](https://docs.github.com/en/enterprise-cloud@latest/rest/releases/assets?apiVersion=2022-11-28).
3. `GET <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>` responds 302 w/ a `Location: <MEDIA SERVER ASSET URL>`.
4. The `followRedirectsClient` follows the redirects to `GET <MEDIA SERVER ASSET URL>` w/ a `Accept: application/octet-stream` header, and `GET <MEDIA SERVER ASSET URL>` responds 200 w/ the asset contents.

google-cla[bot]: Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this [failed invocation](https://github.com/google/go-github/pull/3392/checks?check_run_id=34528008811) of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/google/go-github/pull/3392?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 92.21%. Comparing base [(`2b8c7fa`)](https://app.codecov.io/gh/google/go-github/commit/2b8c7fa86fcd1cb4a89a44c8e7b5a49fe4be8a45?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) to head [(`cd05a32`)](https://app.codecov.io/gh/google/go-github/commit/cd05a32d71ea486bda66a460d0e9934b235bfbf7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).
> Report is 200 commits behind head on master.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #3392      +/-   ##
==========================================
- Coverage   97.72%   92.21%   -5.51%     
==========================================
  Files         153      173      +20     
  Lines       13390    14770    +1380     
==========================================
+ Hits        13085    13620     +535     
- Misses        215     1060     +845     
  Partials       90       90              
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/google/go-github/pull/3392?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).


Patch:
diff --git a/github/git_commits.go b/github/git_commits.go
index fdb3d26c..d7ed3ecb 100644
--- a/github/git_commits.go
+++ b/github/git_commits.go
@@ -49,7 +49,6 @@ type Commit struct {
 	Message      *string                `json:"message,omitempty"`
 	Tree         *Tree                  `json:"tree,omitempty"`
 	Parents      []*Commit              `json:"parents,omitempty"`
-	Stats        *CommitStats           `json:"stats,omitempty"`
 	HTMLURL      *string                `json:"html_url,omitempty"`
 	URL          *string                `json:"url,omitempty"`
 	Verification *SignatureVerification `json:"verification,omitempty"`
diff --git a/github/github-accessors.go b/github/github-accessors.go
index 2d174140..0800c02c 100644
--- a/github/github-accessors.go
+++ b/github/github-accessors.go
@@ -3542,14 +3542,6 @@ func (c *Commit) GetSHA() string {
 	return *c.SHA
 }
 
-// GetStats returns the Stats field.
-func (c *Commit) GetStats() *CommitStats {
-	if c == nil {
-		return nil
-	}
-	return c.Stats
-}
-
 // GetTree returns the Tree field.
 func (c *Commit) GetTree() *Tree {
 	if c == nil {
diff --git a/github/issues_timeline.go b/github/issues_timeline.go
index ffd9b6fe..903f5b89 100644
--- a/github/issues_timeline.go
+++ b/github/issues_timeline.go
@@ -97,6 +97,14 @@ type Timeline struct {
 	//     reviewed
 	//       The pull request was reviewed.
 	//
+	//     review_requested
+	//       The actor requested a review from a user or team.
+	//       Reviewer and Requester/RequestedTeam will be populated.
+	//
+	//     review_request_removed
+	//       The actor removed a review request from a user or team.
+	//       Reviewer and Requester/RequestedTeam will be populated.
+	//
 	//     subscribed
 	//       The actor subscribed to receive notifications for an issue.
 	//
diff --git a/github/repos_releases.go b/github/repos_releases.go
index 7231db6d..6023f632 100644
--- a/github/repos_releases.go
+++ b/github/repos_releases.go
@@ -333,9 +333,10 @@ func (s *RepositoriesService) GetReleaseAsset(ctx context.Context, owner, repo s
 // of the io.ReadCloser. Exactly one of rc and redirectURL will be zero.
 //
 // followRedirectsClient can be passed to download the asset from a redirected
-// location. Passing http.DefaultClient is recommended unless special circumstances
-// exist, but it's possible to pass any http.Client. If nil is passed the
-// redirectURL will be returned instead.
+// location. Specifying any http.Client is possible, but passing http.DefaultClient
+// is recommended, except when the specified repository is private, in which case
+// it's necessary to pass an http.Client that performs authenticated requests.
+// If nil is passed the redirectURL will be returned instead.
 //
 // GitHub API docs: https://docs.github.com/rest/releases/assets#get-a-release-asset
 //
@@ -387,7 +388,7 @@ func (s *RepositoriesService) downloadReleaseAssetFromURL(ctx context.Context, f
 		return nil, err
 	}
 	req = withContext(ctx, req)
-	req.Header.Set("Accept", "*/*")
+	req.Header.Set("Accept", defaultMediaType)
 	resp, err := followRedirectsClient.Do(req)
 	if err != nil {
 		return nil, err


Test patch:
diff --git a/github/git_commits_test.go b/github/git_commits_test.go
index a58a4d98..f0ad44b6 100644
--- a/github/git_commits_test.go
+++ b/github/git_commits_test.go
@@ -71,11 +71,6 @@ func TestCommit_Marshal(t *testing.T) {
 			Truncated: Ptr(false),
 		},
 		Parents: nil,
-		Stats: &CommitStats{
-			Additions: Ptr(1),
-			Deletions: Ptr(1),
-			Total:     Ptr(1),
-		},
 		HTMLURL: Ptr("h"),
 		URL:     Ptr("u"),
 		Verification: &SignatureVerification{
@@ -118,11 +113,6 @@ func TestCommit_Marshal(t *testing.T) {
 			],
 			"truncated": false
 		},
-		"stats": {
-			"additions": 1,
-			"deletions": 1,
-			"total": 1
-		},
 		"html_url": "h",
 		"url": "u",
 		"verification": {
diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go
index 3cf970d3..4b5c9dcb 100644
--- a/github/github-accessors_test.go
+++ b/github/github-accessors_test.go
@@ -4614,14 +4614,6 @@ func TestCommit_GetSHA(tt *testing.T) {
 	c.GetSHA()
 }
 
-func TestCommit_GetStats(tt *testing.T) {
-	tt.Parallel()
-	c := &Commit{}
-	c.GetStats()
-	c = nil
-	c.GetStats()
-}
-
 func TestCommit_GetTree(tt *testing.T) {
 	tt.Parallel()
 	c := &Commit{}
diff --git a/github/github-stringify_test.go b/github/github-stringify_test.go
index e9d8f29d..6fda6a90 100644
--- a/github/github-stringify_test.go
+++ b/github/github-stringify_test.go
@@ -283,14 +283,13 @@ func TestCommit_String(t *testing.T) {
 		Committer:    &CommitAuthor{},
 		Message:      Ptr(""),
 		Tree:         &Tree{},
-		Stats:        &CommitStats{},
 		HTMLURL:      Ptr(""),
 		URL:          Ptr(""),
 		Verification: &SignatureVerification{},
 		NodeID:       Ptr(""),
 		CommentCount: Ptr(0),
 	}
-	want := `github.Commit{SHA:"", Author:github.CommitAuthor{}, Committer:github.CommitAuthor{}, Message:"", Tree:github.Tree{}, Stats:github.CommitStats{}, HTMLURL:"", URL:"", Verification:github.SignatureVerification{}, NodeID:"", CommentCount:0}`
+	want := `github.Commit{SHA:"", Author:github.CommitAuthor{}, Committer:github.CommitAuthor{}, Message:"", Tree:github.Tree{}, HTMLURL:"", URL:"", Verification:github.SignatureVerification{}, NodeID:"", CommentCount:0}`
 	if got := v.String(); got != want {
 		t.Errorf("Commit.String = %v, want %v", got, want)
 	}
diff --git a/github/issues_timeline_test.go b/github/issues_timeline_test.go
index 1e88f500..ed764948 100644
--- a/github/issues_timeline_test.go
+++ b/github/issues_timeline_test.go
@@ -255,3 +255,91 @@ func TestTimeline_Marshal(t *testing.T) {
 
 	testJSONMarshal(t, u, want)
 }
+
+func TestTimeline_ReviewRequests(t *testing.T) {
+	t.Parallel()
+	client, mux, _ := setup(t)
+
+	mux.HandleFunc("/repos/example-org/example-repo/issues/3/timeline", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		fmt.Fprint(w, `[{
+		    "id": 1234567890,
+		    "url": "http://example.com/timeline/1",
+		    "actor": {
+			"login": "actor-user",
+			"id": 1
+		    },
+		    "event": "review_requested",
+		    "created_at": "2006-01-02T15:04:05Z",
+		    "requested_reviewer": {
+			"login": "reviewer-user",
+			"id": 2
+		    },
+		    "review_requester": {
+			"login": "requester-user",
+			"id": 1
+		    }
+		},
+		{
+		    "id": 1234567891,
+		    "url": "http://example.com/timeline/2",
+		    "actor": {
+			"login": "actor-user",
+			"id": 1
+		    },
+		    "event": "review_request_removed",
+		    "created_at": "2006-01-02T15:04:05Z",
+		    "requested_reviewer": {
+			"login": "reviewer-user",
+			"id": 2
+		    }
+		}]`)
+	})
+
+	ctx := context.Background()
+	events, _, err := client.Issues.ListIssueTimeline(ctx, "example-org", "example-repo", 3, nil)
+	if err != nil {
+		t.Errorf("Issues.ListIssueTimeline returned error: %v", err)
+	}
+
+	want := []*Timeline{
+		{
+			ID:  Ptr(int64(1234567890)),
+			URL: Ptr("http://example.com/timeline/1"),
+			Actor: &User{
+				Login: Ptr("actor-user"),
+				ID:    Ptr(int64(1)),
+			},
+			Event:     Ptr("review_requested"),
+			CreatedAt: &Timestamp{referenceTime},
+			Reviewer: &User{
+				Login: Ptr("reviewer-user"),
+				ID:    Ptr(int64(2)),
+			},
+			Requester: &User{
+				Login: Ptr("requester-user"),
+				ID:    Ptr(int64(1)),
+			},
+		},
+		{
+			ID:  Ptr(int64(1234567891)),
+			URL: Ptr("http://example.com/timeline/2"),
+			Actor: &User{
+				Login: Ptr("actor-user"),
+				ID:    Ptr(int64(1)),
+			},
+			Event:     Ptr("review_request_removed"),
+			CreatedAt: &Timestamp{referenceTime},
+			Reviewer: &User{
+				Login: Ptr("reviewer-user"),
+				ID:    Ptr(int64(2)),
+			},
+		},
+	}
+
+	if !cmp.Equal(events, want) {
+		t.Errorf("Issues.ListIssueTimeline review request events = %+v, want %+v", events, want)
+		diff := cmp.Diff(events, want)
+		t.Errorf("Difference: %s", diff)
+	}
+}
diff --git a/github/repos_commits_test.go b/github/repos_commits_test.go
index fb99a75a..eba7e752 100644
--- a/github/repos_commits_test.go
+++ b/github/repos_commits_test.go
@@ -714,11 +714,6 @@ func TestBranchCommit_Marshal(t *testing.T) {
 				Truncated: Ptr(false),
 			},
 			Parents: nil,
-			Stats: &CommitStats{
-				Additions: Ptr(1),
-				Deletions: Ptr(1),
-				Total:     Ptr(1),
-			},
 			HTMLURL: Ptr("h"),
 			URL:     Ptr("u"),
 			Verification: &SignatureVerification{
@@ -765,11 +760,6 @@ func TestBranchCommit_Marshal(t *testing.T) {
 				],
 				"truncated": false
 			},
-			"stats": {
-				"additions": 1,
-				"deletions": 1,
-				"total": 1
-			},
 			"html_url": "h",
 			"url": "u",
 			"verification": {
diff --git a/github/repos_contents_test.go b/github/repos_contents_test.go
index b50e32fc..e75a00e6 100644
--- a/github/repos_contents_test.go
+++ b/github/repos_contents_test.go
@@ -894,11 +894,6 @@ func TestRepositoryContentResponse_Marshal(t *testing.T) {
 				Truncated: Ptr(false),
 			},
 			Parents: nil,
-			Stats: &CommitStats{
-				Additions: Ptr(1),
-				Deletions: Ptr(1),
-				Total:     Ptr(1),
-			},
 			HTMLURL: Ptr("h"),
 			URL:     Ptr("u"),
 			Verification: &SignatureVerification{
@@ -958,11 +953,6 @@ func TestRepositoryContentResponse_Marshal(t *testing.T) {
 				],
 				"truncated": false
 			},
-			"stats": {
-				"additions": 1,
-				"deletions": 1,
-				"total": 1
-			},
 			"html_url": "h",
 			"url": "u",
 			"verification": {
diff --git a/github/repos_releases_test.go b/github/repos_releases_test.go
index 7c9b9e96..011e8cf6 100644
--- a/github/repos_releases_test.go
+++ b/github/repos_releases_test.go
@@ -489,7 +489,7 @@ func TestRepositoriesService_DownloadReleaseAsset_FollowRedirect(t *testing.T) {
 	})
 	mux.HandleFunc("/yo", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
-		testHeader(t, r, "Accept", "*/*")
+		testHeader(t, r, "Accept", defaultMediaType)
 		w.Header().Set("Content-Type", "application/octet-stream")
 		w.Header().Set("Content-Disposition", "attachment; filename=hello-world.txt")
 		fmt.Fprint(w, "Hello World")
@@ -511,6 +511,48 @@ func TestRepositoriesService_DownloadReleaseAsset_FollowRedirect(t *testing.T) {
 	}
 }
 
+func TestRepositoriesService_DownloadReleaseAsset_FollowMultipleRedirects(t *testing.T) {
+	t.Parallel()
+	client, mux, _ := setup(t)
+
+	mux.HandleFunc("/repos/o/r/releases/assets/1", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json; charset=utf-8")
+		testMethod(t, r, "GET")
+		testHeader(t, r, "Accept", defaultMediaType)
+		// /yo, below will be served as baseURLPath/yo
+		http.Redirect(w, r, baseURLPath+"/yo", http.StatusMovedPermanently)
+	})
+	mux.HandleFunc("/yo", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/html;charset=utf-8")
+		testMethod(t, r, "GET")
+		testHeader(t, r, "Accept", defaultMediaType)
+		// /yo2, below will be served as baseURLPath/yo2
+		http.Redirect(w, r, baseURLPath+"/yo2", http.StatusFound)
+	})
+	mux.HandleFunc("/yo2", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/octet-stream")
+		w.Header().Set("Content-Disposition", "attachment; filename=hello-world.txt")
+		testMethod(t, r, "GET")
+		testHeader(t, r, "Accept", defaultMediaType)
+		fmt.Fprint(w, "Hello World")
+	})
+
+	ctx := context.Background()
+	reader, _, err := client.Repositories.DownloadReleaseAsset(ctx, "o", "r", 1, http.DefaultClient)
+	if err != nil {
+		t.Errorf("Repositories.DownloadReleaseAsset returned error: %v", err)
+	}
+	content, err := io.ReadAll(reader)
+	if err != nil {
+		t.Errorf("Reading Repositories.DownloadReleaseAsset returned error: %v", err)
+	}
+	reader.Close()
+	want := []byte("Hello World")
+	if !bytes.Equal(want, content) {
+		t.Errorf("Repositories.DownloadReleaseAsset returned %+v, want %+v", content, want)
+	}
+}
+
 func TestRepositoriesService_DownloadReleaseAsset_FollowRedirectToError(t *testing.T) {
 	t.Parallel()
 	client, mux, _ := setup(t)
@@ -523,7 +565,7 @@ func TestRepositoriesService_DownloadReleaseAsset_FollowRedirectToError(t *testi
 	})
 	mux.HandleFunc("/yo", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
-		testHeader(t, r, "Accept", "*/*")
+		testHeader(t, r, "Accept", defaultMediaType)
 		w.WriteHeader(http.StatusNotFound)
 	})