From a77b2431d34bdc4be60ecfc9ab5e0b9d5c71f94e Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Thu, 28 May 2020 21:20:42 +0200 Subject: [PATCH] Backend: Improve labels, keywords and caching Signed-off-by: Michael Mayer --- Makefile | 3 ++ internal/api/folder.go | 4 +-- internal/api/photo_label.go | 9 ++++-- internal/api/photo_thumbnail.go | 4 +-- internal/classify/const.go | 1 + internal/classify/labels.go | 6 ++-- internal/entity/label.go | 13 ++++++++ internal/entity/photo.go | 41 +++++++++++++++++++++++--- internal/entity/photo_maintenance.go | 2 +- internal/maps/places/location.go | 8 ++--- internal/photoprism/convert.go | 5 ---- internal/photoprism/index_mediafile.go | 6 +++- internal/photoprism/mediafile.go | 4 +++ internal/query/geo.go | 3 +- internal/query/photo_search.go | 3 +- internal/remote/webdav/webdav.go | 2 +- internal/tidb/tidb.go | 4 +-- pkg/fs/zip.go | 4 ++- pkg/txt/words.go | 22 ++++++++++++++ pkg/txt/words_test.go | 11 +++++++ scripts/build.sh | 5 ++++ 21 files changed, 129 insertions(+), 31 deletions(-) diff --git a/Makefile b/Makefile index 9cdc3a989..e16a0ca4c 100644 --- a/Makefile +++ b/Makefile @@ -72,6 +72,9 @@ build-js: build-go: rm -f $(BINARY_NAME) scripts/build.sh debug $(BINARY_NAME) +build-race: + rm -f $(BINARY_NAME) + scripts/build.sh race $(BINARY_NAME) build-static: rm -f $(BINARY_NAME) scripts/build.sh static $(BINARY_NAME) diff --git a/internal/api/folder.go b/internal/api/folder.go index 5f0561209..276c63989 100644 --- a/internal/api/folder.go +++ b/internal/api/folder.go @@ -43,7 +43,7 @@ func GetFolders(router *gin.RouterGroup, conf *config.Config, urlPath, rootName, if cached { if cacheData, ok := gc.Get(cacheKey); ok { log.Debugf("cache hit for %s [%s]", cacheKey, time.Since(start)) - c.JSON(http.StatusOK, cacheData.(FoldersResponse)) + c.JSON(http.StatusOK, cacheData.(*FoldersResponse)) return } } @@ -65,7 +65,7 @@ func GetFolders(router *gin.RouterGroup, conf *config.Config, urlPath, rootName, } if cached { - gc.Set(cacheKey, resp, time.Minute*5) + gc.Set(cacheKey, &resp, time.Minute*5) log.Debugf("cached %s [%s]", cacheKey, time.Since(start)) } diff --git a/internal/api/photo_label.go b/internal/api/photo_label.go index 67a264052..6b94d8851 100644 --- a/internal/api/photo_label.go +++ b/internal/api/photo_label.go @@ -5,6 +5,7 @@ import ( "strconv" "github.com/gin-gonic/gin" + "github.com/photoprism/photoprism/internal/classify" "github.com/photoprism/photoprism/internal/config" "github.com/photoprism/photoprism/internal/entity" "github.com/photoprism/photoprism/internal/event" @@ -114,11 +115,11 @@ func RemovePhotoLabel(router *gin.RouterGroup, conf *config.Config) { return } - if label.LabelSrc == entity.SrcManual { - entity.Db().Delete(&label) + if label.LabelSrc == classify.SrcManual || label.LabelSrc == classify.SrcKeyword{ + logError("label", entity.Db().Delete(&label).Error) } else { label.Uncertainty = 100 - entity.Db().Save(&label) + logError("label", entity.Db().Save(&label).Error) } p, err := query.PhotoPreloadByUID(c.Param("uid")) @@ -128,6 +129,8 @@ func RemovePhotoLabel(router *gin.RouterGroup, conf *config.Config) { return } + logError("label", p.RemoveKeyword(label.Label.LabelName)) + if err := p.Save(); err != nil { c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"error": txt.UcFirst(err.Error())}) return diff --git a/internal/api/photo_thumbnail.go b/internal/api/photo_thumbnail.go index 00dd6be5d..c4bb4ce82 100644 --- a/internal/api/photo_thumbnail.go +++ b/internal/api/photo_thumbnail.go @@ -51,7 +51,7 @@ func GetThumbnail(router *gin.RouterGroup, conf *config.Config) { if cacheData, ok := gc.Get(cacheKey); ok { log.Debugf("cache hit for %s [%s]", cacheKey, time.Since(start)) - cached := cacheData.(ThumbCache) + cached := cacheData.(*ThumbCache) if !fs.FileExists(cached.FileName) { log.Errorf("thumbnail: %s not found", fileHash) @@ -137,7 +137,7 @@ func GetThumbnail(router *gin.RouterGroup, conf *config.Config) { } // Cache thumbnail filename. - gc.Set(cacheKey, ThumbCache{thumbnail, f.ShareFileName()}, time.Hour*24) + gc.Set(cacheKey, &ThumbCache{thumbnail, f.ShareFileName()}, time.Hour*24) log.Debugf("cached %s [%s]", cacheKey, time.Since(start)) if c.Query("download") != "" { diff --git a/internal/classify/const.go b/internal/classify/const.go index c80cd543a..69b2ed111 100644 --- a/internal/classify/const.go +++ b/internal/classify/const.go @@ -6,4 +6,5 @@ const ( SrcManual = "manual" SrcLocation = "location" SrcImage = "image" + SrcKeyword = "keyword" ) diff --git a/internal/classify/labels.go b/internal/classify/labels.go index fdbfbf3ad..302a48f12 100644 --- a/internal/classify/labels.go +++ b/internal/classify/labels.go @@ -13,9 +13,9 @@ type Labels []Label func (l Labels) Len() int { return len(l) } func (l Labels) Swap(i, j int) { l[i], l[j] = l[j], l[i] } func (l Labels) Less(i, j int) bool { - if l[i].Uncertainty >= 100 { + if l[i].Uncertainty >= 100 || l[i].Source == SrcKeyword { return false - } else if l[j].Uncertainty >= 100 { + } else if l[j].Uncertainty >= 100 || l[j].Source == SrcKeyword { return true } else if l[i].Priority == l[j].Priority { return l[i].Uncertainty < l[j].Uncertainty @@ -36,7 +36,7 @@ func (l Labels) AppendLabel(label Label) Labels { // Keywords returns all keywords contains in Labels and their categories func (l Labels) Keywords() (result []string) { for _, label := range l { - if label.Uncertainty >= 100 { + if label.Uncertainty >= 100 || label.Source == SrcKeyword { continue } diff --git a/internal/entity/label.go b/internal/entity/label.go index b56453e9b..87c5c098e 100644 --- a/internal/entity/label.go +++ b/internal/entity/label.go @@ -94,6 +94,19 @@ func FirstOrCreateLabel(m *Label) *Label { return m } +// FindLabel returns an existing row if exists. +func FindLabel(s string) *Label { + labelSlug := slug.Make(txt.Clip(s, txt.ClipSlug)) + + result := Label{} + + if err := Db().Where("label_slug = ? OR custom_slug = ?", labelSlug, labelSlug).First(&result).Error; err == nil { + return &result + } + + return nil +} + // AfterCreate sets the New column used for database callback func (m *Label) AfterCreate(scope *gorm.Scope) error { m.New = true diff --git a/internal/entity/photo.go b/internal/entity/photo.go index 165363010..c93237363 100644 --- a/internal/entity/photo.go +++ b/internal/entity/photo.go @@ -102,12 +102,16 @@ func SavePhotoForm(model Photo, form form.Photo, geoApi string) error { model.Details.Keywords = strings.Join(txt.UniqueWords(w), ", ") } + if err := model.SyncKeywordLabels(); err != nil { + log.Errorf("photo: %s", err) + } + if err := model.UpdateTitle(model.ClassifyLabels()); err != nil { log.Warn(err) } if err := model.IndexKeywords(); err != nil { - log.Error(err) + log.Errorf("photo: %s", err.Error()) } edited := time.Now().UTC() @@ -146,7 +150,7 @@ func (m *Photo) Save() error { } if err := m.IndexKeywords(); err != nil { - log.Error(err) + log.Errorf("photo: %s", err.Error()) } m.PhotoQuality = m.QualityScore() @@ -216,10 +220,39 @@ func (m *Photo) BeforeSave(scope *gorm.Scope) error { return nil } +// RemoveKeyword removes a word from photo keywords. +func (m *Photo) RemoveKeyword(w string) error { + if !m.DetailsLoaded() { + return fmt.Errorf("can't remove keyword, details not loaded") + } + + words := txt.RemoveFromWords(txt.Words(m.Details.Keywords), w) + + m.Details.Keywords = strings.Join(words, ", ") + + return nil +} + +// SyncKeywordLabels maintains the label / photo relationship for existing labels and keywords. +func (m *Photo) SyncKeywordLabels() error { + keywords := txt.UniqueKeywords(m.Details.Keywords) + + var labelIds []uint + + for _, w := range keywords { + if label := FindLabel(w); label != nil { + labelIds = append(labelIds, label.ID) + FirstOrCreatePhotoLabel(NewPhotoLabel(m.ID, label.ID, 5, classify.SrcKeyword)) + } + } + + return Db().Where("label_src = ? AND photo_id = ? AND label_id NOT IN (?)", classify.SrcKeyword, m.ID, labelIds).Delete(&PhotoLabel{}).Error +} + // IndexKeywords adds given keywords to the photo entry func (m *Photo) IndexKeywords() error { if !m.DetailsLoaded() { - return fmt.Errorf("photo: can't index keywords, details not loaded for %s", m.PhotoUID) + return fmt.Errorf("can't index keywords, details not loaded") } db := Db() @@ -240,7 +273,7 @@ func (m *Photo) IndexKeywords() error { kw := FirstOrCreateKeyword(NewKeyword(w)) if kw == nil { - log.Errorf("photo: index keyword should not be nil - bug?") + log.Errorf("index keyword should not be nil - bug?") continue } diff --git a/internal/entity/photo_maintenance.go b/internal/entity/photo_maintenance.go index 9e2232345..a14914168 100644 --- a/internal/entity/photo_maintenance.go +++ b/internal/entity/photo_maintenance.go @@ -59,7 +59,7 @@ func (m *Photo) Maintain() error { } if err := m.IndexKeywords(); err != nil { - log.Error(err) + log.Errorf("photo: %s", err.Error()) } m.PhotoQuality = m.QualityScore() diff --git a/internal/maps/places/location.go b/internal/maps/places/location.go index 456f1554b..12a7f8e80 100644 --- a/internal/maps/places/location.go +++ b/internal/maps/places/location.go @@ -54,9 +54,9 @@ func FindLocation(id string) (result Location, err error) { if hit, ok := cache.Get(id); ok { log.Debugf("api: cache hit for lat %f, lng %f (%s)", lat, lng, ApiName) - result = hit.(Location) - result.Cached = true - return result, nil + cached := hit.(*Location) + cached.Cached = true + return *cached, nil } url := fmt.Sprintf(ReverseLookupURL, id) @@ -101,7 +101,7 @@ func FindLocation(id string) (result Location, err error) { return result, fmt.Errorf("api: no result for %s (%s)", id, ApiName) } - cache.Set(id, result, gc.DefaultExpiration) + cache.Set(id, &result, gc.DefaultExpiration) result.Cached = false diff --git a/internal/photoprism/convert.go b/internal/photoprism/convert.go index 36017912b..78df43a78 100644 --- a/internal/photoprism/convert.go +++ b/internal/photoprism/convert.go @@ -8,7 +8,6 @@ import ( "os" "os/exec" "path/filepath" - "runtime" "sync" "github.com/karrick/godirwalk" @@ -249,10 +248,6 @@ func (c *Convert) ToJpeg(image *MediaFile, hidden bool) (*MediaFile, error) { return nil, err } - // Unclear if this is really necessary here, but safe is safe. - runtime.LockOSThread() - defer runtime.UnlockOSThread() - if useMutex { // Make sure only one command is executed at a time. // See https://photo.stackexchange.com/questions/105969/darktable-cli-fails-because-of-locked-database-file diff --git a/internal/photoprism/index_mediafile.go b/internal/photoprism/index_mediafile.go index 6e68afdcd..eb02c36f7 100644 --- a/internal/photoprism/index_mediafile.go +++ b/internal/photoprism/index_mediafile.go @@ -462,8 +462,12 @@ func (ind *Index) MediaFile(m *MediaFile, o IndexOptions, originalName string) ( return result } + if err := photo.SyncKeywordLabels(); err != nil { + log.Errorf("index: %s for %s", err, quotedName) + } + if err := photo.IndexKeywords(); err != nil { - log.Errorf("%s for %s", err, quotedName) + log.Errorf("index: %s for %s", err, quotedName) } } else { if photo.PhotoQuality >= 0 { diff --git a/internal/photoprism/mediafile.go b/internal/photoprism/mediafile.go index 59316146a..081155043 100644 --- a/internal/photoprism/mediafile.go +++ b/internal/photoprism/mediafile.go @@ -636,6 +636,10 @@ func (m *MediaFile) decodeDimensions() error { if m.IsJpeg() { file, err := os.Open(m.FileName()) + if err != nil || file == nil { + return err + } + defer file.Close() size, _, err := image.DecodeConfig(file) diff --git a/internal/query/geo.go b/internal/query/geo.go index 8117348d8..a42938a79 100644 --- a/internal/query/geo.go +++ b/internal/query/geo.go @@ -33,7 +33,8 @@ func Geo(f form.GeoSearch) (results GeoResults, err error) { Joins(`JOIN files ON files.photo_id = photos.id AND files.file_missing = 0 AND files.file_primary AND files.deleted_at IS NULL`). Where("photos.deleted_at IS NULL"). - Where("photos.photo_lat <> 0") + Where("photos.photo_lat <> 0"). + Group("photos.id, files.id") f.Query = txt.Clip(f.Query, txt.ClipKeyword) diff --git a/internal/query/photo_search.go b/internal/query/photo_search.go index 8408b965e..00c98ed29 100644 --- a/internal/query/photo_search.go +++ b/internal/query/photo_search.go @@ -38,7 +38,8 @@ func PhotoSearch(f form.PhotoSearch) (results PhotoResults, count int, err error Joins("JOIN cameras ON photos.camera_id = cameras.id"). Joins("JOIN lenses ON photos.lens_id = lenses.id"). Joins("JOIN places ON photos.place_uid = places.place_uid"). - Where("files.file_type = 'jpg' OR files.file_video = 1") + Where("files.file_type = 'jpg' OR files.file_video = 1"). + Group("photos.id, files.id") // Shortcut for known photo ids. if f.ID != "" { diff --git a/internal/remote/webdav/webdav.go b/internal/remote/webdav/webdav.go index e253c5cc2..f461a95a7 100644 --- a/internal/remote/webdav/webdav.go +++ b/internal/remote/webdav/webdav.go @@ -180,7 +180,7 @@ func (c Client) CreateDir(dir string) error { func (c Client) Upload(from, to string) error { file, err := os.Open(from) - if err != nil { + if err != nil || file == nil { return err } diff --git a/internal/tidb/tidb.go b/internal/tidb/tidb.go index 3a5c29e23..878472b70 100644 --- a/internal/tidb/tidb.go +++ b/internal/tidb/tidb.go @@ -10,8 +10,6 @@ func InitDatabase(port uint, password string) error { db, err := sql.Open("mysql", fmt.Sprintf("root:@tcp(localhost:%d)/", port)) - defer db.Close() - if err != nil { log.Debug(err.Error()) log.Debug("database login as root with password") @@ -23,6 +21,8 @@ func InitDatabase(port uint, password string) error { return err } + defer db.Close() + if password != "" { log.Debug("set database password") diff --git a/pkg/fs/zip.go b/pkg/fs/zip.go index acdebaf0f..e7c5a7d5c 100644 --- a/pkg/fs/zip.go +++ b/pkg/fs/zip.go @@ -31,11 +31,12 @@ func Zip(filename string, files []string) error { } func AddToZip(zipWriter *zip.Writer, filename string) error { - fileToZip, err := os.Open(filename) + if err != nil { return err } + defer fileToZip.Close() // Get the file information @@ -64,6 +65,7 @@ func AddToZip(zipWriter *zip.Writer, filename string) error { // Extract Zip file in destination directory func Unzip(src, dest string) (fileNames []string, err error) { r, err := zip.OpenReader(src) + if err != nil { return fileNames, err } diff --git a/pkg/txt/words.go b/pkg/txt/words.go index f6b03f536..c9842cf19 100644 --- a/pkg/txt/words.go +++ b/pkg/txt/words.go @@ -72,6 +72,28 @@ func UniqueWords(words []string) (results []string) { return results } +// RemoveFromWords removes words from a string slice and returns the sorted result. +func RemoveFromWords(words []string, remove string) (results []string) { + remove = strings.ToLower(remove) + last := "" + + SortCaseInsensitive(words) + + for _, w := range words { + w = strings.ToLower(w) + + if len(w) < 3 || w == last || strings.Contains(remove, w){ + continue + } + + last = w + + results = append(results, w) + } + + return results +} + // UniqueKeywords returns a slice of unique and sorted keywords without stopwords. func UniqueKeywords(s string) (results []string) { last := "" diff --git a/pkg/txt/words_test.go b/pkg/txt/words_test.go index cd6f1ae02..64d4b6e64 100644 --- a/pkg/txt/words_test.go +++ b/pkg/txt/words_test.go @@ -126,3 +126,14 @@ func TestUniqueKeywords(t *testing.T) { assert.Equal(t, []string(nil), result) }) } + +func TestRemoveFromWords(t *testing.T) { + t.Run("brown apple", func(t *testing.T) { + result := RemoveFromWords([]string{"lazy", "jpg", "Brown", "apple", "brown", "new-york", "JPG"}, "brown apple") + assert.Equal(t, []string{"jpg", "lazy", "new-york"}, result) + }) + t.Run("empty", func(t *testing.T) { + result := RemoveFromWords([]string{"lazy", "jpg", "Brown", "apple"}, "") + assert.Equal(t, []string{"apple", "brown", "jpg", "lazy"}, result) + }) +} diff --git a/scripts/build.sh b/scripts/build.sh index a6b76592f..91c88bd15 100755 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -30,6 +30,11 @@ if [[ $1 == "debug" ]]; then go build -ldflags "-X main.version=${PHOTOPRISM_DATE}-${PHOTOPRISM_VERSION}-${PHOTOPRISM_OS}-${PHOTOPRISM_ARCH}-DEBUG" -o $2 cmd/photoprism/photoprism.go du -h $2 echo "Done." +elif [[ $1 == "race" ]]; then + echo "Building with data race detector..." + go build -race -ldflags "-X main.version=${PHOTOPRISM_DATE}-${PHOTOPRISM_VERSION}-${PHOTOPRISM_OS}-${PHOTOPRISM_ARCH}-DEBUG" -o $2 cmd/photoprism/photoprism.go + du -h $2 + echo "Done." elif [[ $1 == "static" ]]; then echo "Building static production binary..." go build -a -v -ldflags "-linkmode external -extldflags \"-static -L /usr/lib -ltensorflow\" -s -w -X main.version=${PHOTOPRISM_DATE}-${PHOTOPRISM_VERSION}-${PHOTOPRISM_OS}-${PHOTOPRISM_ARCH}" -o $2 cmd/photoprism/photoprism.go