From b8718182e0a966bcc7e54f899ca86e52bd509d9e Mon Sep 17 00:00:00 2001 From: Tomer Heber Date: Sun, 8 Dec 2024 08:15:03 -0600 Subject: [PATCH] Fix: "all_teams" data resource causes 429 status code requests (#988) * Fix: "all_teams" data resource causes 429 status code requests * added a note * linting fixes * Added memoize tests --- client/api_client.go | 7 ++- client/configuration_variable_test.go | 5 +- client/memoize.go | 27 +++++++++++ client/memoize_test.go | 68 +++++++++++++++++++++++++++ client/team.go | 11 +++-- env0/data_teams.go | 1 + 6 files changed, 112 insertions(+), 7 deletions(-) create mode 100644 client/memoize.go create mode 100644 client/memoize_test.go diff --git a/client/api_client.go b/client/api_client.go index 26b46473..ef842b62 100644 --- a/client/api_client.go +++ b/client/api_client.go @@ -10,6 +10,7 @@ type ApiClient struct { http http.HttpClientInterface cachedOrganizationId string defaultOrganizationId string + memoizedGetTeams func(string) ([]Team, error) } type ApiClientInterface interface { @@ -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 } diff --git a/client/configuration_variable_test.go b/client/configuration_variable_test.go index b8760fa8..051d5bb3 100644 --- a/client/configuration_variable_test.go +++ b/client/configuration_variable_test.go @@ -2,7 +2,6 @@ package client_test import ( "encoding/json" - "strings" "testing" . "github.com/env0/terraform-provider-env0/client" @@ -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 @@ -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 diff --git a/client/memoize.go b/client/memoize.go new file mode 100644 index 00000000..c7f47049 --- /dev/null +++ b/client/memoize.go @@ -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) +} diff --git a/client/memoize_test.go b/client/memoize_test.go new file mode 100644 index 00000000..05cf2deb --- /dev/null +++ b/client/memoize_test.go @@ -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 + }) +} diff --git a/client/team.go b/client/team.go index 4f11a2c5..d5898132 100644 --- a/client/team.go +++ b/client/team.go @@ -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 @@ -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) } diff --git a/env0/data_teams.go b/env0/data_teams.go index 21d6e822..88bf9470 100644 --- a/env0/data_teams.go +++ b/env0/data_teams.go @@ -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": {