-
Notifications
You must be signed in to change notification settings - Fork 13.3k
test: add tests for image upload with rotation and EXIF #39252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 1 commit
8d10bb5
bcabff3
bff902d
f632ee0
cdcbb13
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| import path from 'path'; | ||
|
|
||
| import type { Credentials } from '@rocket.chat/api-client'; | ||
| import type { ImageAttachmentProps, IMessage, IRoom, IUser, SettingValue } from '@rocket.chat/core-typings'; | ||
| import { expect } from 'chai'; | ||
| import { after, before, describe, it } from 'mocha'; | ||
| import sharp from 'sharp'; | ||
|
|
||
| import { getCredentials, request } from '../../data/api-data'; | ||
| import { uploadFileToRC } from '../../data/file.helper'; | ||
| import { getSettingValueById, updateSetting } from '../../data/permissions.helper'; | ||
| import { createRoom, deleteRoom } from '../../data/rooms.helper'; | ||
| import type { IRequestConfig, TestUser } from '../../data/users.helper'; | ||
| import { createUser, deleteUser, login } from '../../data/users.helper'; | ||
|
|
||
| const testImagePath = path.join(__dirname, '../../mocks/files/exif-orientation-6.jpg'); | ||
| const testImageName = 'exif-orientation-6.jpg'; | ||
|
|
||
| const downloadBuffer = async (url: string, auth: Credentials): Promise<Buffer> => { | ||
| const response = await request.get(url).set(auth).buffer(true).expect(200); | ||
| return response.body as Buffer; | ||
| }; | ||
|
|
||
| describe('[File Upload - Image Rotation]', () => { | ||
| before((done) => getCredentials(done)); | ||
|
|
||
| let user: TestUser<IUser>; | ||
| const userPassword = `pass${Date.now()}`; | ||
| let userCredentials: Credentials; | ||
| let testRoom: IRoom; | ||
| let rotateImagesSetting: SettingValue; | ||
| let stripExifSetting: SettingValue; | ||
| let thumbnailsEnabledSetting: SettingValue; | ||
|
|
||
| before(async () => { | ||
| user = await createUser({ joinDefaultChannels: false, password: userPassword }); | ||
| userCredentials = await login(user.username, userPassword); | ||
| testRoom = (await createRoom({ type: 'p', name: `rotate-upload-${Date.now()}`, members: [user.username] })).body.group; | ||
|
|
||
| rotateImagesSetting = await getSettingValueById('FileUpload_RotateImages'); | ||
| stripExifSetting = await getSettingValueById('Message_Attachments_Strip_Exif'); | ||
| thumbnailsEnabledSetting = await getSettingValueById('Message_Attachments_Thumbnails_Enabled'); | ||
|
|
||
| await updateSetting('FileUpload_RotateImages', true); | ||
| await updateSetting('Message_Attachments_Strip_Exif', true); | ||
| await updateSetting('Message_Attachments_Thumbnails_Enabled', true); | ||
| }); | ||
|
|
||
| after(async () => { | ||
| await Promise.all([ | ||
| updateSetting('FileUpload_RotateImages', rotateImagesSetting), | ||
| updateSetting('Message_Attachments_Strip_Exif', stripExifSetting), | ||
| updateSetting('Message_Attachments_Thumbnails_Enabled', thumbnailsEnabledSetting), | ||
| deleteRoom({ type: 'p', roomId: testRoom._id }), | ||
| deleteUser(user), | ||
jessicaschelly marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ]); | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }); | ||
|
|
||
| it('should rotate pixels, strip EXIF orientation, and generate thumbnail from rotated image', async () => { | ||
| const fixtureMetadata = await sharp(testImagePath).metadata(); | ||
| expect(fixtureMetadata.width).to.equal(719); | ||
| expect(fixtureMetadata.height).to.equal(479); | ||
| expect(fixtureMetadata.orientation).to.equal(6); | ||
|
|
||
| const requestConfig: IRequestConfig = { request, credentials: userCredentials }; | ||
| const { message } = await uploadFileToRC(testRoom._id, testImagePath, 'rotation-exif-test', requestConfig); | ||
| const uploadMessage = message as IMessage; | ||
jessicaschelly marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| expect(uploadMessage).to.have.property('attachments'); | ||
| const attachment = uploadMessage.attachments?.find((item) => item.title === testImageName); | ||
| expect(attachment).to.be.an('object'); | ||
|
|
||
| const fileUrl = (attachment as { title_link?: string }).title_link; | ||
| const thumbUrl = (attachment as ImageAttachmentProps).image_url; | ||
|
|
||
| expect(fileUrl).to.be.a('string'); | ||
| expect(thumbUrl).to.be.a('string'); | ||
|
|
||
| const originalBuffer = await downloadBuffer(fileUrl as string, userCredentials); | ||
| const originalMetadata = await sharp(originalBuffer).metadata(); | ||
|
|
||
| expect(originalMetadata.width).to.equal(479); | ||
| expect(originalMetadata.height).to.equal(719); | ||
| expect(originalMetadata.exif).to.be.undefined; | ||
|
|
||
| const thumbBuffer = await downloadBuffer(thumbUrl as string, userCredentials); | ||
| const thumbMetadata = await sharp(thumbBuffer).metadata(); | ||
|
|
||
| expect(thumbMetadata.width).to.be.lessThan(thumbMetadata.height as number); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe here is more accurate if we assert that
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, yeah, it makes sense.. I kept the orientation-based assertion because the test is more focused on rotation behavior, not thumbnail sizing config
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'm with you then, ignore my comment. It'd be out of the scope of this task to verify whether the thumbnail sizing or config is correct, but if we don't have any test checking it I'd suggest creating a separate task for it
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But as I see here:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agreed! it mentions the orientation should be applied, so we have the assert for the orientation (height less than width = portrait), but not the sizing.. I could add here the size no problem, i was just worried about tying this test with another setting, wdyt?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as you are validating both width and height instead of just orientation above, I thought it could be more accurate to validate in the same way the thumbnail (perhaps adding |
||
| }); | ||
|
|
||
| it('should NOT rotate pixels when FileUpload_RotateImages is disabled', async () => { | ||
| await updateSetting('FileUpload_RotateImages', false); | ||
jessicaschelly marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| const requestConfig: IRequestConfig = { request, credentials: userCredentials }; | ||
| const { message } = await uploadFileToRC(testRoom._id, testImagePath, 'no-rotation-test', requestConfig); | ||
| const uploadMessage = message as IMessage; | ||
|
|
||
| expect(uploadMessage).to.have.property('attachments'); | ||
| const attachment = uploadMessage.attachments?.find((item) => item.title === testImageName); | ||
| expect(attachment).to.be.an('object'); | ||
|
|
||
| const fileUrl = (attachment as { title_link?: string }).title_link; | ||
| expect(fileUrl).to.be.a('string'); | ||
|
|
||
| const originalBuffer = await downloadBuffer(fileUrl as string, userCredentials); | ||
| const originalMetadata = await sharp(originalBuffer).metadata(); | ||
|
|
||
| expect(originalMetadata.width).to.equal(719); | ||
| expect(originalMetadata.height).to.equal(479); | ||
| }); | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.