From b5206dfc455c119b0dcb897bd7144be6ea4d999d Mon Sep 17 00:00:00 2001 From: Julien Maulny Date: Tue, 3 Dec 2019 21:48:31 +0100 Subject: Fix retrieving of deleted comments when subscribing to a new instance --- .../activitypub/video-comments.ts | 19 ++++++++- server/lib/activitypub/video-comments.ts | 15 ++++---- server/models/video/video-comment.ts | 19 +++++---- server/tests/api/videos/multiple-servers.ts | 45 ++++++++++++++++++++++ .../models/activitypub/objects/common-objects.ts | 2 + 5 files changed, 84 insertions(+), 16 deletions(-) diff --git a/server/helpers/custom-validators/activitypub/video-comments.ts b/server/helpers/custom-validators/activitypub/video-comments.ts index e04c5388f..96655c3f8 100644 --- a/server/helpers/custom-validators/activitypub/video-comments.ts +++ b/server/helpers/custom-validators/activitypub/video-comments.ts @@ -3,11 +3,28 @@ import { ACTIVITY_PUB } from '../../../initializers/constants' import { exists, isArray, isDateValid } from '../misc' import { isActivityPubUrlValid } from './misc' +function isTypeValid (comment: any): boolean { + if (comment.type === 'Note') return true + + if (comment.type === 'Tombstone' && comment.formerType === 'Note') return true + + return false +} + function sanitizeAndCheckVideoCommentObject (comment: any) { - if (!comment || comment.type !== 'Note') return false + if (!comment) return false + + if (!isTypeValid(comment)) return false normalizeComment(comment) + if (comment.type === 'Tombstone') { + return isActivityPubUrlValid(comment.id) && + isDateValid(comment.published) && + isDateValid(comment.deleted) && + isActivityPubUrlValid(comment.url) + } + return isActivityPubUrlValid(comment.id) && isCommentContentValid(comment.content) && isActivityPubUrlValid(comment.inReplyTo) && diff --git a/server/lib/activitypub/video-comments.ts b/server/lib/activitypub/video-comments.ts index 3e8306fa4..1a15842cf 100644 --- a/server/lib/activitypub/video-comments.ts +++ b/server/lib/activitypub/video-comments.ts @@ -131,9 +131,9 @@ async function resolveParentComment (params: ResolveThreadParams) { } const actorUrl = body.attributedTo - if (!actorUrl) throw new Error('Miss attributed to in comment') + if (!actorUrl && body.type !== 'Tombstone') throw new Error('Miss attributed to in comment') - if (checkUrlsSameHost(url, actorUrl) !== true) { + if (actorUrl && checkUrlsSameHost(url, actorUrl) !== true) { throw new Error(`Actor url ${actorUrl} has not the same host than the comment url ${url}`) } @@ -141,18 +141,19 @@ async function resolveParentComment (params: ResolveThreadParams) { throw new Error(`Comment url ${url} host is different from the AP object id ${body.id}`) } - const actor = await getOrCreateActorAndServerAndModel(actorUrl, 'all') + const actor = actorUrl ? await getOrCreateActorAndServerAndModel(actorUrl, 'all') : null const comment = new VideoCommentModel({ url: body.id, - text: body.content, + text: body.content ? body.content : '', videoId: null, - accountId: actor.Account.id, + accountId: actor ? actor.Account.id : null, inReplyToCommentId: null, originCommentId: null, createdAt: new Date(body.published), - updatedAt: new Date(body.updated) + updatedAt: new Date(body.updated), + deletedAt: body.deleted ? new Date(body.deleted) : null }) as MCommentOwner - comment.Account = actor.Account + comment.Account = actor ? actor.Account : null return resolveThread({ url: body.inReplyTo, diff --git a/server/models/video/video-comment.ts b/server/models/video/video-comment.ts index b44d65138..869a42afe 100644 --- a/server/models/video/video-comment.ts +++ b/server/models/video/video-comment.ts @@ -507,27 +507,30 @@ export class VideoCommentModel extends Model { } toActivityPubObject (this: MCommentAP, threadParentComments: MCommentOwner[]): VideoCommentObject | ActivityTombstoneObject { + let inReplyTo: string + // New thread, so in AS we reply to the video + if (this.inReplyToCommentId === null) { + inReplyTo = this.Video.url + } else { + inReplyTo = this.InReplyToVideoComment.url + } + if (this.isDeleted()) { return { id: this.url, type: 'Tombstone', formerType: 'Note', + inReplyTo, published: this.createdAt.toISOString(), updated: this.updatedAt.toISOString(), deleted: this.deletedAt.toISOString() } } - let inReplyTo: string - // New thread, so in AS we reply to the video - if (this.inReplyToCommentId === null) { - inReplyTo = this.Video.url - } else { - inReplyTo = this.InReplyToVideoComment.url - } - const tag: ActivityTagObject[] = [] for (const parentComment of threadParentComments) { + if (!parentComment.Account) continue + const actor = parentComment.Account.Actor tag.push({ diff --git a/server/tests/api/videos/multiple-servers.ts b/server/tests/api/videos/multiple-servers.ts index e7b57ba1f..836bdc658 100644 --- a/server/tests/api/videos/multiple-servers.ts +++ b/server/tests/api/videos/multiple-servers.ts @@ -15,6 +15,7 @@ import { createUser, dateIsValid, doubleFollow, + flushAndRunServer, flushAndRunMultipleServers, getLocalVideos, getVideo, @@ -938,7 +939,51 @@ describe('Test multiple servers', function () { } }) + it('Should retrieve all comments when subscribing to a new server', async function () { + this.timeout(120000) + + const newServer = await flushAndRunServer(4) + + await setAccessTokensToServers([newServer]) + await doubleFollow(newServer, servers[0]) + await doubleFollow(newServer, servers[2]) + await waitJobs([newServer, ...servers]) + + const res = await getVideoCommentThreads(newServer.url, videoUUID, 0, 5) + + expect(res.body.total).to.equal(2) + expect(res.body.data).to.be.an('array') + expect(res.body.data).to.have.lengthOf(2) + + { + const comment: VideoComment = res.body.data[0] + expect(comment).to.not.be.undefined + expect(comment.inReplyToCommentId).to.be.null + expect(comment.account.name).to.equal('root') + expect(comment.account.host).to.equal('localhost:' + servers[2].port) + expect(comment.totalReplies).to.equal(0) + expect(dateIsValid(comment.createdAt as string)).to.be.true + expect(dateIsValid(comment.updatedAt as string)).to.be.true + } + + { + const deletedComment: VideoComment = res.body.data[1] + expect(deletedComment).to.not.be.undefined + expect(deletedComment.isDeleted).to.be.true + expect(deletedComment.deletedAt).to.not.be.null + expect(deletedComment.text).to.equal('') + expect(deletedComment.inReplyToCommentId).to.be.null + expect(deletedComment.account).to.be.null + expect(deletedComment.totalReplies).to.equal(3) + expect(dateIsValid(deletedComment.createdAt as string)).to.be.true + expect(dateIsValid(deletedComment.updatedAt as string)).to.be.true + expect(dateIsValid(deletedComment.deletedAt as string)).to.be.true + } + }) + it('Should delete a remote thread by the origin server', async function () { + this.timeout(5000) + const res = await getVideoCommentThreads(servers[ 0 ].url, videoUUID, 0, 5) const threadId = res.body.data.find(c => c.text === 'my super second comment').id await deleteVideoComment(servers[ 0 ].url, servers[ 0 ].accessToken, videoUUID, threadId) diff --git a/shared/models/activitypub/objects/common-objects.ts b/shared/models/activitypub/objects/common-objects.ts index df287d570..de1116ab3 100644 --- a/shared/models/activitypub/objects/common-objects.ts +++ b/shared/models/activitypub/objects/common-objects.ts @@ -93,9 +93,11 @@ export interface ActivityPubAttributedTo { export interface ActivityTombstoneObject { '@context'?: any id: string + url?: string type: 'Tombstone' name?: string formerType?: string + inReplyTo?: string published: string updated: string deleted: string -- cgit v1.2.3