Improve DownloadContents and DownloadContentsWithMeta methods

github/repos_contents.go

@@ -139,17 +139,24 @@ func (s *RepositoriesService) GetReadme(ctx context.Context, owner, repo string,
 func (s *RepositoriesService) DownloadContents(ctx context.Context, owner, repo, filepath string, opts *RepositoryContentGetOptions) (io.ReadCloser, *Response, error) {
 	dir := path.Dir(filepath)
 	filename := path.Base(filepath)
+	fileContent, _, resp, err := s.GetContents(ctx, owner, repo, filepath, opts)
+	if err == nil && fileContent != nil {
+		content, err := fileContent.GetContent()
+		if err == nil && content != "" {
+			return io.NopCloser(strings.NewReader(content)), resp, nil
+		}
+	}
+
 	_, dirContents, resp, err := s.GetContents(ctx, owner, repo, dir, opts)
 	if err != nil {
 		return nil, resp, err
 	}
 
 	for _, contents := range dirContents {
-		if *contents.Name == filename {
-			if contents.DownloadURL == nil || *contents.DownloadURL == "" {
+		if contents.GetName() == filename {
+			if contents.GetDownloadURL() == "" {
 				return nil, resp, fmt.Errorf("no download link found for %s", filepath)
 			}
-
 			dlReq, err := http.NewRequestWithContext(ctx, http.MethodGet, *contents.DownloadURL, nil)
 			if err != nil {
 				return nil, resp, err
@@ -181,17 +188,24 @@ func (s *RepositoriesService) DownloadContents(ctx context.Context, owner, repo,
 func (s *RepositoriesService) DownloadContentsWithMeta(ctx context.Context, owner, repo, filepath string, opts *RepositoryContentGetOptions) (io.ReadCloser, *RepositoryContent, *Response, error) {
 	dir := path.Dir(filepath)
 	filename := path.Base(filepath)
+	fileContent, _, resp, err := s.GetContents(ctx, owner, repo, filepath, opts)
+	if err == nil && fileContent != nil {
+		content, err := fileContent.GetContent()
+		if err == nil && content != "" {
+			return io.NopCloser(strings.NewReader(content)), fileContent, resp, nil
+		}
+	}
+
 	_, dirContents, resp, err := s.GetContents(ctx, owner, repo, dir, opts)
 	if err != nil {
 		return nil, nil, resp, err
 	}
 
 	for _, contents := range dirContents {
-		if *contents.Name == filename {
-			if contents.DownloadURL == nil || *contents.DownloadURL == "" {
+		if contents.GetName() == filename {
+			if contents.GetDownloadURL() == "" {
 				return nil, contents, resp, fmt.Errorf("no download link found for %s", filepath)
 			}
-
 			dlReq, err := http.NewRequestWithContext(ctx, http.MethodGet, *contents.DownloadURL, nil)
 			if err != nil {
 				return nil, contents, resp, err

github/repos_contents_test.go

@@ -127,10 +127,66 @@ func TestRepositoriesService_GetReadme(t *testing.T) {
 	})
 }
 
-func TestRepositoriesService_DownloadContents_Success(t *testing.T) {
+func TestRepositoriesService_DownloadContents_SuccessForFile(t *testing.T) {
 	t.Parallel()
 	client, mux, serverURL := setup(t)
 
+	mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		fmt.Fprint(w, `{
+		  "type": "file",
+		  "name": "f",
+          "content": "foo",
+		  "download_url": "`+serverURL+baseURLPath+`/download/f"
+		}`)
+	})
+
+	ctx := context.Background()
+	r, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil)
+	if err != nil {
+		t.Errorf("Repositories.DownloadContents returned error: %v", err)
+	}
+
+	if got, want := resp.Response.StatusCode, http.StatusOK; got != want {
+		t.Errorf("Repositories.DownloadContents returned status code %v, want %v", got, want)
+	}
+
+	bytes, err := io.ReadAll(r)
+	if err != nil {
+		t.Errorf("Error reading response body: %v", err)
+	}
+	r.Close()
+
+	if got, want := string(bytes), "foo"; got != want {
+		t.Errorf("Repositories.DownloadContents returned %v, want %v", got, want)
+	}
+
+	const methodName = "DownloadContents"
+	testBadOptions(t, methodName, func() (err error) {
+		_, _, err = client.Repositories.DownloadContents(ctx, "\n", "\n", "\n", nil)
+		return err
+	})
+
+	testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
+		got, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil)
+		if got != nil {
+			t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)
+		}
+		return resp, err
+	})
+}
+
+func TestRepositoriesService_DownloadContents_SuccessForDirectory(t *testing.T) {
+	t.Parallel()
+	client, mux, serverURL := setup(t)
+
+	mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		fmt.Fprint(w, `{
+		  "type": "file",
+		  "name": "f"
+		}`)
+	})
 	mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
 		fmt.Fprint(w, `[{
@@ -183,6 +239,13 @@ func TestRepositoriesService_DownloadContents_FailedResponse(t *testing.T) {
 	t.Parallel()
 	client, mux, serverURL := setup(t)
 
+	mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		fmt.Fprint(w, `{
+			"type": "file",
+			"name": "f"
+		  }`)
+	})
 	mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
 		fmt.Fprint(w, `[{
@@ -222,60 +285,83 @@ func TestRepositoriesService_DownloadContents_NoDownloadURL(t *testing.T) {
 	t.Parallel()
 	client, mux, _ := setup(t)
 
+	mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		fmt.Fprint(w, `{
+		  "type": "file",
+		  "name": "f",
+		  "content": ""
+		}`)
+	})
 	mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
 		fmt.Fprint(w, `[{
 		  "type": "file",
 		  "name": "f",
+		  "content": ""	
 		}]`)
 	})
 
 	ctx := context.Background()
-	_, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil)
+	reader, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil)
 	if err == nil {
 		t.Errorf("Repositories.DownloadContents did not return expected error")
 	}
 
 	if resp == nil {
 		t.Errorf("Repositories.DownloadContents did not return expected response")
 	}
+
+	if reader != nil {
+		t.Errorf("Repositories.DownloadContents did not return expected reader")
+	}
 }
 
 func TestRepositoriesService_DownloadContents_NoFile(t *testing.T) {
 	t.Parallel()
 	client, mux, _ := setup(t)
 
+	mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		fmt.Fprint(w, `{
+		  "type": "file",
+		  "name": "f",
+		  "content": ""
+		}`)
+	})
+
 	mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
 		fmt.Fprint(w, `[]`)
 	})
 
 	ctx := context.Background()
-	_, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil)
+	reader, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil)
 	if err == nil {
 		t.Errorf("Repositories.DownloadContents did not return expected error")
 	}
 
 	if resp == nil {
 		t.Errorf("Repositories.DownloadContents did not return expected response")
 	}
+
+	if reader != nil {
+		t.Errorf("Repositories.DownloadContents did not return expected reader")
+	}
 }
 
-func TestRepositoriesService_DownloadContentsWithMeta_Success(t *testing.T) {
+func TestRepositoriesService_DownloadContentsWithMeta_SuccessForFile(t *testing.T) {
 	t.Parallel()
 	client, mux, serverURL := setup(t)
 
-	mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
+	mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
-		fmt.Fprint(w, `[{
+		fmt.Fprint(w, `{
 		  "type": "file",
 		  "name": "f",
-		  "download_url": "`+serverURL+baseURLPath+`/download/f"
-		}]`)
-	})
-	mux.HandleFunc("/download/f", func(w http.ResponseWriter, r *http.Request) {
-		testMethod(t, r, "GET")
-		fmt.Fprint(w, "foo")
+		  "download_url": "`+serverURL+baseURLPath+`/download/f",
+          "content": "foo"
+		}`)
 	})
 
 	ctx := context.Background()
@@ -324,23 +410,79 @@ func TestRepositoriesService_DownloadContentsWithMeta_Success(t *testing.T) {
 	})
 }
 
-func TestRepositoriesService_DownloadContentsWithMeta_FailedResponse(t *testing.T) {
+func TestRepositoriesService_DownloadContentsWithMeta_SuccessForDirectory(t *testing.T) {
 	t.Parallel()
 	client, mux, serverURL := setup(t)
 
 	mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
 		fmt.Fprint(w, `[{
+		  "type": "file",
+		  "name": "f",
+		  "download_url": "`+serverURL+baseURLPath+`/download/f"
+		}]`)
+	})
+	mux.HandleFunc("/download/f", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		fmt.Fprint(w, "foo")
+	})
+
+	ctx := context.Background()
+	r, c, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d/f", nil)
+	if err != nil {
+		t.Errorf("Repositories.DownloadContentsWithMeta returned error: %v", err)
+	}
+
+	if got, want := resp.Response.StatusCode, http.StatusOK; got != want {
+		t.Errorf("Repositories.DownloadContentsWithMeta returned status code %v, want %v", got, want)
+	}
+
+	bytes, err := io.ReadAll(r)
+	if err != nil {
+		t.Errorf("Error reading response body: %v", err)
+	}
+	r.Close()
+
+	if got, want := string(bytes), "foo"; got != want {
+		t.Errorf("Repositories.DownloadContentsWithMeta returned %v, want %v", got, want)
+	}
+
+	if c != nil && c.Name != nil {
+		if got, want := *c.Name, "f"; got != want {
+			t.Errorf("Repositories.DownloadContentsWithMeta returned content name %v, want %v", got, want)
+		}
+	} else {
+		t.Errorf("Returned RepositoryContent is null")
+	}
+}
+
+func TestRepositoriesService_DownloadContentsWithMeta_FailedResponse(t *testing.T) {
+	t.Parallel()
+	client, mux, serverURL := setup(t)
+
+	downloadURL := fmt.Sprintf("%s%s/download/f", serverURL, baseURLPath)
+
+	mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		fmt.Fprint(w, `{
 			"type": "file",
 			"name": "f",
-			"download_url": "`+serverURL+baseURLPath+`/download/f"
-		  }]`)
+			"download_url": "`+downloadURL+`"
+		  }`)
 	})
 	mux.HandleFunc("/download/f", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
 		w.WriteHeader(http.StatusInternalServerError)
 		fmt.Fprint(w, "foo error")
 	})
+	mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		fmt.Fprint(w, `[{
+			"type": "file",
+			"name": "f",
+			"download_url": "`+downloadURL+`"
+		  }]`)
+	})
 
 	ctx := context.Background()
 	r, c, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d/f", nil)
@@ -375,23 +517,39 @@ func TestRepositoriesService_DownloadContentsWithMeta_NoDownloadURL(t *testing.T
 	t.Parallel()
 	client, mux, _ := setup(t)
 
+	mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		fmt.Fprint(w, `{
+		  "type": "file",
+		  "name": "f",
+		}`)
+	})
 	mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
 		fmt.Fprint(w, `[{
 		  "type": "file",
 		  "name": "f",
+		  "content": ""
 		}]`)
 	})
 
 	ctx := context.Background()
-	_, _, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d/f", nil)
+	reader, contents, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d/f", nil)
 	if err == nil {
 		t.Errorf("Repositories.DownloadContentsWithMeta did not return expected error")
 	}
 
+	if reader != nil {
+		t.Errorf("Repositories.DownloadContentsWithMeta did not return expected reader")
+	}
+
 	if resp == nil {
 		t.Errorf("Repositories.DownloadContentsWithMeta did not return expected response")
 	}
+
+	if contents == nil {
+		t.Errorf("Repositories.DownloadContentsWithMeta did not return expected content")
+	}
 }
 
 func TestRepositoriesService_DownloadContentsWithMeta_NoFile(t *testing.T) {