Better implementation of Bookmarks, using its own table

db/migration/20200801101355_create_bookmark_table.go

@@ -0,0 +1,53 @@
+package migration
+
+import (
+	"database/sql"
+
+	"github.com/pressly/goose"
+)
+
+func init() {
+	goose.AddMigration(upCreateBookmarkTable, downCreateBookmarkTable)
+}
+
+func upCreateBookmarkTable(tx *sql.Tx) error {
+	_, err := tx.Exec(`
+create table bookmark
+(
+    user_id    varchar(255) not null
+        references user
+            on update cascade on delete cascade,
+    item_id    varchar(255) not null,
+    item_type  varchar(255) not null,
+    comment    varchar(255),
+    position   integer,
+    changed_by varchar(255),
+    created_at datetime,
+    updated_at datetime,
+    constraint bookmark_pk
+        unique (user_id, item_id, item_type)
+);
+
+create table playqueue_dg_tmp
+(
+	id varchar(255) not null,
+	user_id varchar(255) not null
+		references user
+			on update cascade on delete cascade,
+	current varchar(255),
+	position real,
+	changed_by varchar(255),
+	items varchar(255),
+	created_at datetime,
+	updated_at datetime
+);
+drop table playqueue;
+alter table playqueue_dg_tmp rename to playqueue;
+`)
+
+	return err
+}
+
+func downCreateBookmarkTable(tx *sql.Tx) error {
+	return nil
+}

engine/common.go

@@ -9,37 +9,37 @@ import (
 )
 
 type Entry struct {
-	Id          string
+	Id               string
-	Title       string
+	Title            string
-	IsDir       bool
+	IsDir            bool
-	Parent      string
+	Parent           string
-	Album       string
+	Album            string
-	Year        int
+	Year             int
-	Artist      string
+	Artist           string
-	Genre       string
+	Genre            string
-	CoverArt    string
+	CoverArt         string
-	Starred     time.Time
+	Starred          time.Time
-	Track       int
+	Track            int
-	Duration    int
+	Duration         int
-	Size        int64
+	Size             int64
-	Suffix      string
+	Suffix           string
-	BitRate     int
+	BitRate          int
-	ContentType string
+	ContentType      string
-	Path        string
+	Path             string
-	PlayCount   int32
+	PlayCount        int32
-	DiscNumber  int
+	DiscNumber       int
-	Created     time.Time
+	Created          time.Time
-	AlbumId     string
+	AlbumId          string
-	ArtistId    string
+	ArtistId         string
-	Type        string
+	Type             string
-	UserRating  int
+	UserRating       int
-	SongCount   int
+	SongCount        int
-
+	UserName         string
-	UserName   string
+	MinutesAgo       int
-	MinutesAgo int
+	PlayerId         int
-	PlayerId   int
+	PlayerName       string
-	PlayerName string
+	AlbumCount       int
-	AlbumCount int
+	BookmarkPosition int64
 }
 
 type Entries []Entry
@@ -112,6 +112,7 @@ func FromMediaFile(mf *model.MediaFile) Entry {
 		e.Starred = mf.StarredAt
 	}
 	e.UserRating = mf.Rating
+	e.BookmarkPosition = mf.BookmarkPosition
 	return e
 }
 

model/bookmark.go

@@ -0,0 +1,28 @@
+package model
+
+import "time"
+
+type Bookmarkable struct {
+	BookmarkPosition int64 `json:"bookmarkPosition"`
+}
+
+type BookmarkableRepository interface {
+	AddBookmark(id, comment string, position int64) error
+	DeleteBookmark(id string) error
+	GetBookmarks() (Bookmarks, error)
+}
+
+type Bookmark struct {
+	Item      MediaFile `json:"item"`
+	Comment   string    `json:"comment"`
+	Position  int64     `json:"position"`
+	ChangedBy string    `json:"changed_by"`
+	CreatedAt time.Time `json:"createdAt"`
+	UpdatedAt time.Time `json:"updatedAt"`
+}
+
+type Bookmarks []Bookmark
+
+// While I can't find a better way to make these fields optional in the models, I keep this list here
+// to be used in other packages
+var BookmarkFields = []string{"bookmarkPosition"}

model/mediafile.go

@@ -7,6 +7,7 @@ import (
 
 type MediaFile struct {
 	Annotations
+	Bookmarkable
 
 	ID                   string    `json:"id"            orm:"pk;column(id)"`
 	Path                 string    `json:"path"`
@@ -63,6 +64,7 @@ type MediaFileRepository interface {
 	DeleteByPath(path string) (int64, error)
 
 	AnnotatedRepository
+	BookmarkableRepository
 }
 
 func (mf MediaFile) GetAnnotations() Annotations {

model/playqueue.go

@@ -7,7 +7,6 @@ import (
 type PlayQueue struct {
 	ID        string     `json:"id"          orm:"column(id)"`
 	UserID    string     `json:"userId"      orm:"column(user_id)"`
-	Comment   string     `json:"comment"`
 	Current   string     `json:"current"`
 	Position  int64      `json:"position"`
 	ChangedBy string     `json:"changedBy"`
@@ -21,17 +20,4 @@ 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 {
-	Item      MediaFile `json:"item"`
-	Comment   string    `json:"comment"`
-	Position  int64     `json:"position"`
-	CreatedAt time.Time `json:"createdAt"`
-	UpdatedAt time.Time `json:"updatedAt"`
-}
-
-type Bookmarks []Bookmark

persistence/helpers.go

@@ -23,7 +23,9 @@ func toSqlArgs(rec interface{}) (map[string]interface{}, error) {
 	err = json.Unmarshal(b, &m)
 	r := make(map[string]interface{}, len(m))
 	for f, v := range m {
-		if !utils.StringInSlice(f, model.AnnotationFields) && v != nil {
+		isAnnotationField := utils.StringInSlice(f, model.AnnotationFields)
+		isBookmarkField := utils.StringInSlice(f, model.BookmarkFields)
+		if !isAnnotationField && !isBookmarkField && v != nil {
 			r[toSnakeCase(f)] = v
 		}
 	}

persistence/mediafile_repository.go

@@ -52,7 +52,8 @@ func (r mediaFileRepository) Put(m *model.MediaFile) error {
 }
 
 func (r mediaFileRepository) selectMediaFile(options ...model.QueryOptions) SelectBuilder {
-	return r.newSelectWithAnnotation("media_file.id", options...).Columns("media_file.*")
+	sql := r.newSelectWithAnnotation("media_file.id", options...).Columns("media_file.*")
+	return r.withBookmark(sql, "media_file.id")
 }
 
 func (r mediaFileRepository) Get(id string) (*model.MediaFile, error) {

persistence/persistence.go

@@ -137,6 +137,11 @@ func (s *SQLStore) GC(ctx context.Context) error {
 		log.Error(ctx, "Error removing orphan artist annotations", err)
 		return err
 	}
+	err = s.MediaFile(ctx).(*mediaFileRepository).cleanBookmarks()
+	if err != nil {
+		log.Error(ctx, "Error removing orphan bookmarks", err)
+		return err
+	}
 	err = s.Playlist(ctx).(*playlistRepository).removeOrphans()
 	if err != nil {
 		log.Error(ctx, "Error tidying up playlists", err)

persistence/playqueue_repository.go

@@ -9,7 +9,6 @@ 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 {
@@ -27,7 +26,6 @@ func NewPlayQueueRepository(ctx context.Context, o orm.Ormer) model.PlayQueueRep
 type playQueue struct {
 	ID        string `orm:"column(id)"`
 	UserID    string `orm:"column(user_id)"`
-	Comment   string
 	Current   string
 	Position  int64
 	ChangedBy string
@@ -64,79 +62,10 @@ 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("*").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
-	}
-
-	// If there is a previous bookmark, override
-	if prev.ID != "" {
-		bm.ID = prev.ID
-		bm.CreatedAt = prev.CreatedAt
-	}
-
-	_, err = r.put(bm.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 {
-		items := r.loadTracks(model.MediaFiles{{ID: pqs[i].Current}})
-		bms[i].Item = items[0]
-		bms[i].Comment = pqs[i].Comment
-		bms[i].Position = pqs[i].Position
-		bms[i].CreatedAt = pqs[i].CreatedAt
-		bms[i].UpdatedAt = pqs[i].UpdatedAt
-	}
-	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,
 		UserID:    q.UserID,
-		Comment:   q.Comment,
 		Current:   q.Current,
 		Position:  q.Position,
 		ChangedBy: q.ChangedBy,
@@ -155,7 +84,6 @@ func (r *playQueueRepository) toModel(pq *playQueue) model.PlayQueue {
 	q := model.PlayQueue{
 		ID:        pq.ID,
 		UserID:    pq.UserID,
-		Comment:   pq.Comment,
 		Current:   pq.Current,
 		Position:  pq.Position,
 		ChangedBy: pq.ChangedBy,
@@ -219,7 +147,7 @@ func (r *playQueueRepository) loadTracks(tracks model.MediaFiles) model.MediaFil
 }
 
 func (r *playQueueRepository) clearPlayQueue(userId string) error {
-	return r.delete(And{Eq{"user_id": userId}, NotEq{"items": ""}})
+	return r.delete(Eq{"user_id": userId})
 }
 
 var _ model.PlayQueueRepository = (*playQueueRepository)(nil)

persistence/playqueue_repository_test.go

@@ -52,55 +52,6 @@ var _ = Describe("PlayQueueRepository", func() {
 			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].Item.ID).To(Equal(songAntenna.ID))
-			Expect(bms[0].Item.Title).To(Equal(songAntenna.Title))
-			Expect(bms[0].Comment).To(Equal("this is a comment"))
-			Expect(bms[0].Position).To(Equal(int64(123)))
-
-			created := bms[0].CreatedAt
-			updated := bms[0].UpdatedAt
-
-			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].Item.ID).To(Equal(songAntenna.ID))
-			Expect(bms[0].Comment).To(Equal("another comment"))
-			Expect(bms[0].Position).To(Equal(int64(333)))
-			Expect(bms[0].CreatedAt).To(Equal(created))
-			Expect(bms[0].UpdatedAt).To(BeTemporally(">", updated))
-
-			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].Item.ID).To(Equal(songComeTogether.ID))
-			Expect(bms[0].Item.Title).To(Equal(songComeTogether.Title))
-		})
-	})
 })
 
 func countPlayQueues(repo model.PlayQueueRepository, userId string) int {
@@ -115,7 +66,6 @@ func countPlayQueues(repo model.PlayQueueRepository, userId string) int {
 func AssertPlayQueue(expected, actual *model.PlayQueue) {
 	Expect(actual.ID).To(Equal(expected.ID))
 	Expect(actual.UserID).To(Equal(expected.UserID))
-	Expect(actual.Comment).To(Equal(expected.Comment))
 	Expect(actual.Current).To(Equal(expected.Current))
 	Expect(actual.Position).To(Equal(expected.Position))
 	Expect(actual.ChangedBy).To(Equal(expected.ChangedBy))
@@ -132,7 +82,6 @@ func aPlayQueue(userId, current string, position int64, items ...model.MediaFile
 	return &model.PlayQueue{
 		ID:        id.String(),
 		UserID:    userId,
-		Comment:   "no_comments",
 		Current:   current,
 		Position:  position,
 		ChangedBy: "test",

persistence/sql_annotations.go

@@ -23,9 +23,9 @@ func (r sqlRepository) newSelectWithAnnotation(idField string, options ...model.
 
 func (r sqlRepository) annId(itemID ...string) And {
 	return And{
-		Eq{"user_id": userId(r.ctx)},
+		Eq{annotationTable + ".user_id": userId(r.ctx)},
-		Eq{"item_type": r.tableName},
+		Eq{annotationTable + ".item_type": r.tableName},
-		Eq{"item_id": itemID},
+		Eq{annotationTable + ".item_id": itemID},
 	}
 }
 

persistence/sql_base_repository.go

@@ -112,7 +112,7 @@ func (r sqlRepository) executeSQL(sq Sqlizer) (int64, error) {
 	return res.RowsAffected()
 }
 
-// Note: Due to a bug in the QueryRow, this method does not map any embedded structs (ex: annotations)
+// Note: Due to a bug in the QueryRow method, this function does not map any embedded structs (ex: annotations)
 // In this case, use the queryAll method and get the first item of the returned list
 func (r sqlRepository) queryOne(sq Sqlizer, response interface{}) error {
 	query, args, err := sq.ToSql()

persistence/sql_bookmarks.go

@@ -0,0 +1,151 @@
+package persistence
+
+import (
+	"time"
+
+	. "github.com/Masterminds/squirrel"
+	"github.com/astaxie/beego/orm"
+	"github.com/deluan/navidrome/log"
+	"github.com/deluan/navidrome/model"
+	"github.com/deluan/navidrome/model/request"
+)
+
+const bookmarkTable = "bookmark"
+
+func (r sqlRepository) withBookmark(sql SelectBuilder, idField string) SelectBuilder {
+	return sql.
+		LeftJoin("bookmark on (" +
+			"bookmark.item_id = " + idField +
+			" AND bookmark.item_type = '" + r.tableName + "'" +
+			" AND bookmark.user_id = '" + userId(r.ctx) + "')").
+		Columns("position as bookmark_position")
+}
+
+func (r sqlRepository) bmkID(itemID ...string) And {
+	return And{
+		Eq{bookmarkTable + ".user_id": userId(r.ctx)},
+		Eq{bookmarkTable + ".item_type": r.tableName},
+		Eq{bookmarkTable + ".item_id": itemID},
+	}
+}
+
+func (r sqlRepository) bmkUpsert(itemID, comment string, position int64) error {
+	client, _ := request.ClientFrom(r.ctx)
+	user, _ := request.UserFrom(r.ctx)
+	values := map[string]interface{}{
+		"comment":    comment,
+		"position":   position,
+		"updated_at": time.Now(),
+		"changed_by": client,
+	}
+
+	upd := Update(bookmarkTable).Where(r.bmkID(itemID)).SetMap(values)
+	c, err := r.executeSQL(upd)
+	if err == nil {
+		log.Debug(r.ctx, "Updated bookmark", "id", itemID, "user", user.UserName, "position", position, "comment", comment)
+	}
+	if c == 0 || err == orm.ErrNoRows {
+		values["user_id"] = user.ID
+		values["item_type"] = r.tableName
+		values["item_id"] = itemID
+		values["created_at"] = time.Now()
+		values["updated_at"] = time.Now()
+		ins := Insert(bookmarkTable).SetMap(values)
+		_, err = r.executeSQL(ins)
+		if err != nil {
+			return err
+		}
+		log.Debug(r.ctx, "Added bookmark", "id", itemID, "user", user.UserName, "position", position, "comment", comment)
+	}
+
+	return err
+}
+
+func (r sqlRepository) AddBookmark(id, comment string, position int64) error {
+	user, _ := request.UserFrom(r.ctx)
+	err := r.bmkUpsert(id, comment, position)
+	if err != nil {
+		log.Error(r.ctx, "Error adding bookmark", "id", id, "user", user.UserName, "position", position, "comment", comment)
+	}
+	return err
+}
+
+func (r sqlRepository) DeleteBookmark(id string) error {
+	user, _ := request.UserFrom(r.ctx)
+	del := Delete(bookmarkTable).Where(r.bmkID(id))
+	_, err := r.executeSQL(del)
+	if err != nil {
+		log.Error(r.ctx, "Error removing bookmark", "id", id, "user", user.UserName)
+	}
+	return err
+}
+
+type bookmark struct {
+	UserID    string    `json:"user_id"         orm:"column(user_id)"`
+	ItemID    string    `json:"item_id"         orm:"column(item_id)"`
+	ItemType  string    `json:"item_type"`
+	Comment   string    `json:"comment"`
+	Position  int64     `json:"position"`
+	ChangedBy string    `json:"changed_by"`
+	CreatedAt time.Time `json:"createdAt"`
+	UpdatedAt time.Time `json:"updatedAt"`
+}
+
+func (r sqlRepository) GetBookmarks() (model.Bookmarks, error) {
+	user, _ := request.UserFrom(r.ctx)
+
+	idField := r.tableName + ".id"
+	sql := r.newSelectWithAnnotation(idField).Columns("*")
+	sql = r.withBookmark(sql, idField).Where(NotEq{bookmarkTable + ".item_id": nil})
+	var mfs model.MediaFiles
+	err := r.queryAll(sql, &mfs)
+	if err != nil {
+		log.Error(r.ctx, "Error getting mediafiles with bookmarks", "user", user.UserName, err)
+		return nil, err
+	}
+
+	ids := make([]string, len(mfs))
+	mfMap := make(map[string]int)
+	for i, mf := range mfs {
+		ids[i] = mf.ID
+		mfMap[mf.ID] = i
+	}
+
+	sql = Select("*").From(bookmarkTable).Where(r.bmkID(ids...))
+	var bmks []bookmark
+	err = r.queryAll(sql, &bmks)
+	if err != nil {
+		log.Error(r.ctx, "Error getting bookmarks", "user", user.UserName, "ids", ids, err)
+		return nil, err
+	}
+
+	resp := make(model.Bookmarks, len(bmks))
+	for i, bmk := range bmks {
+		if itemIdx, ok := mfMap[bmk.ItemID]; !ok {
+			log.Debug(r.ctx, "Invalid bookmark", "id", bmk.ItemID, "user", user.UserName)
+			continue
+		} else {
+			resp[i] = model.Bookmark{
+				Comment:   bmk.Comment,
+				Position:  bmk.Position,
+				CreatedAt: bmk.CreatedAt,
+				UpdatedAt: bmk.UpdatedAt,
+				ChangedBy: bmk.ChangedBy,
+				Item:      mfs[itemIdx],
+			}
+		}
+	}
+	return resp, nil
+}
+
+func (r sqlRepository) cleanBookmarks() error {
+	del := Delete(bookmarkTable).Where(Eq{"item_type": r.tableName}).Where("item_id not in (select id from " + r.tableName + ")")
+	c, err := r.executeSQL(del)
+	if err != nil {
+		return err
+	}
+	if c > 0 {
+		log.Debug(r.ctx, "Clean-up bookmarks", "totalDeleted", c)
+	}
+	return nil
+}

persistence/sql_bookmarks_test.go

@@ -0,0 +1,72 @@
+package persistence
+
+import (
+	"context"
+
+	"github.com/astaxie/beego/orm"
+	"github.com/deluan/navidrome/log"
+	"github.com/deluan/navidrome/model"
+	"github.com/deluan/navidrome/model/request"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("sqlBookmarks", func() {
+	var mr model.MediaFileRepository
+
+	BeforeEach(func() {
+		ctx := log.NewContext(context.TODO())
+		ctx = request.WithUser(ctx, model.User{ID: "user1"})
+		mr = NewMediaFileRepository(ctx, orm.NewOrm())
+	})
+
+	Describe("Bookmarks", func() {
+		It("returns an empty collection if there are no bookmarks", func() {
+			Expect(mr.GetBookmarks()).To(BeEmpty())
+		})
+
+		It("saves and overrides bookmarks", func() {
+			By("Saving the bookmark")
+			Expect(mr.AddBookmark(songAntenna.ID, "this is a comment", 123)).To(BeNil())
+
+			bms, err := mr.GetBookmarks()
+			Expect(err).To(BeNil())
+
+			Expect(bms).To(HaveLen(1))
+			Expect(bms[0].Item.ID).To(Equal(songAntenna.ID))
+			Expect(bms[0].Item.Title).To(Equal(songAntenna.Title))
+			Expect(bms[0].Comment).To(Equal("this is a comment"))
+			Expect(bms[0].Position).To(Equal(int64(123)))
+			created := bms[0].CreatedAt
+			updated := bms[0].UpdatedAt
+			Expect(created.IsZero()).To(BeFalse())
+			Expect(updated).To(BeTemporally(">=", created))
+
+			By("Overriding the bookmark")
+			Expect(mr.AddBookmark(songAntenna.ID, "another comment", 333)).To(BeNil())
+
+			bms, err = mr.GetBookmarks()
+			Expect(err).To(BeNil())
+
+			Expect(bms[0].Item.ID).To(Equal(songAntenna.ID))
+			Expect(bms[0].Comment).To(Equal("another comment"))
+			Expect(bms[0].Position).To(Equal(int64(333)))
+			Expect(bms[0].CreatedAt).To(Equal(created))
+			Expect(bms[0].UpdatedAt).To(BeTemporally(">=", updated))
+
+			By("Saving another bookmark")
+			Expect(mr.AddBookmark(songComeTogether.ID, "one more comment", 444)).To(BeNil())
+			bms, err = mr.GetBookmarks()
+			Expect(err).To(BeNil())
+			Expect(bms).To(HaveLen(2))
+
+			By("Delete bookmark")
+			Expect(mr.DeleteBookmark(songAntenna.ID))
+			bms, err = mr.GetBookmarks()
+			Expect(err).To(BeNil())
+			Expect(bms).To(HaveLen(1))
+			Expect(bms[0].Item.ID).To(Equal(songComeTogether.ID))
+			Expect(bms[0].Item.Title).To(Equal(songComeTogether.Title))
+		})
+	})
+})

server/subsonic/bookmarks.go

@@ -21,8 +21,8 @@ func NewBookmarksController(ds model.DataStore) *BookmarksController {
 func (c *BookmarksController) GetBookmarks(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) {
 	user, _ := request.UserFrom(r.Context())
 
-	repo := c.ds.PlayQueue(r.Context())
+	repo := c.ds.MediaFile(r.Context())
-	bmks, err := repo.GetBookmarks(user.ID)
+	bmks, err := repo.GetBookmarks()
 	if err != nil {
 		return nil, NewError(responses.ErrorGeneric, "Internal Error")
 	}
@@ -52,10 +52,8 @@ func (c *BookmarksController) CreateBookmark(w http.ResponseWriter, r *http.Requ
 	comment := utils.ParamString(r, "comment")
 	position := utils.ParamInt64(r, "position", 0)
 
-	user, _ := request.UserFrom(r.Context())
+	repo := c.ds.MediaFile(r.Context())
-
+	err = repo.AddBookmark(id, comment, position)
-	repo := c.ds.PlayQueue(r.Context())
-	err = repo.AddBookmark(user.ID, id, comment, position)
 	if err != nil {
 		return nil, NewError(responses.ErrorGeneric, "Internal Error")
 	}
@@ -68,10 +66,8 @@ func (c *BookmarksController) DeleteBookmark(w http.ResponseWriter, r *http.Requ
 		return nil, err
 	}
 
-	user, _ := request.UserFrom(r.Context())
+	repo := c.ds.MediaFile(r.Context())
-
+	err = repo.DeleteBookmark(id)
-	repo := c.ds.PlayQueue(r.Context())
-	err = repo.DeleteBookmark(user.ID, id)
 	if err != nil {
 		return nil, NewError(responses.ErrorGeneric, "Internal Error")
 	}

server/subsonic/helpers.go

@@ -142,6 +142,7 @@ func ToChild(ctx context.Context, entry engine.Entry) responses.Child {
 		child.TranscodedSuffix = format
 		child.TranscodedContentType = mime.TypeByExtension("." + format)
 	}
+	child.BookmarkPosition = entry.BookmarkPosition
 	return child
 }
 
@@ -203,6 +204,7 @@ func ChildFromMediaFile(ctx context.Context, mf model.MediaFile) responses.Child
 		child.TranscodedSuffix = format
 		child.TranscodedContentType = mime.TypeByExtension("." + format)
 	}
+	child.BookmarkPosition = mf.BookmarkPosition
 	return child
 }
 

server/subsonic/responses/responses.go

@@ -117,9 +117,9 @@ type Child struct {
 	UserRating            int        `xml:"userRating,attr,omitempty"               json:"userRating,omitempty"`
 	SongCount             int        `xml:"songCount,attr,omitempty"                json:"songCount,omitempty"`
 	IsVideo               bool       `xml:"isVideo,attr"                            json:"isVideo"`
+	BookmarkPosition      int64      `xml:"bookmarkPosition,attr,omitempty"         json:"bookmarkPosition,omitempty"`
 	/*
 	   <xs:attribute name="averageRating" type="sub:AverageRating" use="optional"/>  <!-- Added in 1.6.0 -->
-	   <xs:attribute name="bookmarkPosition" type="xs:long" use="optional"/>  <!-- In millis. Added in 1.10.1 -->
 	   <xs:attribute name="originalWidth" type="xs:int" use="optional"/>  <!-- Added in 1.13.0 -->
 	   <xs:attribute name="originalHeight" type="xs:int" use="optional"/>  <!-- Added in 1.13.0 -->
 	*/