From dd42d2b82342f0a963ca6d3575cd64a98b628feb Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Tue, 26 May 2020 11:00:39 +0200 Subject: [PATCH] Entities: Refactor FirstOrCreate Signed-off-by: Michael Mayer --- internal/api/account.go | 4 +- internal/api/album.go | 8 +++- internal/api/photo_label.go | 26 ++++++----- internal/entity/account.go | 24 +++++++++- internal/entity/account_test.go | 2 +- internal/entity/album.go | 17 ++++++- internal/entity/album_test.go | 2 +- internal/entity/camera.go | 8 +--- internal/entity/country.go | 25 +++++++++-- internal/entity/country_test.go | 7 ++- internal/entity/details.go | 20 +++++++-- internal/entity/details_test.go | 11 ++--- internal/entity/file.go | 2 +- internal/entity/file_share.go | 33 ++++++++++++-- internal/entity/file_share_test.go | 17 +++++-- internal/entity/file_sync.go | 33 ++++++++++++-- internal/entity/file_sync_test.go | 17 +++++-- internal/entity/folder.go | 2 +- internal/entity/keyword.go | 31 +++++++++++-- internal/entity/keyword_test.go | 13 ++++-- internal/entity/label.go | 39 +++++++++++++--- internal/entity/label_test.go | 24 +++++++--- internal/entity/lens.go | 8 +--- internal/entity/location.go | 8 +--- internal/entity/photo.go | 46 ++++++++++--------- internal/entity/photo_album.go | 18 ++++++-- internal/entity/photo_album_test.go | 19 ++++++-- internal/entity/photo_keyword.go | 18 ++++++-- internal/entity/photo_keyword_test.go | 19 ++++++-- internal/entity/photo_label.go | 65 ++++++++++++++++++--------- internal/entity/photo_label_test.go | 27 +++++++---- internal/entity/photo_location.go | 8 +--- internal/entity/place.go | 10 ++--- internal/workers/sync.go | 23 +++++----- internal/workers/sync_refresh.go | 20 +++++---- 35 files changed, 466 insertions(+), 188 deletions(-) diff --git a/internal/api/account.go b/internal/api/account.go index fd767bfc1..578a00a32 100644 --- a/internal/api/account.go +++ b/internal/api/account.go @@ -139,7 +139,7 @@ func ShareWithAccount(router *gin.RouterGroup, conf *config.Config) { dstFileName := dst + "/" + file.ShareFileName() fileShare := entity.NewFileShare(file.ID, m.ID, dstFileName) - fileShare.FirstOrCreate() + entity.FirstOrCreateFileShare(fileShare) } workers.StartShare(conf) @@ -222,7 +222,7 @@ func UpdateAccount(router *gin.RouterGroup, conf *config.Config) { } // 3) Save model with values from form - if err := m.Save(f); err != nil { + if err := m.SaveForm(f); err != nil { log.Error(err) c.AbortWithStatusJSON(http.StatusInternalServerError, ErrSaveFailed) return diff --git a/internal/api/album.go b/internal/api/album.go index ed2b0d330..152c9abc3 100644 --- a/internal/api/album.go +++ b/internal/api/album.go @@ -138,7 +138,7 @@ func UpdateAlbum(router *gin.RouterGroup, conf *config.Config) { return } - if err := m.Save(f); err != nil { + if err := m.SaveForm(f); err != nil { log.Error(err) c.AbortWithStatusJSON(http.StatusInternalServerError, ErrSaveFailed) return @@ -273,7 +273,11 @@ func AddPhotosToAlbum(router *gin.RouterGroup, conf *config.Config) { var added []*entity.PhotoAlbum for _, p := range photos { - added = append(added, entity.NewPhotoAlbum(p.PhotoUID, a.AlbumUID).FirstOrCreate()) + val := entity.FirstOrCreatePhotoAlbum(entity.NewPhotoAlbum(p.PhotoUID, a.AlbumUID)) + + if val != nil { + added = append(added, val) + } } if len(added) == 1 { diff --git a/internal/api/photo_label.go b/internal/api/photo_label.go index 5a491b132..f90e0aa11 100644 --- a/internal/api/photo_label.go +++ b/internal/api/photo_label.go @@ -38,27 +38,29 @@ func AddPhotoLabel(router *gin.RouterGroup, conf *config.Config) { return } - lm := entity.NewLabel(f.LabelName, f.LabelPriority).FirstOrCreate() + labelEntity := entity.FirstOrCreateLabel(entity.NewLabel(f.LabelName, f.LabelPriority)) - if lm.New && f.LabelPriority >= 0 { - event.Publish("count.labels", event.Data{ - "count": 1, - }) + if labelEntity == nil { + c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"error": "failed creating label"}) + return } - plm := entity.NewPhotoLabel(m.ID, lm.ID, f.Uncertainty, "manual").FirstOrCreate() + photoLabel := entity.FirstOrCreatePhotoLabel(entity.NewPhotoLabel(m.ID, labelEntity.ID, f.Uncertainty, "manual")) - if plm.Uncertainty > f.Uncertainty { - plm.Uncertainty = f.Uncertainty - plm.LabelSrc = entity.SrcManual + if photoLabel == nil { + c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"error": "failed updating photo label"}) + return + } - if err := entity.Db().Save(&plm).Error; err != nil { + if photoLabel.Uncertainty > f.Uncertainty { + if err := photoLabel.Updates(map[string]interface{}{ + "Uncertainty": f.Uncertainty, + "LabelSrc": entity.SrcManual, + }); err != nil { log.Errorf("label: %s", err) } } - entity.Db().Save(&lm) - p, err := query.PreloadPhotoByUID(c.Param("uid")) if err != nil { diff --git a/internal/entity/account.go b/internal/entity/account.go index 8da4adfcb..51a9ba87a 100644 --- a/internal/entity/account.go +++ b/internal/entity/account.go @@ -59,13 +59,13 @@ func CreateAccount(form form.Account) (model *Account, err error) { SyncStatus: AccountSyncStatusRefresh, } - err = model.Save(form) + err = model.SaveForm(form) return model, err } // Saves the entity using form data and stores it in the database. -func (m *Account) Save(form form.Account) error { +func (m *Account) SaveForm(form form.Account) error { db := Db() if err := deepcopier.Copy(m).From(form); err != nil { @@ -111,3 +111,23 @@ func (m *Account) Directories() (result fs.FileInfos, err error) { return result, err } + +// Updates multiple columns in the database. +func (m *Account) Updates(values interface{}) error { + return UnscopedDb().Model(m).UpdateColumns(values).Error +} + +// Updates a column in the database. +func (m *Account) Update(attr string, value interface{}) error { + return UnscopedDb().Model(m).UpdateColumn(attr, value).Error +} + +// Save updates the existing or inserts a new row. +func (m *Account) Save() error { + return Db().Save(m).Error +} + +// Create inserts a new row to the database. +func (m *Account) Create() error { + return Db().Create(m).Error +} diff --git a/internal/entity/account_test.go b/internal/entity/account_test.go index 35e6891e5..e8dda1c3a 100644 --- a/internal/entity/account_test.go +++ b/internal/entity/account_test.go @@ -73,7 +73,7 @@ func TestAccount_Save(t *testing.T) { UpdateForm, err := form.NewAccount(accountUpdate) - err = model.Save(UpdateForm) + err = model.SaveForm(UpdateForm) if err != nil { t.Fatal(err) diff --git a/internal/entity/album.go b/internal/entity/album.go index 3ced249c9..9d8f667af 100644 --- a/internal/entity/album.go +++ b/internal/entity/album.go @@ -83,7 +83,7 @@ func (m *Album) SetTitle(title string) { } // Saves the entity using form data and stores it in the database. -func (m *Album) Save(f form.Album) error { +func (m *Album) SaveForm(f form.Album) error { if err := deepcopier.Copy(m).From(f); err != nil { return err } @@ -94,3 +94,18 @@ func (m *Album) Save(f form.Album) error { return Db().Save(m).Error } + +// Updates a column in the database. +func (m *Album) Update(attr string, value interface{}) error { + return UnscopedDb().Model(m).UpdateColumn(attr, value).Error +} + +// Save updates the existing or inserts a new row. +func (m *Album) Save() error { + return Db().Save(m).Error +} + +// Create inserts a new row to the database. +func (m *Album) Create() error { + return Db().Create(m).Error +} diff --git a/internal/entity/album_test.go b/internal/entity/album_test.go index 49a9ba18a..a435abca2 100644 --- a/internal/entity/album_test.go +++ b/internal/entity/album_test.go @@ -78,7 +78,7 @@ func TestAlbum_Save(t *testing.T) { t.Fatal(err) } - err = album.Save(albumForm) + err = album.SaveForm(albumForm) if err != nil { t.Fatal(err) diff --git a/internal/entity/camera.go b/internal/entity/camera.go index a5fe50dce..756689eb1 100644 --- a/internal/entity/camera.go +++ b/internal/entity/camera.go @@ -64,14 +64,10 @@ func NewCamera(modelName string, makeName string) *Camera { // Create inserts a new row to the database. func (m *Camera) Create() error { - if err := Db().Create(m).Error; err != nil { - return err - } - - return nil + return Db().Create(m).Error } -// FirstOrCreateCamera inserts a new row if not exists. +// FirstOrCreateCamera returns the existing row, inserts a new row or nil in case of errors. func FirstOrCreateCamera(m *Camera) *Camera { result := Camera{} diff --git a/internal/entity/country.go b/internal/entity/country.go index 5264f9ce3..c294bc878 100644 --- a/internal/entity/country.go +++ b/internal/entity/country.go @@ -3,6 +3,7 @@ package entity import ( "github.com/gosimple/slug" "github.com/jinzhu/gorm" + "github.com/photoprism/photoprism/internal/event" "github.com/photoprism/photoprism/internal/maps" ) @@ -34,7 +35,7 @@ var UnknownCountry = Country{ // CreateUnknownCountry is used to initialize the database with the default country func CreateUnknownCountry() { - UnknownCountry.FirstOrCreate() + FirstOrCreateCountry(&UnknownCountry) } // NewCountry creates a new country, with default country code if not provided @@ -58,10 +59,26 @@ func NewCountry(countryCode string, countryName string) *Country { return result } -// FirstOrCreate checks if the country exist already in the database (using countryCode) -func (m *Country) FirstOrCreate() *Country { - if err := Db().FirstOrCreate(m, "id = ?", m.ID).Error; err != nil { +// Create inserts a new row to the database. +func (m *Country) Create() error { + return Db().Create(m).Error +} + +// FirstOrCreateCountry returns the existing row, inserts a new row or nil in case of errors. +func FirstOrCreateCountry(m *Country) *Country { + result := Country{} + + if err := Db().Where("id = ?", m.ID).First(&result).Error; err == nil { + return &result + } else if err := m.Create(); err != nil { log.Errorf("country: %s", err) + return nil + } + + if m.ID != UnknownCountry.ID { + event.Publish("count.countries", event.Data{ + "count": 1, + }) } return m diff --git a/internal/entity/country_test.go b/internal/entity/country_test.go index 789d687a6..3a2b611d1 100644 --- a/internal/entity/country_test.go +++ b/internal/entity/country_test.go @@ -25,10 +25,13 @@ func TestNewCountry(t *testing.T) { }) } -func TestCountry_FirstOrCreate(t *testing.T) { +func TestFirstOrCreateCountry(t *testing.T) { t.Run("es", func(t *testing.T) { country := NewCountry("es", "spain") - country.FirstOrCreate() + country = FirstOrCreateCountry(country) + if country == nil { + t.Fatal("country should not be nil") + } }) } diff --git a/internal/entity/details.go b/internal/entity/details.go index 25167064d..90f6eb1df 100644 --- a/internal/entity/details.go +++ b/internal/entity/details.go @@ -11,9 +11,23 @@ type Details struct { License string `gorm:"type:varchar(255);" json:"License" yaml:"License,omitempty"` } -// FirstOrCreate returns the matching entity or creates a new one. -func (m *Details) FirstOrCreate() error { - return Db().FirstOrCreate(m, "photo_id = ?", m.PhotoID).Error +// Create inserts a new row to the database. +func (m *Details) Create() error { + return Db().Create(m).Error +} + +// FirstOrCreateDetails returns the existing row, inserts a new row or nil in case of errors. +func FirstOrCreateDetails(m *Details) *Details { + result := Details{} + + if err := Db().Where("photo_id = ?", m.PhotoID).First(&result).Error; err == nil { + return &result + } else if err := m.Create(); err != nil { + log.Errorf("details: %s", err) + return nil + } + + return m } // NoKeywords checks if the photo has no Keywords diff --git a/internal/entity/details_test.go b/internal/entity/details_test.go index 32ba27611..1ffa08eb9 100644 --- a/internal/entity/details_test.go +++ b/internal/entity/details_test.go @@ -5,11 +5,12 @@ import ( "testing" ) -func TestDetails_FirstOrCreate(t *testing.T) { - description := &Details{PhotoID: 123, Keywords: ""} - err := description.FirstOrCreate() - if err != nil { - t.Fatal(err) +func TestFirstOrCreateDetails(t *testing.T) { + details := &Details{PhotoID: 123, Keywords: ""} + details = FirstOrCreateDetails(details) + + if details == nil { + t.Fatal("details should not be nil") } } diff --git a/internal/entity/file.go b/internal/entity/file.go index a59afb7f0..5489060dc 100644 --- a/internal/entity/file.go +++ b/internal/entity/file.go @@ -168,7 +168,7 @@ func (m *File) UpdateVideoInfos() error { return Db().Model(File{}).Where("photo_id = ? AND file_video = 1", m.PhotoID).Updates(values).Error } -// Updates a model attribute. +// Updates a column in the database. func (m *File) Update(attr string, value interface{}) error { return UnscopedDb().Model(m).UpdateColumn(attr, value).Error } diff --git a/internal/entity/file_share.go b/internal/entity/file_share.go index d9b3e4960..ab8c517fc 100644 --- a/internal/entity/file_share.go +++ b/internal/entity/file_share.go @@ -44,10 +44,35 @@ func NewFileShare(fileID, accountID uint, remoteName string) *FileShare { return result } -// FirstOrCreate returns the matching entity or creates a new one. -func (m *FileShare) FirstOrCreate() *FileShare { - if err := Db().FirstOrCreate(m, "file_id = ? AND account_id = ? AND remote_name = ?", m.FileID, m.AccountID, m.RemoteName).Error; err != nil { - log.Errorf("file push: %s", err) +// Updates multiple columns in the database. +func (m *FileShare) Updates(values interface{}) error { + return UnscopedDb().Model(m).UpdateColumns(values).Error +} + +// Updates a column in the database. +func (m *FileShare) Update(attr string, value interface{}) error { + return UnscopedDb().Model(m).UpdateColumn(attr, value).Error +} + +// Save updates the existing or inserts a new row. +func (m *FileShare) Save() error { + return Db().Save(m).Error +} + +// Create inserts a new row to the database. +func (m *FileShare) Create() error { + return Db().Create(m).Error +} + +// FirstOrCreateFileShare returns the existing row, inserts a new row or nil in case of errors. +func FirstOrCreateFileShare(m *FileShare) *FileShare { + result := FileShare{} + + if err := Db().Where("file_id = ? AND account_id = ? AND remote_name = ?", m.FileID, m.AccountID, m.RemoteName).First(&result).Error; err == nil { + return &result + } else if err := m.Create(); err != nil { + log.Errorf("file-share: %s", err) + return nil } return m diff --git a/internal/entity/file_share_test.go b/internal/entity/file_share_test.go index c5f325440..fcd43d5de 100644 --- a/internal/entity/file_share_test.go +++ b/internal/entity/file_share_test.go @@ -19,8 +19,19 @@ func TestNewFileShare(t *testing.T) { assert.Equal(t, "new", r.Status) } -func TestFileShare_FirstOrCreate(t *testing.T) { +func TestFirstOrCreateFileShare(t *testing.T) { fileShare := &FileShare{FileID: 123, AccountID: 888, RemoteName: "test888"} - r := fileShare.FirstOrCreate() - assert.Equal(t, uint(0x7b), r.FileID) + result := FirstOrCreateFileShare(fileShare) + + if result == nil { + t.Fatal("result share should not be nil") + } + + if result.FileID != fileShare.FileID { + t.Errorf("FileID should be the same: %d %d", result.FileID, fileShare.FileID) + } + + if result.AccountID != fileShare.AccountID { + t.Errorf("AccountID should be the same: %d %d", result.AccountID, fileShare.AccountID) + } } diff --git a/internal/entity/file_sync.go b/internal/entity/file_sync.go index 4250e6f14..6ab8979e2 100644 --- a/internal/entity/file_sync.go +++ b/internal/entity/file_sync.go @@ -44,10 +44,35 @@ func NewFileSync(accountID uint, remoteName string) *FileSync { return result } -// FirstOrCreate returns the matching entity or creates a new one. -func (m *FileSync) FirstOrCreate() *FileSync { - if err := Db().FirstOrCreate(m, "account_id = ? AND remote_name = ?", m.AccountID, m.RemoteName).Error; err != nil { - log.Errorf("file sync: %s", err) +// Updates multiple columns in the database. +func (m *FileSync) Updates(values interface{}) error { + return UnscopedDb().Model(m).UpdateColumns(values).Error +} + +// Updates a column in the database. +func (m *FileSync) Update(attr string, value interface{}) error { + return UnscopedDb().Model(m).UpdateColumn(attr, value).Error +} + +// Save updates the existing or inserts a new row. +func (m *FileSync) Save() error { + return Db().Save(m).Error +} + +// Create inserts a new row to the database. +func (m *FileSync) Create() error { + return Db().Create(m).Error +} + +// FirstOrCreateFileSync returns the existing row, inserts a new row or nil in case of errors. +func FirstOrCreateFileSync(m *FileSync) *FileSync { + result := FileSync{} + + if err := Db().Where("account_id = ? AND remote_name = ?", m.AccountID, m.RemoteName).First(&result).Error; err == nil { + return &result + } else if err := m.Create(); err != nil { + log.Errorf("file-sync: %s", err) + return nil } return m diff --git a/internal/entity/file_sync_test.go b/internal/entity/file_sync_test.go index 8c2a2e51e..601730507 100644 --- a/internal/entity/file_sync_test.go +++ b/internal/entity/file_sync_test.go @@ -18,8 +18,19 @@ func TestNewFileSync(t *testing.T) { assert.Equal(t, "new", r.Status) } -func TestFileSync_FirstOrCreate(t *testing.T) { +func TestFirstOrCreateFileSync(t *testing.T) { fileSync := &FileSync{AccountID: 123, FileID: 888, RemoteName: "test123"} - r := fileSync.FirstOrCreate() - assert.Equal(t, uint(0x7b), r.AccountID) + result := FirstOrCreateFileSync(fileSync) + + if result == nil { + t.Fatal("result should not be nil") + } + + if result.FileID != fileSync.FileID { + t.Errorf("FileID should be the same: %d %d", result.FileID, fileSync.FileID) + } + + if result.AccountID != fileSync.AccountID { + t.Errorf("AccountID should be the same: %d %d", result.AccountID, fileSync.AccountID) + } } diff --git a/internal/entity/folder.go b/internal/entity/folder.go index bfee9285e..4d6fc5ddf 100644 --- a/internal/entity/folder.go +++ b/internal/entity/folder.go @@ -116,7 +116,7 @@ func (m *Folder) Create() error { return nil } -// FirstOrCreateFolder returns the first matching record or creates a new one with the given conditions. +// FirstOrCreateFolder returns the existing row, inserts a new row or nil in case of errors. func FirstOrCreateFolder(m *Folder) *Folder { result := Folder{} diff --git a/internal/entity/keyword.go b/internal/entity/keyword.go index c7326ea97..6129d4b45 100644 --- a/internal/entity/keyword.go +++ b/internal/entity/keyword.go @@ -24,10 +24,35 @@ func NewKeyword(keyword string) *Keyword { return result } -// FirstOrCreate checks if the keyword already exist in the database -func (m *Keyword) FirstOrCreate() *Keyword { - if err := Db().FirstOrCreate(m, "keyword = ?", m.Keyword).Error; err != nil { +// Updates multiple columns in the database. +func (m *Keyword) Updates(values interface{}) error { + return UnscopedDb().Model(m).UpdateColumns(values).Error +} + +// Updates a column in the database. +func (m *Keyword) Update(attr string, value interface{}) error { + return UnscopedDb().Model(m).UpdateColumn(attr, value).Error +} + +// Save updates the existing or inserts a new row. +func (m *Keyword) Save() error { + return Db().Save(m).Error +} + +// Create inserts a new row to the database. +func (m *Keyword) Create() error { + return Db().Create(m).Error +} + +// FirstOrCreateKeyword returns the existing row, inserts a new row or nil in case of errors. +func FirstOrCreateKeyword(m *Keyword) *Keyword { + result := Keyword{} + + if err := Db().Where("keyword = ?", m.Keyword).First(&result).Error; err == nil { + return &result + } else if err := m.Create(); err != nil { log.Errorf("keyword: %s", err) + return nil } return m diff --git a/internal/entity/keyword_test.go b/internal/entity/keyword_test.go index 951d86d48..b20532c54 100644 --- a/internal/entity/keyword_test.go +++ b/internal/entity/keyword_test.go @@ -19,8 +19,15 @@ func TestNewKeyword(t *testing.T) { }) } -func TestKeyword_FirstOrCreate(t *testing.T) { +func TestFirstOrCreateKeyword(t *testing.T) { keyword := NewKeyword("food") - r := keyword.FirstOrCreate() - assert.Equal(t, "food", r.Keyword) + result := FirstOrCreateKeyword(keyword) + + if result == nil { + t.Fatal("result should not be nil") + } + + if result.Keyword != keyword.Keyword { + t.Errorf("Keyword should be the same: %s %s", result.Keyword, keyword.Keyword) + } } diff --git a/internal/entity/label.go b/internal/entity/label.go index 82053e59e..5592ba534 100644 --- a/internal/entity/label.go +++ b/internal/entity/label.go @@ -6,6 +6,7 @@ import ( "github.com/gosimple/slug" "github.com/jinzhu/gorm" "github.com/photoprism/photoprism/internal/classify" + "github.com/photoprism/photoprism/internal/event" "github.com/photoprism/photoprism/pkg/rnd" "github.com/photoprism/photoprism/pkg/txt" ) @@ -61,10 +62,33 @@ func NewLabel(name string, priority int) *Label { return result } -// FirstOrCreate checks if the label already exists in the database -func (m *Label) FirstOrCreate() *Label { - if err := Db().FirstOrCreate(m, "label_slug = ? OR custom_slug = ?", m.LabelSlug, m.CustomSlug).Error; err != nil { +// Save updates the existing or inserts a new row. +func (m *Label) Save() error { + return Db().Save(m).Error +} + +// Create inserts a new row to the database. +func (m *Label) Create() error { + return Db().Create(m).Error +} + +// FirstOrCreateLabel returns the existing row, inserts a new row or nil in case of errors. +func FirstOrCreateLabel(m *Label) *Label { + result := Label{} + + if err := Db().Where("label_slug = ? OR custom_slug = ?", m.LabelSlug, m.CustomSlug).First(&result).Error; err == nil { + return &result + } else if err := m.Create(); err != nil { log.Errorf("label: %s", err) + return nil + } + + if m.LabelPriority >= 0 { + event.EntitiesCreated("labels", []*Label{m}) + + event.Publish("count.labels", event.Data{ + "count": 1, + }) } return m @@ -89,7 +113,7 @@ func (m *Label) SetName(name string) { } // Updates a label if necessary -func (m *Label) Update(label classify.Label) error { +func (m *Label) UpdateClassify(label classify.Label) error { save := false db := Db() @@ -119,7 +143,12 @@ func (m *Label) Update(label classify.Label) error { // Add categories for _, category := range label.Categories { - sn := NewLabel(txt.Title(category), -3).FirstOrCreate() + sn := FirstOrCreateLabel(NewLabel(txt.Title(category), -3)) + + if sn == nil { + continue + } + if err := db.Model(m).Association("LabelCategories").Append(sn).Error; err != nil { return err } diff --git a/internal/entity/label_test.go b/internal/entity/label_test.go index 1a2afa5b9..1ccf7b820 100644 --- a/internal/entity/label_test.go +++ b/internal/entity/label_test.go @@ -52,11 +52,21 @@ func TestLabel_SetName(t *testing.T) { }) } -func TestLabel_FirstOrCreate(t *testing.T) { +func TestFirstOrCreateLabel(t *testing.T) { label := LabelFixtures.Get("flower") - r := label.FirstOrCreate() - assert.Equal(t, "Flower", r.LabelName) - assert.Equal(t, "flower", r.LabelSlug) + result := FirstOrCreateLabel(&label) + + if result == nil { + t.Fatal("result should not be nil") + } + + if result.LabelName != label.LabelName { + t.Errorf("LabelName should be the same: %s %s", result.LabelName, label.LabelName) + } + + if result.LabelSlug != label.LabelSlug { + t.Errorf("LabelName should be the same: %s %s", result.LabelSlug, label.LabelSlug) + } } func TestLabel_Update(t *testing.T) { @@ -69,7 +79,7 @@ func TestLabel_Update(t *testing.T) { assert.Equal(t, "customslug", Label.CustomSlug) assert.Equal(t, "label", Label.LabelName) - err := Label.Update(*classifyLabel) + err := Label.UpdateClassify(*classifyLabel) if err != nil { t.Fatal(err) @@ -89,7 +99,7 @@ func TestLabel_Update(t *testing.T) { assert.Equal(t, "", Label.CustomSlug) assert.Equal(t, "label12", Label.LabelName) - err := Label.Update(*classifyLabel) + err := Label.UpdateClassify(*classifyLabel) if err != nil { t.Fatal(err) } @@ -109,7 +119,7 @@ func TestLabel_Update(t *testing.T) { assert.Equal(t, "labelslug2", Label.CustomSlug) assert.Equal(t, "label34", Label.LabelName) - err := Label.Update(*classifyLabel) + err := Label.UpdateClassify(*classifyLabel) if err != nil { t.Fatal(err) } diff --git a/internal/entity/lens.go b/internal/entity/lens.go index 04275c499..a4736c7b8 100644 --- a/internal/entity/lens.go +++ b/internal/entity/lens.go @@ -58,14 +58,10 @@ func NewLens(modelName string, makeName string) *Lens { // Create inserts a new row to the database. func (m *Lens) Create() error { - if err := Db().Create(m).Error; err != nil { - return err - } - - return nil + return Db().Create(m).Error } -// FirstOrCreateLens inserts a new row if not exists. +// FirstOrCreateLens returns the existing row, inserts a new row or nil in case of errors. func FirstOrCreateLens(m *Lens) *Lens { result := Lens{} diff --git a/internal/entity/location.go b/internal/entity/location.go index 4e2be3e57..236956fe0 100644 --- a/internal/entity/location.go +++ b/internal/entity/location.go @@ -105,14 +105,10 @@ func (m *Location) Find(api string) error { // Create inserts a new row to the database. func (m *Location) Create() error { - if err := Db().Create(m).Error; err != nil { - return err - } - - return nil + return Db().Create(m).Error } -// FirstOrCreateLocation inserts a new row if not exists. +// FirstOrCreateLocation returns the existing row, inserts a new row or nil in case of errors. func FirstOrCreateLocation(m *Location) *Location { if m.LocUID == "" { log.Errorf("location: loc_uid must not be empty") diff --git a/internal/entity/photo.go b/internal/entity/photo.go index 9358d0254..1a05fc7a0 100644 --- a/internal/entity/photo.go +++ b/internal/entity/photo.go @@ -8,7 +8,6 @@ import ( "github.com/jinzhu/gorm" "github.com/photoprism/photoprism/internal/classify" - "github.com/photoprism/photoprism/internal/event" "github.com/photoprism/photoprism/internal/form" "github.com/photoprism/photoprism/pkg/rnd" "github.com/photoprism/photoprism/pkg/txt" @@ -237,7 +236,12 @@ func (m *Photo) IndexKeywords() error { keywords = txt.UniqueWords(keywords) for _, w := range keywords { - kw := NewKeyword(w).FirstOrCreate() + kw := FirstOrCreateKeyword(NewKeyword(w)) + + if kw == nil { + log.Errorf("photo: index keyword should not be nil - bug?") + continue + } if kw.Skip { continue @@ -245,7 +249,7 @@ func (m *Photo) IndexKeywords() error { keywordIds = append(keywordIds, kw.ID) - NewPhotoKeyword(m.ID, kw.ID).FirstOrCreate() + FirstOrCreatePhotoKeyword(NewPhotoKeyword(m.ID, kw.ID)) } return db.Where("photo_id = ? AND keyword_id NOT IN (?)", m.ID, keywordIds).Delete(&PhotoKeyword{}).Error @@ -427,30 +431,30 @@ func (m *Photo) UpdateTitle(labels classify.Labels) error { // AddLabels updates the entity with additional or updated label information. func (m *Photo) AddLabels(labels classify.Labels) { - // TODO: Update classify labels from database - for _, label := range labels { - lm := NewLabel(label.Title(), label.Priority).FirstOrCreate() + for _, classifyLabel := range labels { + labelEntity := FirstOrCreateLabel(NewLabel(classifyLabel.Title(), classifyLabel.Priority)) - if lm.New { - event.EntitiesCreated("labels", []*Label{lm}) - - if label.Priority >= 0 { - event.Publish("count.labels", event.Data{ - "count": 1, - }) - } + if labelEntity == nil { + log.Errorf("index: label %s for photo %d should not be nil - bug?", txt.Quote(classifyLabel.Title()), m.ID) + continue } - if err := lm.Update(label); err != nil { + if err := labelEntity.UpdateClassify(classifyLabel); err != nil { log.Errorf("index: %s", err) } - plm := NewPhotoLabel(m.ID, lm.ID, label.Uncertainty, label.Source).FirstOrCreate() + photoLabel := FirstOrCreatePhotoLabel(NewPhotoLabel(m.ID, labelEntity.ID, classifyLabel.Uncertainty, classifyLabel.Source)) - if plm.Uncertainty > label.Uncertainty && plm.Uncertainty < 100 { - plm.Uncertainty = label.Uncertainty - plm.LabelSrc = label.Source - if err := Db().Save(&plm).Error; err != nil { + if photoLabel == nil { + log.Errorf("index: label %d for photo %d should not be nil - bug?", labelEntity.ID, m.ID) + continue + } + + if photoLabel.Uncertainty > classifyLabel.Uncertainty && photoLabel.Uncertainty < 100 { + if err := photoLabel.Updates(map[string]interface{}{ + "Uncertainty": classifyLabel.Uncertainty, + "LabelSrc": classifyLabel.Source, + }); err != nil { log.Errorf("index: %s", err) } } @@ -591,7 +595,7 @@ func (m *Photo) NoDescription() bool { return m.PhotoDescription == "" } -// Updates a model attribute. +// Updates a column in the database. func (m *Photo) Update(attr string, value interface{}) error { return UnscopedDb().Model(m).UpdateColumn(attr, value).Error } diff --git a/internal/entity/photo_album.go b/internal/entity/photo_album.go index b5196936e..4ec0686e0 100644 --- a/internal/entity/photo_album.go +++ b/internal/entity/photo_album.go @@ -31,10 +31,20 @@ func NewPhotoAlbum(photoUID, albumUID string) *PhotoAlbum { return result } -// FirstOrCreate checks if the PhotoAlbum relation already exist in the database before the creation -func (m *PhotoAlbum) FirstOrCreate() *PhotoAlbum { - if err := Db().FirstOrCreate(m, "photo_uid = ? AND album_uid = ?", m.PhotoUID, m.AlbumUID).Error; err != nil { - log.Errorf("photo album: %s", err) +// Create inserts a new row to the database. +func (m *PhotoAlbum) Create() error { + return Db().Create(m).Error +} + +// FirstOrCreatePhotoAlbum returns the existing row, inserts a new row or nil in case of errors. +func FirstOrCreatePhotoAlbum(m *PhotoAlbum) *PhotoAlbum { + result := PhotoAlbum{} + + if err := Db().Where("photo_uid = ? AND album_uid = ?", m.PhotoUID, m.AlbumUID).First(&result).Error; err == nil { + return &result + } else if err := m.Create(); err != nil { + log.Errorf("photo-album: %s", err) + return nil } return m diff --git a/internal/entity/photo_album_test.go b/internal/entity/photo_album_test.go index 83f7d4259..33e0c4c79 100644 --- a/internal/entity/photo_album_test.go +++ b/internal/entity/photo_album_test.go @@ -21,8 +21,19 @@ func TestPhotoAlbum_TableName(t *testing.T) { assert.Equal(t, "photos_albums", tableName) } -func TestPhotoAlbum_FirstOrCreate(t *testing.T) { - m := PhotoAlbumFixtures.Get("1", "pt9jtdre2lvl0yh7", "at9lxuqxpogaaba8") - r := m.FirstOrCreate() - assert.Equal(t, "at9lxuqxpogaaba8", r.AlbumUID) +func TestFirstOrCreatePhotoAlbum(t *testing.T) { + model := PhotoAlbumFixtures.Get("1", "pt9jtdre2lvl0yh7", "at9lxuqxpogaaba8") + result := FirstOrCreatePhotoAlbum(&model) + + if result == nil { + t.Fatal("result should not be nil") + } + + if result.AlbumUID != model.AlbumUID { + t.Errorf("AlbumUID should be the same: %s %s", result.AlbumUID, model.AlbumUID) + } + + if result.PhotoUID != model.PhotoUID { + t.Errorf("PhotoUID should be the same: %s %s", result.PhotoUID, model.PhotoUID) + } } diff --git a/internal/entity/photo_keyword.go b/internal/entity/photo_keyword.go index 40b8cb76f..14a3c8c09 100644 --- a/internal/entity/photo_keyword.go +++ b/internal/entity/photo_keyword.go @@ -21,10 +21,20 @@ func NewPhotoKeyword(photoID, keywordID uint) *PhotoKeyword { return result } -// FirstOrCreate checks if the Keywords relation already exist in the database before the creation -func (m *PhotoKeyword) FirstOrCreate() *PhotoKeyword { - if err := Db().FirstOrCreate(m, "photo_id = ? AND keyword_id = ?", m.PhotoID, m.KeywordID).Error; err != nil { - log.Errorf("photo keyword: %s", err) +// Create inserts a new row to the database. +func (m *PhotoKeyword) Create() error { + return Db().Create(m).Error +} + +// FirstOrCreatePhotoKeyword returns the existing row, inserts a new row or nil in case of errors. +func FirstOrCreatePhotoKeyword(m *PhotoKeyword) *PhotoKeyword { + result := PhotoKeyword{} + + if err := Db().Where("photo_id = ? AND keyword_id = ?", m.PhotoID, m.KeywordID).First(&result).Error; err == nil { + return &result + } else if err := m.Create(); err != nil { + log.Errorf("photo-keyword: %s", err) + return nil } return m diff --git a/internal/entity/photo_keyword_test.go b/internal/entity/photo_keyword_test.go index 60417f596..36dd11732 100644 --- a/internal/entity/photo_keyword_test.go +++ b/internal/entity/photo_keyword_test.go @@ -21,8 +21,19 @@ func TestPhotoKeyword_TableName(t *testing.T) { assert.Equal(t, "photos_keywords", tableName) } -func TestPhotoKeywords_FirstOrCreate(t *testing.T) { - m := PhotoKeywordFixtures["1"] - r := m.FirstOrCreate() - assert.Equal(t, uint(0xf4244), r.PhotoID) +func TestFirstOrCreatePhotoKeyword(t *testing.T) { + model := PhotoKeywordFixtures["1"] + result := FirstOrCreatePhotoKeyword(&model) + + if result == nil { + t.Fatal("result should not be nil") + } + + if result.PhotoID != model.PhotoID { + t.Errorf("PhotoID should be the same: %d %d", result.PhotoID, model.PhotoID) + } + + if result.KeywordID != model.KeywordID { + t.Errorf("KeywordID should be the same: %d %d", result.KeywordID, model.KeywordID) + } } diff --git a/internal/entity/photo_label.go b/internal/entity/photo_label.go index a9efa9613..678ac3bb7 100644 --- a/internal/entity/photo_label.go +++ b/internal/entity/photo_label.go @@ -32,32 +32,17 @@ func NewPhotoLabel(photoID, labelID uint, uncertainty int, source string) *Photo return result } -// FirstOrCreate checks if the PhotoLabel relation already exist in the database before the creation -func (m *PhotoLabel) FirstOrCreate() *PhotoLabel { - if err := Db().FirstOrCreate(m, "photo_id = ? AND label_id = ?", m.PhotoID, m.LabelID).Error; err != nil { - log.Errorf("photo label: %s", err) - } - - return m +// Updates multiple columns in the database. +func (m *PhotoLabel) Updates(values interface{}) error { + return UnscopedDb().Model(m).UpdateColumns(values).Error } -// ClassifyLabel returns the label as classify.Label -func (m *PhotoLabel) ClassifyLabel() classify.Label { - if m.Label == nil { - panic("photo label: label is nil") - } - - result := classify.Label{ - Name: m.Label.LabelName, - Source: m.LabelSrc, - Uncertainty: m.Uncertainty, - Priority: m.Label.LabelPriority, - } - - return result +// Updates a column in the database. +func (m *PhotoLabel) Update(attr string, value interface{}) error { + return UnscopedDb().Model(m).UpdateColumn(attr, value).Error } -// Save saves the entity in the database and returns an error. +// Save saves the entity in the database. func (m *PhotoLabel) Save() error { if m.Photo != nil { m.Photo = nil @@ -69,3 +54,39 @@ func (m *PhotoLabel) Save() error { return Db().Save(m).Error } + +// Create inserts a new row to the database. +func (m *PhotoLabel) Create() error { + return Db().Create(m).Error +} + +// FirstOrCreatePhotoLabel returns the existing row, inserts a new row or nil in case of errors. +func FirstOrCreatePhotoLabel(m *PhotoLabel) *PhotoLabel { + result := PhotoLabel{} + + if err := Db().Where("photo_id = ? AND label_id = ?", m.PhotoID, m.LabelID).First(&result).Error; err == nil { + return &result + } else if err := m.Create(); err != nil { + log.Errorf("photo-label: %s", err) + return nil + } + + return m +} + +// ClassifyLabel returns the label as classify.Label +func (m *PhotoLabel) ClassifyLabel() classify.Label { + if m.Label == nil { + log.Errorf("photo-label: classify label is nil (photo id %d, label id %d) - bug?", m.PhotoID, m.LabelID) + return classify.Label{} + } + + result := classify.Label{ + Name: m.Label.LabelName, + Source: m.LabelSrc, + Uncertainty: m.Uncertainty, + Priority: m.Label.LabelPriority, + } + + return result +} diff --git a/internal/entity/photo_label_test.go b/internal/entity/photo_label_test.go index 1f5b15d40..82d747427 100644 --- a/internal/entity/photo_label_test.go +++ b/internal/entity/photo_label_test.go @@ -3,6 +3,7 @@ package entity import ( "testing" + "github.com/photoprism/photoprism/internal/classify" "github.com/stretchr/testify/assert" ) @@ -22,10 +23,21 @@ func TestPhotoLabel_TableName(t *testing.T) { assert.Equal(t, "photos_labels", tableName) } -func TestPhotoLabel_FirstOrCreate(t *testing.T) { - pl := LabelFixtures.PhotoLabel(1000000, "flower", 38, "image") - r := pl.FirstOrCreate() - assert.Equal(t, uint(1000000), r.PhotoID) +func TestFirstOrCreatePhotoLabel(t *testing.T) { + model := LabelFixtures.PhotoLabel(1000000, "flower", 38, "image") + result := FirstOrCreatePhotoLabel(&model) + + if result == nil { + t.Fatal("result should not be nil") + } + + if result.PhotoID != model.PhotoID { + t.Errorf("PhotoID should be the same: %d %d", result.PhotoID, model.PhotoID) + } + + if result.LabelID != model.LabelID { + t.Errorf("LabelID should be the same: %d %d", result.LabelID, model.LabelID) + } } func TestPhotoLabel_ClassifyLabel(t *testing.T) { @@ -39,11 +51,8 @@ func TestPhotoLabel_ClassifyLabel(t *testing.T) { t.Run("label = nil", func(t *testing.T) { photoLabel := NewPhotoLabel(1, 3, 80, "source") - assert.Panics(t, func() { - - photoLabel.ClassifyLabel() - - }, "photo label: label is nil") + result := photoLabel.ClassifyLabel() + assert.Equal(t, classify.Label{}, result) }) } diff --git a/internal/entity/photo_location.go b/internal/entity/photo_location.go index f6cef4e36..f4ed7c906 100644 --- a/internal/entity/photo_location.go +++ b/internal/entity/photo_location.go @@ -69,13 +69,7 @@ func (m *Photo) UpdateLocation(geoApi string) (keywords []string, labels classif m.TakenAt = m.GetTakenAt() } - country := NewCountry(location.CountryCode(), location.CountryName()).FirstOrCreate() - - if country.New { - event.Publish("count.countries", event.Data{ - "count": 1, - }) - } + FirstOrCreateCountry(NewCountry(location.CountryCode(), location.CountryName())) locCategory := location.Category() keywords = append(keywords, location.Keywords()...) diff --git a/internal/entity/place.go b/internal/entity/place.go index 1f9543b4d..65ba80d0a 100644 --- a/internal/entity/place.go +++ b/internal/entity/place.go @@ -65,7 +65,7 @@ func FindPlaceByLabel(uid string, label string) *Place { return place } -// Find returns db record of place +// Find updates the entity with values from the database. func (m *Place) Find() error { if err := Db().First(m, "place_uid = ?", m.PlaceUID).Error; err != nil { return err @@ -76,14 +76,10 @@ func (m *Place) Find() error { // Create inserts a new row to the database. func (m *Place) Create() error { - if err := Db().Create(m).Error; err != nil { - return err - } - - return nil + return Db().Create(m).Error } -// FirstOrCreatePlace inserts a new row if not exists. +// FirstOrCreatePlace returns the existing row, inserts a new row or nil in case of errors. func FirstOrCreatePlace(m *Place) *Place { if m.PlaceUID == "" { log.Errorf("place: uid must not be empty") diff --git a/internal/workers/sync.go b/internal/workers/sync.go index 6d626aefd..e1e147ec4 100644 --- a/internal/workers/sync.go +++ b/internal/workers/sync.go @@ -25,6 +25,13 @@ func NewSync(conf *config.Config) *Sync { } } +// Report logs an error message if err is not nil. +func (s *Sync) report(err error) { + if err != nil { + log.Errorf("sync: %s", err.Error()) + } +} + // Start starts the sync worker. func (s *Sync) Start() (err error) { if err := mutex.Sync.Start(); err != nil { @@ -120,18 +127,12 @@ func (s *Sync) Start() (err error) { return nil } - if err := entity.Db().First(&a, a.ID).Error; err != nil { - log.Errorf("sync: %s", err.Error()) - return err - } - // Only update the following fields to avoid overwriting other settings - a.AccError = accError - a.AccErrors = accErrors - a.SyncStatus = syncStatus - a.SyncDate = syncDate - - if err := entity.Db().Save(&a).Error; err != nil { + if err := a.Updates(map[string]interface{}{ + "AccError": accError, + "AccErrors": accErrors, + "SyncStatus": syncStatus, + "SyncDate": syncDate}); err != nil { log.Errorf("sync: %s", err.Error()) } else if synced { event.Publish("sync.synced", event.Data{"account": a}) diff --git a/internal/workers/sync_refresh.go b/internal/workers/sync_refresh.go index b1cd519a4..ba0864c3c 100644 --- a/internal/workers/sync_refresh.go +++ b/internal/workers/sync_refresh.go @@ -14,7 +14,6 @@ func (s *Sync) refresh(a entity.Account) (complete bool, err error) { return false, nil } - db := s.conf.Db() client := webdav.New(a.AccURL, a.AccUser, a.AccPass) subDirs, err := client.Directories(a.SyncPath, true) @@ -62,18 +61,23 @@ func (s *Sync) refresh(a entity.Account) (complete bool, err error) { } } - f.FirstOrCreate() + f = entity.FirstOrCreateFileSync(f) + + if f == nil { + log.Errorf("sync: file sync entity should not be nil - bug?") + continue + } if f.Status == entity.FileSyncIgnore && mediaType == fs.MediaRaw && a.SyncRaw { - f.Status = entity.FileSyncNew - db.Save(&f) + s.report(f.Update("Status", entity.FileSyncNew)) } if f.Status == entity.FileSyncDownloaded && !f.RemoteDate.Equal(file.Date) { - f.Status = entity.FileSyncNew - f.RemoteDate = file.Date - f.RemoteSize = file.Size - db.Save(&f) + s.report(f.Updates(map[string]interface{}{ + "Status": entity.FileSyncNew, + "RemoteDate": file.Date, + "RemoteSize": file.Size, + })) } } }