From ffa19d72c6ba35343929cfd94af6170833dc81ee Mon Sep 17 00:00:00 2001 From: kirillbilchenko Date: Mon, 11 May 2020 11:54:36 +0200 Subject: [PATCH 1/5] List releases paginations added for corner case when there is a lot of draft releases on the first page Signed-off-by: kirillbilchenko --- github.go | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/github.go b/github.go index 989399a..8ea9108 100644 --- a/github.go +++ b/github.go @@ -49,7 +49,7 @@ func NewGitHubClient(source Source) (*GitHubClient, error) { if source.Insecure { httpClient.Transport = &http.Transport{ - Proxy: http.ProxyFromEnvironment, + Proxy: http.ProxyFromEnvironment, TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, } ctx = context.WithValue(ctx, oauth2.HTTPClient, httpClient) @@ -99,17 +99,27 @@ func NewGitHubClient(source Source) (*GitHubClient, error) { } func (g *GitHubClient) ListReleases() ([]*github.RepositoryRelease, error) { - releases, res, err := g.client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, nil) - if err != nil { - return []*github.RepositoryRelease{}, err + opt := &github.RepositoryListByOrgOptions{ + ListOptions: github.ListOptions{PerPage: 10}, } - - err = res.Body.Close() - if err != nil { - return nil, err + var allReleases []*github.RepositoryRelease + for { + releases, res, err := g.client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, nil) + if err != nil { + return []*github.RepositoryRelease{}, err + } + allReleases = append(allReleases, releases...) + if res.NextPage == 0 { + err = res.Body.Close() + if err != nil { + return nil, err + } + break + } + opt.Page = res.NextPage } - return releases, nil + return allReleases, nil } func (g *GitHubClient) GetReleaseByTag(tag string) (*github.RepositoryRelease, error) { From ccf3a09d928502cd62110d5a01b8b71692a8c5af Mon Sep 17 00:00:00 2001 From: kirillbilchenko Date: Mon, 11 May 2020 18:40:59 +0200 Subject: [PATCH 2/5] Update after CR added check for number of http request and fix issue in base method handler Signed-off-by: kirillbilchenko Signed-off-by: kirillbilchenko --- github.go | 6 ++---- github_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/github.go b/github.go index 8ea9108..79bd546 100644 --- a/github.go +++ b/github.go @@ -99,12 +99,10 @@ func NewGitHubClient(source Source) (*GitHubClient, error) { } func (g *GitHubClient) ListReleases() ([]*github.RepositoryRelease, error) { - opt := &github.RepositoryListByOrgOptions{ - ListOptions: github.ListOptions{PerPage: 10}, - } + opt := &github.ListOptions{PerPage: 100} var allReleases []*github.RepositoryRelease for { - releases, res, err := g.client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, nil) + releases, res, err := g.client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, opt) if err != nil { return []*github.RepositoryRelease{}, err } diff --git a/github_test.go b/github_test.go index 7150a27..0486084 100644 --- a/github_test.go +++ b/github_test.go @@ -1,6 +1,8 @@ package resource_test import ( + "bytes" + "encoding/json" "net/http" . "github.com/concourse/github-release-resource" @@ -119,6 +121,44 @@ var _ = Describe("GitHub Client", func() { }) }) + Describe("ListReleases", func() { + BeforeEach(func() { + source = Source{ + Owner: "concourse", + Repository: "concourse", + } + }) + Context("When list of releases return more then 100 items", func() { + BeforeEach(func() { + var result []*github.RepositoryRelease + for i := 1; i < 102; i++ { + result = append(result, &github.RepositoryRelease{ID: github.Int(i)}) + + } + reqBodyBytes := new(bytes.Buffer) + json.NewEncoder(reqBodyBytes).Encode(result) + + server.AppendHandlers( + ghttp.CombineHandlers( + ghttp.VerifyRequest("GET", "/repos/concourse/concourse/releases", "per_page=100"), + ghttp.RespondWithJSONEncoded(200, result[:100], http.Header{"Link": []string{`; rel="next"`}}), + ), + ghttp.CombineHandlers( + ghttp.VerifyRequest("GET", "/repos/concourse/concourse/releases", "per_page=100&page=2"), + ghttp.RespondWithJSONEncoded(200, result[100:]), + ), + ) + }) + + It("list releases", func() { + releases, err := client.ListReleases() + Ω(err).ShouldNot(HaveOccurred()) + Expect(releases).To(HaveLen(101)) + Expect(server.ReceivedRequests()).To(HaveLen(2)) + }) + }) + }) + Describe("GetRelease", func() { BeforeEach(func() { source = Source{ From 5ada04c8343c857405cf7f8cf17a5da779e37e9b Mon Sep 17 00:00:00 2001 From: kirillbilchenko Date: Mon, 11 May 2020 18:40:59 +0200 Subject: [PATCH 3/5] Add test to check pagination functionality Signed-off-by: kirillbilchenko Add test to check pagination functionality Signed-off-by: kirillbilchenko --- github.go | 6 ++++-- github_test.go | 9 ++------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/github.go b/github.go index 79bd546..a021a30 100644 --- a/github.go +++ b/github.go @@ -99,10 +99,12 @@ func NewGitHubClient(source Source) (*GitHubClient, error) { } func (g *GitHubClient) ListReleases() ([]*github.RepositoryRelease, error) { - opt := &github.ListOptions{PerPage: 100} + opt := &github.RepositoryListByOrgOptions{ + ListOptions: github.ListOptions{PerPage: 100}, + } var allReleases []*github.RepositoryRelease for { - releases, res, err := g.client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, opt) + releases, res, err := g.client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, nil) if err != nil { return []*github.RepositoryRelease{}, err } diff --git a/github_test.go b/github_test.go index 0486084..290eaae 100644 --- a/github_test.go +++ b/github_test.go @@ -140,12 +140,8 @@ var _ = Describe("GitHub Client", func() { server.AppendHandlers( ghttp.CombineHandlers( - ghttp.VerifyRequest("GET", "/repos/concourse/concourse/releases", "per_page=100"), - ghttp.RespondWithJSONEncoded(200, result[:100], http.Header{"Link": []string{`; rel="next"`}}), - ), - ghttp.CombineHandlers( - ghttp.VerifyRequest("GET", "/repos/concourse/concourse/releases", "per_page=100&page=2"), - ghttp.RespondWithJSONEncoded(200, result[100:]), + ghttp.VerifyRequest("GET", "/repos/concourse/concourse/releases"), + ghttp.RespondWith(200, reqBodyBytes.Bytes()), ), ) }) @@ -154,7 +150,6 @@ var _ = Describe("GitHub Client", func() { releases, err := client.ListReleases() Ω(err).ShouldNot(HaveOccurred()) Expect(releases).To(HaveLen(101)) - Expect(server.ReceivedRequests()).To(HaveLen(2)) }) }) }) From 25ee28f307d65dcfa6681a193ad88ceb2999a543 Mon Sep 17 00:00:00 2001 From: kirillbilchenko Date: Tue, 12 May 2020 16:22:39 +0200 Subject: [PATCH 4/5] Update after CR added check for number of http request and fix issue in base method handler Signed-off-by: kirillbilchenko Signed-off-by: kirillbilchenko --- github.go | 6 ++---- github_test.go | 9 +++++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/github.go b/github.go index a021a30..79bd546 100644 --- a/github.go +++ b/github.go @@ -99,12 +99,10 @@ func NewGitHubClient(source Source) (*GitHubClient, error) { } func (g *GitHubClient) ListReleases() ([]*github.RepositoryRelease, error) { - opt := &github.RepositoryListByOrgOptions{ - ListOptions: github.ListOptions{PerPage: 100}, - } + opt := &github.ListOptions{PerPage: 100} var allReleases []*github.RepositoryRelease for { - releases, res, err := g.client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, nil) + releases, res, err := g.client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, opt) if err != nil { return []*github.RepositoryRelease{}, err } diff --git a/github_test.go b/github_test.go index 290eaae..0486084 100644 --- a/github_test.go +++ b/github_test.go @@ -140,8 +140,12 @@ var _ = Describe("GitHub Client", func() { server.AppendHandlers( ghttp.CombineHandlers( - ghttp.VerifyRequest("GET", "/repos/concourse/concourse/releases"), - ghttp.RespondWith(200, reqBodyBytes.Bytes()), + ghttp.VerifyRequest("GET", "/repos/concourse/concourse/releases", "per_page=100"), + ghttp.RespondWithJSONEncoded(200, result[:100], http.Header{"Link": []string{`; rel="next"`}}), + ), + ghttp.CombineHandlers( + ghttp.VerifyRequest("GET", "/repos/concourse/concourse/releases", "per_page=100&page=2"), + ghttp.RespondWithJSONEncoded(200, result[100:]), ), ) }) @@ -150,6 +154,7 @@ var _ = Describe("GitHub Client", func() { releases, err := client.ListReleases() Ω(err).ShouldNot(HaveOccurred()) Expect(releases).To(HaveLen(101)) + Expect(server.ReceivedRequests()).To(HaveLen(2)) }) }) }) From 4144622a9cfab7baae30a9baf504f5fba31a3d53 Mon Sep 17 00:00:00 2001 From: kirillbilchenko Date: Tue, 12 May 2020 16:46:07 +0200 Subject: [PATCH 5/5] Remove unused calls Signed-off-by: kirillbilchenko --- github_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/github_test.go b/github_test.go index 0486084..f4ce287 100644 --- a/github_test.go +++ b/github_test.go @@ -1,8 +1,6 @@ package resource_test import ( - "bytes" - "encoding/json" "net/http" . "github.com/concourse/github-release-resource" @@ -135,8 +133,6 @@ var _ = Describe("GitHub Client", func() { result = append(result, &github.RepositoryRelease{ID: github.Int(i)}) } - reqBodyBytes := new(bytes.Buffer) - json.NewEncoder(reqBodyBytes).Encode(result) server.AppendHandlers( ghttp.CombineHandlers(