From 2d3ed85311a97b3c4b361abea6696f00dd9232a1 Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 31 Jul 2020 15:32:08 -0400 Subject: [PATCH] Add bookmark in persistence layer --- ...20200731095603_create_play_queues_table.go | 6 +- model/playqueue.go | 14 +++- persistence/playqueue_repository.go | 63 +++++++++++++- persistence/playqueue_repository_test.go | 82 ++++++++++++++----- server/subsonic/bookmarks.go | 4 +- utils/request_helpers.go | 12 +++ utils/request_helpers_test.go | 18 ++++ 7 files changed, 173 insertions(+), 26 deletions(-) diff --git a/db/migration/20200731095603_create_play_queues_table.go b/db/migration/20200731095603_create_play_queues_table.go index 7c5956d8c9c..db7f641ff8d 100644 --- a/db/migration/20200731095603_create_play_queues_table.go +++ b/db/migration/20200731095603_create_play_queues_table.go @@ -14,13 +14,13 @@ func upCreatePlayQueuesTable(tx *sql.Tx) error { _, err := tx.Exec(` create table playqueue ( - id varchar(255) not null, + id varchar(255) not null primary key, user_id varchar(255) not null references user (id) on update cascade on delete cascade, comment varchar(255), - current varchar(255), - position real, + current varchar(255) not null, + position integer, changed_by varchar(255), items varchar(255), created_at datetime, diff --git a/model/playqueue.go b/model/playqueue.go index 2c482807eaa..97cda7e9d1a 100644 --- a/model/playqueue.go +++ b/model/playqueue.go @@ -9,7 +9,7 @@ type PlayQueue struct { UserID string `json:"userId" orm:"column(user_id)"` Comment string `json:"comment"` Current string `json:"current"` - Position float32 `json:"position"` + Position int64 `json:"position"` ChangedBy string `json:"changedBy"` Items MediaFiles `json:"items,omitempty"` CreatedAt time.Time `json:"createdAt"` @@ -21,4 +21,16 @@ type PlayQueues []PlayQueue type PlayQueueRepository interface { Store(queue *PlayQueue) error Retrieve(userId string) (*PlayQueue, error) + AddBookmark(userId, id, comment string, position int64) error + GetBookmarks(userId string) (Bookmarks, error) + DeleteBookmark(userId, id string) error } + +type Bookmark struct { + ID string `json:"id" orm:"column(id)"` + Comment string `json:"comment"` + Position int64 `json:"position"` + CreatedAt time.Time `json:"createdAt"` +} + +type Bookmarks []Bookmark diff --git a/persistence/playqueue_repository.go b/persistence/playqueue_repository.go index b178b0e86c7..99ce51b835e 100644 --- a/persistence/playqueue_repository.go +++ b/persistence/playqueue_repository.go @@ -9,6 +9,7 @@ import ( "github.com/astaxie/beego/orm" "github.com/deluan/navidrome/log" "github.com/deluan/navidrome/model" + "github.com/deluan/navidrome/model/request" ) type playQueueRepository struct { @@ -28,7 +29,7 @@ type playQueue struct { UserID string `orm:"column(user_id)"` Comment string Current string - Position float32 + Position int64 ChangedBy string Items string CreatedAt time.Time @@ -63,6 +64,66 @@ func (r *playQueueRepository) Retrieve(userId string) (*model.PlayQueue, error) return &pls, err } +func (r *playQueueRepository) AddBookmark(userId, id, comment string, position int64) error { + u := loggedUser(r.ctx) + client, _ := request.ClientFrom(r.ctx) + bm := &playQueue{ + UserID: userId, + Comment: comment, + Current: id, + Position: position, + ChangedBy: client, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + } + + sel := r.newSelect().Column("id").Where(And{ + Eq{"user_id": userId}, + Eq{"items": ""}, + Eq{"current": id}, + }) + var prev model.PlayQueue + err := r.queryOne(sel, &prev) + if err != nil && err != model.ErrNotFound { + log.Error(r.ctx, "Error retrieving previous bookmark", "user", u.UserName, err, "mediaFileId", id, err) + return err + } + + _, err = r.put(prev.ID, bm) + if err != nil { + log.Error(r.ctx, "Error saving bookmark", "user", u.UserName, err, "mediaFileId", id, err) + return err + } + return nil +} + +func (r *playQueueRepository) GetBookmarks(userId string) (model.Bookmarks, error) { + u := loggedUser(r.ctx) + sel := r.newSelect().Column("*").Where(And{Eq{"user_id": userId}, Eq{"items": ""}}) + var pqs model.PlayQueues + err := r.queryAll(sel, &pqs) + if err != nil { + log.Error(r.ctx, "Error retrieving bookmarks", "user", u.UserName, err) + return nil, err + } + bms := make(model.Bookmarks, len(pqs)) + for i := range pqs { + bms[i].ID = pqs[i].Current + bms[i].Comment = pqs[i].Comment + bms[i].Position = int64(pqs[i].Position) + bms[i].CreatedAt = pqs[i].CreatedAt + } + return bms, nil +} + +func (r *playQueueRepository) DeleteBookmark(userId, id string) error { + return r.delete(And{ + Eq{"user_id": userId}, + Eq{"items": ""}, + Eq{"current": id}, + }) +} + func (r *playQueueRepository) fromModel(q *model.PlayQueue) playQueue { pq := playQueue{ ID: q.ID, diff --git a/persistence/playqueue_repository_test.go b/persistence/playqueue_repository_test.go index 6fae30c942f..308f38999b6 100644 --- a/persistence/playqueue_repository_test.go +++ b/persistence/playqueue_repository_test.go @@ -23,32 +23,76 @@ var _ = Describe("PlayQueueRepository", func() { repo = NewPlayQueueRepository(ctx, orm.NewOrm()) }) - It("returns notfound error if there's no playqueue for the user", func() { - _, err := repo.Retrieve("user999") - Expect(err).To(MatchError(model.ErrNotFound)) - }) + Describe("PlayQueues", func() { + It("returns notfound error if there's no playqueue for the user", func() { + _, err := repo.Retrieve("user999") + Expect(err).To(MatchError(model.ErrNotFound)) + }) + + It("stores and retrieves the playqueue for the user", func() { + By("Storing a playqueue for the user") - It("stores and retrieves the playqueue for the user", func() { - By("Storing a playqueue for the user") + expected := aPlayQueue("user1", songDayInALife.ID, 123, songComeTogether, songDayInALife) + Expect(repo.Store(expected)).To(BeNil()) - expected := aPlayQueue("user1", songDayInALife.ID, 123, songComeTogether, songDayInALife) - Expect(repo.Store(expected)).To(BeNil()) + actual, err := repo.Retrieve("user1") + Expect(err).To(BeNil()) - actual, err := repo.Retrieve("user1") - Expect(err).To(BeNil()) + AssertPlayQueue(expected, actual) - AssertPlayQueue(expected, actual) + By("Storing a new playqueue for the same user") - By("Storing a new playqueue for the same user") + new := aPlayQueue("user1", songRadioactivity.ID, 321, songAntenna, songRadioactivity) + Expect(repo.Store(new)).To(BeNil()) - new := aPlayQueue("user1", songRadioactivity.ID, 321, songAntenna, songRadioactivity) - Expect(repo.Store(new)).To(BeNil()) + actual, err = repo.Retrieve("user1") + Expect(err).To(BeNil()) - actual, err = repo.Retrieve("user1") - Expect(err).To(BeNil()) + AssertPlayQueue(new, actual) + Expect(countPlayQueues(repo, "user1")).To(Equal(1)) + }) + }) - AssertPlayQueue(new, actual) - Expect(countPlayQueues(repo, "user1")).To(Equal(1)) + Describe("Bookmarks", func() { + It("returns an empty collection if there are no bookmarks", func() { + Expect(repo.GetBookmarks("user999")).To(BeEmpty()) + }) + + It("saves and overrides bookmarks", func() { + By("Saving the bookmark") + Expect(repo.AddBookmark("user5", songAntenna.ID, "this is a comment", 123)).To(BeNil()) + + bms, err := repo.GetBookmarks("user5") + Expect(err).To(BeNil()) + + Expect(bms).To(HaveLen(1)) + Expect(bms[0].ID).To(Equal(songAntenna.ID)) + Expect(bms[0].Comment).To(Equal("this is a comment")) + Expect(bms[0].Position).To(Equal(int64(123))) + + By("Overriding the bookmark") + Expect(repo.AddBookmark("user5", songAntenna.ID, "another comment", 333)).To(BeNil()) + + bms, err = repo.GetBookmarks("user5") + Expect(err).To(BeNil()) + + Expect(bms[0].ID).To(Equal(songAntenna.ID)) + Expect(bms[0].Comment).To(Equal("another comment")) + Expect(bms[0].Position).To(Equal(int64(333))) + + By("Saving another bookmark") + Expect(repo.AddBookmark("user5", songComeTogether.ID, "one more comment", 444)).To(BeNil()) + bms, err = repo.GetBookmarks("user5") + Expect(err).To(BeNil()) + Expect(bms).To(HaveLen(2)) + + By("Delete bookmark") + Expect(repo.DeleteBookmark("user5", songAntenna.ID)) + bms, err = repo.GetBookmarks("user5") + Expect(err).To(BeNil()) + Expect(bms).To(HaveLen(1)) + Expect(bms[0].ID).To(Equal(songComeTogether.ID)) + }) }) }) @@ -74,7 +118,7 @@ func AssertPlayQueue(expected, actual *model.PlayQueue) { } } -func aPlayQueue(userId, current string, position float32, items ...model.MediaFile) *model.PlayQueue { +func aPlayQueue(userId, current string, position int64, items ...model.MediaFile) *model.PlayQueue { createdAt := time.Now() updatedAt := createdAt.Add(time.Minute) id, _ := uuid.NewRandom() diff --git a/server/subsonic/bookmarks.go b/server/subsonic/bookmarks.go index 4ffa29fdae6..4fc24155d32 100644 --- a/server/subsonic/bookmarks.go +++ b/server/subsonic/bookmarks.go @@ -46,7 +46,7 @@ func (c *BookmarksController) SavePlayQueue(w http.ResponseWriter, r *http.Reque } current := utils.ParamString(r, "current") - position := utils.ParamInt(r, "position", 0) + position := utils.ParamInt64(r, "position", 0) user, _ := request.UserFrom(r.Context()) client, _ := request.ClientFrom(r.Context()) @@ -59,7 +59,7 @@ func (c *BookmarksController) SavePlayQueue(w http.ResponseWriter, r *http.Reque pq := &model.PlayQueue{ UserID: user.ID, Current: current, - Position: float32(position), + Position: position, ChangedBy: client, Items: items, CreatedAt: time.Time{}, diff --git a/utils/request_helpers.go b/utils/request_helpers.go index 4ec7292e9c5..b9d5800c484 100644 --- a/utils/request_helpers.go +++ b/utils/request_helpers.go @@ -51,6 +51,18 @@ func ParamInt(r *http.Request, param string, def int) int { return int(value) } +func ParamInt64(r *http.Request, param string, def int64) int64 { + v := ParamString(r, param) + if v == "" { + return def + } + value, err := strconv.ParseInt(v, 10, 64) + if err != nil { + return def + } + return value +} + func ParamInts(r *http.Request, param string) []int { pStr := ParamStrings(r, param) ints := make([]int, 0, len(pStr)) diff --git a/utils/request_helpers_test.go b/utils/request_helpers_test.go index 0e1a1eaa235..c4343f8575a 100644 --- a/utils/request_helpers_test.go +++ b/utils/request_helpers_test.go @@ -98,6 +98,24 @@ var _ = Describe("Request Helpers", func() { }) }) + Describe("ParamInt64", func() { + BeforeEach(func() { + r = httptest.NewRequest("GET", "/ping?i=123&inv=123.45", nil) + }) + + It("returns default value if param does not exist", func() { + Expect(ParamInt64(r, "xx", 999)).To(Equal(int64(999))) + }) + + It("returns default value if param is an invalid int", func() { + Expect(ParamInt64(r, "inv", 999)).To(Equal(int64(999))) + }) + + It("returns parsed time", func() { + Expect(ParamInt64(r, "i", 999)).To(Equal(int64(123))) + }) + }) + Describe("ParamInts", func() { BeforeEach(func() { r = httptest.NewRequest("GET", "/ping?i=123&i=456", nil)