Skip to content

Commit

Permalink
Remove deprecated video query filter
Browse files Browse the repository at this point in the history
  • Loading branch information
Chocobozzz committed Jul 28, 2023
1 parent ce8d0b5 commit 982edf3
Show file tree
Hide file tree
Showing 9 changed files with 6 additions and 189 deletions.
11 changes: 3 additions & 8 deletions server/helpers/custom-validators/videos.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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)
}
Expand Down Expand Up @@ -217,7 +213,6 @@ export {
isVideoFileSizeValid,
isVideoImageValid,
isVideoSupportValid,
isVideoFilterValid,
isPasswordValid,
isValidPasswordProtectedPrivacy
}
24 changes: 2 additions & 22 deletions server/middlewares/validators/videos/videos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -26,7 +26,6 @@ import {
isValidPasswordProtectedPrivacy,
isVideoCategoryValid,
isVideoDescriptionValid,
isVideoFilterValid,
isVideoImageValid,
isVideoIncludeValid,
isVideoLanguageValid,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)) {
Expand Down
2 changes: 1 addition & 1 deletion server/models/video/video.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1639,7 +1639,7 @@ export class VideoModel extends Model<Partial<AttributesOnly<VideoModel>>> {

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')
}
}

Expand Down
73 changes: 0 additions & 73 deletions server/tests/api/check-params/videos-common-filters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
74 changes: 0 additions & 74 deletions server/tests/api/videos/videos-common-filters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
6 changes: 0 additions & 6 deletions shared/models/search/videos-common-query.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -45,7 +42,4 @@ export interface VideosCommonQueryAfterSanitize extends VideosCommonQuery {
start: number
count: number
sort: string

// FIXME: deprecated in 4.0, to remove
filter?: never
}
3 changes: 0 additions & 3 deletions shared/models/search/videos-search-query.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,4 @@ export interface VideosSearchQueryAfterSanitize extends VideosSearchQuery {
start: number
count: number
sort: string

// FIXME: deprecated in 4.0, to remove
filter?: never
}
1 change: 0 additions & 1 deletion shared/models/videos/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down
1 change: 0 additions & 1 deletion shared/models/videos/video-filter.type.ts

This file was deleted.

0 comments on commit 982edf3

Please sign in to comment.