Better view counter

4 files changed, 48 insertions, 28 deletions

diff --git a/server/controllers/api/videos/index.ts b/server/controllers/api/videos/index.ts
index 244d91914..e2798830e 100644
--- a/server/controllers/api/videos/index.ts
+++ b/server/controllers/api/videos/index.ts
@@ -104,6 +104,10 @@ videosRouter.get('/:id',
   asyncMiddleware(videosGetValidator),
   getVideo
 )
+videosRouter.post('/:id/views',
+  asyncMiddleware(videosGetValidator),
+  asyncMiddleware(viewVideo)
+)
 
 videosRouter.delete('/:id',
   authenticate,
@@ -311,25 +315,25 @@ async function updateVideo (req: express.Request, res: express.Response) {
   }
 }
 
-async function getVideo (req: express.Request, res: express.Response) {
+function getVideo (req: express.Request, res: express.Response) {
+  const videoInstance = res.locals.video
+
+  return res.json(videoInstance.toFormattedDetailsJSON())
+}
+
+async function viewVideo (req: express.Request, res: express.Response) {
   const videoInstance = res.locals.video
 
-  const baseIncrementPromise = videoInstance.increment('views')
-    .then(() => getServerAccount())
+  await videoInstance.increment('views')
+  const serverAccount = await getServerAccount()
 
   if (videoInstance.isOwned()) {
-    // The increment is done directly in the database, not using the instance value
-    baseIncrementPromise
-      .then(serverAccount => sendCreateViewToVideoFollowers(serverAccount, videoInstance, undefined))
-      .catch(err => logger.error('Cannot add view to video/send view to followers for %s.', videoInstance.uuid, err))
+    await sendCreateViewToVideoFollowers(serverAccount, videoInstance, undefined)
   } else {
-    baseIncrementPromise
-      .then(serverAccount => sendCreateViewToOrigin(serverAccount, videoInstance, undefined))
-      .catch(err => logger.error('Cannot send view to origin server for %s.', videoInstance.uuid, err))
+    await sendCreateViewToOrigin(serverAccount, videoInstance, undefined)
   }
 
-  // Do not wait the view system
-  return res.json(videoInstance.toFormattedDetailsJSON())
+  return res.status(204).end()
 }
 
 async function getVideoDescription (req: express.Request, res: express.Response) {
diff --git a/server/tests/api/multiple-servers.ts b/server/tests/api/multiple-servers.ts
index 052b0231f..c80ded862 100644
--- a/server/tests/api/multiple-servers.ts
+++ b/server/tests/api/multiple-servers.ts
@@ -25,6 +25,7 @@ import {
   doubleFollow
 } from '../utils'
 import { createUser } from '../utils/users'
+import { viewVideo } from '../utils/videos'
 
 const expect = chai.expect
 
@@ -486,10 +487,10 @@ describe('Test multiple servers', function () {
       this.timeout(10000)
 
       const tasks: Promise<any>[] = []
-      tasks.push(getVideo(servers[2].url, localVideosServer3[0]))
-      tasks.push(getVideo(servers[2].url, localVideosServer3[0]))
-      tasks.push(getVideo(servers[2].url, localVideosServer3[0]))
-      tasks.push(getVideo(servers[2].url, localVideosServer3[1]))
+      tasks.push(viewVideo(servers[2].url, localVideosServer3[0]))
+      tasks.push(viewVideo(servers[2].url, localVideosServer3[0]))
+      tasks.push(viewVideo(servers[2].url, localVideosServer3[0]))
+      tasks.push(viewVideo(servers[2].url, localVideosServer3[1]))
 
       await Promise.all(tasks)
 
@@ -502,8 +503,8 @@ describe('Test multiple servers', function () {
         const video0 = videos.find(v => v.uuid === localVideosServer3[0])
         const video1 = videos.find(v => v.uuid === localVideosServer3[1])
 
-        expect(video0.views).to.equal(7)
-        expect(video1.views).to.equal(5)
+        expect(video0.views).to.equal(3)
+        expect(video1.views).to.equal(1)
       }
     })
 
@@ -511,16 +512,16 @@ describe('Test multiple servers', function () {
       this.timeout(15000)
 
       const tasks: Promise<any>[] = []
-      tasks.push(getVideo(servers[0].url, remoteVideosServer1[0]))
-      tasks.push(getVideo(servers[1].url, remoteVideosServer2[0]))
-      tasks.push(getVideo(servers[1].url, remoteVideosServer2[0]))
-      tasks.push(getVideo(servers[2].url, remoteVideosServer3[0]))
-      tasks.push(getVideo(servers[2].url, remoteVideosServer3[1]))
-      tasks.push(getVideo(servers[2].url, remoteVideosServer3[1]))
-      tasks.push(getVideo(servers[2].url, remoteVideosServer3[1]))
-      tasks.push(getVideo(servers[2].url, localVideosServer3[1]))
-      tasks.push(getVideo(servers[2].url, localVideosServer3[1]))
-      tasks.push(getVideo(servers[2].url, localVideosServer3[1]))
+      tasks.push(viewVideo(servers[0].url, remoteVideosServer1[0]))
+      tasks.push(viewVideo(servers[1].url, remoteVideosServer2[0]))
+      tasks.push(viewVideo(servers[1].url, remoteVideosServer2[0]))
+      tasks.push(viewVideo(servers[2].url, remoteVideosServer3[0]))
+      tasks.push(viewVideo(servers[2].url, remoteVideosServer3[1]))
+      tasks.push(viewVideo(servers[2].url, remoteVideosServer3[1]))
+      tasks.push(viewVideo(servers[2].url, remoteVideosServer3[1]))
+      tasks.push(viewVideo(servers[2].url, localVideosServer3[1]))
+      tasks.push(viewVideo(servers[2].url, localVideosServer3[1]))
+      tasks.push(viewVideo(servers[2].url, localVideosServer3[1]))
 
       await Promise.all(tasks)
 
diff --git a/server/tests/api/single-server.ts b/server/tests/api/single-server.ts
index 40e2c64fe..041d13225 100644
--- a/server/tests/api/single-server.ts
+++ b/server/tests/api/single-server.ts
@@ -33,6 +33,7 @@ import {
   searchVideoWithSort,
   updateVideo
 } from '../utils'
+import { viewVideo } from '../utils/videos'
 
 describe('Test a single server', function () {
   let server: ServerInfo = null
@@ -214,6 +215,10 @@ describe('Test a single server', function () {
   })
 
   it('Should have the views updated', async function () {
+    await viewVideo(server.url, videoId)
+    await viewVideo(server.url, videoId)
+    await viewVideo(server.url, videoId)
+
     const res = await getVideo(server.url, videoId)
 
     const video = res.body
diff --git a/server/tests/utils/videos.ts b/server/tests/utils/videos.ts
index dababe924..73a9f1a0a 100644
--- a/server/tests/utils/videos.ts
+++ b/server/tests/utils/videos.ts
@@ -55,6 +55,15 @@ function getVideo (url: string, id: number | string, expectedStatus = 200) {
           .expect(expectedStatus)
 }
 
+function viewVideo (url: string, id: number | string, expectedStatus = 204) {
+  const path = '/api/v1/videos/' + id + '/views'
+
+  return request(url)
+    .post(path)
+    .set('Accept', 'application/json')
+    .expect(expectedStatus)
+}
+
 function getVideoWithToken (url: string, token: string, id: number | string, expectedStatus = 200) {
   const path = '/api/v1/videos/' + id
 
@@ -313,5 +322,6 @@ export {
   uploadVideo,
   updateVideo,
   rateVideo,
+  viewVideo,
   parseTorrentVideo
 }