From e7d862d07a62b961207355a7671f7cb88f8b089c Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Sun, 2 Apr 2023 10:27:57 +0200 Subject: [PATCH] Albums: Improve database error handling #3320 Signed-off-by: Michael Mayer --- internal/entity/album.go | 64 ++++++++++++++++++++++++++---- internal/entity/album_test.go | 52 +++++++++++++----------- internal/entity/auth_user_share.go | 2 +- internal/entity/link.go | 2 +- internal/entity/photo.go | 4 +- internal/entity/place.go | 2 +- internal/entity/reaction.go | 2 +- 7 files changed, 90 insertions(+), 38 deletions(-) diff --git a/internal/entity/album.go b/internal/entity/album.go index 0663550ff..14ac82580 100644 --- a/internal/entity/album.go +++ b/internal/entity/album.go @@ -273,7 +273,7 @@ func FindMonthAlbum(year, month int) *Album { return nil } - if UnscopedDb().First(&m, "album_year = ? AND album_month = ? AND album_type = ?", year, month, AlbumMonth).RecordNotFound() { + if UnscopedDb().First(&m, "album_year = ? AND album_month = ? AND album_type = ?", year, month, AlbumMonth).Error != nil { return nil } @@ -288,7 +288,7 @@ func FindAlbumBySlug(albumSlug, albumType string) *Album { return nil } - if UnscopedDb().First(&m, "album_slug = ? AND album_type = ?", albumSlug, albumType).RecordNotFound() { + if UnscopedDb().First(&m, "album_slug = ? AND album_type = ?", albumSlug, albumType).Error != nil { return nil } @@ -315,7 +315,7 @@ func FindAlbumByAttr(slugs, filters []string, albumType string) *Album { stmt = stmt.Where("album_slug IN (?) OR album_filter IN (?)", slugs, filters) } - if stmt.First(&m).RecordNotFound() { + if stmt.First(&m).Error != nil { return nil } @@ -336,7 +336,7 @@ func FindFolderAlbum(albumPath string) *Album { stmt := UnscopedDb().Where("album_type = ?", AlbumFolder). Where("album_slug = ? OR album_path = ?", albumSlug, albumPath) - if stmt.First(&m).RecordNotFound() { + if stmt.First(&m).Error != nil { return nil } @@ -349,7 +349,7 @@ func FindAlbum(find Album) *Album { // Search by UID. if rnd.IsUID(find.AlbumUID, AlbumUID) { - if UnscopedDb().First(&m, "album_uid = ?", find.AlbumUID).RecordNotFound() { + if UnscopedDb().First(&m, "album_uid = ?", find.AlbumUID).Error != nil { return nil } else if m.AlbumUID != "" { albumCache.SetDefault(m.AlbumUID, m) @@ -382,7 +382,7 @@ func FindAlbum(find Album) *Album { } // Find first matching record. - if stmt.First(&m).RecordNotFound() { + if stmt.First(&m).Error != nil { return nil } @@ -490,6 +490,10 @@ func (m *Album) SetLocation(location, state, country string) *Album { // UpdateTitleAndLocation updates title, location, and slug of generated albums if needed. func (m *Album) UpdateTitleAndLocation(title, location, state, country, slug string) error { + if !m.HasID() { + return fmt.Errorf("album does not exist") + } + title = txt.Clip(title, txt.ClipDefault) slug = txt.Clip(slug, txt.ClipSlug) @@ -533,6 +537,10 @@ func (m *Album) UpdateTitleAndLocation(title, location, state, country, slug str // UpdateTitleAndState updates the album location. func (m *Album) UpdateTitleAndState(title, slug, stateName, countryCode string) error { + if !m.HasID() { + return fmt.Errorf("album does not exist") + } + title = txt.Clip(title, txt.ClipDefault) slug = txt.Clip(slug, txt.ClipSlug) @@ -593,16 +601,28 @@ func (m *Album) SaveForm(f form.Album) error { // Update sets a new value for a database column. func (m *Album) Update(attr string, value interface{}) error { + if !m.HasID() { + return fmt.Errorf("album does not exist") + } + return UnscopedDb().Model(m).Update(attr, value).Error } // Updates multiple columns in the database. func (m *Album) Updates(values interface{}) error { + if !m.HasID() { + return fmt.Errorf("album does not exist") + } + return UnscopedDb().Model(m).Updates(values).Error } // UpdateFolder updates the path, filter and slug for a folder album. func (m *Album) UpdateFolder(albumPath, albumFilter string) error { + if !m.HasID() { + return fmt.Errorf("album does not exist") + } + albumPath = strings.Trim(albumPath, string(os.PathSeparator)) albumSlug := txt.Slug(albumPath) @@ -610,11 +630,11 @@ func (m *Album) UpdateFolder(albumPath, albumFilter string) error { return fmt.Errorf("folder album must have a path and filter") } - if err := UnscopedDb().Model(m).Updates(map[string]interface{}{ + if err := m.Updates(map[string]interface{}{ "AlbumPath": albumPath, "AlbumFilter": albumFilter, "AlbumSlug": albumSlug, - }).Error; err != nil { + }); err != nil { return err } else if err = UnscopedDb().Exec("UPDATE albums SET album_path = NULL WHERE album_type = ? AND album_path = ? AND id <> ?", AlbumFolder, albumPath, m.ID).Error; err != nil { return err @@ -663,6 +683,10 @@ func (m *Album) PublishCountChange(n int) { // Delete marks the entity as deleted in the database. func (m *Album) Delete() error { + if !m.HasID() { + return fmt.Errorf("album does not exist") + } + if m.Deleted() { return nil } @@ -679,6 +703,10 @@ func (m *Album) Delete() error { // DeletePermanently permanently removes an album from the index. func (m *Album) DeletePermanently() error { + if !m.HasID() { + return fmt.Errorf("album does not exist") + } + wasDeleted := m.Deleted() if err := UnscopedDb().Delete(m).Error; err != nil { @@ -704,6 +732,10 @@ func (m *Album) Deleted() bool { // Restore restores the entity in the database. func (m *Album) Restore() error { + if !m.HasID() { + return fmt.Errorf("album does not exist") + } + if !m.Deleted() { return nil } @@ -738,7 +770,15 @@ func (m *Album) ZipName() string { // AddPhotos adds photos to an existing album. func (m *Album) AddPhotos(UIDs []string) (added PhotoAlbums) { + if !m.HasID() { + return added + } + for _, uid := range UIDs { + if !rnd.IsUID(uid, PhotoUID) { + continue + } + entry := PhotoAlbum{AlbumUID: m.AlbumUID, PhotoUID: uid, Hidden: false} if err := entry.Save(); err != nil { @@ -753,7 +793,15 @@ func (m *Album) AddPhotos(UIDs []string) (added PhotoAlbums) { // RemovePhotos removes photos from an album. func (m *Album) RemovePhotos(UIDs []string) (removed PhotoAlbums) { + if !m.HasID() { + return removed + } + for _, uid := range UIDs { + if !rnd.IsUID(uid, PhotoUID) { + continue + } + entry := PhotoAlbum{AlbumUID: m.AlbumUID, PhotoUID: uid, Hidden: true} if err := entry.Save(); err != nil { diff --git a/internal/entity/album_test.go b/internal/entity/album_test.go index 679b20acb..34d797e34 100644 --- a/internal/entity/album_test.go +++ b/internal/entity/album_test.go @@ -373,26 +373,28 @@ func TestAlbum_IsMoment(t *testing.T) { } func TestAlbum_Update(t *testing.T) { - t.Run("success", func(t *testing.T) { - album := Album{ - AlbumUID: "abc123", - AlbumSlug: "test-slug", - AlbumType: AlbumManual, - AlbumTitle: "Test Title", - } - assert.Equal(t, "test-slug", album.AlbumSlug) - - err := album.Update("AlbumSlug", "new-slug") - - if err != nil { + t.Run("Success", func(t *testing.T) { + album := NewAlbum("Test Title", AlbumManual) + if err := album.Save(); err != nil { t.Fatal(err) } + + assert.Equal(t, "test-title", album.AlbumSlug) + + if err := album.Update("AlbumSlug", "new-slug"); err != nil { + t.Fatal(err) + } + assert.Equal(t, "new-slug", album.AlbumSlug) + + if err := album.DeletePermanently(); err != nil { + t.Fatal(err) + } }) } func TestAlbum_Save(t *testing.T) { - t.Run("success", func(t *testing.T) { + t.Run("Success", func(t *testing.T) { album := NewStateAlbum("Dogs", "dogs", "label:dog") initialDate := album.UpdatedAt @@ -411,7 +413,7 @@ func TestAlbum_Save(t *testing.T) { } func TestAlbum_Create(t *testing.T) { - t.Run("album", func(t *testing.T) { + t.Run("Album", func(t *testing.T) { album := Album{ AlbumType: AlbumManual, } @@ -422,7 +424,7 @@ func TestAlbum_Create(t *testing.T) { t.Fatal(err) } }) - t.Run("moment", func(t *testing.T) { + t.Run("Moment", func(t *testing.T) { album := Album{ AlbumType: AlbumMoment, } @@ -433,7 +435,7 @@ func TestAlbum_Create(t *testing.T) { t.Fatal(err) } }) - t.Run("month", func(t *testing.T) { + t.Run("Month", func(t *testing.T) { album := Album{ AlbumType: AlbumMonth, } @@ -444,7 +446,7 @@ func TestAlbum_Create(t *testing.T) { t.Fatal(err) } }) - t.Run("folder", func(t *testing.T) { + t.Run("Folder", func(t *testing.T) { album := Album{ AlbumType: AlbumFolder, } @@ -458,7 +460,7 @@ func TestAlbum_Create(t *testing.T) { } func TestAlbum_Title(t *testing.T) { - t.Run("success", func(t *testing.T) { + t.Run("Success", func(t *testing.T) { album := Album{ AlbumUID: "abc123", AlbumSlug: "test-slug", @@ -470,7 +472,7 @@ func TestAlbum_Title(t *testing.T) { } func TestAlbum_Links(t *testing.T) { - t.Run("1 result", func(t *testing.T) { + t.Run("OneResult", func(t *testing.T) { album := AlbumFixtures.Get("christmas2030") links := album.Links() assert.Equal(t, "4jxf3jfn2k", links[0].LinkToken) @@ -478,14 +480,15 @@ func TestAlbum_Links(t *testing.T) { } func TestAlbum_AddPhotos(t *testing.T) { - t.Run("success", func(t *testing.T) { + t.Run("Success", func(t *testing.T) { album := Album{ - AlbumUID: "abc123", + ID: 1000000, + AlbumUID: "at9lxuqxpogaaba7", AlbumSlug: "test-slug", AlbumType: AlbumManual, AlbumTitle: "Test Title", } - added := album.AddPhotos([]string{"ab", "cd"}) + added := album.AddPhotos([]string{"pt9jtdre2lvl0yh7", "pt9jtdre2lvl0yh8"}) assert.Equal(t, 2, len(added)) }) } @@ -493,12 +496,13 @@ func TestAlbum_AddPhotos(t *testing.T) { func TestAlbum_RemovePhotos(t *testing.T) { t.Run("success", func(t *testing.T) { album := Album{ - AlbumUID: "abc123", + ID: 1000000, + AlbumUID: "at9lxuqxpogaaba7", AlbumSlug: "test-slug", AlbumType: AlbumManual, AlbumTitle: "Test Title", } - removed := album.RemovePhotos([]string{"ab", "cd"}) + removed := album.RemovePhotos([]string{"pt9jtdre2lvl0yh7", "pt9jtdre2lvl0yh8"}) assert.Equal(t, 2, len(removed)) }) } diff --git a/internal/entity/auth_user_share.go b/internal/entity/auth_user_share.go index ba2249c18..eec325da8 100644 --- a/internal/entity/auth_user_share.go +++ b/internal/entity/auth_user_share.go @@ -103,7 +103,7 @@ func FindUserShare(find UserShare) *UserShare { m := &UserShare{} // Find matching record. - if UnscopedDb().First(m, "user_uid = ? AND share_uid = ?", find.UserUID, find.ShareUID).RecordNotFound() { + if UnscopedDb().First(m, "user_uid = ? AND share_uid = ?", find.UserUID, find.ShareUID).Error != nil { return nil } diff --git a/internal/entity/link.go b/internal/entity/link.go index b7aa609ba..a3bd538e8 100644 --- a/internal/entity/link.go +++ b/internal/entity/link.go @@ -200,7 +200,7 @@ func FindLink(linkUid string) *Link { result := Link{} - if Db().Where("link_uid = ?", linkUid).First(&result).RecordNotFound() { + if Db().Where("link_uid = ?", linkUid).First(&result).Error != nil { event.AuditWarn([]string{"link %s", "not found"}, clean.Log(linkUid)) return nil } diff --git a/internal/entity/photo.go b/internal/entity/photo.go index 512cb17d5..8bb422261 100644 --- a/internal/entity/photo.go +++ b/internal/entity/photo.go @@ -302,14 +302,14 @@ func FindPhoto(find Photo) *Photo { // Search for UID. if rnd.IsUID(find.PhotoUID, PhotoUID) { - if !stmt.First(&m, "photo_uid = ?", find.PhotoUID).RecordNotFound() { + if stmt.First(&m, "photo_uid = ?", find.PhotoUID).Error == nil { return &m } } // Search for ID. if find.ID > 0 { - if !stmt.First(&m, "id = ?", find.ID).RecordNotFound() { + if stmt.First(&m, "id = ?", find.ID).Error == nil { return &m } } diff --git a/internal/entity/place.go b/internal/entity/place.go index bfaacf163..0a4696859 100644 --- a/internal/entity/place.go +++ b/internal/entity/place.go @@ -53,7 +53,7 @@ func CreateUnknownPlace() { func FindPlace(id string) *Place { m := Place{} - if Db().First(&m, "id = ?", id).RecordNotFound() { + if Db().First(&m, "id = ?", id).Error != nil { log.Debugf("place: %s not found", clean.Log(id)) return nil } diff --git a/internal/entity/reaction.go b/internal/entity/reaction.go index 9d08fed4c..59b839167 100644 --- a/internal/entity/reaction.go +++ b/internal/entity/reaction.go @@ -39,7 +39,7 @@ func FindReaction(uid, userUid string) (m *Reaction) { m = &Reaction{} - if Db().First(m, "uid = ? AND user_uid = ?", uid, userUid).RecordNotFound() { + if Db().First(m, "uid = ? AND user_uid = ?", uid, userUid).Error != nil { return nil }