From b536024f2b8b674412f4afe338746eb9ce3c51ad Mon Sep 17 00:00:00 2001 From: kontrollanten <6680299+kontrollanten@users.noreply.github.com> Date: Wed, 25 Sep 2024 12:12:26 +0200 Subject: [PATCH 1/2] feat(sitemap): remove empty accounts/channels closes #6607 --- .../src/videos/videos-command.ts | 15 ++++++----- packages/tests/src/misc-endpoints.ts | 27 ++++++++++++++----- server/core/models/account/account.ts | 17 +++++++++++- server/core/models/video/video-channel.ts | 10 ++++++- 4 files changed, 54 insertions(+), 15 deletions(-) diff --git a/packages/server-commands/src/videos/videos-command.ts b/packages/server-commands/src/videos/videos-command.ts index ddafc0d3f51..9e5a71679fb 100644 --- a/packages/server-commands/src/videos/videos-command.ts +++ b/packages/server-commands/src/videos/videos-command.ts @@ -417,14 +417,17 @@ export class VideosCommand extends AbstractCommand { mode?: 'legacy' | 'resumable' // default legacy waitTorrentGeneration?: boolean // default true completedExpectedStatus?: HttpStatusCodeType + videoChannelId?: number } = {}) { - const { mode = 'legacy', waitTorrentGeneration = true } = options + const { mode = 'legacy', videoChannelId, waitTorrentGeneration = true } = options let defaultChannelId = 1 - try { - const { videoChannels } = await this.server.users.getMyInfo({ token: options.token }) - defaultChannelId = videoChannels[0].id - } catch (e) { /* empty */ } + if (!videoChannelId) { + try { + const { videoChannels } = await this.server.users.getMyInfo({ token: options.token }) + defaultChannelId = videoChannels[0].id + } catch (e) { /* empty */ } + } // Override default attributes const attributes = { @@ -432,7 +435,7 @@ export class VideosCommand extends AbstractCommand { category: 5, licence: 4, language: 'zh', - channelId: defaultChannelId, + channelId: videoChannelId || defaultChannelId, nsfw: true, waitTranscoding: false, description: 'my super description', diff --git a/packages/tests/src/misc-endpoints.ts b/packages/tests/src/misc-endpoints.ts index 4c163f59a5b..59fa89defd7 100644 --- a/packages/tests/src/misc-endpoints.ts +++ b/packages/tests/src/misc-endpoints.ts @@ -193,15 +193,26 @@ describe('Test misc endpoints', function () { it('Should add videos, channel and accounts and get sitemap', async function () { this.timeout(35000) - await server.videos.upload({ attributes: { name: 'video 1', nsfw: false } }) - await server.videos.upload({ attributes: { name: 'video 2', nsfw: false } }) - await server.videos.upload({ attributes: { name: 'video 3', privacy: VideoPrivacy.PRIVATE } }) + const { token: user1Token } = await server.users.generate('user1') + const { token: user2Token } = await server.users.generate('user2') + const { token: user3Token } = await server.users.generate('user3') - await server.channels.create({ attributes: { name: 'channel1', displayName: 'channel 1' } }) - await server.channels.create({ attributes: { name: 'channel2', displayName: 'channel 2' } }) + const { id: channel1Id } = await server.channels.create({ + attributes: { name: 'channel1', displayName: 'channel 1' }, + token: user1Token + }) + const { id: channel2Id } = await server.channels.create({ + attributes: { name: 'channel2', displayName: 'channel 2' }, + token: user2Token + }) + const { id: channel3Id } = await server.channels.create({ + attributes: { name: 'channel3', displayName: 'channel 3' }, + token: user3Token + }) - await server.users.create({ username: 'user1', password: 'password' }) - await server.users.create({ username: 'user2', password: 'password' }) + await server.videos.upload({ attributes: { name: 'video 1', nsfw: false }, videoChannelId: channel1Id }) + await server.videos.upload({ attributes: { name: 'video 2', nsfw: false }, videoChannelId: channel2Id }) + await server.videos.upload({ attributes: { name: 'video 3', privacy: VideoPrivacy.PRIVATE }, videoChannelId: channel3Id }) const res = await makeGetRequest({ url: server.url, @@ -218,9 +229,11 @@ describe('Test misc endpoints', function () { expect(res.text).to.contain('' + server.url + '/c/channel1/videos') expect(res.text).to.contain('' + server.url + '/c/channel2/videos') + expect(res.text).to.not.contain('' + server.url + '/c/channel3/videos') expect(res.text).to.contain('' + server.url + '/a/user1/video-channels') expect(res.text).to.contain('' + server.url + '/a/user2/video-channels') + expect(res.text).to.not.contain('' + server.url + '/a/user3/video-channels') }) it('Should not fail with big title/description videos', async function () { diff --git a/server/core/models/account/account.ts b/server/core/models/account/account.ts index 9144a982377..2619781588a 100644 --- a/server/core/models/account/account.ts +++ b/server/core/models/account/account.ts @@ -1,4 +1,4 @@ -import { Account, AccountSummary } from '@peertube/peertube-models' +import { Account, AccountSummary, VideoPrivacy } from '@peertube/peertube-models' import { ModelCache } from '@server/models/shared/model-cache.js' import { FindOptions, IncludeOptions, Includeable, Op, Transaction, WhereOptions } from 'sequelize' import { @@ -433,6 +433,21 @@ export class AccountModel extends SequelizeModel { where: { serverId: null } + }, + { + attributes: [ 'id' ], + model: VideoChannelModel.unscoped(), + required: true, + include: [ + { + attributes: [ 'id' ], + model: VideoModel.unscoped(), + required: true, + where: { + privacy: VideoPrivacy.PUBLIC + } + } + ] } ] } diff --git a/server/core/models/video/video-channel.ts b/server/core/models/video/video-channel.ts index bb920c68b53..7d6c612b145 100644 --- a/server/core/models/video/video-channel.ts +++ b/server/core/models/video/video-channel.ts @@ -1,5 +1,5 @@ import { forceNumber, pick } from '@peertube/peertube-core-utils' -import { ActivityPubActor, VideoChannel, VideoChannelSummary } from '@peertube/peertube-models' +import { ActivityPubActor, VideoChannel, VideoChannelSummary, VideoPrivacy } from '@peertube/peertube-models' import { CONFIG } from '@server/initializers/config.js' import { InternalEventEmitter } from '@server/lib/internal-event-emitter.js' import { MAccountHost } from '@server/types/models/index.js' @@ -522,6 +522,14 @@ export class VideoChannelModel extends SequelizeModel { where: { serverId: null } + }, + { + attributes: [ 'id' ], + model: VideoModel.unscoped(), + required: true, + where: { + privacy: VideoPrivacy.PUBLIC + } } ] } From 71bc56d4e8035496b1363f241b0fe9bd8baf621a Mon Sep 17 00:00:00 2001 From: kontrollanten <6680299+kontrollanten@users.noreply.github.com> Date: Fri, 27 Sep 2024 14:57:55 +0200 Subject: [PATCH 2/2] feat(sitemap): add more video tags https://developers.google.com/search/docs/crawling-indexing/sitemaps/video-sitemaps closes #6606 --- packages/tests/src/misc-endpoints.ts | 21 +++++++- server/core/controllers/sitemap.ts | 48 ++++++++++++------- .../sql/video/shared/video-model-builder.ts | 3 +- .../sql/video/videos-id-list-query-builder.ts | 1 + .../video/videos-model-list-query-builder.ts | 4 ++ server/core/models/video/video.ts | 2 + 6 files changed, 61 insertions(+), 18 deletions(-) diff --git a/packages/tests/src/misc-endpoints.ts b/packages/tests/src/misc-endpoints.ts index 59fa89defd7..0dff6e0cebd 100644 --- a/packages/tests/src/misc-endpoints.ts +++ b/packages/tests/src/misc-endpoints.ts @@ -210,10 +210,17 @@ describe('Test misc endpoints', function () { token: user3Token }) - await server.videos.upload({ attributes: { name: 'video 1', nsfw: false }, videoChannelId: channel1Id }) + const { id: video1Id } = await server.videos.upload({ attributes: { name: 'video 1', nsfw: false }, videoChannelId: channel1Id }) await server.videos.upload({ attributes: { name: 'video 2', nsfw: false }, videoChannelId: channel2Id }) await server.videos.upload({ attributes: { name: 'video 3', privacy: VideoPrivacy.PRIVATE }, videoChannelId: channel3Id }) + await server.videos.update({ + id: video1Id, + attributes: { + tags: [ 'fish', 'chips' ] + } + }) + const res = await makeGetRequest({ url: server.url, path: '/sitemap.xml?t=1', // avoid using cache @@ -227,6 +234,18 @@ describe('Test misc endpoints', function () { expect(res.text).to.contain('video 2') expect(res.text).to.not.contain('video 3') + expect(res.text).to.match(/.*\.jpg<\/video:thumbnail_loc>/) + expect(res.text).to.match(/.*\.webm<\/video:content_loc>/) + expect(res.text).to.match(/.*\/videos\/embed\/.*<\/video:player_loc>/) + expect(res.text).to.match(/.*<\/video:duration>/) + expect(res.text).to.match(/0<\/video:rating>/) + expect(res.text).to.match(/0<\/video:view_count>/) + expect(res.text).to.match(/.*<\/video:publication_date>/) + expect(res.text).to.match(/fish<\/video:tag>/) + expect(res.text).to.match(/chips<\/video:tag>/) + expect(res.text).to.match(/channel 1<\/video:uploader>/) + expect(res.text).to.match(/NO<\/video:live>/) + expect(res.text).to.contain('' + server.url + '/c/channel1/videos') expect(res.text).to.contain('' + server.url + '/c/channel2/videos') expect(res.text).to.not.contain('' + server.url + '/c/channel3/videos') diff --git a/server/core/controllers/sitemap.ts b/server/core/controllers/sitemap.ts index 42fb54f7c6b..dd7526afc1d 100644 --- a/server/core/controllers/sitemap.ts +++ b/server/core/controllers/sitemap.ts @@ -5,11 +5,11 @@ import { logger } from '@server/helpers/logger.js' import { getServerActor } from '@server/models/application/application.js' import { buildNSFWFilter } from '../helpers/express-utils.js' import { ROUTE_CACHE_LIFETIME, WEBSERVER } from '../initializers/constants.js' -import { apiRateLimiter, asyncMiddleware } from '../middlewares/index.js' -import { cacheRoute } from '../middlewares/cache/cache.js' +import { apiRateLimiter, asyncMiddleware, cacheRoute } from '../middlewares/index.js' import { AccountModel } from '../models/account/account.js' import { VideoModel } from '../models/video/video.js' import { VideoChannelModel } from '../models/video/video-channel.js' +import { VideoFileStream, VideoInclude } from '@peertube/peertube-models' const sitemapRouter = express.Router() @@ -83,22 +83,38 @@ async function getSitemapLocalVideoUrls () { }, isLocal: true, nsfw: buildNSFWFilter(), - countVideos: false + countVideos: false, + includeTags: true, + include: VideoInclude.FILES }) - return data.map(v => ({ - url: WEBSERVER.URL + v.getWatchStaticPath(), - video: [ - { - // Sitemap title should be < 100 characters - title: truncate(v.name, { length: 100, omission: '...' }), - // Sitemap description should be < 2000 characters - description: truncate(v.description || v.name, { length: 2000, omission: '...' }), - player_loc: WEBSERVER.URL + v.getEmbedStaticPath(), - thumbnail_loc: WEBSERVER.URL + v.getMiniatureStaticPath() - } - ] - })) + return data.map(v => { + const contentLoc = v.getHLSPlaylist()?.getMasterPlaylistUrl(v) || v.getMaxQualityFile(VideoFileStream.VIDEO).getFileUrl(v) + + return { + url: WEBSERVER.URL + v.getWatchStaticPath(), + video: [ + { + // Sitemap title should be < 100 characters + 'title': truncate(v.name, { length: 100, omission: '...' }), + // Sitemap description should be < 2000 characters + 'description': truncate(v.description || v.name, { length: 2000, omission: '...' }), + 'player_loc': WEBSERVER.URL + v.getEmbedStaticPath(), + 'thumbnail_loc': WEBSERVER.URL + v.getMiniatureStaticPath(), + 'content_loc': contentLoc, + 'duration': v.duration, + 'view_count': v.views, + 'publication_date': v.publishedAt.toISOString(), + 'uploader': v.VideoChannel.getDisplayName(), + 'uploader:info': WEBSERVER.URL + '/c/' + v.VideoChannel.Actor.preferredUsername, + 'live': v.isLive ? 'YES' : 'NO', + 'family_friendly': v.nsfw ? 'NO' : 'YES', + 'rating': (v.likes * 5) / (v.likes + v.dislikes) || 0, + 'tag': v.Tags.map(t => t.name) + } + ] + } + }) } function getSitemapBasicUrls () { diff --git a/server/core/models/video/sql/video/shared/video-model-builder.ts b/server/core/models/video/sql/video/shared/video-model-builder.ts index cb182a38b0c..246c9a76007 100644 --- a/server/core/models/video/sql/video/shared/video-model-builder.ts +++ b/server/core/models/video/sql/video/shared/video-model-builder.ts @@ -98,8 +98,9 @@ export class VideoModelBuilder { this.addStreamingPlaylistFile(row) } + this.addTag(row, videoModel) + if (this.mode === 'get') { - this.addTag(row, videoModel) this.addTracker(row, videoModel) this.setBlacklisted(row, videoModel) this.setScheduleVideoUpdate(row, videoModel) diff --git a/server/core/models/video/sql/video/videos-id-list-query-builder.ts b/server/core/models/video/sql/video/videos-id-list-query-builder.ts index f4c9a7c66cc..3ef617fd7f8 100644 --- a/server/core/models/video/sql/video/videos-id-list-query-builder.ts +++ b/server/core/models/video/sql/video/videos-id-list-query-builder.ts @@ -36,6 +36,7 @@ export type BuildVideosListQueryOptions = { isLive?: boolean isLocal?: boolean include?: VideoIncludeType + includeTags?: boolean categoryOneOf?: number[] licenceOneOf?: number[] diff --git a/server/core/models/video/sql/video/videos-model-list-query-builder.ts b/server/core/models/video/sql/video/videos-model-list-query-builder.ts index 1cbde1b76fd..88f76bbd2c5 100644 --- a/server/core/models/video/sql/video/videos-model-list-query-builder.ts +++ b/server/core/models/video/sql/video/videos-model-list-query-builder.ts @@ -109,6 +109,10 @@ export class VideosModelListQueryBuilder extends AbstractVideoQueryBuilder { this.includeAutomaticTags(serverActor.Account.id) } + if (options.includeTags) { + this.includeTags() + } + const select = this.buildSelect() this.query = `${select} FROM (${this.innerQuery}) AS "tmp" ${this.joins} ${this.innerSort}` diff --git a/server/core/models/video/video.ts b/server/core/models/video/video.ts index b6aa854869a..2f9b8fb66fc 100644 --- a/server/core/models/video/video.ts +++ b/server/core/models/video/video.ts @@ -1151,6 +1151,7 @@ export class VideoModel extends SequelizeModel { isLive?: boolean isLocal?: boolean include?: VideoIncludeType + includeTags?: boolean hasFiles?: boolean // default false @@ -1215,6 +1216,7 @@ export class VideoModel extends SequelizeModel { 'privacyOneOf', 'isLocal', 'include', + 'includeTags', 'displayOnlyForFollower', 'hasFiles', 'accountId',