Skip to content

Commit c577c08

Browse files
demeyerthomdemeyerthom
authored
Merge pull request #570 from labd/fix/custom-fields-map-panic
Fix/custom fields map panic
2 parents 374cd53 + 7e4603c commit c577c08

File tree

8 files changed

+62
-29
lines changed

8 files changed

+62
-29
lines changed

‎.changes/unreleased/Fixed-20250206-113217.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
kind: Fixed
2+
body: Custom fields cache concurrent writes
3+
time: 2025-02-06T11:32:17.345811+01:00

‎Taskfile.yml

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,21 +42,15 @@ tasks:
4242
- go tool cover -func=coverage.txt
4343

4444
testacc:
45-
cmds:
46-
- TF_ACC=1 go test -v ./...
47-
48-
testacct:
49-
cmds:
50-
- TF_ACC=1 go test -race -coverprofile=coverage.txt -covermode=atomic -coverpkg=./... -v ./...
51-
52-
mockacc:
53-
cmds:
54-
- go test -count=1 -v ./...
5545
env:
56-
TF_ACC: 1
57-
CTP_CLIENT_ID: unittest
58-
CTP_CLIENT_SECRET: x
59-
CTP_PROJECT_KEY: unittest
60-
CTP_SCOPES: manage_project:projectkey
46+
TF_ACC: true
47+
CTP_CLIENT_ID: client-id
48+
CTP_CLIENT_SECRET: client-secret
49+
CTP_PROJECT_KEY: project-key
50+
CTP_SCOPES: "manage_project:project-key"
6151
CTP_API_URL: http://localhost:8989
6252
CTP_AUTH_URL: http://localhost:8989
53+
cmds:
54+
- docker-compose up -d
55+
- go test -v ./...
56+
- docker-compose down

‎commercetools/custom_fields.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@ import (
66
"fmt"
77
"strconv"
88
"strings"
9+
"sync"
910
"time"
1011

1112
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1213
"github.com/labd/commercetools-go-sdk/platform"
1314
)
1415

1516
var cacheTypes map[string]*platform.Type
17+
var cacheTypesLock sync.Mutex
1618

1719
func CustomFieldSchema() *schema.Schema {
1820
return &schema.Schema{
@@ -248,15 +250,23 @@ func getTypeResourceFromResourceData(ctx context.Context, client *platform.ByPro
248250
}
249251

250252
if typeId, ok := data["type_id"].(string); ok {
251-
return GetTypeResource(ctx, client, typeId)
253+
return GetTypeResource(ctx, CreateTypeFetcher(client), typeId)
252254
}
253255
return nil, fmt.Errorf("missing type_id for custom fields")
254256
}
255257

258+
func CreateTypeFetcher(client *platform.ByProjectKeyRequestBuilder) func(ctx context.Context, id string) (*platform.Type, error) {
259+
return func(ctx context.Context, id string) (*platform.Type, error) {
260+
return client.Types().WithId(id).Get().Execute(ctx)
261+
}
262+
}
263+
256264
// GetTypeResource returns the platform.Type for the type_id in the custom
257265
// field. The type_id is cached to minimize API calls when multiple resource
258266
// use the same type
259-
func GetTypeResource(ctx context.Context, client *platform.ByProjectKeyRequestBuilder, typeId string) (*platform.Type, error) {
267+
func GetTypeResource(ctx context.Context, getTypeByIdFunc func(ctx context.Context, id string) (*platform.Type, error), typeId string) (*platform.Type, error) {
268+
cacheTypesLock.Lock()
269+
defer cacheTypesLock.Unlock()
260270
if cacheTypes == nil {
261271
cacheTypes = make(map[string]*platform.Type)
262272
}
@@ -267,9 +277,15 @@ func GetTypeResource(ctx context.Context, client *platform.ByProjectKeyRequestBu
267277
return t, nil
268278
}
269279

270-
t, err := client.Types().WithId(typeId).Get().Execute(ctx)
280+
t, err := getTypeByIdFunc(ctx, typeId)
281+
if err != nil {
282+
cacheTypes[typeId] = nil
283+
return nil, err
284+
}
285+
271286
cacheTypes[typeId] = t
272-
return t, err
287+
288+
return t, nil
273289
}
274290

275291
func CustomFieldUpdateActions[T SetCustomTypeAction, F SetCustomFieldAction](ctx context.Context, client *platform.ByProjectKeyRequestBuilder, d *schema.ResourceData) ([]any, error) {

‎commercetools/custom_fields_test.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@ import (
44
"bytes"
55
"context"
66
"fmt"
7+
"math/rand"
8+
"sync"
79
"testing"
810
"text/template"
11+
"time"
912

1013
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
1114
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -85,7 +88,6 @@ var customFieldTypes = []string{"String", "Boolean", "Number", "LocalizedString"
8588

8689
func TestAccCustomField_SetAndRemove(t *testing.T) {
8790
for _, customFieldResourceType := range customFieldResourceTypes {
88-
fmt.Println("Testing custom fields for:", customFieldResourceType)
8991
resourceShortName := "ct" + acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
9092
resourceFullName := customFieldResourceType + "." + resourceShortName
9193
resourceKey := "key" + resourceShortName
@@ -249,3 +251,21 @@ func testGetShippingMethod(s *terraform.State, identifier string) (*platform.Shi
249251
}
250252
return result, nil
251253
}
254+
255+
func TestGetTypeResourceConcurrent(t *testing.T) {
256+
var ids = []string{"1", "2", "3", "4", "5", "6", "7", "8", "9", "10"}
257+
258+
wg := &sync.WaitGroup{}
259+
for pid := 0; pid < 1000; pid++ {
260+
wg.Add(1)
261+
go func(pid int) {
262+
defer wg.Done()
263+
_, err := GetTypeResource(context.Background(), func(_ context.Context, id string) (*platform.Type, error) {
264+
time.Sleep(time.Duration(rand.Intn(1000)) * time.Millisecond)
265+
return &platform.Type{ID: id}, nil
266+
}, ids[pid%len(ids)])
267+
assert.NoError(t, err)
268+
}(pid)
269+
}
270+
wg.Wait()
271+
}

‎internal/resources/associate_role/resource.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ func (r *associateRoleResource) Create(ctx context.Context, req resource.CreateR
185185
var customType *platform.Type
186186
var err error
187187
if plan.Custom.IsSet() {
188-
customType, err = commercetools.GetTypeResource(ctx, r.client, *plan.Custom.TypeID)
188+
customType, err = commercetools.GetTypeResource(ctx, commercetools.CreateTypeFetcher(r.client), *plan.Custom.TypeID)
189189
if err != nil {
190190
resp.Diagnostics.AddError(
191191
"Error getting custom type",
@@ -294,7 +294,7 @@ func (r *associateRoleResource) Read(ctx context.Context, req resource.ReadReque
294294

295295
var customType *platform.Type
296296
if state.Custom.IsSet() {
297-
customType, err = commercetools.GetTypeResource(ctx, r.client, *state.Custom.TypeID)
297+
customType, err = commercetools.GetTypeResource(ctx, commercetools.CreateTypeFetcher(r.client), *state.Custom.TypeID)
298298
if err != nil {
299299
resp.Diagnostics.AddError(
300300
"Error getting custom type",
@@ -352,7 +352,7 @@ func (r *associateRoleResource) Update(ctx context.Context, req resource.UpdateR
352352

353353
var customType *platform.Type
354354
if plan.Custom.IsSet() {
355-
customType, err = commercetools.GetTypeResource(ctx, r.client, *plan.Custom.TypeID)
355+
customType, err = commercetools.GetTypeResource(ctx, commercetools.CreateTypeFetcher(r.client), *plan.Custom.TypeID)
356356
if err != nil {
357357
resp.Diagnostics.AddError(
358358
"Error getting custom type",

‎internal/resources/business_unit_company/resource.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ func (b *companyResource) Create(ctx context.Context, req resource.CreateRequest
143143
var customType *platform.Type
144144
var err error
145145
if plan.Custom.IsSet() {
146-
customType, err = commercetools.GetTypeResource(ctx, b.client, *plan.Custom.TypeID)
146+
customType, err = commercetools.GetTypeResource(ctx, commercetools.CreateTypeFetcher(b.client), *plan.Custom.TypeID)
147147
if err != nil {
148148
res.Diagnostics.AddError(
149149
"Error getting custom type",
@@ -251,7 +251,7 @@ func (b *companyResource) Update(ctx context.Context, req resource.UpdateRequest
251251
var customType *platform.Type
252252
var err error
253253
if plan.Custom.IsSet() {
254-
customType, err = commercetools.GetTypeResource(ctx, b.client, *plan.Custom.TypeID)
254+
customType, err = commercetools.GetTypeResource(ctx, commercetools.CreateTypeFetcher(b.client), *plan.Custom.TypeID)
255255
if err != nil {
256256
res.Diagnostics.AddError(
257257
"Error getting custom type",

‎internal/resources/business_unit_division/resource.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ func (b *divisionResource) Create(ctx context.Context, req resource.CreateReques
208208
var customType *platform.Type
209209
var err error
210210
if plan.Custom.IsSet() {
211-
customType, err = commercetools.GetTypeResource(ctx, b.client, *plan.Custom.TypeID)
211+
customType, err = commercetools.GetTypeResource(ctx, commercetools.CreateTypeFetcher(b.client), *plan.Custom.TypeID)
212212
if err != nil {
213213
res.Diagnostics.AddError(
214214
"Error getting custom type",
@@ -350,7 +350,7 @@ func (b *divisionResource) Update(ctx context.Context, req resource.UpdateReques
350350
var customType *platform.Type
351351
var err error
352352
if plan.Custom.IsSet() {
353-
customType, err = commercetools.GetTypeResource(ctx, b.client, *plan.Custom.TypeID)
353+
customType, err = commercetools.GetTypeResource(ctx, commercetools.CreateTypeFetcher(b.client), *plan.Custom.TypeID)
354354
if err != nil {
355355
res.Diagnostics.AddError(
356356
"Error getting custom type",

‎internal/resources/product_selection/resource.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func (r *productSelectionResource) Create(ctx context.Context, req resource.Crea
109109
var customType *platform.Type
110110
var err error
111111
if plan.Custom.IsSet() {
112-
customType, err = commercetools.GetTypeResource(ctx, r.client, *plan.Custom.TypeID)
112+
customType, err = commercetools.GetTypeResource(ctx, commercetools.CreateTypeFetcher(r.client), *plan.Custom.TypeID)
113113
if err != nil {
114114
resp.Diagnostics.AddError(
115115
"Error getting custom type",
@@ -252,7 +252,7 @@ func (r *productSelectionResource) Update(ctx context.Context, req resource.Upda
252252
var customType *platform.Type
253253
var err error
254254
if plan.Custom.IsSet() {
255-
customType, err = commercetools.GetTypeResource(ctx, r.client, *plan.Custom.TypeID)
255+
customType, err = commercetools.GetTypeResource(ctx, commercetools.CreateTypeFetcher(r.client), *plan.Custom.TypeID)
256256
if err != nil {
257257
resp.Diagnostics.AddError(
258258
"Error getting custom type",

0 commit comments

Comments
 (0)