Update bookmarks API endpoints to use new user model

server/controllers/MeController.js

@@ -3,7 +3,7 @@ const Logger = require('../Logger')
 const SocketAuthority = require('../SocketAuthority')
 const Database = require('../Database')
 const { sort } = require('../libs/fastSort')
-const { toNumber } = require('../utils/index')
+const { toNumber, isNullOrNaN } = require('../utils/index')
 const userStats = require('../utils/queries/userStats')
 
 /**
@@ -193,45 +193,71 @@ class MeController {
     if (!(await Database.libraryItemModel.checkExistsById(req.params.id))) return res.sendStatus(404)
 
     const { time, title } = req.body
-    const bookmark = req.user.createBookmark(req.params.id, time, title)
+    if (isNullOrNaN(time)) {
-    await Database.updateUser(req.user)
+      Logger.error(`[MeController] createBookmark invalid time`, time)
-    SocketAuthority.clientEmitter(req.userNew.id, 'user_updated', req.user.toJSONForBrowser())
+      return res.status(400).send('Invalid time')
+    }
+    if (!title || typeof title !== 'string') {
+      Logger.error(`[MeController] createBookmark invalid title`, title)
+      return res.status(400).send('Invalid title')
+    }
+
+    const bookmark = await req.userNew.createBookmark(req.params.id, time, title)
+    SocketAuthority.clientEmitter(req.userNew.id, 'user_updated', req.userNew.toOldJSONForBrowser())
     res.json(bookmark)
   }
 
-  // PATCH: api/me/item/:id/bookmark
+  /**
+   * PATCH: /api/me/item/:id/bookmark
+   *
+   * @param {RequestWithUser} req
+   * @param {Response} res
+   */
   async updateBookmark(req, res) {
     if (!(await Database.libraryItemModel.checkExistsById(req.params.id))) return res.sendStatus(404)
 
     const { time, title } = req.body
-    if (!req.user.findBookmark(req.params.id, time)) {
+    if (isNullOrNaN(time)) {
-      Logger.error(`[MeController] updateBookmark not found`)
+      Logger.error(`[MeController] updateBookmark invalid time`, time)
+      return res.status(400).send('Invalid time')
+    }
+    if (!title || typeof title !== 'string') {
+      Logger.error(`[MeController] updateBookmark invalid title`, title)
+      return res.status(400).send('Invalid title')
+    }
+
+    const bookmark = await req.userNew.updateBookmark(req.params.id, time, title)
+    if (!bookmark) {
+      Logger.error(`[MeController] updateBookmark not found for library item id "${req.params.id}" and time "${time}"`)
       return res.sendStatus(404)
     }
 
-    const bookmark = req.user.updateBookmark(req.params.id, time, title)
+    SocketAuthority.clientEmitter(req.userNew.id, 'user_updated', req.userNew.toOldJSONForBrowser())
-    if (!bookmark) return res.sendStatus(500)
-
-    await Database.updateUser(req.user)
-    SocketAuthority.clientEmitter(req.user.id, 'user_updated', req.user.toJSONForBrowser())
     res.json(bookmark)
   }
 
-  // DELETE: api/me/item/:id/bookmark/:time
+  /**
+   * DELETE: /api/me/item/:id/bookmark/:time
+   *
+   * @param {RequestWithUser} req
+   * @param {Response} res
+   */
   async removeBookmark(req, res) {
     if (!(await Database.libraryItemModel.checkExistsById(req.params.id))) return res.sendStatus(404)
 
     const time = Number(req.params.time)
-    if (isNaN(time)) return res.sendStatus(500)
+    if (isNaN(time)) {
+      return res.status(400).send('Invalid time')
+    }
 
-    if (!req.user.findBookmark(req.params.id, time)) {
+    if (!req.userNew.findBookmark(req.params.id, time)) {
       Logger.error(`[MeController] removeBookmark not found`)
       return res.sendStatus(404)
     }
 
-    req.user.removeBookmark(req.params.id, time)
+    await req.userNew.removeBookmark(req.params.id, time)
-    await Database.updateUser(req.user)
+
-    SocketAuthority.clientEmitter(req.user.id, 'user_updated', req.user.toJSONForBrowser())
+    SocketAuthority.clientEmitter(req.userNew.id, 'user_updated', req.userNew.toOldJSONForBrowser())
     res.sendStatus(200)
   }
 

server/models/User.js

@@ -2,11 +2,20 @@ const uuidv4 = require('uuid').v4
 const sequelize = require('sequelize')
 const Logger = require('../Logger')
 const oldUser = require('../objects/user/User')
+const AudioBookmark = require('../objects/user/AudioBookmark')
 const SocketAuthority = require('../SocketAuthority')
 const { isNullOrNaN } = require('../utils')
 
 const { DataTypes, Model } = sequelize
 
+/**
+ * @typedef AudioBookmarkObject
+ * @property {string} libraryItemId
+ * @property {string} title
+ * @property {number} time
+ * @property {number} createdAt
+ */
+
 class User extends Model {
   constructor(values, options) {
     super(values, options)
@@ -31,7 +40,7 @@ class User extends Model {
     this.lastSeen
     /** @type {Object} */
     this.permissions
-    /** @type {Object} */
+    /** @type {AudioBookmarkObject[]} */
     this.bookmarks
     /** @type {Object} */
     this.extraData
@@ -689,6 +698,88 @@ class User extends Model {
       mediaProgress
     }
   }
+
+  /**
+   * Find bookmark
+   * TODO: Bookmarks should use mediaItemId instead of libraryItemId to support podcast episodes
+   *
+   * @param {string} libraryItemId
+   * @param {number} time
+   * @returns {AudioBookmarkObject|null}
+   */
+  findBookmark(libraryItemId, time) {
+    return this.bookmarks.find((bm) => bm.libraryItemId === libraryItemId && bm.time == time)
+  }
+
+  /**
+   * Create bookmark
+   *
+   * @param {string} libraryItemId
+   * @param {number} time
+   * @param {string} title
+   * @returns {Promise<AudioBookmarkObject>}
+   */
+  async createBookmark(libraryItemId, time, title) {
+    const existingBookmark = this.findBookmark(libraryItemId, time)
+    if (existingBookmark) {
+      Logger.warn('[User] Create Bookmark already exists for this time')
+      if (existingBookmark.title !== title) {
+        existingBookmark.title = title
+        this.changed('bookmarks', true)
+        await this.save()
+      }
+      return existingBookmark
+    }
+
+    const newBookmark = {
+      libraryItemId,
+      time,
+      title,
+      createdAt: Date.now()
+    }
+    this.bookmarks.push(newBookmark)
+    this.changed('bookmarks', true)
+    await this.save()
+    return newBookmark
+  }
+
+  /**
+   * Update bookmark
+   *
+   * @param {string} libraryItemId
+   * @param {number} time
+   * @param {string} title
+   * @returns {Promise<AudioBookmarkObject>}
+   */
+  async updateBookmark(libraryItemId, time, title) {
+    const bookmark = this.findBookmark(libraryItemId, time)
+    if (!bookmark) {
+      Logger.error(`[User] updateBookmark not found`)
+      return null
+    }
+    bookmark.title = title
+    this.changed('bookmarks', true)
+    await this.save()
+    return bookmark
+  }
+
+  /**
+   * Remove bookmark
+   *
+   * @param {string} libraryItemId
+   * @param {number} time
+   * @returns {Promise<boolean>} - true if bookmark was removed
+   */
+  async removeBookmark(libraryItemId, time) {
+    if (!this.findBookmark(libraryItemId, time)) {
+      Logger.error(`[User] removeBookmark not found`)
+      return false
+    }
+    this.bookmarks = this.bookmarks.filter((bm) => bm.libraryItemId !== libraryItemId || bm.time !== time)
+    this.changed('bookmarks', true)
+    await this.save()
+    return true
+  }
 }
 
 module.exports = User

server/objects/user/User.js

@@ -450,37 +450,6 @@ class User {
     return this.checkCanAccessLibraryItemWithTags(tags)
   }
 
-  findBookmark(libraryItemId, time) {
-    return this.bookmarks.find((bm) => bm.libraryItemId === libraryItemId && bm.time == time)
-  }
-
-  createBookmark(libraryItemId, time, title) {
-    var existingBookmark = this.findBookmark(libraryItemId, time)
-    if (existingBookmark) {
-      Logger.warn('[User] Create Bookmark already exists for this time')
-      existingBookmark.title = title
-      return existingBookmark
-    }
-    var newBookmark = new AudioBookmark()
-    newBookmark.setData(libraryItemId, time, title)
-    this.bookmarks.push(newBookmark)
-    return newBookmark
-  }
-
-  updateBookmark(libraryItemId, time, title) {
-    var bookmark = this.findBookmark(libraryItemId, time)
-    if (!bookmark) {
-      Logger.error(`[User] updateBookmark not found`)
-      return null
-    }
-    bookmark.title = title
-    return bookmark
-  }
-
-  removeBookmark(libraryItemId, time) {
-    this.bookmarks = this.bookmarks.filter((bm) => bm.libraryItemId !== libraryItemId || bm.time !== time)
-  }
-
   checkShouldHideSeriesFromContinueListening(seriesId) {
     return this.seriesHideFromContinueListening.includes(seriesId)
   }