Skip to content

Commit

Permalink
Fix: "all_teams" data resource causes 429 status code requests (#988)
Browse files Browse the repository at this point in the history
* Fix: "all_teams" data resource causes 429 status code requests

* added a note

* linting fixes

* Added memoize tests
  • Loading branch information
TomerHeber authored Dec 8, 2024
1 parent 8a80a6a commit b871818
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 7 deletions.
7 changes: 6 additions & 1 deletion client/api_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ type ApiClient struct {
http http.HttpClientInterface
cachedOrganizationId string
defaultOrganizationId string
memoizedGetTeams func(string) ([]Team, error)
}

type ApiClientInterface interface {
Expand Down Expand Up @@ -172,9 +173,13 @@ type ApiClientInterface interface {
}

func NewApiClient(client http.HttpClientInterface, defaultOrganizationId string) ApiClientInterface {
return &ApiClient{
apiClient := &ApiClient{
http: client,
cachedOrganizationId: "",
defaultOrganizationId: defaultOrganizationId,
}

apiClient.memoizedGetTeams = memoize(apiClient.GetTeams)

return apiClient
}
5 changes: 2 additions & 3 deletions client/configuration_variable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package client_test

import (
"encoding/json"
"strings"
"testing"

. "github.com/env0/terraform-provider-env0/client"
Expand Down Expand Up @@ -309,7 +308,7 @@ func TestConfigurationVariableMarshelling(t *testing.T) {

b, err := json.Marshal(&variable)
if assert.NoError(t, err) {
assert.False(t, strings.Contains(string(b), str))
assert.NotContains(t, string(b), str)
}

type ConfigurationVariableDummy ConfigurationVariable
Expand All @@ -318,7 +317,7 @@ func TestConfigurationVariableMarshelling(t *testing.T) {

b, err = json.Marshal(&dummy)
if assert.NoError(t, err) {
assert.True(t, strings.Contains(string(b), str))
assert.Contains(t, string(b), str)
}

var variable2 ConfigurationVariable
Expand Down
27 changes: 27 additions & 0 deletions client/memoize.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package client

type memoizedResult[V any] struct {
value V
err error
}

func memoize[K comparable, V any](f func(K) (V, error)) func(K) (V, error) {
cache := make(map[K]memoizedResult[V])

return func(key K) (V, error) {
if res, ok := cache[key]; ok {
return res.value, res.err
}

value, err := f(key)

cache[key] = memoizedResult[V]{value: value, err: err}

return value, err
}
}

// MemoizeExported exports the memoize function for testing
func MemoizeExported[K comparable, V any](f func(K) (V, error)) func(K) (V, error) {
return memoize(f)
}
68 changes: 68 additions & 0 deletions client/memoize_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package client_test

import (
"errors"
"testing"

"github.com/env0/terraform-provider-env0/client"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestMemoize(t *testing.T) {
t.Run("should cache successful results", func(t *testing.T) {
callCount := 0
f := func(s string) ([]client.Team, error) {
callCount++

return []client.Team{{Name: s}}, nil
}

memoized := client.MemoizeExported(f)

// First call
result1, err1 := memoized("")
require.NoError(t, err1)
assert.Len(t, result1, 1)
assert.Equal(t, 1, callCount)

// Second call with same input - should use cache
result2, err2 := memoized("")
require.NoError(t, err2)
assert.Len(t, result2, 1)
assert.Equal(t, 1, callCount) // Count shouldn't increase

// Different input - should call function again
result3, err3 := memoized("test")
require.NoError(t, err3)
assert.Len(t, result3, 1)
assert.Equal(t, "test", result3[0].Name)
assert.Equal(t, 2, callCount)
})

t.Run("should cache errors", func(t *testing.T) {
callCount := 0
expectedError := errors.New("test error")
f := func(s string) ([]client.Team, error) {
callCount++

return nil, expectedError
}

memoized := client.MemoizeExported(f)

// First call
result1, err1 := memoized("")
require.Error(t, err1)
assert.Equal(t, expectedError, err1)
assert.Nil(t, result1)
assert.Equal(t, 1, callCount)

// Second call - should return cached error
result2, err2 := memoized("")
require.Error(t, err2)
assert.Equal(t, expectedError, err2)
assert.Nil(t, result2)
assert.Equal(t, 1, callCount) // Count shouldn't increase
})
}
11 changes: 8 additions & 3 deletions client/team.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,12 @@ func (client *ApiClient) TeamUpdate(id string, payload TeamUpdatePayload) (Team,
return result, nil
}

func (client *ApiClient) GetTeams(params map[string]string) ([]Team, error) {
func (client *ApiClient) GetTeams(name string) ([]Team, error) {
params := map[string]string{"limit": "100"}
if name != "" {
params["name"] = name
}

organizationId, err := client.OrganizationId()
if err != nil {
return nil, err
Expand Down Expand Up @@ -112,9 +117,9 @@ func (client *ApiClient) GetTeams(params map[string]string) ([]Team, error) {
}

func (client *ApiClient) Teams() ([]Team, error) {
return client.GetTeams(map[string]string{"limit": "100"})
return client.memoizedGetTeams("")
}

func (client *ApiClient) TeamsByName(name string) ([]Team, error) {
return client.GetTeams(map[string]string{"name": name, "limit": "100"})
return client.memoizedGetTeams(name)
}
1 change: 1 addition & 0 deletions env0/data_teams.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
func dataTeams() *schema.Resource {
return &schema.Resource{
ReadContext: dataTeamsRead,
Description: "Note: this data source is cached, once fetched it will not be updated until the next plan/apply",

Schema: map[string]*schema.Schema{
"names": {
Expand Down

0 comments on commit b871818

Please sign in to comment.