From 28be89161aab245526d64f6fb7dd29391a97fe0a Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 7 Jun 2018 09:43:18 +0200 Subject: [PATCH] Improve create import file job Fix federation of .ogv videos --- scripts/create-import-video-file-job.ts | 1 + server/helpers/custom-validators/videos.ts | 4 +-- server/initializers/constants.ts | 3 ++ server/lib/job-queue/handlers/video-file.ts | 10 +++--- server/lib/job-queue/job-queue.ts | 6 ++-- server/models/video/video.ts | 27 +++++++++------- .../tests/cli/create-import-video-file-job.ts | 31 ++++++++++++------- 7 files changed, 48 insertions(+), 34 deletions(-) diff --git a/scripts/create-import-video-file-job.ts b/scripts/create-import-video-file-job.ts index a2f4f38f2..2b636014a 100644 --- a/scripts/create-import-video-file-job.ts +++ b/scripts/create-import-video-file-job.ts @@ -27,6 +27,7 @@ async function run () { const video = await VideoModel.loadByUUID(program['video']) if (!video) throw new Error('Video not found.') + if (video.isOwned() === false) throw new Error('Cannot import files of a non owned video.') const dataInput = { videoUUID: video.uuid, diff --git a/server/helpers/custom-validators/videos.ts b/server/helpers/custom-validators/videos.ts index 0c268a684..f365df985 100644 --- a/server/helpers/custom-validators/videos.ts +++ b/server/helpers/custom-validators/videos.ts @@ -7,8 +7,8 @@ import { UserRight, VideoRateType } from '../../../shared' import { CONSTRAINTS_FIELDS, VIDEO_CATEGORIES, - VIDEO_LANGUAGES, - VIDEO_LICENCES, VIDEO_MIMETYPE_EXT, + VIDEO_LICENCES, + VIDEO_MIMETYPE_EXT, VIDEO_PRIVACIES, VIDEO_RATE_TYPES } from '../../initializers' diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts index 482db2d5c..54f22c95a 100644 --- a/server/initializers/constants.ts +++ b/server/initializers/constants.ts @@ -7,6 +7,7 @@ import { VideoPrivacy } from '../../shared/models/videos' // Do not use barrels, remain constants as independent as possible import { buildPath, isTestInstance, root, sanitizeHost, sanitizeUrl } from '../helpers/core-utils' import { NSFWPolicyType } from '../../shared/models/videos/nsfw-policy.type' +import { invert } from 'lodash' // Use a variable to reload the configuration if we need let config: IConfig = require('config') @@ -330,6 +331,7 @@ const VIDEO_MIMETYPE_EXT = { 'video/ogg': '.ogv', 'video/mp4': '.mp4' } +const VIDEO_EXT_MIMETYPE = invert(VIDEO_MIMETYPE_EXT) const IMAGE_MIMETYPE_EXT = { 'image/png': '.png', @@ -501,6 +503,7 @@ export { SCHEDULER_INTERVAL, STATIC_DOWNLOAD_PATHS, RATES_LIMIT, + VIDEO_EXT_MIMETYPE, JOB_COMPLETED_LIFETIME, VIDEO_VIEW_LIFETIME, buildLanguages diff --git a/server/lib/job-queue/handlers/video-file.ts b/server/lib/job-queue/handlers/video-file.ts index 38eb3511c..a6fce4279 100644 --- a/server/lib/job-queue/handlers/video-file.ts +++ b/server/lib/job-queue/handlers/video-file.ts @@ -16,14 +16,14 @@ export type VideoFilePayload = { isPortraitMode?: boolean } -export type VideoImportPayload = { +export type VideoFileImportPayload = { videoUUID: string, filePath: string } -async function processVideoImport (job: kue.Job) { - const payload = job.data as VideoImportPayload - logger.info('Processing video import in job %d.', job.id) +async function processVideoFileImport (job: kue.Job) { + const payload = job.data as VideoFileImportPayload + logger.info('Processing video file import in job %d.', job.id) const video = await VideoModel.loadByUUIDAndPopulateAccountAndServerAndTags(payload.videoUUID) // No video, maybe deleted? @@ -132,5 +132,5 @@ async function onVideoFileOptimizerSuccess (video: VideoModel, isNewVideo: boole export { processVideoFile, - processVideoImport + processVideoFileImport } diff --git a/server/lib/job-queue/job-queue.ts b/server/lib/job-queue/job-queue.ts index 69335acf0..bdfa19b61 100644 --- a/server/lib/job-queue/job-queue.ts +++ b/server/lib/job-queue/job-queue.ts @@ -7,7 +7,7 @@ import { ActivitypubHttpBroadcastPayload, processActivityPubHttpBroadcast } from import { ActivitypubHttpFetcherPayload, processActivityPubHttpFetcher } from './handlers/activitypub-http-fetcher' import { ActivitypubHttpUnicastPayload, processActivityPubHttpUnicast } from './handlers/activitypub-http-unicast' import { EmailPayload, processEmail } from './handlers/email' -import { processVideoFile, processVideoImport, VideoFilePayload, VideoImportPayload } from './handlers/video-file' +import { processVideoFile, processVideoFileImport, VideoFilePayload, VideoFileImportPayload } from './handlers/video-file' import { ActivitypubFollowPayload, processActivityPubFollow } from './handlers/activitypub-follow' type CreateJobArgument = @@ -15,7 +15,7 @@ type CreateJobArgument = { type: 'activitypub-http-unicast', payload: ActivitypubHttpUnicastPayload } | { type: 'activitypub-http-fetcher', payload: ActivitypubHttpFetcherPayload } | { type: 'activitypub-follow', payload: ActivitypubFollowPayload } | - { type: 'video-file-import', payload: VideoImportPayload } | + { type: 'video-file-import', payload: VideoFileImportPayload } | { type: 'video-file', payload: VideoFilePayload } | { type: 'email', payload: EmailPayload } @@ -24,7 +24,7 @@ const handlers: { [ id in JobType ]: (job: kue.Job) => Promise} = { 'activitypub-http-unicast': processActivityPubHttpUnicast, 'activitypub-http-fetcher': processActivityPubHttpFetcher, 'activitypub-follow': processActivityPubFollow, - 'video-file-import': processVideoImport, + 'video-file-import': processVideoFileImport, 'video-file': processVideoFile, 'email': processEmail } diff --git a/server/models/video/video.ts b/server/models/video/video.ts index 2875e6685..faa6f3f9b 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -2,7 +2,7 @@ import * as Bluebird from 'bluebird' import { map, maxBy } from 'lodash' import * as magnetUtil from 'magnet-uri' import * as parseTorrent from 'parse-torrent' -import { join, extname } from 'path' +import { extname, join } from 'path' import * as Sequelize from 'sequelize' import { AllowNull, @@ -30,9 +30,9 @@ import { VideoTorrentObject } from '../../../shared/models/activitypub/objects' import { Video, VideoDetails, VideoFile } from '../../../shared/models/videos' import { VideoFilter } from '../../../shared/models/videos/video-query.type' import { + copyFilePromise, createTorrentPromise, peertubeTruncate, - copyFilePromise, renamePromise, statPromise, unlinkPromise, @@ -63,6 +63,7 @@ import { STATIC_PATHS, THUMBNAILS_SIZE, VIDEO_CATEGORIES, + VIDEO_EXT_MIMETYPE, VIDEO_LANGUAGES, VIDEO_LICENCES, VIDEO_PRIVACIES @@ -1166,7 +1167,7 @@ export class VideoModel extends Model { for (const file of this.VideoFiles) { url.push({ type: 'Link', - mimeType: 'video/' + file.extname.replace('.', ''), + mimeType: VIDEO_EXT_MIMETYPE[file.extname], href: this.getVideoFileUrl(file, baseUrlHttp), width: file.resolution, size: file.size @@ -1328,14 +1329,18 @@ export class VideoModel extends Model { await copyFilePromise(inputFilePath, outputPath) const currentVideoFile = this.VideoFiles.find(videoFile => videoFile.resolution === updatedVideoFile.resolution) - const isNewVideoFile = !currentVideoFile - if (!isNewVideoFile) { - if (currentVideoFile.extname !== updatedVideoFile.extname) { - await this.removeFile(currentVideoFile) - currentVideoFile.set('extname', updatedVideoFile.extname) - } + if (currentVideoFile) { + // Remove old file and old torrent + await this.removeFile(currentVideoFile) + await this.removeTorrent(currentVideoFile) + // Remove the old video file from the array + this.VideoFiles = this.VideoFiles.filter(f => f !== currentVideoFile) + + // Update the database + currentVideoFile.set('extname', updatedVideoFile.extname) currentVideoFile.set('size', updatedVideoFile.size) + updatedVideoFile = currentVideoFile } @@ -1343,9 +1348,7 @@ export class VideoModel extends Model { await updatedVideoFile.save() - if (isNewVideoFile) { - this.VideoFiles.push(updatedVideoFile) - } + this.VideoFiles.push(updatedVideoFile) } getOriginalFileResolution () { diff --git a/server/tests/cli/create-import-video-file-job.ts b/server/tests/cli/create-import-video-file-job.ts index d486db600..251088882 100644 --- a/server/tests/cli/create-import-video-file-job.ts +++ b/server/tests/cli/create-import-video-file-job.ts @@ -3,29 +3,31 @@ import 'mocha' import * as chai from 'chai' import { VideoDetails, VideoFile } from '../../../shared/models/videos' -const expect = chai.expect - import { + doubleFollow, execCLI, + flushAndRunMultipleServers, flushTests, getEnvCli, + getVideo, getVideosList, killallServers, - parseTorrentVideo, - runServer, ServerInfo, setAccessTokensToServers, uploadVideo, - wait, - getVideo, flushAndRunMultipleServers, doubleFollow + wait } from '../utils' -function assertVideoProperties (video: VideoFile, resolution: number, extname: string) { +const expect = chai.expect + +function assertVideoProperties (video: VideoFile, resolution: number, extname: string, size?: number) { expect(video).to.have.nested.property('resolution.id', resolution) expect(video).to.have.property('magnetUri').that.includes(`.${extname}`) expect(video).to.have.property('torrentUrl').that.includes(`-${resolution}.torrent`) expect(video).to.have.property('fileUrl').that.includes(`.${extname}`) expect(video).to.have.property('size').that.is.above(0) + + if (size) expect(video.size).to.equal(size) } describe('Test create import video jobs', function () { @@ -51,6 +53,7 @@ describe('Test create import video jobs', function () { const res2 = await uploadVideo(servers[1].url, servers[1].accessToken, { name: 'video2' }) video2UUID = res2.body.video.uuid + // Transcoding await wait(40000) }) @@ -60,12 +63,11 @@ describe('Test create import video jobs', function () { await wait(30000) + let magnetUri: string for (const server of servers) { const { data: videos } = (await getVideosList(server.url)).body expect(videos).to.have.lengthOf(2) - let infoHashes: { [ id: number ]: string } = {} - const video = videos.find(({ uuid }) => uuid === video1UUID) const videoDetail: VideoDetails = (await getVideo(server.url, video.uuid)).body @@ -73,6 +75,9 @@ describe('Test create import video jobs', function () { const [originalVideo, transcodedVideo] = videoDetail.files assertVideoProperties(originalVideo, 720, 'webm') assertVideoProperties(transcodedVideo, 480, 'webm') + + if (!magnetUri) magnetUri = transcodedVideo.magnetUri + else expect(transcodedVideo.magnetUri).to.equal(magnetUri) } }) @@ -82,21 +87,23 @@ describe('Test create import video jobs', function () { await wait(30000) + let magnetUri: string for (const server of servers.reverse()) { const { data: videos } = (await getVideosList(server.url)).body expect(videos).to.have.lengthOf(2) - let infoHashes: { [ id: number ]: string } - const video = videos.find(({ uuid }) => uuid === video2UUID) const videoDetail: VideoDetails = (await getVideo(server.url, video.uuid)).body expect(videoDetail.files).to.have.lengthOf(4) const [originalVideo, transcodedVideo420, transcodedVideo320, transcodedVideo240] = videoDetail.files - assertVideoProperties(originalVideo, 720, 'ogv') + assertVideoProperties(originalVideo, 720, 'ogv', 140849) assertVideoProperties(transcodedVideo420, 480, 'mp4') assertVideoProperties(transcodedVideo320, 360, 'mp4') assertVideoProperties(transcodedVideo240, 240, 'mp4') + + if (!magnetUri) magnetUri = originalVideo.magnetUri + else expect(originalVideo.magnetUri).to.equal(magnetUri) } }) -- 2.25.1