Implement pagination for overviews endpoint

7 files changed, 185 insertions, 70 deletions

diff --git a/server/controllers/api/overviews.ts b/server/controllers/api/overviews.ts
index 75f3baedb..fb31932aa 100644
--- a/server/controllers/api/overviews.ts
+++ b/server/controllers/api/overviews.ts
@@ -1,17 +1,18 @@
 import * as express from 'express'
 import { buildNSFWFilter } from '../../helpers/express-utils'
 import { VideoModel } from '../../models/video/video'
-import { asyncMiddleware } from '../../middlewares'
+import { asyncMiddleware, optionalAuthenticate, videosOverviewValidator } from '../../middlewares'
 import { TagModel } from '../../models/video/tag'
-import { VideosOverview } from '../../../shared/models/overviews'
-import { MEMOIZE_TTL, OVERVIEWS, ROUTE_CACHE_LIFETIME } from '../../initializers/constants'
-import { cacheRoute } from '../../middlewares/cache'
+import { CategoryOverview, ChannelOverview, TagOverview, VideosOverview } from '../../../shared/models/overviews'
+import { MEMOIZE_TTL, OVERVIEWS } from '../../initializers/constants'
 import * as memoizee from 'memoizee'
+import { logger } from '@server/helpers/logger'
 
 const overviewsRouter = express.Router()
 
 overviewsRouter.get('/videos',
-  asyncMiddleware(cacheRoute()(ROUTE_CACHE_LIFETIME.OVERVIEWS.VIDEOS)),
+  videosOverviewValidator,
+  optionalAuthenticate,
   asyncMiddleware(getVideosOverview)
 )
 
@@ -28,17 +29,28 @@ const buildSamples = memoizee(async function () {
     TagModel.getRandomSamples(OVERVIEWS.VIDEOS.SAMPLE_THRESHOLD, OVERVIEWS.VIDEOS.SAMPLES_COUNT)
   ])
 
-  return { categories, channels, tags }
+  const result = { categories, channels, tags }
+
+  logger.debug('Building samples for overview endpoint.', { result })
+
+  return result
 }, { maxAge: MEMOIZE_TTL.OVERVIEWS_SAMPLE })
 
 // This endpoint could be quite long, but we cache it
 async function getVideosOverview (req: express.Request, res: express.Response) {
   const attributes = await buildSamples()
 
-  const [ categories, channels, tags ] = await Promise.all([
-    Promise.all(attributes.categories.map(c => getVideosByCategory(c, res))),
-    Promise.all(attributes.channels.map(c => getVideosByChannel(c, res))),
-    Promise.all(attributes.tags.map(t => getVideosByTag(t, res)))
+  const page = req.query.page || 1
+  const index = page - 1
+
+  const categories: CategoryOverview[] = []
+  const channels: ChannelOverview[] = []
+  const tags: TagOverview[] = []
+
+  await Promise.all([
+    getVideosByCategory(attributes.categories, index, res, categories),
+    getVideosByChannel(attributes.channels, index, res, channels),
+    getVideosByTag(attributes.tags, index, res, tags)
   ])
 
   const result: VideosOverview = {
@@ -47,45 +59,49 @@ async function getVideosOverview (req: express.Request, res: express.Response) {
     tags
   }
 
-  // Cleanup our object
-  for (const key of Object.keys(result)) {
-    result[key] = result[key].filter(v => v !== undefined)
-  }
-
   return res.json(result)
 }
 
-async function getVideosByTag (tag: string, res: express.Response) {
+async function getVideosByTag (tagsSample: string[], index: number, res: express.Response, acc: TagOverview[]) {
+  if (tagsSample.length <= index) return
+
+  const tag = tagsSample[index]
   const videos = await getVideos(res, { tagsOneOf: [ tag ] })
 
-  if (videos.length === 0) return undefined
+  if (videos.length === 0) return
 
-  return {
+  acc.push({
     tag,
     videos
-  }
+  })
 }
 
-async function getVideosByCategory (category: number, res: express.Response) {
+async function getVideosByCategory (categoriesSample: number[], index: number, res: express.Response, acc: CategoryOverview[]) {
+  if (categoriesSample.length <= index) return
+
+  const category = categoriesSample[index]
   const videos = await getVideos(res, { categoryOneOf: [ category ] })
 
-  if (videos.length === 0) return undefined
+  if (videos.length === 0) return
 
-  return {
+  acc.push({
     category: videos[0].category,
     videos
-  }
+  })
 }
 
-async function getVideosByChannel (channelId: number, res: express.Response) {
+async function getVideosByChannel (channelsSample: number[], index: number, res: express.Response, acc: ChannelOverview[]) {
+  if (channelsSample.length <= index) return
+
+  const channelId = channelsSample[index]
   const videos = await getVideos(res, { videoChannelId: channelId })
 
-  if (videos.length === 0) return undefined
+  if (videos.length === 0) return
 
-  return {
+  acc.push({
     channel: videos[0].channel,
     videos
-  }
+  })
 }
 
 async function getVideos (
diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts
index 8b040aa2c..13448ffed 100644
--- a/server/initializers/constants.ts
+++ b/server/initializers/constants.ts
@@ -90,9 +90,6 @@ const ROUTE_CACHE_LIFETIME = {
   SECURITYTXT: '2 hours',
   NODEINFO: '10 minutes',
   DNT_POLICY: '1 week',
-  OVERVIEWS: {
-    VIDEOS: '1 hour'
-  },
   ACTIVITY_PUB: {
     VIDEOS: '1 second' // 1 second, cache concurrent requests after a broadcast for example
   },
@@ -446,7 +443,7 @@ MIMETYPES.IMAGE.EXT_MIMETYPE = invert(MIMETYPES.IMAGE.MIMETYPE_EXT)
 const OVERVIEWS = {
   VIDEOS: {
     SAMPLE_THRESHOLD: 6,
-    SAMPLES_COUNT: 2
+    SAMPLES_COUNT: 20
   }
 }
 
@@ -687,8 +684,8 @@ if (isTestInstance() === true) {
   JOB_ATTEMPTS['email'] = 1
 
   FILES_CACHE.VIDEO_CAPTIONS.MAX_AGE = 3000
-  MEMOIZE_TTL.OVERVIEWS_SAMPLE = 1
-  ROUTE_CACHE_LIFETIME.OVERVIEWS.VIDEOS = '0ms'
+  MEMOIZE_TTL.OVERVIEWS_SAMPLE = 3000
+  OVERVIEWS.VIDEOS.SAMPLE_THRESHOLD = 2
 }
 
 updateWebserverUrls()
diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts
index 96e0d6600..3a7869354 100644
--- a/server/middlewares/validators/videos/videos.ts
+++ b/server/middlewares/validators/videos/videos.ts
@@ -29,7 +29,7 @@ import {
 } from '../../../helpers/custom-validators/videos'
 import { getDurationFromVideoFile } from '../../../helpers/ffmpeg-utils'
 import { logger } from '../../../helpers/logger'
-import { CONSTRAINTS_FIELDS } from '../../../initializers/constants'
+import { CONSTRAINTS_FIELDS, OVERVIEWS } from '../../../initializers/constants'
 import { authenticatePromiseIfNeeded } from '../../oauth'
 import { areValidationErrors } from '../utils'
 import { cleanUpReqFiles } from '../../../helpers/express-utils'
@@ -301,6 +301,19 @@ const videosAcceptChangeOwnershipValidator = [
   }
 ]
 
+const videosOverviewValidator = [
+  query('page')
+    .optional()
+    .isInt({ min: 1, max: OVERVIEWS.VIDEOS.SAMPLES_COUNT })
+    .withMessage('Should have a valid pagination'),
+
+  (req: express.Request, res: express.Response, next: express.NextFunction) => {
+    if (areValidationErrors(req, res)) return
+
+    return next()
+  }
+]
+
 function getCommonVideoEditAttributes () {
   return [
     body('thumbnailfile')
@@ -442,7 +455,9 @@ export {
 
   getCommonVideoEditAttributes,
 
-  commonVideosFiltersValidator
+  commonVideosFiltersValidator,
+
+  videosOverviewValidator
 }
 
 // ---------------------------------------------------------------------------
diff --git a/server/tests/api/check-params/index.ts b/server/tests/api/check-params/index.ts
index 924c0df76..ef152f55c 100644
--- a/server/tests/api/check-params/index.ts
+++ b/server/tests/api/check-params/index.ts
@@ -23,3 +23,4 @@ import './video-playlists'
 import './videos'
 import './videos-filter'
 import './videos-history'
+import './videos-overviews'
diff --git a/server/tests/api/check-params/videos-overviews.ts b/server/tests/api/check-params/videos-overviews.ts
new file mode 100644
index 000000000..69d7fc471
--- /dev/null
+++ b/server/tests/api/check-params/videos-overviews.ts
@@ -0,0 +1,33 @@
+/* eslint-disable @typescript-eslint/no-unused-expressions,@typescript-eslint/require-await */
+
+import 'mocha'
+import { cleanupTests, flushAndRunServer, ServerInfo } from '../../../../shared/extra-utils'
+import { getVideosOverview } from '@shared/extra-utils/overviews/overviews'
+
+describe('Test videos overview', function () {
+  let server: ServerInfo
+
+  // ---------------------------------------------------------------
+
+  before(async function () {
+    this.timeout(30000)
+
+    server = await flushAndRunServer(1)
+  })
+
+  describe('When getting videos overview', function () {
+
+    it('Should fail with a bad pagination', async function () {
+      await getVideosOverview(server.url, 0, 400)
+      await getVideosOverview(server.url, 100, 400)
+    })
+
+    it('Should succeed with a good pagination', async function () {
+      await getVideosOverview(server.url, 1)
+    })
+  })
+
+  after(async function () {
+    await cleanupTests([ server ])
+  })
+})
diff --git a/server/tests/api/videos/video-nsfw.ts b/server/tests/api/videos/video-nsfw.ts
index 7eba8d7d9..b16b484b9 100644
--- a/server/tests/api/videos/video-nsfw.ts
+++ b/server/tests/api/videos/video-nsfw.ts
@@ -19,12 +19,20 @@ import {
   updateCustomConfig,
   updateMyUser
 } from '../../../../shared/extra-utils'
-import { ServerConfig } from '../../../../shared/models'
+import { ServerConfig, VideosOverview } from '../../../../shared/models'
 import { CustomConfig } from '../../../../shared/models/server/custom-config.model'
 import { User } from '../../../../shared/models/users'
+import { getVideosOverview, getVideosOverviewWithToken } from '@shared/extra-utils/overviews/overviews'
 
 const expect = chai.expect
 
+function createOverviewRes (res: any) {
+  const overview = res.body as VideosOverview
+
+  const videos = overview.categories[0].videos
+  return { body: { data: videos, total: videos.length } }
+}
+
 describe('Test video NSFW policy', function () {
   let server: ServerInfo
   let userAccessToken: string
@@ -36,22 +44,38 @@ describe('Test video NSFW policy', function () {
         const user: User = res.body
         const videoChannelName = user.videoChannels[0].name
         const accountName = user.account.name + '@' + user.account.host
+        const hasQuery = Object.keys(query).length !== 0
+        let promises: Promise<any>[]
 
         if (token) {
-          return Promise.all([
+          promises = [
             getVideosListWithToken(server.url, token, query),
             searchVideoWithToken(server.url, 'n', token, query),
             getAccountVideos(server.url, token, accountName, 0, 5, undefined, query),
             getVideoChannelVideos(server.url, token, videoChannelName, 0, 5, undefined, query)
-          ])
+          ]
+
+          // Overviews do not support video filters
+          if (!hasQuery) {
+            promises.push(getVideosOverviewWithToken(server.url, 1, token).then(res => createOverviewRes(res)))
+          }
+
+          return Promise.all(promises)
         }
 
-        return Promise.all([
+        promises = [
           getVideosList(server.url),
           searchVideo(server.url, 'n'),
           getAccountVideos(server.url, undefined, accountName, 0, 5),
           getVideoChannelVideos(server.url, undefined, videoChannelName, 0, 5)
-        ])
+        ]
+
+        // Overviews do not support video filters
+        if (!hasQuery) {
+          promises.push(getVideosOverview(server.url, 1).then(res => createOverviewRes(res)))
+        }
+
+        return Promise.all(promises)
       })
   }
 
@@ -63,12 +87,12 @@ describe('Test video NSFW policy', function () {
     await setAccessTokensToServers([ server ])
 
     {
-      const attributes = { name: 'nsfw', nsfw: true }
+      const attributes = { name: 'nsfw', nsfw: true, category: 1 }
       await uploadVideo(server.url, server.accessToken, attributes)
     }
 
     {
-      const attributes = { name: 'normal', nsfw: false }
+      const attributes = { name: 'normal', nsfw: false, category: 1 }
       await uploadVideo(server.url, server.accessToken, attributes)
     }
 
diff --git a/server/tests/api/videos/videos-overview.ts b/server/tests/api/videos/videos-overview.ts
index ca08ab5b1..d38bcb6eb 100644
--- a/server/tests/api/videos/videos-overview.ts
+++ b/server/tests/api/videos/videos-overview.ts
@@ -2,7 +2,7 @@
 
 import * as chai from 'chai'
 import 'mocha'
-import { cleanupTests, flushAndRunServer, ServerInfo, setAccessTokensToServers, uploadVideo } from '../../../../shared/extra-utils'
+import { cleanupTests, flushAndRunServer, ServerInfo, setAccessTokensToServers, uploadVideo, wait } from '../../../../shared/extra-utils'
 import { getVideosOverview } from '../../../../shared/extra-utils/overviews/overviews'
 import { VideosOverview } from '../../../../shared/models/overviews'
 
@@ -20,7 +20,7 @@ describe('Test a videos overview', function () {
   })
 
   it('Should send empty overview', async function () {
-    const res = await getVideosOverview(server.url)
+    const res = await getVideosOverview(server.url, 1)
 
     const overview: VideosOverview = res.body
     expect(overview.tags).to.have.lengthOf(0)
@@ -31,15 +31,15 @@ describe('Test a videos overview', function () {
   it('Should upload 5 videos in a specific category, tag and channel but not include them in overview', async function () {
     this.timeout(15000)
 
-    for (let i = 0; i < 5; i++) {
-      await uploadVideo(server.url, server.accessToken, {
-        name: 'video ' + i,
-        category: 3,
-        tags: [ 'coucou1', 'coucou2' ]
-      })
-    }
+    await wait(3000)
+
+    await uploadVideo(server.url, server.accessToken, {
+      name: 'video 0',
+      category: 3,
+      tags: [ 'coucou1', 'coucou2' ]
+    })
 
-    const res = await getVideosOverview(server.url)
+    const res = await getVideosOverview(server.url, 1)
 
     const overview: VideosOverview = res.body
     expect(overview.tags).to.have.lengthOf(0)
@@ -48,27 +48,55 @@ describe('Test a videos overview', function () {
   })
 
   it('Should upload another video and include all videos in the overview', async function () {
-    await uploadVideo(server.url, server.accessToken, {
-      name: 'video 5',
-      category: 3,
-      tags: [ 'coucou1', 'coucou2' ]
-    })
+    this.timeout(15000)
 
-    const res = await getVideosOverview(server.url)
+    for (let i = 1; i < 6; i++) {
+      await uploadVideo(server.url, server.accessToken, {
+        name: 'video ' + i,
+        category: 3,
+        tags: [ 'coucou1', 'coucou2' ]
+      })
+    }
 
-    const overview: VideosOverview = res.body
-    expect(overview.tags).to.have.lengthOf(2)
-    expect(overview.categories).to.have.lengthOf(1)
-    expect(overview.channels).to.have.lengthOf(1)
+    await wait(3000)
+
+    {
+      const res = await getVideosOverview(server.url, 1)
+
+      const overview: VideosOverview = res.body
+      expect(overview.tags).to.have.lengthOf(1)
+      expect(overview.categories).to.have.lengthOf(1)
+      expect(overview.channels).to.have.lengthOf(1)
+    }
+
+    {
+      const res = await getVideosOverview(server.url, 2)
+
+      const overview: VideosOverview = res.body
+      expect(overview.tags).to.have.lengthOf(1)
+      expect(overview.categories).to.have.lengthOf(0)
+      expect(overview.channels).to.have.lengthOf(0)
+    }
   })
 
   it('Should have the correct overview', async function () {
-    const res = await getVideosOverview(server.url)
+    const res1 = await getVideosOverview(server.url, 1)
+    const res2 = await getVideosOverview(server.url, 2)
 
-    const overview: VideosOverview = res.body
+    const overview1: VideosOverview = res1.body
+    const overview2: VideosOverview = res2.body
+
+    const tmp = [
+      overview1.tags,
+      overview1.categories,
+      overview1.channels,
+      overview2.tags
+    ]
+
+    for (const arr of tmp) {
+      expect(arr).to.have.lengthOf(1)
 
-    for (const attr of [ 'tags', 'categories', 'channels' ]) {
-      const obj = overview[attr][0]
+      const obj = arr[0]
 
       expect(obj.videos).to.have.lengthOf(6)
       expect(obj.videos[0].name).to.equal('video 5')
@@ -79,12 +107,13 @@ describe('Test a videos overview', function () {
       expect(obj.videos[5].name).to.equal('video 0')
     }
 
-    expect(overview.tags.find(t => t.tag === 'coucou1')).to.not.be.undefined
-    expect(overview.tags.find(t => t.tag === 'coucou2')).to.not.be.undefined
+    const tags = [ overview1.tags[0].tag, overview2.tags[0].tag ]
+    expect(tags.find(t => t === 'coucou1')).to.not.be.undefined
+    expect(tags.find(t => t === 'coucou2')).to.not.be.undefined
 
-    expect(overview.categories[0].category.id).to.equal(3)
+    expect(overview1.categories[0].category.id).to.equal(3)
 
-    expect(overview.channels[0].channel.name).to.equal('root_channel')
+    expect(overview1.channels[0].channel.name).to.equal('root_channel')
   })
 
   after(async function () {