From 80109b2ddb14ec4a54cede7885611cb9244da3cb Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 18 Apr 2018 10:20:13 +0200 Subject: [PATCH 1/5] Handle when autoplay fails --- .../+video-watch/video-watch.component.scss | 2 +- client/src/assets/player/peertube-player.ts | 3 +- .../assets/player/peertube-videojs-plugin.ts | 35 +++++++++++++------ .../assets/player/peertube-videojs-typings.ts | 1 + 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/client/src/app/videos/+video-watch/video-watch.component.scss b/client/src/app/videos/+video-watch/video-watch.component.scss index 3ebeccd4b..8bc9f8d9a 100644 --- a/client/src/app/videos/+video-watch/video-watch.component.scss +++ b/client/src/app/videos/+video-watch/video-watch.component.scss @@ -332,7 +332,7 @@ .video-actions-rates { margin-top: 20px; - align-items: left; + align-items: start; .video-info-likes-dislikes-bar { margin-top: 10px; diff --git a/client/src/assets/player/peertube-player.ts b/client/src/assets/player/peertube-player.ts index e8a258065..f02fe5d75 100644 --- a/client/src/assets/player/peertube-player.ts +++ b/client/src/assets/player/peertube-player.ts @@ -27,11 +27,12 @@ function getVideojsOptions (options: { const videojsOptions = { controls: true, poster: options.poster, - autoplay: options.autoplay, + autoplay: false, inactivityTimeout: options.inactivityTimeout, playbackRates: [ 0.5, 1, 1.5, 2 ], plugins: { peertube: { + autoplay: options.autoplay, // Use peertube plugin autoplay because we get the file by webtorrent videoFiles: options.videoFiles, playerElement: options.playerElement, videoViewUrl: options.videoViewUrl, diff --git a/client/src/assets/player/peertube-videojs-plugin.ts b/client/src/assets/player/peertube-videojs-plugin.ts index 60c291a50..290d88724 100644 --- a/client/src/assets/player/peertube-videojs-plugin.ts +++ b/client/src/assets/player/peertube-videojs-plugin.ts @@ -68,9 +68,7 @@ class PeerTubePlugin extends Plugin { constructor (player: videojs.Player, options: PeertubePluginOptions) { super(player, options) - // Fix canplay event on google chrome by disabling default videojs autoplay - this.autoplay = this.player.options_.autoplay - this.player.options_.autoplay = false + this.autoplay = options.autoplay this.startTime = options.startTime this.videoFiles = options.videoFiles @@ -190,12 +188,7 @@ class PeerTubePlugin extends Plugin { if (err) return this.fallbackToHttp(done) - if (!this.player.paused()) { - const playPromise = this.player.play() - if (playPromise !== undefined) return playPromise.then(done) - - return done() - } + if (!this.player.paused()) return this.tryToPlay(done) return done() }) @@ -264,6 +257,25 @@ class PeerTubePlugin extends Plugin { this.trigger('autoResolutionUpdate') } + private tryToPlay (done?: Function) { + if (!done) done = function () { /* empty */ } + + const playPromise = this.player.play() + if (playPromise !== undefined) { + return playPromise.then(done) + .catch(err => { + console.error(err) + this.player.pause() + this.player.posterImage.show() + this.player.removeClass('vjs-has-autoplay') + + return done() + }) + } + + return done() + } + private seek (time: number) { this.player.currentTime(time) this.player.handleTechSeeked_() @@ -317,7 +329,10 @@ class PeerTubePlugin extends Plugin { if (this.autoplay === true) { this.player.posterImage.hide() - this.updateVideoFile(undefined, 0, () => this.seek(this.startTime)) + this.updateVideoFile(undefined, 0, () => { + this.seek(this.startTime) + this.tryToPlay() + }) } else { // Proxy first play const oldPlay = this.player.play.bind(this.player) diff --git a/client/src/assets/player/peertube-videojs-typings.ts b/client/src/assets/player/peertube-videojs-typings.ts index a66caa30b..abdf333e1 100644 --- a/client/src/assets/player/peertube-videojs-typings.ts +++ b/client/src/assets/player/peertube-videojs-typings.ts @@ -22,6 +22,7 @@ type PeertubePluginOptions = { videoViewUrl: string videoDuration: number startTime: number + autoplay: boolean } // videojs typings don't have some method we need From c9ffd53217e69395c303ba36532a6eef39645e4e Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 18 Apr 2018 15:00:42 +0200 Subject: [PATCH 2/5] Better responsive design on many comment children --- .../comment/video-comment.component.scss | 27 ++++++++++++++++++- .../comment/video-comments.component.scss | 10 +++---- .../+video-watch/video-watch.component.scss | 3 ++- 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/client/src/app/videos/+video-watch/comment/video-comment.component.scss b/client/src/app/videos/+video-watch/comment/video-comment.component.scss index 3b0b7eafd..3a3f32b83 100644 --- a/client/src/app/videos/+video-watch/comment/video-comment.component.scss +++ b/client/src/app/videos/+video-watch/comment/video-comment.component.scss @@ -14,6 +14,8 @@ .comment { flex-grow: 1; + // Fix word-wrap with flex + min-width: 1px; .highlighted-comment { display: inline-block; @@ -44,8 +46,8 @@ } .comment-html { - word-wrap: initial; word-break: normal; + word-wrap: break-word; text-align: justify; /deep/ a { @@ -76,3 +78,26 @@ } } } + +// Decrease the space of child comments on small screens +@media screen and (max-width: 1600px) { + .children { + margin-left: -20px; + } +} + +@media screen and (max-width: 1200px) { + .children { + margin-left: -30px; + } +} + +@media screen and (max-width: 600px) { + .children { + margin-left: -40px; + } + + .root-comment { + img { margin-right: 10px; } + } +} \ No newline at end of file diff --git a/client/src/app/videos/+video-watch/comment/video-comments.component.scss b/client/src/app/videos/+video-watch/comment/video-comments.component.scss index 7aadc2866..0b8aa1854 100644 --- a/client/src/app/videos/+video-watch/comment/video-comments.component.scss +++ b/client/src/app/videos/+video-watch/comment/video-comments.component.scss @@ -19,8 +19,8 @@ font-size: 13px; } -.comment-html { - word-wrap: normal; - word-break: normal; - text-align: justify; -} +@media screen and (max-width: 600px) { + .view-replies { + margin-left: 46px; + } +} \ No newline at end of file diff --git a/client/src/app/videos/+video-watch/video-watch.component.scss b/client/src/app/videos/+video-watch/video-watch.component.scss index 8bc9f8d9a..fe3cc4ded 100644 --- a/client/src/app/videos/+video-watch/video-watch.component.scss +++ b/client/src/app/videos/+video-watch/video-watch.component.scss @@ -215,7 +215,8 @@ font-size: 15px; .video-info-description-html { - word-wrap: break-word; + word-break: normal; + word-wrap: normal; text-align: justify; } From bf6e8e3e3dd6516a6125c9c2c12587e7bac8c107 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 18 Apr 2018 15:27:33 +0200 Subject: [PATCH 3/5] Fix tests --- server/tests/utils/server/servers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/tests/utils/server/servers.ts b/server/tests/utils/server/servers.ts index 8373c73ab..1372c03c3 100644 --- a/server/tests/utils/server/servers.ts +++ b/server/tests/utils/server/servers.ts @@ -88,7 +88,7 @@ function runServer (serverNumber: number, configOverride?: Object) { // These actions are async so we need to be sure that they have both been done const serverRunString = { - 'Server listening on port': false + 'Server listening': false } const key = 'Database peertube_test' + serverNumber + ' is ready' serverRunString[key] = false From 5350fd8e5b2b2d017b16d97828893a8a4a40bd89 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 18 Apr 2018 15:32:40 +0200 Subject: [PATCH 4/5] Move server follow in the job queue It helps to track follow errors --- server/controllers/api/server/follows.ts | 78 ++++--------------- server/initializers/constants.ts | 2 + .../job-queue/handlers/activitypub-follow.ts | 68 ++++++++++++++++ server/lib/job-queue/job-queue.ts | 5 +- shared/models/server/job.model.ts | 1 + 5 files changed, 90 insertions(+), 64 deletions(-) create mode 100644 server/lib/job-queue/handlers/activitypub-follow.ts diff --git a/server/controllers/api/server/follows.ts b/server/controllers/api/server/follows.ts index bb0063473..e78361c9a 100644 --- a/server/controllers/api/server/follows.ts +++ b/server/controllers/api/server/follows.ts @@ -1,20 +1,22 @@ import * as express from 'express' import { UserRight } from '../../../../shared/models/users' -import { sanitizeHost } from '../../../helpers/core-utils' -import { retryTransactionWrapper } from '../../../helpers/database-utils' import { logger } from '../../../helpers/logger' import { getFormattedObjects, getServerActor } from '../../../helpers/utils' -import { loadActorUrlOrGetFromWebfinger } from '../../../helpers/webfinger' -import { REMOTE_SCHEME, sequelizeTypescript, SERVER_ACTOR_NAME } from '../../../initializers' -import { getOrCreateActorAndServerAndModel } from '../../../lib/activitypub/actor' -import { sendFollow, sendUndoFollow } from '../../../lib/activitypub/send' +import { sequelizeTypescript } from '../../../initializers' +import { sendUndoFollow } from '../../../lib/activitypub/send' import { - asyncMiddleware, authenticate, ensureUserHasRight, paginationValidator, removeFollowingValidator, setBodyHostsPort, setDefaultSort, - setDefaultPagination + asyncMiddleware, + authenticate, + ensureUserHasRight, + paginationValidator, + removeFollowingValidator, + setBodyHostsPort, + setDefaultPagination, + setDefaultSort } from '../../../middlewares' import { followersSortValidator, followingSortValidator, followValidator } from '../../../middlewares/validators' -import { ActorModel } from '../../../models/activitypub/actor' import { ActorFollowModel } from '../../../models/activitypub/actor-follow' +import { JobQueue } from '../../../lib/job-queue' const serverFollowsRouter = express.Router() serverFollowsRouter.get('/following', @@ -30,7 +32,7 @@ serverFollowsRouter.post('/following', ensureUserHasRight(UserRight.MANAGE_SERVER_FOLLOW), followValidator, setBodyHostsPort, - asyncMiddleware(followRetry) + asyncMiddleware(followInstance) ) serverFollowsRouter.delete('/following/:host', @@ -70,67 +72,17 @@ async function listFollowers (req: express.Request, res: express.Response, next: return res.json(getFormattedObjects(resultList.data, resultList.total)) } -async function followRetry (req: express.Request, res: express.Response, next: express.NextFunction) { +async function followInstance (req: express.Request, res: express.Response, next: express.NextFunction) { const hosts = req.body.hosts as string[] - const fromActor = await getServerActor() - - const tasks: Promise[] = [] - const actorName = SERVER_ACTOR_NAME for (const host of hosts) { - const sanitizedHost = sanitizeHost(host, REMOTE_SCHEME.HTTP) - - // We process each host in a specific transaction - // First, we add the follow request in the database - // Then we send the follow request to other actor - const p = loadActorUrlOrGetFromWebfinger(actorName, sanitizedHost) - .then(actorUrl => getOrCreateActorAndServerAndModel(actorUrl)) - .then(targetActor => { - const options = { - arguments: [ fromActor, targetActor ], - errorMessage: 'Cannot follow with many retries.' - } - - return retryTransactionWrapper(follow, options) - }) - .catch(err => logger.warn('Cannot follow server %s.', sanitizedHost, { err })) - - tasks.push(p) + JobQueue.Instance.createJob({ type: 'activitypub-follow', payload: { host } }) + .catch(err => logger.error('Cannot create follow job for %s.', host, err)) } - // Don't make the client wait the tasks - Promise.all(tasks) - .catch(err => logger.error('Error in follow.', { err })) - return res.status(204).end() } -function follow (fromActor: ActorModel, targetActor: ActorModel) { - if (fromActor.id === targetActor.id) { - throw new Error('Follower is the same than target actor.') - } - - return sequelizeTypescript.transaction(async t => { - const [ actorFollow ] = await ActorFollowModel.findOrCreate({ - where: { - actorId: fromActor.id, - targetActorId: targetActor.id - }, - defaults: { - state: 'pending', - actorId: fromActor.id, - targetActorId: targetActor.id - }, - transaction: t - }) - actorFollow.ActorFollowing = targetActor - actorFollow.ActorFollower = fromActor - - // Send a notification to remote server - await sendFollow(actorFollow) - }) -} - async function removeFollow (req: express.Request, res: express.Response, next: express.NextFunction) { const follow: ActorFollowModel = res.locals.follow diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts index 9fde989c5..ffcbe69b8 100644 --- a/server/initializers/constants.ts +++ b/server/initializers/constants.ts @@ -65,6 +65,7 @@ const JOB_ATTEMPTS: { [ id in JobType ]: number } = { 'activitypub-http-broadcast': 5, 'activitypub-http-unicast': 5, 'activitypub-http-fetcher': 5, + 'activitypub-follow': 5, 'video-file': 1, 'email': 5 } @@ -72,6 +73,7 @@ const JOB_CONCURRENCY: { [ id in JobType ]: number } = { 'activitypub-http-broadcast': 1, 'activitypub-http-unicast': 5, 'activitypub-http-fetcher': 1, + 'activitypub-follow': 3, 'video-file': 1, 'email': 5 } diff --git a/server/lib/job-queue/handlers/activitypub-follow.ts b/server/lib/job-queue/handlers/activitypub-follow.ts new file mode 100644 index 000000000..6764a4037 --- /dev/null +++ b/server/lib/job-queue/handlers/activitypub-follow.ts @@ -0,0 +1,68 @@ +import * as kue from 'kue' +import { logger } from '../../../helpers/logger' +import { getServerActor } from '../../../helpers/utils' +import { REMOTE_SCHEME, sequelizeTypescript, SERVER_ACTOR_NAME } from '../../../initializers' +import { sendFollow } from '../../activitypub/send' +import { sanitizeHost } from '../../../helpers/core-utils' +import { loadActorUrlOrGetFromWebfinger } from '../../../helpers/webfinger' +import { getOrCreateActorAndServerAndModel } from '../../activitypub/actor' +import { retryTransactionWrapper } from '../../../helpers/database-utils' +import { ActorFollowModel } from '../../../models/activitypub/actor-follow' +import { ActorModel } from '../../../models/activitypub/actor' + +export type ActivitypubFollowPayload = { + host: string +} + +async function processActivityPubFollow (job: kue.Job) { + const payload = job.data as ActivitypubFollowPayload + const host = payload.host + + logger.info('Processing ActivityPub follow in job %d.', job.id) + + const sanitizedHost = sanitizeHost(host, REMOTE_SCHEME.HTTP) + + const actorUrl = await loadActorUrlOrGetFromWebfinger(SERVER_ACTOR_NAME, sanitizedHost) + const targetActor = await getOrCreateActorAndServerAndModel(actorUrl) + + const fromActor = await getServerActor() + const options = { + arguments: [ fromActor, targetActor ], + errorMessage: 'Cannot follow with many retries.' + } + + return retryTransactionWrapper(follow, options) +} +// --------------------------------------------------------------------------- + +export { + processActivityPubFollow +} + +// --------------------------------------------------------------------------- + +function follow (fromActor: ActorModel, targetActor: ActorModel) { + if (fromActor.id === targetActor.id) { + throw new Error('Follower is the same than target actor.') + } + + return sequelizeTypescript.transaction(async t => { + const [ actorFollow ] = await ActorFollowModel.findOrCreate({ + where: { + actorId: fromActor.id, + targetActorId: targetActor.id + }, + defaults: { + state: 'pending', + actorId: fromActor.id, + targetActorId: targetActor.id + }, + transaction: t + }) + actorFollow.ActorFollowing = targetActor + actorFollow.ActorFollower = fromActor + + // Send a notification to remote server + await sendFollow(actorFollow) + }) +} diff --git a/server/lib/job-queue/job-queue.ts b/server/lib/job-queue/job-queue.ts index 1dc28755e..bf40a9206 100644 --- a/server/lib/job-queue/job-queue.ts +++ b/server/lib/job-queue/job-queue.ts @@ -8,11 +8,13 @@ import { ActivitypubHttpFetcherPayload, processActivityPubHttpFetcher } from './ import { ActivitypubHttpUnicastPayload, processActivityPubHttpUnicast } from './handlers/activitypub-http-unicast' import { EmailPayload, processEmail } from './handlers/email' import { processVideoFile, VideoFilePayload } from './handlers/video-file' +import { ActivitypubFollowPayload, processActivityPubFollow } from './handlers/activitypub-follow' type CreateJobArgument = { type: 'activitypub-http-broadcast', payload: ActivitypubHttpBroadcastPayload } | { type: 'activitypub-http-unicast', payload: ActivitypubHttpUnicastPayload } | { type: 'activitypub-http-fetcher', payload: ActivitypubHttpFetcherPayload } | + { type: 'activitypub-follow', payload: ActivitypubFollowPayload } | { type: 'video-file', payload: VideoFilePayload } | { type: 'email', payload: EmailPayload } @@ -20,6 +22,7 @@ const handlers: { [ id in JobType ]: (job: kue.Job) => Promise} = { 'activitypub-http-broadcast': processActivityPubHttpBroadcast, 'activitypub-http-unicast': processActivityPubHttpUnicast, 'activitypub-http-fetcher': processActivityPubHttpFetcher, + 'activitypub-follow': processActivityPubFollow, 'video-file': processVideoFile, 'email': processEmail } @@ -50,7 +53,7 @@ class JobQueue { } }) - this.jobQueue.setMaxListeners(15) + this.jobQueue.setMaxListeners(20) this.jobQueue.on('error', err => { logger.error('Error in job queue.', { err }) diff --git a/shared/models/server/job.model.ts b/shared/models/server/job.model.ts index 5ebb75a5c..0fa36820e 100644 --- a/shared/models/server/job.model.ts +++ b/shared/models/server/job.model.ts @@ -3,6 +3,7 @@ export type JobState = 'active' | 'complete' | 'failed' | 'inactive' | 'delayed' export type JobType = 'activitypub-http-unicast' | 'activitypub-http-broadcast' | 'activitypub-http-fetcher' | + 'activitypub-follow' | 'video-file' | 'email' From f55e5a7bf81c2c27db1514273e3366511aabf4ae Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 18 Apr 2018 16:04:49 +0200 Subject: [PATCH 5/5] Process broadcast requests in parallel --- server.ts | 7 ++++--- server/initializers/constants.ts | 2 ++ .../handlers/activitypub-http-broadcast.ts | 17 +++++++---------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/server.ts b/server.ts index 5323bae2b..8024655a3 100644 --- a/server.ts +++ b/server.ts @@ -215,7 +215,8 @@ async function startApplication () { Redis.Instance.init() // Make server listening - server.listen(port, hostname) - logger.info('Server listening on %s:%d', hostname, port) - logger.info('Web server: %s', CONFIG.WEBSERVER.URL) + server.listen(port, hostname, () => { + logger.info('Server listening on %s:%d', hostname, port) + logger.info('Web server: %s', CONFIG.WEBSERVER.URL) + }) } diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts index ffcbe69b8..5ee13389d 100644 --- a/server/initializers/constants.ts +++ b/server/initializers/constants.ts @@ -77,6 +77,7 @@ const JOB_CONCURRENCY: { [ id in JobType ]: number } = { 'video-file': 1, 'email': 5 } +const BROADCAST_CONCURRENCY = 5 // How many requests in parallel we do in activitypub-http-broadcast job // 2 days const JOB_COMPLETED_LIFETIME = 60000 * 60 * 24 * 2 @@ -463,6 +464,7 @@ export { LAST_MIGRATION_VERSION, OAUTH_LIFETIME, OPENGRAPH_AND_OEMBED_COMMENT, + BROADCAST_CONCURRENCY, PAGINATION_COUNT_DEFAULT, ACTOR_FOLLOW_SCORE, PREVIEWS_SIZE, diff --git a/server/lib/job-queue/handlers/activitypub-http-broadcast.ts b/server/lib/job-queue/handlers/activitypub-http-broadcast.ts index 78878fc01..38b8393f4 100644 --- a/server/lib/job-queue/handlers/activitypub-http-broadcast.ts +++ b/server/lib/job-queue/handlers/activitypub-http-broadcast.ts @@ -1,8 +1,10 @@ import * as kue from 'kue' +import * as Bluebird from 'bluebird' import { logger } from '../../../helpers/logger' import { doRequest } from '../../../helpers/requests' import { ActorFollowModel } from '../../../models/activitypub/actor-follow' import { buildSignedRequestOptions, computeBody } from './utils/activitypub-http-utils' +import { BROADCAST_CONCURRENCY } from '../../../initializers' export type ActivitypubHttpBroadcastPayload = { uris: string[] @@ -28,16 +30,11 @@ async function processActivityPubHttpBroadcast (job: kue.Job) { const badUrls: string[] = [] const goodUrls: string[] = [] - for (const uri of payload.uris) { - options.uri = uri - - try { - await doRequest(options) - goodUrls.push(uri) - } catch (err) { - badUrls.push(uri) - } - } + await Bluebird.map(payload.uris, uri => { + return doRequest(Object.assign({}, options, { uri })) + .then(() => goodUrls.push(uri)) + .catch(() => badUrls.push(uri)) + }, { concurrency: BROADCAST_CONCURRENCY }) return ActorFollowModel.updateActorFollowsScore(goodUrls, badUrls, undefined) }