From 213e30ef90806369529684ac9c247d73b8dc7928 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 7 Apr 2021 10:36:13 +0200 Subject: Add banner tests --- server/controllers/api/users/me.ts | 2 +- server/controllers/api/video-channel.ts | 2 +- server/helpers/custom-validators/actor-images.ts | 17 +++++ server/helpers/custom-validators/users.ts | 13 +--- server/lib/activitypub/actor.ts | 72 +++++++++---------- server/lib/activitypub/process/process-update.ts | 9 +-- server/lib/actor-image.ts | 3 +- server/middlewares/validators/actor-image.ts | 30 ++++++++ server/middlewares/validators/avatar.ts | 30 -------- server/middlewares/validators/index.ts | 1 + server/models/video/video-channel.ts | 9 ++- server/tests/api/check-params/video-channels.ts | 70 ++++++++++-------- server/tests/api/videos/video-channels.ts | 86 ++++++++++++++++++----- server/tests/fixtures/banner-resized.jpg | Bin 0 -> 88780 bytes server/tests/fixtures/banner.jpg | Bin 0 -> 31648 bytes server/typings/express/index.d.ts | 1 - 16 files changed, 206 insertions(+), 139 deletions(-) create mode 100644 server/helpers/custom-validators/actor-images.ts create mode 100644 server/middlewares/validators/actor-image.ts delete mode 100644 server/middlewares/validators/avatar.ts create mode 100644 server/tests/fixtures/banner-resized.jpg create mode 100644 server/tests/fixtures/banner.jpg (limited to 'server') diff --git a/server/controllers/api/users/me.ts b/server/controllers/api/users/me.ts index 25a18caa5..9f9d2d77f 100644 --- a/server/controllers/api/users/me.ts +++ b/server/controllers/api/users/me.ts @@ -25,7 +25,7 @@ import { usersVideoRatingValidator } from '../../../middlewares' import { deleteMeValidator, videoImportsSortValidator, videosSortValidator } from '../../../middlewares/validators' -import { updateAvatarValidator } from '../../../middlewares/validators/avatar' +import { updateAvatarValidator } from '../../../middlewares/validators/actor-image' import { AccountModel } from '../../../models/account/account' import { AccountVideoRateModel } from '../../../models/account/account-video-rate' import { UserModel } from '../../../models/account/user' diff --git a/server/controllers/api/video-channel.ts b/server/controllers/api/video-channel.ts index 1c926722d..149d6cfb4 100644 --- a/server/controllers/api/video-channel.ts +++ b/server/controllers/api/video-channel.ts @@ -33,7 +33,7 @@ import { videoPlaylistsSortValidator } from '../../middlewares' import { videoChannelsNameWithHostValidator, videoChannelsOwnSearchValidator, videosSortValidator } from '../../middlewares/validators' -import { updateAvatarValidator, updateBannerValidator } from '../../middlewares/validators/avatar' +import { updateAvatarValidator, updateBannerValidator } from '../../middlewares/validators/actor-image' import { commonVideoPlaylistFiltersValidator } from '../../middlewares/validators/videos/video-playlists' import { AccountModel } from '../../models/account/account' import { VideoModel } from '../../models/video/video' diff --git a/server/helpers/custom-validators/actor-images.ts b/server/helpers/custom-validators/actor-images.ts new file mode 100644 index 000000000..4fb0b7c70 --- /dev/null +++ b/server/helpers/custom-validators/actor-images.ts @@ -0,0 +1,17 @@ + +import { CONSTRAINTS_FIELDS } from '../../initializers/constants' +import { isFileValid } from './misc' + +const imageMimeTypes = CONSTRAINTS_FIELDS.ACTORS.IMAGE.EXTNAME + .map(v => v.replace('.', '')) + .join('|') +const imageMimeTypesRegex = `image/(${imageMimeTypes})` +function isActorImageFile (files: { [ fieldname: string ]: Express.Multer.File[] } | Express.Multer.File[], fieldname: string) { + return isFileValid(files, imageMimeTypesRegex, fieldname, CONSTRAINTS_FIELDS.ACTORS.IMAGE.FILE_SIZE.max) +} + +// --------------------------------------------------------------------------- + +export { + isActorImageFile +} diff --git a/server/helpers/custom-validators/users.ts b/server/helpers/custom-validators/users.ts index 85f3634c8..5b21c3529 100644 --- a/server/helpers/custom-validators/users.ts +++ b/server/helpers/custom-validators/users.ts @@ -3,7 +3,7 @@ import validator from 'validator' import { UserRole } from '../../../shared' import { isEmailEnabled } from '../../initializers/config' import { CONSTRAINTS_FIELDS, NSFW_POLICY_TYPES } from '../../initializers/constants' -import { exists, isArray, isBooleanValid, isFileValid } from './misc' +import { exists, isArray, isBooleanValid } from './misc' const USERS_CONSTRAINTS_FIELDS = CONSTRAINTS_FIELDS.USERS @@ -97,14 +97,6 @@ function isUserRoleValid (value: any) { return exists(value) && validator.isInt('' + value) && UserRole[value] !== undefined } -const avatarMimeTypes = CONSTRAINTS_FIELDS.ACTORS.IMAGE.EXTNAME - .map(v => v.replace('.', '')) - .join('|') -const avatarMimeTypesRegex = `image/(${avatarMimeTypes})` -function isAvatarFile (files: { [ fieldname: string ]: Express.Multer.File[] } | Express.Multer.File[]) { - return isFileValid(files, avatarMimeTypesRegex, 'avatarfile', CONSTRAINTS_FIELDS.ACTORS.IMAGE.FILE_SIZE.max) -} - // --------------------------------------------------------------------------- export { @@ -128,6 +120,5 @@ export { isUserDisplayNameValid, isUserDescriptionValid, isNoInstanceConfigWarningModal, - isNoWelcomeModal, - isAvatarFile + isNoWelcomeModal } diff --git a/server/lib/activitypub/actor.ts b/server/lib/activitypub/actor.ts index fe4796a3d..917fed6ec 100644 --- a/server/lib/activitypub/actor.ts +++ b/server/lib/activitypub/actor.ts @@ -34,6 +34,7 @@ import { MActorFull, MActorFullActor, MActorId, + MActorImage, MActorImages, MChannel } from '../../types/models' @@ -169,38 +170,34 @@ async function updateActorInstance (actorInstance: ActorModel, attributes: Activ } } -type AvatarInfo = { name: string, onDisk: boolean, fileUrl: string, type: ActorImageType } -async function updateActorImageInstance (actor: MActorImages, info: AvatarInfo, t: Transaction) { - if (!info.name) return actor - - const oldImageModel = info.type === ActorImageType.AVATAR +type ImageInfo = { name: string, onDisk?: boolean, fileUrl: string } +async function updateActorImageInstance (actor: MActorImages, type: ActorImageType, imageInfo: ImageInfo | null, t: Transaction) { + const oldImageModel = type === ActorImageType.AVATAR ? actor.Avatar : actor.Banner if (oldImageModel) { // Don't update the avatar if the file URL did not change - if (info.fileUrl && oldImageModel.fileUrl === info.fileUrl) return actor + if (imageInfo?.fileUrl && oldImageModel.fileUrl === imageInfo.fileUrl) return actor try { await oldImageModel.destroy({ transaction: t }) + + setActorImage(actor, type, null) } catch (err) { logger.error('Cannot remove old actor image of actor %s.', actor.url, { err }) } } - const imageModel = await ActorImageModel.create({ - filename: info.name, - onDisk: info.onDisk, - fileUrl: info.fileUrl, - type: info.type - }, { transaction: t }) + if (imageInfo) { + const imageModel = await ActorImageModel.create({ + filename: imageInfo.name, + onDisk: imageInfo.onDisk ?? false, + fileUrl: imageInfo.fileUrl, + type: type + }, { transaction: t }) - if (info.type === ActorImageType.AVATAR) { - actor.avatarId = imageModel.id - actor.Avatar = imageModel - } else { - actor.bannerId = imageModel.id - actor.Banner = imageModel + setActorImage(actor, type, imageModel) } return actor @@ -310,27 +307,8 @@ async function refreshActorIfNeeded { updateInstanceWithAnother(actor, result.actor) - if (result.avatar !== undefined) { - const avatarInfo = { - name: result.avatar.name, - fileUrl: result.avatar.fileUrl, - onDisk: false, - type: ActorImageType.AVATAR - } - - await updateActorImageInstance(actor, avatarInfo, t) - } - - if (result.banner !== undefined) { - const bannerInfo = { - name: result.banner.name, - fileUrl: result.banner.fileUrl, - onDisk: false, - type: ActorImageType.BANNER - } - - await updateActorImageInstance(actor, bannerInfo, t) - } + await updateActorImageInstance(actor, ActorImageType.AVATAR, result.avatar, t) + await updateActorImageInstance(actor, ActorImageType.BANNER, result.banner, t) // Force update actor.setDataValue('updatedAt', new Date()) @@ -381,6 +359,22 @@ export { // --------------------------------------------------------------------------- +function setActorImage (actorModel: MActorImages, type: ActorImageType, imageModel: MActorImage) { + const id = imageModel + ? imageModel.id + : null + + if (type === ActorImageType.AVATAR) { + actorModel.avatarId = id + actorModel.Avatar = imageModel + } else { + actorModel.bannerId = id + actorModel.Banner = imageModel + } + + return actorModel +} + function saveActorAndServerAndModelIfNotExist ( result: FetchRemoteActorResult, ownerActor?: MActorFullActor, diff --git a/server/lib/activitypub/process/process-update.ts b/server/lib/activitypub/process/process-update.ts index ad3bb392d..6df9b93b2 100644 --- a/server/lib/activitypub/process/process-update.ts +++ b/server/lib/activitypub/process/process-update.ts @@ -134,13 +134,8 @@ async function processUpdateActor (actor: ActorModel, activity: ActivityUpdate) await updateActorInstance(actor, actorAttributesToUpdate) - for (const imageInfo of [ avatarInfo, bannerInfo ]) { - if (!imageInfo) continue - - const imageOptions = Object.assign({}, imageInfo, { onDisk: false }) - - await updateActorImageInstance(actor, imageOptions, t) - } + await updateActorImageInstance(actor, ActorImageType.AVATAR, avatarInfo, t) + await updateActorImageInstance(actor, ActorImageType.BANNER, bannerInfo, t) await actor.save({ transaction: t }) diff --git a/server/lib/actor-image.ts b/server/lib/actor-image.ts index 59afa93bd..fa1a2a18a 100644 --- a/server/lib/actor-image.ts +++ b/server/lib/actor-image.ts @@ -34,11 +34,10 @@ async function updateLocalActorImageFile ( const actorImageInfo = { name: imageName, fileUrl: null, - type, onDisk: true } - const updatedActor = await updateActorImageInstance(accountOrChannel.Actor, actorImageInfo, t) + const updatedActor = await updateActorImageInstance(accountOrChannel.Actor, type, actorImageInfo, t) await updatedActor.save({ transaction: t }) await sendUpdateActor(accountOrChannel, t) diff --git a/server/middlewares/validators/actor-image.ts b/server/middlewares/validators/actor-image.ts new file mode 100644 index 000000000..961d7a7e5 --- /dev/null +++ b/server/middlewares/validators/actor-image.ts @@ -0,0 +1,30 @@ +import * as express from 'express' +import { body } from 'express-validator' +import { isActorImageFile } from '@server/helpers/custom-validators/actor-images' +import { cleanUpReqFiles } from '../../helpers/express-utils' +import { logger } from '../../helpers/logger' +import { CONSTRAINTS_FIELDS } from '../../initializers/constants' +import { areValidationErrors } from './utils' + +const updateActorImageValidatorFactory = (fieldname: string) => ([ + body(fieldname).custom((value, { req }) => isActorImageFile(req.files, fieldname)).withMessage( + 'This file is not supported or too large. Please, make sure it is of the following type : ' + + CONSTRAINTS_FIELDS.ACTORS.IMAGE.EXTNAME.join(', ') + ), + + (req: express.Request, res: express.Response, next: express.NextFunction) => { + logger.debug('Checking updateActorImageValidator parameters', { files: req.files }) + + if (areValidationErrors(req, res)) return cleanUpReqFiles(req) + + return next() + } +]) + +const updateAvatarValidator = updateActorImageValidatorFactory('avatarfile') +const updateBannerValidator = updateActorImageValidatorFactory('bannerfile') + +export { + updateAvatarValidator, + updateBannerValidator +} diff --git a/server/middlewares/validators/avatar.ts b/server/middlewares/validators/avatar.ts deleted file mode 100644 index f7eb367bd..000000000 --- a/server/middlewares/validators/avatar.ts +++ /dev/null @@ -1,30 +0,0 @@ -import * as express from 'express' -import { body } from 'express-validator' -import { isAvatarFile } from '../../helpers/custom-validators/users' -import { areValidationErrors } from './utils' -import { CONSTRAINTS_FIELDS } from '../../initializers/constants' -import { logger } from '../../helpers/logger' -import { cleanUpReqFiles } from '../../helpers/express-utils' - -const updateActorImageValidatorFactory = (fieldname: string) => ([ - body(fieldname).custom((value, { req }) => isAvatarFile(req.files)).withMessage( - 'This file is not supported or too large. Please, make sure it is of the following type : ' + - CONSTRAINTS_FIELDS.ACTORS.IMAGE.EXTNAME.join(', ') - ), - - (req: express.Request, res: express.Response, next: express.NextFunction) => { - logger.debug('Checking updateActorImageValidator parameters', { files: req.files }) - - if (areValidationErrors(req, res)) return cleanUpReqFiles(req) - - return next() - } -]) - -const updateAvatarValidator = updateActorImageValidatorFactory('avatarfile') -const updateBannerValidator = updateActorImageValidatorFactory('bannerfile') - -export { - updateAvatarValidator, - updateBannerValidator -} diff --git a/server/middlewares/validators/index.ts b/server/middlewares/validators/index.ts index 4086d77aa..24faeea3e 100644 --- a/server/middlewares/validators/index.ts +++ b/server/middlewares/validators/index.ts @@ -1,5 +1,6 @@ export * from './abuse' export * from './account' +export * from './actor-image' export * from './blocklist' export * from './oembed' export * from './activitypub' diff --git a/server/models/video/video-channel.ts b/server/models/video/video-channel.ts index 74885edfb..d2a055f5b 100644 --- a/server/models/video/video-channel.ts +++ b/server/models/video/video-channel.ts @@ -99,7 +99,14 @@ export type SummaryOptions = { } } ] - } + }, + include: [ + { + model: ActorImageModel, + as: 'Banner', + required: false + } + ] }, { model: AccountModel, diff --git a/server/tests/api/check-params/video-channels.ts b/server/tests/api/check-params/video-channels.ts index 0dd436426..bc2e6192e 100644 --- a/server/tests/api/check-params/video-channels.ts +++ b/server/tests/api/check-params/video-channels.ts @@ -234,7 +234,8 @@ describe('Test video channels API validator', function () { }) }) - describe('When updating video channel avatar', function () { + describe('When updating video channel avatar/banner', function () { + const types = [ 'avatar', 'banner' ] let path: string before(async function () { @@ -242,48 +243,57 @@ describe('Test video channels API validator', function () { }) it('Should fail with an incorrect input file', async function () { - const fields = {} - const attaches = { - avatarfile: join(__dirname, '..', '..', 'fixtures', 'video_short.mp4') + for (const type of types) { + const fields = {} + const attaches = { + [type + 'file']: join(__dirname, '..', '..', 'fixtures', 'video_short.mp4') + } + + await makeUploadRequest({ url: server.url, path: `${path}/${type}/pick`, token: server.accessToken, fields, attaches }) } - await makeUploadRequest({ url: server.url, path: path + '/avatar/pick', token: server.accessToken, fields, attaches }) }) it('Should fail with a big file', async function () { - const fields = {} - const attaches = { - avatarfile: join(__dirname, '..', '..', 'fixtures', 'avatar-big.png') + for (const type of types) { + const fields = {} + const attaches = { + [type + 'file']: join(__dirname, '..', '..', 'fixtures', 'avatar-big.png') + } + await makeUploadRequest({ url: server.url, path: `${path}/${type}/pick`, token: server.accessToken, fields, attaches }) } - await makeUploadRequest({ url: server.url, path: path + '/avatar/pick', token: server.accessToken, fields, attaches }) }) it('Should fail with an unauthenticated user', async function () { - const fields = {} - const attaches = { - avatarfile: join(__dirname, '..', '..', 'fixtures', 'avatar.png') + for (const type of types) { + const fields = {} + const attaches = { + [type + 'file']: join(__dirname, '..', '..', 'fixtures', 'avatar.png') + } + await makeUploadRequest({ + url: server.url, + path: `${path}/${type}/pick`, + fields, + attaches, + statusCodeExpected: HttpStatusCode.UNAUTHORIZED_401 + }) } - await makeUploadRequest({ - url: server.url, - path: path + '/avatar/pick', - fields, - attaches, - statusCodeExpected: HttpStatusCode.UNAUTHORIZED_401 - }) }) it('Should succeed with the correct params', async function () { - const fields = {} - const attaches = { - avatarfile: join(__dirname, '..', '..', 'fixtures', 'avatar.png') + for (const type of types) { + const fields = {} + const attaches = { + [type + 'file']: join(__dirname, '..', '..', 'fixtures', 'avatar.png') + } + await makeUploadRequest({ + url: server.url, + path: `${path}/${type}/pick`, + token: server.accessToken, + fields, + attaches, + statusCodeExpected: HttpStatusCode.OK_200 + }) } - await makeUploadRequest({ - url: server.url, - path: path + '/avatar/pick', - token: server.accessToken, - fields, - attaches, - statusCodeExpected: HttpStatusCode.OK_200 - }) }) }) diff --git a/server/tests/api/videos/video-channels.ts b/server/tests/api/videos/video-channels.ts index 367f99fdd..8033b9ba5 100644 --- a/server/tests/api/videos/video-channels.ts +++ b/server/tests/api/videos/video-channels.ts @@ -5,13 +5,14 @@ import * as chai from 'chai' import { cleanupTests, createUser, + deleteVideoChannelImage, doubleFollow, flushAndRunMultipleServers, getVideo, getVideoChannelVideos, testImage, updateVideo, - updateVideoChannelAvatar, + updateVideoChannelImage, uploadVideo, userLogin, wait @@ -21,7 +22,6 @@ import { deleteVideoChannel, getAccountVideoChannelsList, getMyUserInformation, - getVideoChannel, getVideoChannelsList, ServerInfo, setAccessTokensToServers, @@ -33,6 +33,13 @@ import { User, Video, VideoChannel, VideoDetails } from '../../../../shared/inde const expect = chai.expect +async function findChannel (server: ServerInfo, channelId: number) { + const res = await getVideoChannelsList(server.url, 0, 5, '-name') + const videoChannel = res.body.data.find(c => c.id === channelId) + + return videoChannel as VideoChannel +} + describe('Test video channels', function () { let servers: ServerInfo[] let userInfo: User @@ -262,38 +269,85 @@ describe('Test video channels', function () { }) it('Should update video channel avatar', async function () { - this.timeout(5000) + this.timeout(15000) const fixture = 'avatar.png' - await updateVideoChannelAvatar({ + await updateVideoChannelImage({ url: servers[0].url, accessToken: servers[0].accessToken, videoChannelName: 'second_video_channel', - fixture + fixture, + type: 'avatar' }) await waitJobs(servers) + + for (const server of servers) { + const videoChannel = await findChannel(server, secondVideoChannelId) + + await testImage(server.url, 'avatar-resized', videoChannel.avatar.path, '.png') + } }) - it('Should have video channel avatar updated', async function () { + it('Should update video channel banner', async function () { + this.timeout(15000) + + const fixture = 'banner.jpg' + + await updateVideoChannelImage({ + url: servers[0].url, + accessToken: servers[0].accessToken, + videoChannelName: 'second_video_channel', + fixture, + type: 'banner' + }) + + await waitJobs(servers) + for (const server of servers) { - const res = await getVideoChannelsList(server.url, 0, 1, '-name') + const videoChannel = await findChannel(server, secondVideoChannelId) - const videoChannel = res.body.data.find(c => c.id === secondVideoChannelId) + await testImage(server.url, 'banner-resized', videoChannel.banner.path) + } + }) - await testImage(server.url, 'avatar-resized', videoChannel.avatar.path, '.png') + it('Should delete the video channel avatar', async function () { + this.timeout(15000) + + await deleteVideoChannelImage({ + url: servers[0].url, + accessToken: servers[0].accessToken, + videoChannelName: 'second_video_channel', + type: 'avatar' + }) + + await waitJobs(servers) + + for (const server of servers) { + const videoChannel = await findChannel(server, secondVideoChannelId) + + expect(videoChannel.avatar).to.be.null } }) - it('Should get video channel', async function () { - const res = await getVideoChannel(servers[0].url, 'second_video_channel') + it('Should delete the video channel banner', async function () { + this.timeout(15000) + + await deleteVideoChannelImage({ + url: servers[0].url, + accessToken: servers[0].accessToken, + videoChannelName: 'second_video_channel', + type: 'banner' + }) - const videoChannel = res.body - expect(videoChannel.name).to.equal('second_video_channel') - expect(videoChannel.displayName).to.equal('video channel updated') - expect(videoChannel.description).to.equal('video channel description updated') - expect(videoChannel.support).to.equal('video channel support text updated') + await waitJobs(servers) + + for (const server of servers) { + const videoChannel = await findChannel(server, secondVideoChannelId) + + expect(videoChannel.banner).to.be.null + } }) it('Should list the second video channel videos', async function () { diff --git a/server/tests/fixtures/banner-resized.jpg b/server/tests/fixtures/banner-resized.jpg new file mode 100644 index 000000000..13ea422cb Binary files /dev/null and b/server/tests/fixtures/banner-resized.jpg differ diff --git a/server/tests/fixtures/banner.jpg b/server/tests/fixtures/banner.jpg new file mode 100644 index 000000000..e5f284f59 Binary files /dev/null and b/server/tests/fixtures/banner.jpg differ diff --git a/server/typings/express/index.d.ts b/server/typings/express/index.d.ts index ee4faa35d..cf3e7ae34 100644 --- a/server/typings/express/index.d.ts +++ b/server/typings/express/index.d.ts @@ -3,7 +3,6 @@ import { MAbuseMessage, MAbuseReporter, MAccountBlocklist, - MActorFollowActors, MActorFollowActorsDefault, MActorUrl, MChannelBannerAccountDefault, -- cgit v1.2.3