From 1f3e9feca2caf68024168b0ea9ed39d8438fa235 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 30 Nov 2017 09:21:11 +0100 Subject: [PATCH] Better view counter --- .../+video-watch/video-watch.component.ts | 39 ++++++++++++------- client/src/app/videos/shared/video.service.ts | 6 +++ server/controllers/api/videos/index.ts | 28 +++++++------ server/tests/api/multiple-servers.ts | 33 ++++++++-------- server/tests/api/single-server.ts | 5 +++ server/tests/utils/videos.ts | 10 +++++ 6 files changed, 79 insertions(+), 42 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 { Observable } from 'rxjs/Observable' -import { Subscription } from 'rxjs/Subscription' - -import videojs from 'video.js' -import '../../../assets/player/peertube-videojs-plugin' - import { MetaService } from '@ngx-meta/core' import { NotificationsService } from 'angular2-notifications' - -import { AuthService, ConfirmService } from '../../core' -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 { Observable } from 'rxjs/Observable' +import { Subscription } from 'rxjs/Subscription' +import videojs from 'video.js' import { UserVideoRateType, VideoRateType } from '../../../../../shared' -import { BehaviorSubject } from 'rxjs/BehaviorSubject' +import '../../../assets/player/peertube-videojs-plugin' +import { AuthService, ConfirmService } from '../../core' +import { VideoBlacklistService } from '../../shared' +import { MarkdownService, VideoDetails, VideoService } from '../shared' +import { VideoDownloadComponent } from './video-download.component' +import { VideoReportComponent } from './video-report.component' +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 { + 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 - const baseIncrementPromise = videoInstance.increment('views') - .then(() => getServerAccount()) + return res.json(videoInstance.toFormattedDetailsJSON()) +} + +async function viewVideo (req: express.Request, res: express.Response) { + const videoInstance = res.locals.video + + 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[] = [] - 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[] = [] - 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 }