Labels: Refactor label entity and cache in label.go and label_cache.go

Signed-off-by: Michael Mayer <michael@photoprism.app>

internal/entity/label.go

@@ -11,7 +11,6 @@ import (
 	"github.com/photoprism/photoprism/internal/ai/classify"
 	"github.com/photoprism/photoprism/internal/event"
 	"github.com/photoprism/photoprism/internal/form"
-
 	"github.com/photoprism/photoprism/pkg/clean"
 	"github.com/photoprism/photoprism/pkg/rnd"
 	"github.com/photoprism/photoprism/pkg/txt"
@@ -65,6 +64,13 @@ func (m *Label) AfterDelete(tx *gorm.DB) (err error) {
 	return
 }
 
+// AfterCreate sets the New column used for database callback
+func (m *Label) AfterCreate(scope *gorm.Scope) error {
+	m.New = true
+	FlushLabelCache()
+	return nil
+}
+
 // BeforeCreate creates a random UID if needed before inserting a new row to the database.
 func (m *Label) BeforeCreate(scope *gorm.Scope) error {
 	if rnd.IsUnique(m.LabelUID, LabelUID) {
@@ -158,6 +164,37 @@ func (m *Label) Restore() error {
 	return nil
 }
 
+// HasID tests if the entity has an ID and a valid UID.
+func (m *Label) HasID() bool {
+	if m == nil {
+		return false
+	}
+
+	return m.ID > 0 && m.HasUID()
+}
+
+// HasUID tests if the entity has a valid UID.
+func (m *Label) HasUID() bool {
+	if m == nil {
+		return false
+	}
+
+	return rnd.IsUID(m.LabelUID, LabelUID)
+}
+
+// Skip tests if the entity has invalid IDs or has been deleted and therefore should not be assigned.
+func (m *Label) Skip() bool {
+	if m == nil {
+		return true
+	} else if !m.HasID() {
+		return true
+	} else if m.Deleted() {
+		return true
+	}
+
+	return false
+}
+
 // Update a label property in the database.
 func (m *Label) Update(attr string, value interface{}) error {
 	return UnscopedDb().Model(m).UpdateColumn(attr, value).Error
@@ -188,43 +225,6 @@ func FirstOrCreateLabel(m *Label) *Label {
 	return nil
 }
 
-// FindLabel find the matching label based on the string provided or an error if not found.
-func FindLabel(s string, cached bool) (*Label, error) {
-	labelSlug := txt.Slug(s)
-
-	if labelSlug == "" {
-		return &Label{}, fmt.Errorf("invalid label slug %s", clean.LogQuote(labelSlug))
-	}
-
-	// Return cached label, if found.
-	if cached {
-		if cacheData, ok := labelCache.Get(labelSlug); ok {
-			log.Tracef("label: cache hit for %s", labelSlug)
-			return cacheData.(*Label), nil
-		}
-	}
-
-	// Fetch and cache label from database.
-	result := &Label{}
-
-	if find := Db().First(result, "label_slug = ? OR custom_slug = ?", labelSlug, labelSlug); find.RecordNotFound() {
-		labelCache.Delete(labelSlug)
-		return result, fmt.Errorf("label not found")
-	} else if find.Error != nil {
-		labelCache.Delete(labelSlug)
-		return result, find.Error
-	} else {
-		labelCache.SetDefault(result.LabelSlug, result)
-		return result, nil
-	}
-}
-
-// AfterCreate sets the New column used for database callback
-func (m *Label) AfterCreate(scope *gorm.Scope) error {
-	m.New = true
-	return nil
-}
-
 // SetName changes the label name.
 func (m *Label) SetName(name string) {
 	name = clean.NameCapitalized(name)

internal/entity/label_cache.go

@@ -1,14 +1,60 @@
 package entity
 
 import (
+	"fmt"
 	"time"
 
 	gc "github.com/patrickmn/go-cache"
+
+	"github.com/photoprism/photoprism/pkg/clean"
+	"github.com/photoprism/photoprism/pkg/txt"
 )
 
-var labelCache = gc.New(15*time.Minute, 15*time.Minute)
+// labelCache expiration times and cleanup interval.
+const (
+	labelDefaultExpiration = 15 * time.Minute
+	labelErrorExpiration   = 5 * time.Minute
+	labelCleanupInterval   = 5 * time.Minute
+)
 
-// FlushLabelCache resets the label cache.
+// labelCache stores Label entities for faster indexing.
+var labelCache = gc.New(labelDefaultExpiration, labelCleanupInterval)
+
+// FindLabel find the matching label based on the string provided or an error if not found.
+func FindLabel(s string, cached bool) (*Label, error) {
+	labelSlug := txt.Slug(s)
+
+	if labelSlug == "" {
+		return &Label{}, fmt.Errorf("invalid label slug %s", clean.LogQuote(labelSlug))
+	}
+
+	// Return cached label, if found.
+	if cached {
+		if cacheData, ok := labelCache.Get(labelSlug); ok {
+			log.Tracef("label: cache hit for %s", labelSlug)
+			return cacheData.(*Label), nil
+		}
+	}
+
+	// Fetch and cache label from database.
+	result := &Label{}
+
+	if find := Db().First(result, "label_slug = ? OR custom_slug = ?", labelSlug, labelSlug); find.RecordNotFound() {
+		labelCache.Delete(labelSlug)
+		labelCache.Set(result.LabelSlug, result, labelErrorExpiration)
+		return result, fmt.Errorf("label not found")
+	} else if find.Error != nil {
+		labelCache.Delete(labelSlug)
+		labelCache.Set(result.LabelSlug, result, labelErrorExpiration)
+		return result, find.Error
+	} else {
+		labelCache.SetDefault(result.LabelSlug, result)
+	}
+
+	return result, nil
+}
+
+// FlushLabelCache removes all cached Label entities from the cache.
 func FlushLabelCache() {
 	labelCache.Flush()
 }

internal/entity/label_cache_test.go

@@ -0,0 +1,40 @@
+package entity
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFindLabel(t *testing.T) {
+	t.Run("Success", func(t *testing.T) {
+		label := &Label{LabelSlug: "find-me-label", LabelName: "Find Me"}
+
+		if err := label.Save(); err != nil {
+			t.Fatal(err)
+		}
+
+		uncached, findErr := FindLabel("find-me-label", false)
+
+		assert.NoError(t, findErr)
+		assert.Equal(t, "Find Me", uncached.LabelName)
+
+		cached, cacheErr := FindLabel("find-me-label", true)
+
+		assert.NoError(t, cacheErr)
+		assert.Equal(t, "Find Me", cached.LabelName)
+		assert.Equal(t, uncached.LabelSlug, cached.LabelSlug)
+		assert.Equal(t, uncached.ID, cached.ID)
+		assert.Equal(t, uncached.LabelUID, cached.LabelUID)
+	})
+	t.Run("NotFound", func(t *testing.T) {
+		result, err := FindLabel("XXX", true)
+		assert.Error(t, err)
+		assert.NotNil(t, result)
+	})
+	t.Run("EmptyName", func(t *testing.T) {
+		result, err := FindLabel("", true)
+		assert.Error(t, err)
+		assert.NotNil(t, result)
+	})
+}

internal/entity/label_test.go

@@ -80,62 +80,62 @@ func TestFirstOrCreateLabel(t *testing.T) {
 func TestLabel_UpdateClassify(t *testing.T) {
 	t.Run("update priority and label slug", func(t *testing.T) {
 		classifyLabel := &classify.Label{Name: "classify", Uncertainty: 30, Source: "manual", Priority: 5}
-		Label := &Label{LabelName: "label", LabelSlug: "", CustomSlug: "customslug", LabelPriority: 4}
+		result := &Label{LabelName: "label", LabelSlug: "", CustomSlug: "customslug", LabelPriority: 4}
 
-		assert.Equal(t, 4, Label.LabelPriority)
-		assert.Equal(t, "", Label.LabelSlug)
-		assert.Equal(t, "customslug", Label.CustomSlug)
-		assert.Equal(t, "label", Label.LabelName)
+		assert.Equal(t, 4, result.LabelPriority)
+		assert.Equal(t, "", result.LabelSlug)
+		assert.Equal(t, "customslug", result.CustomSlug)
+		assert.Equal(t, "label", result.LabelName)
 
-		err := Label.UpdateClassify(*classifyLabel)
+		err := result.UpdateClassify(*classifyLabel)
 
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		assert.Equal(t, 5, Label.LabelPriority)
-		assert.Equal(t, "customslug", Label.LabelSlug)
-		assert.Equal(t, "classify", Label.CustomSlug)
-		assert.Equal(t, "Classify", Label.LabelName)
+		assert.Equal(t, 5, result.LabelPriority)
+		assert.Equal(t, "customslug", result.LabelSlug)
+		assert.Equal(t, "classify", result.CustomSlug)
+		assert.Equal(t, "Classify", result.LabelName)
 	})
 	t.Run("update custom slug", func(t *testing.T) {
 		classifyLabel := &classify.Label{Name: "classify", Uncertainty: 30, Source: "manual", Priority: 5}
-		Label := &Label{LabelName: "label12", LabelSlug: "labelslug", CustomSlug: "", LabelPriority: 5}
+		result := &Label{LabelName: "label12", LabelSlug: "labelslug", CustomSlug: "", LabelPriority: 5}
 
-		assert.Equal(t, 5, Label.LabelPriority)
-		assert.Equal(t, "labelslug", Label.LabelSlug)
-		assert.Equal(t, "", Label.CustomSlug)
-		assert.Equal(t, "label12", Label.LabelName)
+		assert.Equal(t, 5, result.LabelPriority)
+		assert.Equal(t, "labelslug", result.LabelSlug)
+		assert.Equal(t, "", result.CustomSlug)
+		assert.Equal(t, "label12", result.LabelName)
 
-		err := Label.UpdateClassify(*classifyLabel)
+		err := result.UpdateClassify(*classifyLabel)
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		assert.Equal(t, 5, Label.LabelPriority)
-		assert.Equal(t, "labelslug", Label.LabelSlug)
-		assert.Equal(t, "classify", Label.CustomSlug)
-		assert.Equal(t, "Classify", Label.LabelName)
+		assert.Equal(t, 5, result.LabelPriority)
+		assert.Equal(t, "labelslug", result.LabelSlug)
+		assert.Equal(t, "classify", result.CustomSlug)
+		assert.Equal(t, "Classify", result.LabelName)
 
 	})
 	t.Run("update name and Categories", func(t *testing.T) {
 		classifyLabel := &classify.Label{Name: "classify", Uncertainty: 30, Source: "manual", Priority: 5, Categories: []string{"flower", "plant"}}
-		Label := &Label{LabelName: "label34", LabelSlug: "labelslug2", CustomSlug: "labelslug2", LabelPriority: 5, LabelCategories: []*Label{LabelFixtures.Pointer("flower")}}
+		result := &Label{LabelName: "label34", LabelSlug: "labelslug2", CustomSlug: "labelslug2", LabelPriority: 5, LabelCategories: []*Label{LabelFixtures.Pointer("flower")}}
 
-		assert.Equal(t, 5, Label.LabelPriority)
-		assert.Equal(t, "labelslug2", Label.LabelSlug)
-		assert.Equal(t, "labelslug2", Label.CustomSlug)
-		assert.Equal(t, "label34", Label.LabelName)
+		assert.Equal(t, 5, result.LabelPriority)
+		assert.Equal(t, "labelslug2", result.LabelSlug)
+		assert.Equal(t, "labelslug2", result.CustomSlug)
+		assert.Equal(t, "label34", result.LabelName)
 
-		err := Label.UpdateClassify(*classifyLabel)
+		err := result.UpdateClassify(*classifyLabel)
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		assert.Equal(t, 5, Label.LabelPriority)
-		assert.Equal(t, "labelslug2", Label.LabelSlug)
-		assert.Equal(t, "classify", Label.CustomSlug)
-		assert.Equal(t, "Classify", Label.LabelName)
+		assert.Equal(t, 5, result.LabelPriority)
+		assert.Equal(t, "labelslug2", result.LabelSlug)
+		assert.Equal(t, "classify", result.CustomSlug)
+		assert.Equal(t, "Classify", result.LabelName)
 
 	})
 }
@@ -149,6 +149,7 @@ func TestLabel_Save(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
+
 		afterDate := label.UpdatedAt
 
 		assert.True(t, afterDate.After(initialDate))
@@ -164,7 +165,7 @@ func TestLabel_Delete(t *testing.T) {
 
 		var labels Labels
 
-		if err := Db().Where("label_name = ?", label.LabelName).Find(&labels).Error; err != nil {
+		if err = Db().Where("label_name = ?", label.LabelName).Find(&labels).Error; err != nil {
 			t.Fatal(err)
 		}
 
@@ -175,7 +176,7 @@ func TestLabel_Delete(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		if err := Db().Where("label_name = ?", label.LabelName).Find(&labels).Error; err != nil {
+		if err = Db().Where("label_name = ?", label.LabelName).Find(&labels).Error; err != nil {
 			t.Fatal(err)
 		}
 
@@ -185,57 +186,29 @@ func TestLabel_Delete(t *testing.T) {
 
 func TestLabel_Restore(t *testing.T) {
 	t.Run("Success", func(t *testing.T) {
-		var deleteTime = time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
+		var deletedAt = time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
+		label := &Label{DeletedAt: &deletedAt, LabelName: "ToBeRestored"}
 
-		label := &Label{DeletedAt: &deleteTime, LabelName: "ToBeRestored"}
-		err := label.Save()
-		if err != nil {
+		if err := label.Save(); err != nil {
 			t.Fatal(err)
 		}
+
 		assert.True(t, label.Deleted())
 
-		err = label.Restore()
-		if err != nil {
+		if err := label.Restore(); err != nil {
 			t.Fatal(err)
 		}
+
 		assert.False(t, label.Deleted())
 	})
 	t.Run("label not deleted", func(t *testing.T) {
 		label := &Label{DeletedAt: nil, LabelName: "NotDeleted1234"}
-		err := label.Restore()
-		if err != nil {
+
+		if err := label.Restore(); err != nil {
 			t.Fatal(err)
 		}
-		assert.False(t, label.Deleted())
-	})
-}
 
-func TestFindLabel(t *testing.T) {
-	t.Run("SaveAndFindWithCache", func(t *testing.T) {
-		label := &Label{LabelSlug: "find-me-label", LabelName: "Find Me"}
-		saveErr := label.Save()
-		if saveErr != nil {
-			t.Fatal(saveErr)
-		}
-		uncached, findErr := FindLabel("find-me-label", false)
-		assert.NoError(t, findErr)
-		assert.Equal(t, "Find Me", uncached.LabelName)
-		cached, cacheErr := FindLabel("find-me-label", true)
-		assert.NoError(t, cacheErr)
-		assert.Equal(t, "Find Me", cached.LabelName)
-		assert.Equal(t, uncached.LabelSlug, cached.LabelSlug)
-		assert.Equal(t, uncached.ID, cached.ID)
-		assert.Equal(t, uncached.LabelUID, cached.LabelUID)
-	})
-	t.Run("NotFound", func(t *testing.T) {
-		r, err := FindLabel("XXX", true)
-		assert.Error(t, err)
-		assert.NotNil(t, r)
-	})
-	t.Run("Empty", func(t *testing.T) {
-		r, err := FindLabel("", true)
-		assert.Error(t, err)
-		assert.NotNil(t, r)
+		assert.False(t, label.Deleted())
 	})
 }
 
@@ -250,16 +223,19 @@ func TestLabel_Links(t *testing.T) {
 func TestLabel_Update(t *testing.T) {
 	t.Run("Success", func(t *testing.T) {
 		label := &Label{LabelSlug: "to-be-updated", LabelName: "Update Me Please"}
+
 		err := label.Save()
+
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		err2 := label.Update("LabelSlug", "my-unique-slug")
-		if err2 != nil {
-			t.Fatal(err2)
+		err = label.Update("LabelSlug", "my-unique-slug")
+
+		if err != nil {
+			t.Fatal(err)
 		}
+
 		assert.Equal(t, "my-unique-slug", label.LabelSlug)
 	})
-
 }

internal/entity/photo.go

@@ -479,7 +479,7 @@ func (m *Photo) UpdateTitleLabels() error {
 
 	for _, w := range keywords {
 		if label, err := FindLabel(w, true); err == nil {
-			if label.Deleted() {
+			if label.Skip() {
 				continue
 			}
 
@@ -509,7 +509,7 @@ func (m *Photo) UpdateSubjectLabels() error {
 
 	for _, w := range keywords {
 		if label, err := FindLabel(w, true); err == nil {
-			if label.Deleted() {
+			if label.Skip() {
 				continue
 			}
 
@@ -537,7 +537,7 @@ func (m *Photo) UpdateKeywordLabels() error {
 
 	for _, w := range keywords {
 		if label, err := FindLabel(w, true); err == nil {
-			if label.Deleted() {
+			if label.Skip() {
 				continue
 			}