fix: parents update, race conditions

.gitignore

@@ -16,3 +16,5 @@ bin
 *.idea
 /.vscode/*
 /untracked_*.go
+cover.html
+cover.out

Makefile

@@ -16,12 +16,18 @@ build:
 ## Run tests
 test:
 	go run ./main.go -exportfile ./test/test-space-export.json ./test/testapi
-	go test -count=1 ./...
+	go test ./...
 
 race:
 	go run ./main.go -exportfile ./test/test-space-export.json ./test/testapi
 	go test -race ./...
 
+cover:
+	rm cover.out cover.html
+	go run ./main.go -exportfile ./test/test-space-export.json ./test/testapi
+	go test -cover -coverprofile cover.out -coverpkg=./test/testapi ./...
+	go tool cover -html=cover.out -o cover.html; open cover.html
+
 .PHONY: lint
 ## Run linter
 lint:

erm/templates/contentful_vo_lib.gotmpl

@@ -210,6 +210,8 @@ func (cc *ContentfulClient) CacheHasContentType(contentTypeID string) bool {
 	if cc.Cache == nil {
 		return false
 	}
+	cc.cacheMutex.sharedDataGcLock.RLock()
+	defer cc.cacheMutex.sharedDataGcLock.RUnlock()
 	for _, cachedContentType := range cc.Cache.contentTypes {
 		if cachedContentType == contentTypeID {
 			return true
@@ -223,6 +225,8 @@ func (cc *ContentfulClient) ClientStats() {
 		cc.logFn(nil, LogWarn, "ClientStats: no client available")
 		return
 	}
+	cc.cacheMutex.sharedDataGcLock.RLock()
+	defer cc.cacheMutex.sharedDataGcLock.RUnlock()
 	if cc.logFn != nil {
 		fieldsMap := map[string]interface{}{
 			"space ID":     cc.SpaceID,
@@ -235,7 +239,14 @@ func (cc *ContentfulClient) ClientStats() {
 		if cc.cacheInit {
 			fieldsMap["cache asset count"] = len(cc.Cache.assets)
 			fieldsMap["cache entry count"] = len(cc.Cache.idContentTypeMap)
-			fieldsMap["cache parentMap length"] = len(cc.Cache.parentMap)
+			fieldsMap["cache parentMap children"] = len(cc.Cache.parentMap)
+			cc.cacheMutex.parentMapGcLock.RLock()
+			defer cc.cacheMutex.parentMapGcLock.RUnlock()
+			referenceCount := 0
+			for _, parents := range cc.Cache.parentMap {
+				referenceCount += len(parents)
+			}
+			fieldsMap["cache parentMap parents"] = referenceCount		
 		}
 		cc.logFn(fieldsMap, LogInfo, "Contentful ClientStats")
 	}
@@ -283,10 +294,13 @@ func (cc *ContentfulClient) GetAssetByID(id string, forceNoCache ...bool) (*cont
 	if cc == nil || cc.Client == nil {
 		return nil, errors.New("GetAssetByID: No client available")
 	}
-	if cc.cacheInit && cc.Cache.assets != nil && (len(forceNoCache) == 0 || !forceNoCache[0]) {
-		cc.cacheMutex.assetsGcLock.Lock()
+	cc.cacheMutex.sharedDataGcLock.Lock()
+	cacheInit := cc.cacheInit
+	cc.cacheMutex.sharedDataGcLock.Unlock()
+	cc.cacheMutex.assetsGcLock.Lock()
+	defer cc.cacheMutex.assetsGcLock.Unlock()
+	if cacheInit && cc.Cache.assets != nil && (len(forceNoCache) == 0 || !forceNoCache[0]) {
 		asset, okAsset := cc.Cache.assets[id]
-		cc.cacheMutex.assetsGcLock.Unlock()
 		if okAsset {
 			return asset, nil
 		} else {
@@ -583,6 +597,8 @@ func (cc *ContentfulClient) SetOfflineFallback(filename string) error {
 }
 
 func (cc *ContentfulClient) SetSyncMode(mode bool) error {
+	cc.cacheMutex.sharedDataGcLock.RLock()
+	defer cc.cacheMutex.sharedDataGcLock.RUnlock()
 	if cc.offline {
 		return errors.New("SetSyncMode: client is set offline, can't enable sync")
 	}
@@ -595,14 +611,18 @@ func (cc *ContentfulClient) ResetSync() {
 }
 
 func (cc *ContentfulClient) UpdateCache(ctx context.Context, contentTypes []string, cacheAssets bool) error {
+	cc.cacheMutex.sharedDataGcLock.RLock()
     ctxAtWork, cancel := context.WithTimeout(ctx, time.Second*time.Duration(cc.cacheUpdateTimeout))
-    defer cancel()
-    if cc.offline {
-        ctxAtWork = ctx
-    }
-    if !cc.offline {
-        time.Sleep(time.Second * 2)
-    }
+	defer cancel()
+	localOffline := cc.offline
+	cc.cacheMutex.sharedDataGcLock.RUnlock()
+
+	if localOffline {
+		ctxAtWork = ctx
+	}
+	if !localOffline {
+		time.Sleep(time.Second * 2)
+	}
 	if contentTypes == nil {
 		contentTypes = spaceContentTypes
 	} else {
@@ -647,7 +667,9 @@ func (cc *ContentfulClient) UpdateCache(ctx context.Context, contentTypes []stri
 
 func (cc *ContentfulClient) syncCache(ctx context.Context, contentTypes []string) error {
 	start := time.Now()
+	cc.cacheMutex.sharedDataGcLock.Lock()
 	cc.Cache.contentTypes = contentTypes
+	cc.cacheMutex.sharedDataGcLock.Unlock()
 	if cc.logFn != nil && cc.logLevel <= LogInfo {
 			cc.logFn(map[string]interface{}{"task": "syncCache"}, LogInfo, InfoCacheUpdateQueued)
 	}
@@ -658,8 +680,10 @@ func (cc *ContentfulClient) syncCache(ctx context.Context, contentTypes []string
 			cc.syncToken,
 		)
 		col.GetAll()
+		cc.cacheMutex.sharedDataGcLock.Lock()
 		cc.syncToken = col.SyncToken
 		cc.cacheInit = true
+		cc.cacheMutex.sharedDataGcLock.Unlock()
 		if len(col.Items) == 0 {
 			if cc.logFn != nil && cc.logLevel <= LogInfo {
 				cc.logFn(map[string]interface{}{"time elapsed": fmt.Sprint(time.Since(start)), "task": "syncCache"}, LogInfo, InfoUpdateCacheTime)
@@ -805,11 +829,21 @@ func (cc *ContentfulClient) cacheSpace(ctx context.Context, contentTypes []strin
         cc.logFn(map[string]interface{}{"time elapsed": fmt.Sprint(time.Since(start)), "task": "UpdateCache"}, LogInfo, InfoUpdateCacheTime)
     }
     cc.cacheMutex.fullCacheGcLock.Lock()
-    defer cc.cacheMutex.fullCacheGcLock.Unlock()
-    cc.Cache = tempCache
     cc.cacheMutex.sharedDataGcLock.Lock()
+    cc.cacheMutex.assetsGcLock.Lock()
+    cc.cacheMutex.idContentTypeMapGcLock.Lock()
+    cc.cacheMutex.parentMapGcLock.Lock()
+    {{ range $index , $contentType := $contentTypes }}cc.cacheMutex.{{ $contentType.Sys.ID }}GcLock.Lock()
+    {{ end }}
+	cc.Cache = tempCache
     cc.offline = offlinePreviousState
+    {{ range $index , $contentType := $contentTypes }}cc.cacheMutex.{{ $contentType.Sys.ID }}GcLock.Unlock()
+    {{ end }}
+    cc.cacheMutex.parentMapGcLock.Unlock()
+    cc.cacheMutex.idContentTypeMapGcLock.Unlock()
+	cc.cacheMutex.assetsGcLock.Unlock()
     cc.cacheMutex.sharedDataGcLock.Unlock()
+	cc.cacheMutex.fullCacheGcLock.Unlock()
 }
 
 func ToAssetReference(asset *contentful.Asset) (refSys ContentTypeSys) {
@@ -922,12 +956,17 @@ func (cc *ContentfulClient) getAllAssets(tryCacheFirst bool) (map[string]*conten
 	if cc == nil || cc.Client == nil {
 		return nil, errors.New("getAllAssets: No client available")
 	}
-	if cc.cacheInit && cc.Cache.assets != nil && tryCacheFirst {
+	cc.cacheMutex.sharedDataGcLock.RLock()
+	offline := cc.offline
+	cacheInit := cc.cacheInit
+	cc.cacheMutex.sharedDataGcLock.RUnlock()
+
+	if cacheInit && cc.Cache.assets != nil && tryCacheFirst {
 		return cc.Cache.assets, nil
 	}
 	allItems := []interface{}{}
 	assets := map[string]*contentful.Asset{}
-	if cc.offline {
+	if offline {
 		for _, asset := range cc.offlineTemp.Assets {
 			allItems = append(allItems,asset)
 		}

erm/templates/contentful_vo_lib_contenttype.gotmpl

@@ -20,10 +20,14 @@ func (cc *ContentfulClient) GetAll{{ firstCap $contentType.Sys.ID }}() (voMap ma
 	if cc == nil {
 		return nil, errors.New("GetAll{{ firstCap $contentType.Sys.ID }}: No client available")
 	}
-	if cc.cacheInit {
+	cc.cacheMutex.sharedDataGcLock.RLock()
+	cacheInit := cc.cacheInit
+	optimisticPageSize := cc.optimisticPageSize
+	cc.cacheMutex.sharedDataGcLock.RUnlock()
+	if cacheInit {
 		return cc.Cache.entryMaps.{{ $contentType.Sys.ID }}, nil
 	}
-	col, err := cc.optimisticPageSizeGetAll("{{ $contentType.Sys.ID }}", cc.optimisticPageSize)
+	col, err := cc.optimisticPageSizeGetAll("{{ $contentType.Sys.ID }}", optimisticPageSize)
 	if err != nil {
 		return nil, err
 	}
@@ -740,9 +744,9 @@ func (cc *ContentfulClient) cache{{ firstCap $contentType.Sys.ID }}ByID(ctx cont
 		// delete as child
 		delete(cc.Cache.parentMap, id)
 		// delete as parent
-		for childID, child := range cc.Cache.parentMap {
+		for childID, parents := range cc.Cache.parentMap {
 			newParents := []EntryReference{}
-			for _, parent := range child {
+			for _, parent := range parents {
 				if parent.ID != id {
 					newParents = append(newParents, parent)
 				}
@@ -761,12 +765,14 @@ func (cc *ContentfulClient) cache{{ firstCap $contentType.Sys.ID }}ByID(ctx cont
 	}
 	cc.Cache.entryMaps.{{ $contentType.Sys.ID }}[id] = {{ $contentType.Sys.ID }}
 			cc.Cache.idContentTypeMap[id] = {{ $contentType.Sys.ID }}.Sys.ContentType.Sys.ID
+	allChildrensIds := map[string]bool{}
 	{{ range $fieldIndex, $field := $contentType.Fields }}
 	{{ if fieldIsMultipleReference $field }}
 	for _, loc := range cc.locales {
 		children, okChildren := {{ $contentType.Sys.ID }}.Fields.{{ firstCap $field.ID }}[string(loc)]
 		if okChildren {
 			for _, child := range children {
+				allChildrensIds[child.Sys.ID] = true
 				if cc.Cache.parentMap[child.Sys.ID] == nil {
 					cc.Cache.parentMap[child.Sys.ID] = []EntryReference{}
 				}
@@ -787,6 +793,7 @@ func (cc *ContentfulClient) cache{{ firstCap $contentType.Sys.ID }}ByID(ctx cont
 		for _, loc := range cc.locales {
 			child, okChild := {{ $contentType.Sys.ID }}.Fields.{{ firstCap $field.ID }}[string(loc)]
 			if okChild {
+				allChildrensIds[child.Sys.ID] = true
 				if cc.Cache.parentMap[child.Sys.ID] == nil {
 					cc.Cache.parentMap[child.Sys.ID] = []EntryReference{}
 				}
@@ -803,6 +810,20 @@ func (cc *ContentfulClient) cache{{ firstCap $contentType.Sys.ID }}ByID(ctx cont
 		}
 	{{ end }}
 	{{ end }}
+	_ = allChildrensIds // safety net
+	// clean up child-parents that don't exist anymore
+	for childID, parents := range cc.Cache.parentMap {
+		if _, isCollectedChildID := allChildrensIds[childID]; isCollectedChildID {
+			continue
+		} 
+		newParents := []EntryReference{}
+		for _, parent := range parents {
+			if parent.ID != id {
+				newParents = append(newParents, parent)
+			}
+		}
+		cc.Cache.parentMap[childID] = newParents
+	}
 	return nil
 }
 

main.go

@@ -13,7 +13,7 @@ import (
 	"github.com/foomo/gocontentful/erm"
 )
 
-var VERSION = "v1.0.17"
+var VERSION = "v1.0.18"
 
 type contentfulRc struct {
 	ManagementToken string `json:"managementToken"`

test/cache_test.go

@@ -10,13 +10,16 @@ import (
 
 func TestCache(t *testing.T) {
 	contentfulClient, err := getTestClient()
+	contentfulClient.ClientStats()
 	require.NoError(t, err)
 	stats, err := contentfulClient.GetCacheStats()
 	require.NoError(t, err)
 	require.Equal(t, 3, len(stats.ContentTypes))
 	require.Equal(t, 12, stats.AssetCount)
 	require.Equal(t, 9, stats.EntryCount)
 	require.Equal(t, 6, stats.ParentCount)
+	err = contentfulClient.SetSyncMode(true)
+	require.Error(t, err)
 }
 
 func TestBrokenReferences(t *testing.T) {
@@ -32,6 +35,15 @@ func TestCacheHasContentType(t *testing.T) {
 	require.True(t, contentfulClient.CacheHasContentType("brand"))
 }
 
+func TestGetAsset(t *testing.T) {
+	contentfulClient, err := getTestClient()
+	require.NoError(t, err)
+	_, err = contentfulClient.GetAssetByID("Xc0ny7GWsMEMCeASWO2um")
+	require.NoError(t, err)
+	newAsset := testapi.NewAssetFromURL("12345", "https://example.com", "PNG", "New Asset")
+	require.NotNil(t, newAsset)
+}
+
 func TestDeleteAssetFromCache(t *testing.T) {
 	contentfulClient, err := getTestClient()
 	require.NoError(t, err)
@@ -91,7 +103,7 @@ func TestPreserveCacheIfNewer(t *testing.T) {
 	require.Equal(t, 2.0, brand.Sys.Version)
 }
 
-func TestAddEntryAndSet(t *testing.T) {
+func TestEntry(t *testing.T) {
 	contentfulClient, err := getTestClient()
 	require.NoError(t, err)
 	cfProduct := testapi.NewCfProduct(contentfulClient)

test/concurrency_test.go

@@ -19,8 +19,26 @@ func readWorker(contentfulClient *testapi.ContentfulClient, i int) error {
 	if err != nil {
 		return err
 	}
+	_, err = contentfulClient.GetAllProduct()
+	if err != nil {
+		return err
+	}
 	price := product.Price()
 	testLogger.Infof("Read worker %d read price: %f", i, price)
+	_ = product.Brand()
+	_ = product.Categories()
+	_ = product.Image()
+	_ = product.Nodes()
+	_ = product.ProductDescription()
+	_ = product.ProductName()
+	_ = product.Quantity()
+	_ = product.SeoText()
+	_ = product.Sizetypecolor()
+	_ = product.Sku()
+	_ = product.Slug()
+	_ = product.Tags()
+	_ = product.Website()
+	_ = product.GetPublishingStatus()
 	return nil
 }
 
@@ -48,6 +66,19 @@ func writeWorker(contentfulClient *testapi.ContentfulClient, i int) error {
 	}
 	contentfulClient.SetProductInCache(product)
 	testLogger.Infof("Write worker %d set price: %d", i, i)
+	product.SetBrand(testapi.ContentTypeSys{})
+	product.SetCategories([]testapi.ContentTypeSys{})
+	product.SetImage([]testapi.ContentTypeSys{})
+	product.SetNodes(nil)
+	product.SetProductDescription("")
+	product.SetProductName("")
+	product.SetQuantity(1)
+	product.SetSeoText("")
+	product.SetSizetypecolor("")
+	product.SetSku("")
+	product.SetSlug("")
+	product.SetTags([]string{""})
+	product.SetWebsite("")
 	return nil
 }