From 7be401ac7629c7af586986f45f9109f2bbfcd7e3 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 28 Feb 2024 15:55:37 +0100 Subject: [PATCH] Fix playlist elements merge on import --- .../src/videos/playlists-command.ts | 17 +++++++++ packages/tests/src/api/users/user-import.ts | 21 ++++++----- server/core/controllers/api/video-playlist.ts | 33 ++--------------- .../importers/video-playlists-importer.ts | 20 ++++++++--- server/core/lib/video-playlist.ts | 36 +++++++++++++++---- server/core/models/video/video-playlist.ts | 9 ++++- 6 files changed, 85 insertions(+), 51 deletions(-) diff --git a/packages/server-commands/src/videos/playlists-command.ts b/packages/server-commands/src/videos/playlists-command.ts index c687ab001..821058c72 100644 --- a/packages/server-commands/src/videos/playlists-command.ts +++ b/packages/server-commands/src/videos/playlists-command.ts @@ -14,6 +14,7 @@ import { VideoPlaylistPrivacy, VideoPlaylistPrivacyType, VideoPlaylistReorder, + VideoPlaylistType, VideoPlaylistType_Type, VideoPlaylistUpdate } from '@peertube/peertube-models' @@ -82,6 +83,8 @@ export class PlaylistsCommand extends AbstractCommand { }) } + // --------------------------------------------------------------------------- + get (options: OverrideCommandOptions & { playlistId: number | string }) { @@ -97,6 +100,20 @@ export class PlaylistsCommand extends AbstractCommand { }) } + async getWatchLater (options: OverrideCommandOptions & { + handle: string + }) { + const { data: playlists } = await this.listByAccount({ + ...options, + + playlistType: VideoPlaylistType.WATCH_LATER + }) + + return playlists[0] + } + + // --------------------------------------------------------------------------- + listVideos (options: OverrideCommandOptions & { playlistId: number | string start?: number diff --git a/packages/tests/src/api/users/user-import.ts b/packages/tests/src/api/users/user-import.ts index f67bb0178..217ca04be 100644 --- a/packages/tests/src/api/users/user-import.ts +++ b/packages/tests/src/api/users/user-import.ts @@ -97,15 +97,14 @@ function runTest (withObjectStorage: boolean) { }) // Add a video in watch later playlist - const { data: playlists } = await server.playlists.listByAccount({ - token: noahToken, - handle: 'noah', - playlistType: VideoPlaylistType.WATCH_LATER + await server.playlists.addElement({ + playlistId: (await server.playlists.getWatchLater({ token: noahToken, handle: 'noah' })).id, + attributes: { videoId: noahVideo.uuid } }) - await server.playlists.addElement({ - playlistId: playlists[0].id, - attributes: { videoId: noahVideo.uuid } + await remoteServer.playlists.addElement({ + playlistId: (await remoteServer.playlists.getWatchLater({ token: remoteNoahToken, handle: 'noah_remote' })).id, + attributes: { videoId: mouskaVideo.uuid } }) await waitJobs([ server, remoteServer, blockedServer ]) @@ -285,11 +284,15 @@ function runTest (withObjectStorage: boolean) { expect(watchLater.privacy.id).to.equal(VideoPlaylistPrivacy.PRIVATE) // Playlists were merged - expect(watchLater.videosLength).to.equal(1) + expect(watchLater.videosLength).to.equal(2) const { data: videos } = await remoteServer.playlists.listVideos({ playlistId: watchLater.id, token: remoteNoahToken }) + expect(videos[0].position).to.equal(1) - expect(videos[0].video.uuid).to.equal(noahVideo.uuid) + // Mouska is muted + expect(videos[0].video).to.not.exist + expect(videos[1].position).to.equal(2) + expect(videos[1].video.uuid).to.equal(noahVideo.uuid) // Not federated await server.playlists.get({ playlistId: watchLater.uuid, expectedStatus: HttpStatusCode.NOT_FOUND_404 }) diff --git a/server/core/controllers/api/video-playlist.ts b/server/core/controllers/api/video-playlist.ts index aef073ceb..9220e3e71 100644 --- a/server/core/controllers/api/video-playlist.ts +++ b/server/core/controllers/api/video-playlist.ts @@ -13,10 +13,9 @@ import { VideoPlaylistUpdate } from '@peertube/peertube-models' import { scheduleRefreshIfNeeded } from '@server/lib/activitypub/playlists/index.js' -import { VideoMiniaturePermanentFileCache } from '@server/lib/files-cache/index.js' import { Hooks } from '@server/lib/plugins/hooks.js' import { getServerActor } from '@server/models/application/application.js' -import { MVideoPlaylistFull, MVideoPlaylistThumbnail, MVideoThumbnail } from '@server/types/models/index.js' +import { MVideoPlaylistFull, MVideoPlaylistThumbnail } from '@server/types/models/index.js' import { uuidToShort } from '@peertube/peertube-node-utils' import { resetSequelizeInstance } from '../../helpers/database-utils.js' import { createReqFiles } from '../../helpers/express-utils.js' @@ -51,6 +50,7 @@ import { import { AccountModel } from '../../models/account/account.js' import { VideoPlaylistElementModel } from '../../models/video/video-playlist-element.js' import { VideoPlaylistModel } from '../../models/video/video-playlist.js' +import { generateThumbnailForPlaylist } from '@server/lib/video-playlist.js' const reqThumbnailFile = createReqFiles([ 'thumbnailfile' ], MIMETYPES.IMAGE.MIMETYPE_EXT) @@ -329,7 +329,7 @@ async function addVideoInPlaylist (req: express.Request, res: express.Response) }) // If the user did not set a thumbnail, automatically take the video thumbnail - if (videoPlaylist.hasThumbnail() === false || (videoPlaylist.hasGeneratedThumbnail() && playlistElement.position === 1)) { + if (videoPlaylist.shouldGenerateThumbnailWithNewElement(playlistElement)) { await generateThumbnailForPlaylist(videoPlaylist, video) } @@ -488,30 +488,3 @@ async function regeneratePlaylistThumbnail (videoPlaylist: MVideoPlaylistThumbna const firstElement = await VideoPlaylistElementModel.loadFirstElementWithVideoThumbnail(videoPlaylist.id) if (firstElement) await generateThumbnailForPlaylist(videoPlaylist, firstElement.Video) } - -async function generateThumbnailForPlaylist (videoPlaylist: MVideoPlaylistThumbnail, video: MVideoThumbnail) { - logger.info('Generating default thumbnail to playlist %s.', videoPlaylist.url) - - const videoMiniature = video.getMiniature() - if (!videoMiniature) { - logger.info('Cannot generate thumbnail for playlist %s because video %s does not have any.', videoPlaylist.url, video.url) - return - } - - // Ensure the file is on disk - const videoMiniaturePermanentFileCache = new VideoMiniaturePermanentFileCache() - const inputPath = videoMiniature.isOwned() - ? videoMiniature.getPath() - : await videoMiniaturePermanentFileCache.downloadRemoteFile(videoMiniature) - - const thumbnailModel = await updateLocalPlaylistMiniatureFromExisting({ - inputPath, - playlist: videoPlaylist, - automaticallyGenerated: true, - keepOriginal: true - }) - - thumbnailModel.videoPlaylistId = videoPlaylist.id - - videoPlaylist.Thumbnail = await thumbnailModel.save() -} diff --git a/server/core/lib/user-import-export/importers/video-playlists-importer.ts b/server/core/lib/user-import-export/importers/video-playlists-importer.ts index 66bc25864..cd10e03e7 100644 --- a/server/core/lib/user-import-export/importers/video-playlists-importer.ts +++ b/server/core/lib/user-import-export/importers/video-playlists-importer.ts @@ -2,8 +2,7 @@ import { VideoPlaylistPrivacy, VideoPlaylistType, VideoPlaylistsExportJSON } fro import { logger, loggerTagsFactory } from '@server/helpers/logger.js' import { buildUUID } from '@peertube/peertube-node-utils' import { - MChannelBannerAccountDefault, MVideoPlaylist, - MVideoPlaylistFull, + MChannelBannerAccountDefault, MVideoPlaylistFull, MVideoPlaylistThumbnail } from '@server/types/models/index.js' import { getLocalVideoPlaylistActivityPubUrl, getLocalVideoPlaylistElementActivityPubUrl } from '@server/lib/activitypub/url.js' @@ -28,6 +27,8 @@ import { saveInTransactionWithRetries } from '@server/helpers/database-utils.js' import { isArray } from '@server/helpers/custom-validators/misc.js' import { isUrlValid } from '@server/helpers/custom-validators/activitypub/misc.js' import { pick } from '@peertube/peertube-core-utils' +import { VideoModel } from '@server/models/video/video.js' +import { generateThumbnailForPlaylist } from '@server/lib/video-playlist.js' const lTags = loggerTagsFactory('user-import') @@ -135,7 +136,7 @@ export class VideoPlaylistsImporter extends AbstractUserImporter { - for (let position = 1; position <= elementsToCreate.length; position++) { - const elementToCreate = elementsToCreate[position - 1] + let position = await VideoPlaylistElementModel.getNextPositionOf(playlist.id, t) + for (const elementToCreate of elementsToCreate) { const playlistElement = new VideoPlaylistElementModel({ position, startTimestamp: elementToCreate.startTimestamp, @@ -168,6 +169,15 @@ export class VideoPlaylistsImporter extends AbstractUserImporter logger.error('Cannot generate thumbnail from playlist', { err })) + } + + position++ } }) } diff --git a/server/core/lib/video-playlist.ts b/server/core/lib/video-playlist.ts index 5c97f9c9a..546ddba4f 100644 --- a/server/core/lib/video-playlist.ts +++ b/server/core/lib/video-playlist.ts @@ -1,11 +1,14 @@ import * as Sequelize from 'sequelize' import { VideoPlaylistPrivacy, VideoPlaylistType } from '@peertube/peertube-models' import { VideoPlaylistModel } from '../models/video/video-playlist.js' -import { MAccount } from '../types/models/index.js' -import { MVideoPlaylistOwner } from '../types/models/video/video-playlist.js' +import { MAccount, MVideoThumbnail } from '../types/models/index.js' +import { MVideoPlaylistOwner, MVideoPlaylistThumbnail } from '../types/models/video/video-playlist.js' import { getLocalVideoPlaylistActivityPubUrl } from './activitypub/url.js' +import { VideoMiniaturePermanentFileCache } from './files-cache/video-miniature-permanent-file-cache.js' +import { updateLocalPlaylistMiniatureFromExisting } from './thumbnail.js' +import { logger } from '@server/helpers/logger.js' -async function createWatchLaterPlaylist (account: MAccount, t: Sequelize.Transaction) { +export async function createWatchLaterPlaylist (account: MAccount, t: Sequelize.Transaction) { const videoPlaylist: MVideoPlaylistOwner = new VideoPlaylistModel({ name: 'Watch later', privacy: VideoPlaylistPrivacy.PRIVATE, @@ -22,8 +25,29 @@ async function createWatchLaterPlaylist (account: MAccount, t: Sequelize.Transac return videoPlaylist } -// --------------------------------------------------------------------------- +export async function generateThumbnailForPlaylist (videoPlaylist: MVideoPlaylistThumbnail, video: MVideoThumbnail) { + logger.info('Generating default thumbnail to playlist %s.', videoPlaylist.url) -export { - createWatchLaterPlaylist + const videoMiniature = video.getMiniature() + if (!videoMiniature) { + logger.info('Cannot generate thumbnail for playlist %s because video %s does not have any.', videoPlaylist.url, video.url) + return + } + + // Ensure the file is on disk + const videoMiniaturePermanentFileCache = new VideoMiniaturePermanentFileCache() + const inputPath = videoMiniature.isOwned() + ? videoMiniature.getPath() + : await videoMiniaturePermanentFileCache.downloadRemoteFile(videoMiniature) + + const thumbnailModel = await updateLocalPlaylistMiniatureFromExisting({ + inputPath, + playlist: videoPlaylist, + automaticallyGenerated: true, + keepOriginal: true + }) + + thumbnailModel.videoPlaylistId = videoPlaylist.id + + videoPlaylist.Thumbnail = await thumbnailModel.save() } diff --git a/server/core/models/video/video-playlist.ts b/server/core/models/video/video-playlist.ts index a6b2d4e73..6122a3fab 100644 --- a/server/core/models/video/video-playlist.ts +++ b/server/core/models/video/video-playlist.ts @@ -10,7 +10,7 @@ import { } from '@peertube/peertube-models' import { buildUUID, uuidToShort } from '@peertube/peertube-node-utils' import { activityPubCollectionPagination } from '@server/lib/activitypub/collection.js' -import { MAccountId, MChannelId } from '@server/types/models/index.js' +import { MAccountId, MChannelId, MVideoPlaylistElement } from '@server/types/models/index.js' import { join } from 'path' import { FindOptions, Includeable, literal, Op, ScopeOptions, Sequelize, Transaction, WhereOptions } from 'sequelize' import { @@ -629,6 +629,13 @@ export class VideoPlaylistModel extends SequelizeModel { return this.hasThumbnail() && this.Thumbnail.automaticallyGenerated === true } + shouldGenerateThumbnailWithNewElement (newElement: MVideoPlaylistElement) { + if (this.hasThumbnail() === false) return true + if (newElement.position === 1 && this.hasGeneratedThumbnail()) return true + + return false + } + generateThumbnailName () { const extension = '.jpg'