Improve create import file job

Fix federation of .ogv videos

6 files changed, 47 insertions, 34 deletions

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<any>} = {
   '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<VideoModel> {
     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<VideoModel> {
     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<VideoModel> {
 
     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)
     }
   })