Skip to content

Commit

Permalink
Fix: support fetching environments with the limit+offset feature (#356)
Browse files Browse the repository at this point in the history
* Fix: support fetching environments with the limit+offset feature

* revert generics

* replaced 100 and 33 with consts
  • Loading branch information
TomerHeber committed May 2, 2022
1 parent dbedc77 commit 4d9a448
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 16 deletions.
1 change: 1 addition & 0 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var (
mockHttpClient *http.MockHttpClientInterface
apiClient ApiClientInterface
httpCall *gomock.Call
httpCall2 *gomock.Call
organizationIdCall *gomock.Call
)

Expand Down
20 changes: 6 additions & 14 deletions client/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,24 +92,16 @@ type EnvironmentDeployResponse struct {
Id string `json:"id"`
}

func (Environment) getEndpoint() string {
return "/environments"
}

func (client *ApiClient) Environments() ([]Environment, error) {
var result []Environment
err := client.http.Get("/environments", nil, &result)
if err != nil {
return []Environment{}, err
}
return result, nil
return getAll(client, nil)
}

func (client *ApiClient) ProjectEnvironments(projectId string) ([]Environment, error) {

var result []Environment
err := client.http.Get("/environments", map[string]string{"projectId": projectId}, &result)

if err != nil {
return []Environment{}, err
}
return result, nil
return getAll(client, map[string]string{"projectId": projectId})
}

func (client *ApiClient) Environment(id string) (Environment, error) {
Expand Down
84 changes: 82 additions & 2 deletions client/environment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import (
. "github.com/onsi/gomega"
)

const full_page = 100
const partial_page = 33

var _ = Describe("Environment Client", func() {
const (
environmentId = "env-id"
Expand All @@ -28,7 +31,7 @@ var _ = Describe("Environment Client", func() {
Describe("Success", func() {
BeforeEach(func() {
httpCall = mockHttpClient.EXPECT().
Get("/environments", nil, gomock.Any()).
Get("/environments", gomock.Any(), gomock.Any()).
Do(func(path string, request interface{}, response *[]Environment) {
*response = mockEnvironments
})
Expand All @@ -45,11 +48,88 @@ var _ = Describe("Environment Client", func() {
})
})

Describe("SuccessMultiPages", func() {
var environmentsP1, environmentsP2 []Environment
for i := 0; i < full_page; i++ {
environmentsP1 = append(environmentsP1, mockEnvironment)
}

for i := 0; i < partial_page; i++ {
environmentsP2 = append(environmentsP2, mockEnvironment)
}

BeforeEach(func() {
httpCall = mockHttpClient.EXPECT().
Get("/environments", map[string]string{
"offset": "0",
"limit": "100",
}, gomock.Any()).
Do(func(path string, request interface{}, response *[]Environment) {
*response = environmentsP1
}).Times(1)

httpCall2 = mockHttpClient.EXPECT().
Get("/environments", map[string]string{
"offset": "100",
"limit": "100",
}, gomock.Any()).
Do(func(path string, request interface{}, response *[]Environment) {
*response = environmentsP2
}).Times(1)

environments, err = apiClient.Environments()
})

It("Should return the environments", func() {
Expect(environments).To(Equal(append(environmentsP1, environmentsP2...)))
})
})

Describe("SuccessMultiPagesWithProject", func() {
projectId := "proj123"
var environmentsP1, environmentsP2 []Environment
for i := 0; i < full_page; i++ {
environmentsP1 = append(environmentsP1, mockEnvironment)
}

for i := 0; i < partial_page; i++ {
environmentsP2 = append(environmentsP2, mockEnvironment)
}

BeforeEach(func() {
httpCall = mockHttpClient.EXPECT().
Get("/environments", map[string]string{
"offset": "0",
"limit": "100",
"projectId": projectId,
}, gomock.Any()).
Do(func(path string, request interface{}, response *[]Environment) {
*response = environmentsP1
}).Times(1)

httpCall2 = mockHttpClient.EXPECT().
Get("/environments", map[string]string{
"offset": "100",
"limit": "100",
"projectId": projectId,
}, gomock.Any()).
Do(func(path string, request interface{}, response *[]Environment) {
*response = environmentsP2
}).Times(1)

environments, err = apiClient.ProjectEnvironments(projectId)
})

It("Should return the environments", func() {
Expect(environments).To(Equal(append(environmentsP1, environmentsP2...)))
})
})

Describe("Failure", func() {
It("On error from server return the error", func() {
expectedErr := errors.New("some error")
httpCall = mockHttpClient.EXPECT().
Get("/environments", nil, gomock.Any()).
Get("/environments", gomock.Any(), gomock.Any()).
Return(expectedErr)

_, err = apiClient.Environments()
Expand Down
67 changes: 67 additions & 0 deletions client/pagination.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package client

import "strconv"

const limit = 100

type Pagination struct {
offset int
params map[string]string
}

// Note: all the FIXME should be uncommented when Alpine adds support for go 1.18.
// This will enable generics.

type Paginated interface {
// FIXME: Environment
getEndpoint() string
}

func (p *Pagination) getParams() map[string]string {
return map[string]string{
"limit": strconv.Itoa(limit),
"offset": strconv.Itoa(p.offset),
}
}

// returns true if there is more data.
func (p *Pagination) next(currentPageSize int) bool {
p.offset += currentPageSize
return currentPageSize == limit
}

// params - additional params. may be nil.
// FIXME: func getAll[P Paginated](client *ApiClient, params map[string]string) ([]P, error) {
func getAll(client *ApiClient, params map[string]string) ([]Environment, error) {
p := Pagination{
offset: 0,
params: params,
}

// FIXME: var allResults []P
var allResults []Environment

for {
pageParams := p.getParams()
for k, v := range params {
pageParams[k] = v
}

// FIXME: var pageResults []P
var pageResults []Environment

// FIXME: err := client.http.Get(P{}.getEndpoint(), pageParams, &pageResults)
err := client.http.Get(Environment{}.getEndpoint(), pageParams, &pageResults)
if err != nil {
return nil, err
}

allResults = append(allResults, pageResults...)

if more := p.next(len(pageResults)); !more {
break
}
}

return allResults, nil
}

0 comments on commit 4d9a448

Please sign in to comment.