From f2eb23cd87cf32b8fe545178143b5f49e06a58da Mon Sep 17 00:00:00 2001 From: Rigel Kent Date: Tue, 8 Dec 2020 21:16:10 +0100 Subject: emit more specific status codes on video upload (#3423) - reduce http status codes list to potentially useful codes - convert more codes to typed ones - factorize html generator for error responses --- server/tests/api/videos/multiple-servers.ts | 5 +++-- server/tests/api/videos/video-change-ownership.ts | 14 +++++++++++--- server/tests/api/videos/video-hls.ts | 9 +++++---- server/tests/api/videos/video-playlists.ts | 13 +++++++------ server/tests/api/videos/video-privacy.ts | 13 +++++++------ server/tests/api/videos/videos-history.ts | 3 ++- 6 files changed, 35 insertions(+), 22 deletions(-) (limited to 'server/tests/api/videos') diff --git a/server/tests/api/videos/multiple-servers.ts b/server/tests/api/videos/multiple-servers.ts index fdd5e33f3..f754df04e 100644 --- a/server/tests/api/videos/multiple-servers.ts +++ b/server/tests/api/videos/multiple-servers.ts @@ -41,6 +41,7 @@ import { findCommentId } from '../../../../shared/extra-utils/videos/video-comments' import { waitJobs } from '../../../../shared/extra-utils/server/jobs' +import { HttpStatusCode } from '../../../../shared/core-utils/miscs/http-error-codes' const expect = chai.expect @@ -999,7 +1000,7 @@ describe('Test multiple servers', function () { expect(res.body.downloadEnabled).to.be.false const text = 'my super forbidden comment' - await addVideoCommentThread(server.url, server.accessToken, videoUUID, text, 409) + await addVideoCommentThread(server.url, server.accessToken, videoUUID, text, HttpStatusCode.CONFLICT_409) } }) }) @@ -1021,7 +1022,7 @@ describe('Test multiple servers', function () { const filePath = join(__dirname, '..', '..', 'fixtures', 'video_short.webm') await req.attach('videofile', filePath) - .expect(200) + .expect(HttpStatusCode.OK_200) await waitJobs(servers) diff --git a/server/tests/api/videos/video-change-ownership.ts b/server/tests/api/videos/video-change-ownership.ts index dee6575b9..fad4c8b1f 100644 --- a/server/tests/api/videos/video-change-ownership.ts +++ b/server/tests/api/videos/video-change-ownership.ts @@ -23,6 +23,7 @@ import { import { waitJobs } from '../../../../shared/extra-utils/server/jobs' import { User } from '../../../../shared/models/users' import { VideoDetails } from '../../../../shared/models/videos' +import { HttpStatusCode } from '../../../../shared/core-utils/miscs/http-error-codes' const expect = chai.expect @@ -140,7 +141,7 @@ describe('Test video change ownership - nominal', function () { it('Should not be possible to refuse the change of ownership from first user', async function () { this.timeout(10000) - await refuseChangeOwnership(servers[0].url, firstUserAccessToken, lastRequestChangeOwnershipId, 403) + await refuseChangeOwnership(servers[0].url, firstUserAccessToken, lastRequestChangeOwnershipId, HttpStatusCode.FORBIDDEN_403) }) it('Should be possible to refuse the change of ownership from second user', async function () { @@ -177,7 +178,7 @@ describe('Test video change ownership - nominal', function () { const secondUserInformationResponse = await getMyUserInformation(servers[0].url, secondUserAccessToken) const secondUserInformation: User = secondUserInformationResponse.body const channelId = secondUserInformation.videoChannels[0].id - await acceptChangeOwnership(servers[0].url, firstUserAccessToken, lastRequestChangeOwnershipId, channelId, 403) + await acceptChangeOwnership(servers[0].url, firstUserAccessToken, lastRequestChangeOwnershipId, channelId, HttpStatusCode.FORBIDDEN_403) }) it('Should be possible to accept the change of ownership from second user', async function () { @@ -294,7 +295,14 @@ describe('Test video change ownership - quota too small', function () { const secondUserInformationResponse = await getMyUserInformation(server.url, secondUserAccessToken) const secondUserInformation: User = secondUserInformationResponse.body const channelId = secondUserInformation.videoChannels[0].id - await acceptChangeOwnership(server.url, secondUserAccessToken, lastRequestChangeOwnershipId, channelId, 403) + + await acceptChangeOwnership( + server.url, + secondUserAccessToken, + lastRequestChangeOwnershipId, + channelId, + HttpStatusCode.PAYLOAD_TOO_LARGE_413 + ) }) after(async function () { diff --git a/server/tests/api/videos/video-hls.ts b/server/tests/api/videos/video-hls.ts index 3a65cc1d2..f3dbbb114 100644 --- a/server/tests/api/videos/video-hls.ts +++ b/server/tests/api/videos/video-hls.ts @@ -26,6 +26,7 @@ import { import { VideoDetails } from '../../../../shared/models/videos' import { VideoStreamingPlaylistType } from '../../../../shared/models/videos/video-streaming-playlist.type' import { DEFAULT_AUDIO_RESOLUTION } from '../../../initializers/constants' +import { HttpStatusCode } from '../../../../shared/core-utils/miscs/http-error-codes' const expect = chai.expect @@ -57,8 +58,8 @@ async function checkHlsPlaylist (servers: ServerInfo[], videoUUID: string, hlsOn ) expect(file.resolution.label).to.equal(resolution + 'p') - await makeRawRequest(file.torrentUrl, 200) - await makeRawRequest(file.fileUrl, 200) + await makeRawRequest(file.torrentUrl, HttpStatusCode.OK_200) + await makeRawRequest(file.fileUrl, HttpStatusCode.OK_200) const torrent = await webtorrentAdd(file.magnetUri, true) expect(torrent.files).to.be.an('array') @@ -144,8 +145,8 @@ describe('Test HLS videos', function () { await waitJobs(servers) for (const server of servers) { - await getVideo(server.url, videoUUID, 404) - await getVideo(server.url, videoAudioUUID, 404) + await getVideo(server.url, videoUUID, HttpStatusCode.NOT_FOUND_404) + await getVideo(server.url, videoAudioUUID, HttpStatusCode.NOT_FOUND_404) } }) diff --git a/server/tests/api/videos/video-playlists.ts b/server/tests/api/videos/video-playlists.ts index 7d1215990..0a96ea9a0 100644 --- a/server/tests/api/videos/video-playlists.ts +++ b/server/tests/api/videos/video-playlists.ts @@ -61,6 +61,7 @@ import { removeServerFromAccountBlocklist, removeServerFromServerBlocklist } from '../../../../shared/extra-utils/users/blocklist' +import { HttpStatusCode } from '../../../../shared/core-utils/miscs/http-error-codes' const expect = chai.expect @@ -1091,7 +1092,7 @@ describe('Test video playlists', function () { await waitJobs(servers) for (const server of servers) { - await getVideoPlaylist(server.url, videoPlaylistIds.uuid, 200) + await getVideoPlaylist(server.url, videoPlaylistIds.uuid, HttpStatusCode.OK_200) } const playlistAttrs = { privacy: VideoPlaylistPrivacy.PRIVATE } @@ -1100,11 +1101,11 @@ describe('Test video playlists', function () { await waitJobs(servers) for (const server of [ servers[1], servers[2] ]) { - await getVideoPlaylist(server.url, videoPlaylistIds.uuid, 404) + await getVideoPlaylist(server.url, videoPlaylistIds.uuid, HttpStatusCode.NOT_FOUND_404) } - await getVideoPlaylist(servers[0].url, videoPlaylistIds.uuid, 401) + await getVideoPlaylist(servers[0].url, videoPlaylistIds.uuid, HttpStatusCode.UNAUTHORIZED_401) - await getVideoPlaylistWithToken(servers[0].url, servers[0].accessToken, videoPlaylistIds.uuid, 200) + await getVideoPlaylistWithToken(servers[0].url, servers[0].accessToken, videoPlaylistIds.uuid, HttpStatusCode.OK_200) }) }) @@ -1118,7 +1119,7 @@ describe('Test video playlists', function () { await waitJobs(servers) for (const server of servers) { - await getVideoPlaylist(server.url, playlistServer1UUID, 404) + await getVideoPlaylist(server.url, playlistServer1UUID, HttpStatusCode.NOT_FOUND_404) } }) @@ -1178,7 +1179,7 @@ describe('Test video playlists', function () { expect(res3.body.displayName).to.equal('channel playlist') expect(res3.body.privacy.id).to.equal(VideoPlaylistPrivacy.PRIVATE) - await getVideoPlaylist(servers[1].url, videoPlaylistUUID, 404) + await getVideoPlaylist(servers[1].url, videoPlaylistUUID, HttpStatusCode.NOT_FOUND_404) }) it('Should delete an account and delete its playlists', async function () { diff --git a/server/tests/api/videos/video-privacy.ts b/server/tests/api/videos/video-privacy.ts index 38e93bbe6..f25d75af4 100644 --- a/server/tests/api/videos/video-privacy.ts +++ b/server/tests/api/videos/video-privacy.ts @@ -18,6 +18,7 @@ import { createUser } from '../../../../shared/extra-utils/users/users' import { getMyVideos, getVideo, getVideoWithToken, updateVideo } from '../../../../shared/extra-utils/videos/videos' import { waitJobs } from '../../../../shared/extra-utils/server/jobs' import { Video } from '@shared/models' +import { HttpStatusCode } from '@shared/core-utils/miscs/http-error-codes' const expect = chai.expect @@ -110,8 +111,8 @@ describe('Test video privacy', function () { }) it('Should not be able to watch the private/internal video with non authenticated user', async function () { - await getVideo(servers[0].url, privateVideoUUID, 401) - await getVideo(servers[0].url, internalVideoUUID, 401) + await getVideo(servers[0].url, privateVideoUUID, HttpStatusCode.UNAUTHORIZED_401) + await getVideo(servers[0].url, internalVideoUUID, HttpStatusCode.UNAUTHORIZED_401) }) it('Should not be able to watch the private video with another user', async function () { @@ -124,15 +125,15 @@ describe('Test video privacy', function () { await createUser({ url: servers[0].url, accessToken: servers[0].accessToken, username: user.username, password: user.password }) anotherUserToken = await userLogin(servers[0], user) - await getVideoWithToken(servers[0].url, anotherUserToken, privateVideoUUID, 403) + await getVideoWithToken(servers[0].url, anotherUserToken, privateVideoUUID, HttpStatusCode.FORBIDDEN_403) }) it('Should be able to watch the internal video with another user', async function () { - await getVideoWithToken(servers[0].url, anotherUserToken, internalVideoUUID, 200) + await getVideoWithToken(servers[0].url, anotherUserToken, internalVideoUUID, HttpStatusCode.OK_200) }) it('Should be able to watch the private video with the correct user', async function () { - await getVideoWithToken(servers[0].url, servers[0].accessToken, privateVideoUUID, 200) + await getVideoWithToken(servers[0].url, servers[0].accessToken, privateVideoUUID, HttpStatusCode.OK_200) }) it('Should upload an unlisted video on server 2', async function () { @@ -202,7 +203,7 @@ describe('Test video privacy', function () { }) it('Should not be able to get non-federated unlisted video from federated server', async function () { - await getVideo(servers[1].url, nonFederatedUnlistedVideoUUID, 404) + await getVideo(servers[1].url, nonFederatedUnlistedVideoUUID, HttpStatusCode.NOT_FOUND_404) }) it('Should update the private and internal videos to public on server 1', async function () { diff --git a/server/tests/api/videos/videos-history.ts b/server/tests/api/videos/videos-history.ts index 6f90e9a57..661d603cb 100644 --- a/server/tests/api/videos/videos-history.ts +++ b/server/tests/api/videos/videos-history.ts @@ -20,6 +20,7 @@ import { } from '../../../../shared/extra-utils' import { Video, VideoDetails } from '../../../../shared/models/videos' import { listMyVideosHistory, removeMyVideosHistory, userWatchVideo } from '../../../../shared/extra-utils/videos/video-history' +import { HttpStatusCode } from '../../../../shared/core-utils/miscs/http-error-codes' const expect = chai.expect @@ -171,7 +172,7 @@ describe('Test videos history', function () { videosHistoryEnabled: false }) - await userWatchVideo(server.url, server.accessToken, video2UUID, 8, 409) + await userWatchVideo(server.url, server.accessToken, video2UUID, 8, HttpStatusCode.CONFLICT_409) }) it('Should re-enable videos history', async function () { -- cgit v1.2.3