Better view counter

6 files changed, 75 insertions, 38 deletions

diff --git a/client/src/app/videos/+video-watch/video-watch.component.ts b/client/src/app/videos/+video-watch/video-watch.component.ts
index 2a7290cbd..b26f3092f 100644
--- a/client/src/app/videos/+video-watch/video-watch.component.ts
+++ b/client/src/app/videos/+video-watch/video-watch.component.ts
@@ -1,22 +1,18 @@
 import { Component, ElementRef, OnDestroy, OnInit, ViewChild } from '@angular/core'
 import { ActivatedRoute, Router } from '@angular/router'
+import { MetaService } from '@ngx-meta/core'
+import { NotificationsService } from 'angular2-notifications'
 import { Observable } from 'rxjs/Observable'
 import { Subscription } from 'rxjs/Subscription'
-
 import videojs from 'video.js'
+import { UserVideoRateType, VideoRateType } from '../../../../../shared'
 import '../../../assets/player/peertube-videojs-plugin'
-
-import { MetaService } from '@ngx-meta/core'
-import { NotificationsService } from 'angular2-notifications'
-
 import { AuthService, ConfirmService } from '../../core'
+import { VideoBlacklistService } from '../../shared'
+import { MarkdownService, VideoDetails, VideoService } from '../shared'
 import { VideoDownloadComponent } from './video-download.component'
-import { VideoShareComponent } from './video-share.component'
 import { VideoReportComponent } from './video-report.component'
-import { VideoDetails, VideoService, MarkdownService } from '../shared'
-import { VideoBlacklistService } from '../../shared'
-import { UserVideoRateType, VideoRateType } from '../../../../../shared'
-import { BehaviorSubject } from 'rxjs/BehaviorSubject'
+import { VideoShareComponent } from './video-share.component'
 
 @Component({
   selector: 'my-video-watch',
@@ -320,6 +316,8 @@ export class VideoWatchComponent implements OnInit, OnDestroy {
 
         this.setOpenGraphTags()
         this.checkUserRating()
+
+        this.prepareViewAdd()
       }
     )
   }
@@ -360,4 +358,17 @@ export class VideoWatchComponent implements OnInit, OnDestroy {
     this.metaService.setTag('og:url', window.location.href)
     this.metaService.setTag('url', window.location.href)
   }
+
+  private prepareViewAdd () {
+    // After 30 seconds (or 3/4 of the video), increment add a view
+    let viewTimeoutSeconds = 30
+    if (this.video.duration < viewTimeoutSeconds) viewTimeoutSeconds = (this.video.duration * 3) / 4
+
+    setTimeout(() => {
+      this.videoService
+        .viewVideo(this.video.uuid)
+        .subscribe()
+
+    }, viewTimeoutSeconds * 1000)
+  }
 }
diff --git a/client/src/app/videos/shared/video.service.ts b/client/src/app/videos/shared/video.service.ts
index b1ab5f8b9..5d25a26d4 100644
--- a/client/src/app/videos/shared/video.service.ts
+++ b/client/src/app/videos/shared/video.service.ts
@@ -41,6 +41,12 @@ export class VideoService {
                         .catch((res) => this.restExtractor.handleError(res))
   }
 
+  viewVideo (uuid: string): Observable<VideoDetails> {
+    return this.authHttp.post(VideoService.BASE_VIDEO_URL + uuid + '/views', {})
+      .map(this.restExtractor.extractDataBool)
+      .catch(this.restExtractor.handleError)
+  }
+
   updateVideo (video: VideoEdit) {
     const language = video.language ? video.language : null
 
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
 }