Fix duplicates when syncing a channel

packages/tests/src/api/videos/video-channel-syncs.ts

@@ -22,7 +22,6 @@ describe('Test channel synchronizations', function () {
   if (areYoutubeImportTestsDisabled()) return
 
   function runSuite (mode: 'youtube-dl' | 'yt-dlp') {
-
     describe('Sync using ' + mode, function () {
       let servers: PeerTubeServer[]
       let sqlCommands: SQLCommand[] = []
@@ -30,6 +29,8 @@ describe('Test channel synchronizations', function () {
       let startTestDate: Date
 
       let rootChannelSyncId: number
+      let videoToDelete: number
+
       const userInfo = {
         accessToken: '',
         username: 'user1',
@@ -41,8 +42,8 @@ describe('Test channel synchronizations', function () {
       async function changeDateForSync (channelSyncId: number, newDate: string) {
         await sqlCommands[0].updateQuery(
           `UPDATE "videoChannelSync" ` +
-          `SET "createdAt"='${newDate}', "lastSyncAt"='${newDate}' ` +
+            `SET "createdAt"='${newDate}', "lastSyncAt"='${newDate}' ` +
-          `WHERE id=${channelSyncId}`
+            `WHERE id=${channelSyncId}`
         )
       }
 
@@ -288,21 +289,34 @@ describe('Test channel synchronizations', function () {
           }
         })
 
-        const { videoChannelSync: { id: videoChannelSyncId } } = await servers[0].channelSyncs.create({
+        const { videoChannelSync } = await servers[0].channelSyncs.create({
           attributes: {
             externalChannelUrl: FIXTURE_URLS.youtubePlaylist,
             videoChannelId: channelId
           }
         })
+        rootChannelSyncId = videoChannelSync.id
 
-        await forceSyncAll(videoChannelSyncId)
+        await forceSyncAll(rootChannelSyncId)
 
         {
-
           const { total, data } = await listAllVideosOfChannel('channel2')
           expect(total).to.equal(2)
           expect(data[0].name).to.equal('test')
           expect(data[1].name).to.equal('small video - youtube')
+
+          videoToDelete = data[1].id
+        }
+      })
+
+      it('Should not re-import deleted videos', async function () {
+        await servers[0].videos.remove({ id: videoToDelete })
+        await forceSyncAll(rootChannelSyncId)
+
+        {
+          const { total, data } = await listAllVideosOfChannel('channel2')
+          expect(total).to.equal(1)
+          expect(data[0].name).to.equal('test')
         }
       })
 

server/core/lib/sync-channel.ts

@@ -1,11 +1,11 @@
+import { VideoChannelSyncState, VideoPrivacy } from '@peertube/peertube-models'
 import { logger, loggerTagsFactory } from '@server/helpers/logger.js'
 import { YoutubeDLWrapper } from '@server/helpers/youtube-dl/index.js'
 import { CONFIG } from '@server/initializers/config.js'
 import { buildYoutubeDLImport } from '@server/lib/video-pre-import.js'
 import { UserModel } from '@server/models/user/user.js'
 import { VideoImportModel } from '@server/models/video/video-import.js'
-import { MChannel, MChannelAccountDefault, MChannelSync } from '@server/types/models/index.js'
+import { MChannelAccountDefault, MChannelSync } from '@server/types/models/index.js'
-import { VideoChannelSyncState, VideoPrivacy } from '@peertube/peertube-models'
 import { CreateJobArgument, JobQueue } from './job-queue/index.js'
 import { ServerConfigManager } from './server-config-manager.js'
 
@@ -38,7 +38,9 @@ export async function synchronizeChannel (options: {
 
     logger.info(
       'Fetched %d candidate URLs for sync channel %s.',
-      targetUrls.length, channel.Actor.preferredUsername, { targetUrls, ...lTags() }
+      targetUrls.length,
+      channel.Actor.preferredUsername,
+      { targetUrls, ...lTags() }
     )
 
     if (targetUrls.length === 0) {
@@ -56,7 +58,7 @@ export async function synchronizeChannel (options: {
       logger.debug(`Import candidate: ${targetUrl}`, lTags())
 
       try {
-        if (await skipImport(channel, targetUrl, onlyAfter)) continue
+        if (await skipImport({ channel, channelSync, targetUrl, onlyAfter })) continue
 
         const { job } = await buildYoutubeDLImport({
           user,
@@ -92,9 +94,19 @@ export async function synchronizeChannel (options: {
 
 // ---------------------------------------------------------------------------
 
-async function skipImport (channel: MChannel, targetUrl: string, onlyAfter?: Date) {
+async function skipImport (options: {
-  if (await VideoImportModel.urlAlreadyImported(channel.id, targetUrl)) {
+  channel: MChannelAccountDefault
-    logger.debug('%s is already imported for channel %s, skipping video channel synchronization.', targetUrl, channel.name, lTags())
+  channelSync: MChannelSync
+  targetUrl: string
+  onlyAfter?: Date
+}) {
+  const { channel, channelSync, targetUrl, onlyAfter } = options
+
+  if (await VideoImportModel.urlAlreadyImported({ channelId: channel.id, channelSyncId: channelSync?.id, targetUrl })) {
+    logger.debug(
+      `${targetUrl} is already imported for channel ${channel.name}, skipping video channel synchronization.`,
+      { channelSync, ...lTags() }
+    )
     return true
   }
 

server/core/models/video/video-import.ts

@@ -208,18 +208,44 @@ export class VideoImportModel extends SequelizeModel<VideoImportModel> {
     ]).then(([ total, data ]) => ({ total, data }))
   }
 
-  static async urlAlreadyImported (channelId: number, targetUrl: string): Promise<boolean> {
+  static async urlAlreadyImported (options: {
-    const element = await VideoImportModel.unscoped().findOne({
+    targetUrl: string
-      where: {
+    channelId: number
-        targetUrl,
+    channelSyncId?: number
-        state: {
+  }): Promise<boolean> {
-          [Op.in]: [ VideoImportState.PENDING, VideoImportState.PROCESSING, VideoImportState.SUCCESS ]
+    const { channelSyncId, channelId, targetUrl } = options
-        },
+
-        videoChannelSyncId: channelId
+    const baseWhere = {
+      targetUrl,
+      state: {
+        [Op.in]: [ VideoImportState.PENDING, VideoImportState.PROCESSING, VideoImportState.SUCCESS ]
       }
+    }
+
+    const bySyncId = channelSyncId
+      ? VideoImportModel.unscoped().findOne({
+        where: {
+          ...baseWhere,
+
+          videoChannelSyncId: channelSyncId
+        }
+      })
+      : Promise.resolve(undefined)
+
+    const byChannelId = VideoImportModel.unscoped().findOne({
+      where: baseWhere,
+      include: [
+        {
+          model: VideoModel.unscoped(),
+          required: true,
+          where: {
+            channelId
+          }
+        }
+      ]
     })
 
-    return !!element
+    return (await Promise.all([ bySyncId, byChannelId ])).some(e => !!e)
   }
 
   getTargetIdentifier () {