Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixes for Media Library upload #5971

Merged
merged 12 commits into from
Jun 25, 2020
Merged

Fixes for Media Library upload #5971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
43599ef
Fix formatFileInfo
AHgPuK Apr 27, 2020
e0e5674
Fix file name for generated media
AHgPuK Apr 27, 2020
4bff446
Generate hash based on file name without extension
AHgPuK May 12, 2020
a1444b4
Merge branch 'master' into master
alexandrebodin Jun 15, 2020
d7e3bf8
Merge branch 'master' into master
alexandrebodin Jun 15, 2020
3f11f1a
cleanup PR
alexandrebodin Jun 15, 2020
8630f3a
Fix unit tests
Jun 15, 2020
a6dab2d
Remove path delimiter from filename because it is correct character f…
Jun 15, 2020
fa4857a
Fix e2e
Jun 19, 2020
c8935a2
Continue fixes for e2e
AHgPuK Jun 19, 2020
bd8e421
Fix createRequest content type
AHgPuK Jun 20, 2020
868a3dd
Correct thumbnail name in e2e test
AHgPuK Jun 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/strapi-plugin-upload/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
"koa-range": "0.3.0",
"koa-static": "^5.0.0",
"lodash": "^4.17.11",
"mime-types": "2.1.26",
"node-fetch": "2.6.0",
"react": "^16.9.0",
"react-copy-to-clipboard": "^5.0.1",
Expand Down
9 changes: 4 additions & 5 deletions packages/strapi-plugin-upload/services/Upload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ const crypto = require('crypto');
const _ = require('lodash');
const util = require('util');
const { nameToSlug } = require('strapi-utils');
const mime = require('mime-types');

const { bytesToKbytes } = require('../utils/file');

Expand Down Expand Up @@ -43,16 +42,16 @@ const combineFilters = params => {

module.exports = {
formatFileInfo({ filename, type, size }, fileInfo = {}, metas = {}) {
const ext = '.' + mime.extension(type) || path.extname(filename);
const baseName = path.basename(filename, path.extname(filename));
const ext = path.extname(filename);
alexandrebodin marked this conversation as resolved.
Show resolved Hide resolved
const basename = path.basename(fileInfo.name || filename, ext);

const usedName = fileInfo.name || baseName;
const usedName = fileInfo.name || filename;
alexandrebodin marked this conversation as resolved.
Show resolved Hide resolved

const entity = {
name: usedName,
alternativeText: fileInfo.alternativeText,
caption: fileInfo.caption,
hash: generateFileName(usedName),
hash: generateFileName(basename),
ext,
mime: type,
size: bytesToKbytes(size),
Expand Down
12 changes: 6 additions & 6 deletions packages/strapi-plugin-upload/services/__tests__/upload.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('Upload service', () => {
};

expect(uploadService.formatFileInfo(fileData)).toMatchObject({
name: 'File Name',
name: 'File Name.png',
hash: expect.stringContaining('File_Name'),
ext: '.png',
mime: 'image/png',
Expand All @@ -20,13 +20,13 @@ describe('Upload service', () => {

test('Replaces reserved and unsafe characters for URLs and files in hash', () => {
const fileData = {
filename: 'File%&Näme\\<>:"|?*.png',
filename: 'File%&Näme<>:"|?*.png',
type: 'image/png',
size: 1000 * 1000,
};

expect(uploadService.formatFileInfo(fileData)).toMatchObject({
name: 'File%&Näme\\<>:"|?*',
name: 'File%&Näme<>:"|?*.png',
hash: expect.stringContaining('File_and_Naeme'),
ext: '.png',
mime: 'image/png',
Expand All @@ -42,7 +42,7 @@ describe('Upload service', () => {
};

const fileInfo = {
name: 'Custom File Name',
name: 'Custom File Name.png',
};

expect(uploadService.formatFileInfo(fileData, fileInfo)).toMatchObject({
Expand All @@ -67,7 +67,7 @@ describe('Upload service', () => {
};

expect(uploadService.formatFileInfo(fileData, fileInfo)).toMatchObject({
name: 'File Name',
name: 'File Name.png',
caption: fileInfo.caption,
alternativeText: fileInfo.alternativeText,
hash: expect.stringContaining('File_Name'),
Expand All @@ -89,7 +89,7 @@ describe('Upload service', () => {
};

expect(uploadService.formatFileInfo(fileData, {}, fileMetas)).toMatchObject({
name: 'File Name',
name: 'File Name.png',
ext: '.png',
mime: 'image/png',
size: 1000,
Expand Down
2 changes: 2 additions & 0 deletions packages/strapi-plugin-upload/services/image-manipulation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const generateThumbnail = async file => {
const { width, height, size } = await getMetadatas(newBuff);

return {
name: `thumbnail_${file.name}`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the filename might be really strange here as it can be set by the user. What was the need for it here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name attribute should be set for all media files because it goes to the same handler for upload. A user can use file name as a property for a media library in custom plugin.
For example, to provide uniqueness of file names in a library. Only hash for this is not enough because it is random generated every time.

hash: `thumbnail_${file.hash}`,
ext: file.ext,
mime: file.mime,
Expand Down Expand Up @@ -122,6 +123,7 @@ const generateBreakpoint = async (key, { file, breakpoint }) => {
return {
key,
file: {
name: `${key}_${file.name}`,
hash: `${key}_${file.hash}`,
ext: file.ext,
mime: file.mime,
Expand Down
4 changes: 2 additions & 2 deletions packages/strapi-plugin-upload/test/graphqlUpload.test.e2e.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe('Upload plugin end to end tests', () => {
data: {
upload: {
id: expect.anything(),
name: 'rec',
name: 'rec.jpg',
},
},
});
Expand Down Expand Up @@ -122,7 +122,7 @@ describe('Upload plugin end to end tests', () => {
multipleUpload: expect.arrayContaining([
expect.objectContaining({
id: expect.anything(),
name: 'rec',
name: 'rec.jpg',
}),
]),
},
Expand Down
7 changes: 4 additions & 3 deletions packages/strapi-plugin-upload/test/upload.test.e2e.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ describe('Upload plugin end to end tests', () => {
expect(res.body[0]).toEqual(
expect.objectContaining({
id: expect.anything(),
name: 'rec',
ext: '.jpeg',
name: 'rec.jpg',
ext: '.jpg',
mime: 'image/jpeg',
hash: expect.any(String),
size: expect.any(Number),
Expand Down Expand Up @@ -105,7 +105,7 @@ describe('Upload plugin end to end tests', () => {
expect(res.body[0]).toEqual(
expect.objectContaining({
id: expect.anything(),
name: 'thumbnail_target',
name: 'thumbnail_target.png',
ext: '.png',
mime: 'image/png',
hash: expect.any(String),
Expand All @@ -116,6 +116,7 @@ describe('Upload plugin end to end tests', () => {
provider: 'local',
formats: {
thumbnail: {
name: 'thumbnail_thumbnail_target.png',
hash: expect.any(String),
ext: '.png',
mime: 'image/png',
Expand Down
1 change: 1 addition & 0 deletions test/helpers/request.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const createAuthRequest = token => {
return createRequest({
headers: {
Authorization: `Bearer ${token}`,
'Content-Type': 'application/json',
},
});
};
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11825,7 +11825,7 @@ mime-db@1.43.0, "mime-db@>= 1.43.0 < 2":
resolved "https://registry.yarnpkg.com/mime-db/-/mime-db-1.43.0.tgz#0a12e0502650e473d735535050e7c8f4eb4fae58"
integrity sha512-+5dsGEEovYbT8UY9yD7eE4XTc4UwJ1jBYlgaQQF38ENsKR3wj/8q8RFZrF9WIZpB2V1ArTVFUva8sAul1NzRzQ==

mime-types@2.1.26, mime-types@^2.0.8, mime-types@^2.1.12, mime-types@^2.1.18, mime-types@~2.1.17, mime-types@~2.1.19, mime-types@~2.1.24:
mime-types@^2.0.8, mime-types@^2.1.12, mime-types@^2.1.18, mime-types@~2.1.17, mime-types@~2.1.19, mime-types@~2.1.24:
version "2.1.26"
resolved "https://registry.yarnpkg.com/mime-types/-/mime-types-2.1.26.tgz#9c921fc09b7e149a65dfdc0da4d20997200b0a06"
integrity sha512-01paPWYgLrkqAyrlDorC1uDwl2p3qZT7yl806vW7DvDoxwXi46jsjFbg+WdwotBIk6/MbEhO/dh5aZ5sNj/dWQ==
Expand Down