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/avatar.ts b/server/middlewares/validators/actor-image.ts similarity index 84% rename from server/middlewares/validators/avatar.ts rename to server/middlewares/validators/actor-image.ts index f7eb367bd..961d7a7e5 100644 --- a/server/middlewares/validators/avatar.ts +++ b/server/middlewares/validators/actor-image.ts @@ -1,13 +1,13 @@ 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 { 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 }) => isAvatarFile(req.files)).withMessage( + 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(', ') ), 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) - }) - it('Should have video channel avatar updated', async function () { for (const server of servers) { - const res = await getVideoChannelsList(server.url, 0, 1, '-name') - - const videoChannel = res.body.data.find(c => c.id === secondVideoChannelId) + const videoChannel = await findChannel(server, secondVideoChannelId) await testImage(server.url, 'avatar-resized', videoChannel.avatar.path, '.png') } }) - it('Should get video channel', async function () { - const res = await getVideoChannel(servers[0].url, 'second_video_channel') + it('Should update video channel banner', async function () { + this.timeout(15000) - 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') + 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 videoChannel = await findChannel(server, secondVideoChannelId) + + await testImage(server.url, 'banner-resized', videoChannel.banner.path) + } + }) + + 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 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' + }) + + 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, diff --git a/shared/extra-utils/requests/requests.ts b/shared/extra-utils/requests/requests.ts index 3e773ee03..8b5cddf4a 100644 --- a/shared/extra-utils/requests/requests.ts +++ b/shared/extra-utils/requests/requests.ts @@ -152,11 +152,12 @@ function makeHTMLRequest (url: string, path: string) { .expect(HttpStatusCode.OK_200) } -function updateAvatarRequest (options: { +function updateImageRequest (options: { url: string path: string accessToken: string fixture: string + fieldname: string }) { let filePath = '' if (isAbsolute(options.fixture)) { @@ -170,7 +171,7 @@ function updateAvatarRequest (options: { path: options.path, token: options.accessToken, fields: {}, - attaches: { avatarfile: filePath }, + attaches: { [options.fieldname]: filePath }, statusCodeExpected: HttpStatusCode.OK_200 }) } @@ -191,5 +192,5 @@ export { makePutBodyRequest, makeDeleteRequest, makeRawRequest, - updateAvatarRequest + updateImageRequest } diff --git a/shared/extra-utils/users/users.ts b/shared/extra-utils/users/users.ts index db532dbb0..6040dd9c0 100644 --- a/shared/extra-utils/users/users.ts +++ b/shared/extra-utils/users/users.ts @@ -4,7 +4,7 @@ import { UserUpdateMe } from '../../models/users' import { UserAdminFlag } from '../../models/users/user-flag.model' import { UserRegister } from '../../models/users/user-register.model' import { UserRole } from '../../models/users/user-role' -import { makeGetRequest, makePostBodyRequest, makePutBodyRequest, updateAvatarRequest } from '../requests/requests' +import { makeGetRequest, makePostBodyRequest, makePutBodyRequest, updateImageRequest } from '../requests/requests' import { ServerInfo } from '../server/servers' import { userLogin } from './login' import { HttpStatusCode } from '../../../shared/core-utils/miscs/http-error-codes' @@ -275,7 +275,7 @@ function updateMyAvatar (options: { }) { const path = '/api/v1/users/me/avatar/pick' - return updateAvatarRequest(Object.assign(options, { path })) + return updateImageRequest({ ...options, path, fieldname: 'avatarfile' }) } function updateUser (options: { diff --git a/shared/extra-utils/videos/video-channels.ts b/shared/extra-utils/videos/video-channels.ts index 3ff445c2a..d0dfb5856 100644 --- a/shared/extra-utils/videos/video-channels.ts +++ b/shared/extra-utils/videos/video-channels.ts @@ -3,7 +3,7 @@ import * as request from 'supertest' import { VideoChannelUpdate } from '../../models/videos/channel/video-channel-update.model' import { VideoChannelCreate } from '../../models/videos/channel/video-channel-create.model' -import { makeGetRequest, updateAvatarRequest } from '../requests/requests' +import { makeDeleteRequest, makeGetRequest, updateImageRequest } from '../requests/requests' import { ServerInfo } from '../server/servers' import { User } from '../../models/users/user.model' import { getMyUserInformation } from '../users/users' @@ -129,16 +129,32 @@ function getVideoChannel (url: string, channelName: string) { .expect('Content-Type', /json/) } -function updateVideoChannelAvatar (options: { +function updateVideoChannelImage (options: { url: string accessToken: string fixture: string videoChannelName: string | number + type: 'avatar' | 'banner' }) { + const path = `/api/v1/video-channels/${options.videoChannelName}/${options.type}/pick` - const path = '/api/v1/video-channels/' + options.videoChannelName + '/avatar/pick' + return updateImageRequest({ ...options, path, fieldname: options.type + 'file' }) +} - return updateAvatarRequest(Object.assign(options, { path })) +function deleteVideoChannelImage (options: { + url: string + accessToken: string + videoChannelName: string | number + type: 'avatar' | 'banner' +}) { + const path = `/api/v1/video-channels/${options.videoChannelName}/${options.type}` + + return makeDeleteRequest({ + url: options.url, + token: options.accessToken, + path, + statusCodeExpected: 204 + }) } function setDefaultVideoChannel (servers: ServerInfo[]) { @@ -157,12 +173,13 @@ function setDefaultVideoChannel (servers: ServerInfo[]) { // --------------------------------------------------------------------------- export { - updateVideoChannelAvatar, + updateVideoChannelImage, getVideoChannelsList, getAccountVideoChannelsList, addVideoChannel, updateVideoChannel, deleteVideoChannel, getVideoChannel, - setDefaultVideoChannel + setDefaultVideoChannel, + deleteVideoChannelImage }