From 982edf32ae436424e9b1ebf7f13696e4871ce202 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 28 Jul 2023 16:28:16 +0200 Subject: [PATCH] Remove deprecated video query filter --- server/helpers/custom-validators/videos.ts | 11 +-- .../middlewares/validators/videos/videos.ts | 24 +----- server/models/video/video.ts | 2 +- .../api/check-params/videos-common-filters.ts | 73 ------------------ .../tests/api/videos/videos-common-filters.ts | 74 ------------------- .../search/videos-common-query.model.ts | 6 -- .../search/videos-search-query.model.ts | 3 - shared/models/videos/index.ts | 1 - shared/models/videos/video-filter.type.ts | 1 - 9 files changed, 6 insertions(+), 189 deletions(-) delete mode 100644 shared/models/videos/video-filter.type.ts diff --git a/server/helpers/custom-validators/videos.ts b/server/helpers/custom-validators/videos.ts index 91109217c..00c6deed4 100644 --- a/server/helpers/custom-validators/videos.ts +++ b/server/helpers/custom-validators/videos.ts @@ -1,7 +1,8 @@ -import { Response, Request, UploadFilesForCheck } from 'express' +import { Request, Response, UploadFilesForCheck } from 'express' import { decode as magnetUriDecode } from 'magnet-uri' import validator from 'validator' -import { HttpStatusCode, VideoFilter, VideoInclude, VideoPrivacy, VideoRateType } from '@shared/models' +import { getVideoWithAttributes } from '@server/helpers/video' +import { HttpStatusCode, VideoInclude, VideoPrivacy, VideoRateType } from '@shared/models' import { CONSTRAINTS_FIELDS, MIMETYPES, @@ -13,14 +14,9 @@ import { VIDEO_STATES } from '../../initializers/constants' import { exists, isArray, isDateValid, isFileValid } from './misc' -import { getVideoWithAttributes } from '@server/helpers/video' const VIDEOS_CONSTRAINTS_FIELDS = CONSTRAINTS_FIELDS.VIDEOS -function isVideoFilterValid (filter: VideoFilter) { - return filter === 'local' || filter === 'all-local' || filter === 'all' -} - function isVideoIncludeValid (include: VideoInclude) { return exists(include) && validator.isInt('' + include) } @@ -217,7 +213,6 @@ export { isVideoFileSizeValid, isVideoImageValid, isVideoSupportValid, - isVideoFilterValid, isPasswordValid, isValidPasswordProtectedPrivacy } diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts index aea3453b5..5a49779ed 100644 --- a/server/middlewares/validators/videos/videos.ts +++ b/server/middlewares/validators/videos/videos.ts @@ -7,8 +7,8 @@ import { uploadx } from '@server/lib/uploadx' import { getServerActor } from '@server/models/application/application' import { ExpressPromiseHandler } from '@server/types/express-handler' import { MUserAccountId, MVideoFullLight } from '@server/types/models' -import { arrayify, getAllPrivacies } from '@shared/core-utils' -import { HttpStatusCode, ServerErrorCode, UserRight, VideoInclude, VideoState } from '@shared/models' +import { arrayify } from '@shared/core-utils' +import { HttpStatusCode, ServerErrorCode, UserRight, VideoState } from '@shared/models' import { exists, isBooleanValid, @@ -26,7 +26,6 @@ import { isValidPasswordProtectedPrivacy, isVideoCategoryValid, isVideoDescriptionValid, - isVideoFilterValid, isVideoImageValid, isVideoIncludeValid, isVideoLanguageValid, @@ -464,9 +463,6 @@ const commonVideosFiltersValidator = [ .optional() .customSanitizer(toBooleanOrNull) .custom(isBooleanValid).withMessage('Should have a valid isLive boolean'), - query('filter') - .optional() - .custom(isVideoFilterValid), query('include') .optional() .custom(isVideoIncludeValid), @@ -501,22 +497,6 @@ const commonVideosFiltersValidator = [ (req: express.Request, res: express.Response, next: express.NextFunction) => { if (areValidationErrors(req, res)) return - // FIXME: deprecated in 4.0, to remove - { - if (req.query.filter === 'all-local') { - req.query.include = VideoInclude.NOT_PUBLISHED_STATE - req.query.isLocal = true - req.query.privacyOneOf = getAllPrivacies() - } else if (req.query.filter === 'all') { - req.query.include = VideoInclude.NOT_PUBLISHED_STATE - req.query.privacyOneOf = getAllPrivacies() - } else if (req.query.filter === 'local') { - req.query.isLocal = true - } - - req.query.filter = undefined - } - const user = res.locals.oauth?.token.User if ((!user || user.hasRight(UserRight.SEE_ALL_VIDEOS) !== true)) { diff --git a/server/models/video/video.ts b/server/models/video/video.ts index 2fe701436..73308182d 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -1639,7 +1639,7 @@ export class VideoModel extends Model>> { private static throwIfPrivateIncludeWithoutUser (include: VideoInclude, user: MUserAccountId) { if (VideoModel.isPrivateInclude(include) && !user?.hasRight(UserRight.SEE_ALL_VIDEOS)) { - throw new Error('Try to filter all-local but user cannot see all videos') + throw new Error('Try to include protected videos but user cannot see all videos') } } diff --git a/server/tests/api/check-params/videos-common-filters.ts b/server/tests/api/check-params/videos-common-filters.ts index 3e44e2f67..603f7f777 100644 --- a/server/tests/api/check-params/videos-common-filters.ts +++ b/server/tests/api/check-params/videos-common-filters.ts @@ -35,79 +35,6 @@ describe('Test video filters validators', function () { moderatorAccessToken = await server.login.getAccessToken(moderator) }) - describe('When setting a deprecated video filter', function () { - - async function testEndpoints (token: string, filter: string, expectedStatus: HttpStatusCode) { - const paths = [ - '/api/v1/video-channels/root_channel/videos', - '/api/v1/accounts/root/videos', - '/api/v1/videos', - '/api/v1/search/videos', - '/api/v1/users/me/subscriptions/videos' - ] - - for (const path of paths) { - await makeGetRequest({ - url: server.url, - path, - token, - query: { - filter - }, - expectedStatus - }) - } - } - - it('Should fail with a bad filter', async function () { - await testEndpoints(server.accessToken, 'bad-filter', HttpStatusCode.BAD_REQUEST_400) - }) - - it('Should succeed with a good filter', async function () { - await testEndpoints(server.accessToken, 'local', HttpStatusCode.OK_200) - }) - - it('Should fail to list all-local/all with a simple user', async function () { - await testEndpoints(userAccessToken, 'all-local', HttpStatusCode.UNAUTHORIZED_401) - await testEndpoints(userAccessToken, 'all', HttpStatusCode.UNAUTHORIZED_401) - }) - - it('Should succeed to list all-local/all with a moderator', async function () { - await testEndpoints(moderatorAccessToken, 'all-local', HttpStatusCode.OK_200) - await testEndpoints(moderatorAccessToken, 'all', HttpStatusCode.OK_200) - }) - - it('Should succeed to list all-local/all with an admin', async function () { - await testEndpoints(server.accessToken, 'all-local', HttpStatusCode.OK_200) - await testEndpoints(server.accessToken, 'all', HttpStatusCode.OK_200) - }) - - // Because we cannot authenticate the user on the RSS endpoint - it('Should fail on the feeds endpoint with the all-local/all filter', async function () { - for (const filter of [ 'all', 'all-local' ]) { - await makeGetRequest({ - url: server.url, - path: '/feeds/videos.json', - expectedStatus: HttpStatusCode.UNAUTHORIZED_401, - query: { - filter - } - }) - } - }) - - it('Should succeed on the feeds endpoint with the local filter', async function () { - await makeGetRequest({ - url: server.url, - path: '/feeds/videos.json', - expectedStatus: HttpStatusCode.OK_200, - query: { - filter: 'local' - } - }) - }) - }) - describe('When setting video filters', function () { const validIncludes = [ diff --git a/server/tests/api/videos/videos-common-filters.ts b/server/tests/api/videos/videos-common-filters.ts index fac0f5dc5..48de7c537 100644 --- a/server/tests/api/videos/videos-common-filters.ts +++ b/server/tests/api/videos/videos-common-filters.ts @@ -74,80 +74,6 @@ describe('Test videos filter', function () { ] }) - describe('Check deprecated videos filter', function () { - - async function getVideosNames (options: { - server: PeerTubeServer - token: string - filter: string - skipSubscription?: boolean - expectedStatus?: HttpStatusCode - }) { - const { server, token, filter, skipSubscription = false, expectedStatus = HttpStatusCode.OK_200 } = options - - const videosResults: Video[][] = [] - - for (const path of paths) { - if (skipSubscription && path === subscriptionVideosPath) continue - - const res = await makeGetRequest({ - url: server.url, - path, - token, - query: { - sort: 'createdAt', - filter - }, - expectedStatus - }) - - videosResults.push(res.body.data.map(v => v.name)) - } - - return videosResults - } - - it('Should display local videos', async function () { - for (const server of servers) { - const namesResults = await getVideosNames({ server, token: server.accessToken, filter: 'local' }) - for (const names of namesResults) { - expect(names).to.have.lengthOf(1) - expect(names[0]).to.equal('public ' + server.serverNumber) - } - } - }) - - it('Should display all local videos by the admin or the moderator', async function () { - for (const server of servers) { - for (const token of [ server.accessToken, server['moderatorAccessToken'] ]) { - - const namesResults = await getVideosNames({ server, token, filter: 'all-local', skipSubscription: true }) - for (const names of namesResults) { - expect(names).to.have.lengthOf(3) - - expect(names[0]).to.equal('public ' + server.serverNumber) - expect(names[1]).to.equal('unlisted ' + server.serverNumber) - expect(names[2]).to.equal('private ' + server.serverNumber) - } - } - } - }) - - it('Should display all videos by the admin or the moderator', async function () { - for (const server of servers) { - for (const token of [ server.accessToken, server['moderatorAccessToken'] ]) { - - const [ channelVideos, accountVideos, videos, searchVideos ] = await getVideosNames({ server, token, filter: 'all' }) - expect(channelVideos).to.have.lengthOf(3) - expect(accountVideos).to.have.lengthOf(3) - - expect(videos).to.have.lengthOf(5) - expect(searchVideos).to.have.lengthOf(5) - } - } - }) - }) - describe('Check videos filters', function () { async function listVideos (options: { diff --git a/shared/models/search/videos-common-query.model.ts b/shared/models/search/videos-common-query.model.ts index 2c52ca8cf..f783d7534 100644 --- a/shared/models/search/videos-common-query.model.ts +++ b/shared/models/search/videos-common-query.model.ts @@ -12,9 +12,6 @@ export interface VideosCommonQuery { isLive?: boolean - // FIXME: deprecated in 4.0 in favour of isLocal and include, to remove - filter?: never - isLocal?: boolean include?: VideoInclude @@ -45,7 +42,4 @@ export interface VideosCommonQueryAfterSanitize extends VideosCommonQuery { start: number count: number sort: string - - // FIXME: deprecated in 4.0, to remove - filter?: never } diff --git a/shared/models/search/videos-search-query.model.ts b/shared/models/search/videos-search-query.model.ts index 447c72806..a5436879d 100644 --- a/shared/models/search/videos-search-query.model.ts +++ b/shared/models/search/videos-search-query.model.ts @@ -23,7 +23,4 @@ export interface VideosSearchQueryAfterSanitize extends VideosSearchQuery { start: number count: number sort: string - - // FIXME: deprecated in 4.0, to remove - filter?: never } diff --git a/shared/models/videos/index.ts b/shared/models/videos/index.ts index b3ce6ad3f..f8f1ce081 100644 --- a/shared/models/videos/index.ts +++ b/shared/models/videos/index.ts @@ -22,7 +22,6 @@ export * from './video-constant.model' export * from './video-create.model' export * from './video-privacy.enum' -export * from './video-filter.type' export * from './video-include.enum' export * from './video-rate.type' diff --git a/shared/models/videos/video-filter.type.ts b/shared/models/videos/video-filter.type.ts deleted file mode 100644 index e641a401c..000000000 --- a/shared/models/videos/video-filter.type.ts +++ /dev/null @@ -1 +0,0 @@ -export type VideoFilter = 'local' | 'all-local' | 'all'