From d624a3f232efe86e21de4582716bc305183c58d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 21 May 2019 00:39:26 +0200 Subject: [PATCH 1/5] Write tests for fileUpload --- backend/public/uploads/.gitkeep | 0 backend/src/resolvers/fileUpload/index.js | 27 +++++++++++ backend/src/resolvers/fileUpload/spec.js | 56 +++++++++++++++++++++++ backend/src/resolvers/users.js | 28 +++--------- 4 files changed, 90 insertions(+), 21 deletions(-) create mode 100644 backend/public/uploads/.gitkeep create mode 100644 backend/src/resolvers/fileUpload/index.js create mode 100644 backend/src/resolvers/fileUpload/spec.js diff --git a/backend/public/uploads/.gitkeep b/backend/public/uploads/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/backend/src/resolvers/fileUpload/index.js b/backend/src/resolvers/fileUpload/index.js new file mode 100644 index 000000000..65a48a34d --- /dev/null +++ b/backend/src/resolvers/fileUpload/index.js @@ -0,0 +1,27 @@ +import { createWriteStream } from 'fs' +import path from 'path' +import slug from 'slug' + +const storeUpload = ({ createReadStream, fileLocation}) => + new Promise((resolve, reject) => + createReadStream() + .pipe(createWriteStream(`public${fileLocation}`)) + .on("finish", resolve) + .on("error", reject) + ); + +export default async function fileUpload(params, { file, url}, uploadCallback = storeUpload) { + const upload = params[file] + + if (upload) { + const { createReadStream, filename } = await upload; + const { name } = path.parse(filename) + const fileLocation = `/uploads/${Date.now()}-${slug(name)}` + await uploadCallback({ createReadStream, fileLocation }); + delete params[file] + + params[url] = fileLocation + } + + return params +} diff --git a/backend/src/resolvers/fileUpload/spec.js b/backend/src/resolvers/fileUpload/spec.js new file mode 100644 index 000000000..93e98adfc --- /dev/null +++ b/backend/src/resolvers/fileUpload/spec.js @@ -0,0 +1,56 @@ +import fileUpload from '.' + +describe('fileUpload', () => { + let params + let uploadCallback + + beforeEach(() => { + params = { + uploadAttribute: { + filename: 'avatar.jpg', + mimetype: 'image/jpeg', + encoding: '7bit', + createReadStream: jest.fn() + } + } + uploadCallback = jest.fn() + }) + + it('calls uploadCallback', async () => { + await fileUpload(params, { file: 'uploadAttribute', url: 'attribute'}, uploadCallback) + expect(uploadCallback).toHaveBeenCalled() + }) + + describe('file name', () => { + it('saves the upload url in params[url]', async () => { + await fileUpload(params, { file: 'uploadAttribute', url: 'attribute'}, uploadCallback) + expect(params.attribute).toMatch(/^\/uploads\/\d+-avatar$/) + }) + + it('uses the name without file ending', async () => { + params.uploadAttribute.filename = 'somePng.png' + await fileUpload(params, { file: 'uploadAttribute', url: 'attribute'}, uploadCallback) + expect(params.attribute).toMatch(/^\/uploads\/\d+-somePng/) + }) + + it('creates a url safe name', async () => { + params.uploadAttribute.filename = '/path/to/awkward?/ file-location/?foo- bar-avatar.jpg?foo- bar' + await fileUpload(params, { file: 'uploadAttribute', url: 'attribute'}, uploadCallback) + expect(params.attribute).toMatch(/^\/uploads\/\d+-foo-bar-avatar$/) + }) + + describe('in case of duplicates', () => { + it('creates unique names to avoid overwriting existing files', async () => { + const { attribute: first } = await fileUpload({ + ...params + }, { file: 'uploadAttribute', url: 'attribute'}, uploadCallback) + + await new Promise(resolve => setTimeout(resolve, 1000)); + const { attribute: second } = await fileUpload({ + ...params + }, { file: 'uploadAttribute', url: 'attribute'}, uploadCallback) + expect(first).not.toEqual(second) + }) + }) + }) +}) diff --git a/backend/src/resolvers/users.js b/backend/src/resolvers/users.js index 33ba8c36b..838f45138 100644 --- a/backend/src/resolvers/users.js +++ b/backend/src/resolvers/users.js @@ -1,29 +1,15 @@ import { neo4jgraphql } from 'neo4j-graphql-js' -import { createWriteStream } from 'fs' - - -const storeUpload = ({ stream, fileLocation}) => - new Promise((resolve, reject) => - stream - .pipe(createWriteStream(`public${fileLocation}`)) - .on("finish", () => resolve()) - .on("error", reject) - ); +import fileUpload from './fileUpload' export default { Mutation: { UpdateUser: async (object, params, context, resolveInfo) => { - const { avatarUpload } = params - - if (avatarUpload) { - const { stream, filename } = await avatarUpload ; - const fileLocation = `/uploads/${filename}` - await storeUpload({ stream, fileLocation }); - delete params.avatarUpload - - params.avatar = fileLocation - } + params = await fileUpload(params, { file: 'avatarUpload', url: 'avatar'}) + return await neo4jgraphql(object, params, context, resolveInfo, false) + }, + CreateUser: async (object, params, context, resolveInfo) => { + params = await fileUpload(params, { file: 'avatarUpload', url: 'avatar'}) return await neo4jgraphql(object, params, context, resolveInfo, false) } - }, + } }; From 2b03b515e23472a592cd4c25992ab3801b82b774 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 21 May 2019 01:52:18 +0200 Subject: [PATCH 2/5] Implement image upload for posts --- backend/src/resolvers/posts.js | 7 +++++++ backend/src/schema.graphql | 1 + 2 files changed, 8 insertions(+) diff --git a/backend/src/resolvers/posts.js b/backend/src/resolvers/posts.js index 5b06c38fa..02239eebb 100644 --- a/backend/src/resolvers/posts.js +++ b/backend/src/resolvers/posts.js @@ -1,8 +1,15 @@ import { neo4jgraphql } from 'neo4j-graphql-js' +import fileUpload from './fileUpload' export default { Mutation: { + UpdatePost: async (object, params, context, resolveInfo) => { + params = await fileUpload(params, { file: 'imageUpload', url: 'image'}) + return neo4jgraphql(object, params, context, resolveInfo, false) + }, + CreatePost: async (object, params, context, resolveInfo) => { + params = await fileUpload(params, { file: 'imageUpload', url: 'image'}) const result = await neo4jgraphql(object, params, context, resolveInfo, false) const session = context.driver.session() diff --git a/backend/src/schema.graphql b/backend/src/schema.graphql index d364ba4b3..a581d287c 100644 --- a/backend/src/schema.graphql +++ b/backend/src/schema.graphql @@ -179,6 +179,7 @@ type Post { content: String! contentExcerpt: String image: String + imageUpload: Upload visibility: VisibilityEnum deleted: Boolean disabled: Boolean From 2a22aaa907a408042ff2de9fd019f856197e64e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 21 May 2019 01:57:41 +0200 Subject: [PATCH 3/5] Fix edge case bug in `extractIds` --- .../{extractMentions.js => extractIds/index.js} | 1 + .../{extractMentions.spec.js => extractIds/spec.js} | 10 ++++++++-- backend/src/middleware/notifications/index.js | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) rename backend/src/middleware/notifications/{extractMentions.js => extractIds/index.js} (93%) rename backend/src/middleware/notifications/{extractMentions.spec.js => extractIds/spec.js} (93%) diff --git a/backend/src/middleware/notifications/extractMentions.js b/backend/src/middleware/notifications/extractIds/index.js similarity index 93% rename from backend/src/middleware/notifications/extractMentions.js rename to backend/src/middleware/notifications/extractIds/index.js index f2b28444f..d5b1823f2 100644 --- a/backend/src/middleware/notifications/extractMentions.js +++ b/backend/src/middleware/notifications/extractIds/index.js @@ -2,6 +2,7 @@ import cheerio from 'cheerio' const ID_REGEX = /\/profile\/([\w\-.!~*'"(),]+)/g export default function (content) { + if (!content) return [] const $ = cheerio.load(content) const urls = $('.mention').map((_, el) => { return $(el).attr('href') diff --git a/backend/src/middleware/notifications/extractMentions.spec.js b/backend/src/middleware/notifications/extractIds/spec.js similarity index 93% rename from backend/src/middleware/notifications/extractMentions.spec.js rename to backend/src/middleware/notifications/extractIds/spec.js index 625b1d8fe..73c0ce0a1 100644 --- a/backend/src/middleware/notifications/extractMentions.spec.js +++ b/backend/src/middleware/notifications/extractIds/spec.js @@ -1,6 +1,12 @@ -import extractIds from './extractMentions' +import extractIds from '.' + +describe('extractIds', () => { + describe('content undefined', () => { + it('returns empty array', () => { + expect(extractIds()).toEqual([]) + }) + }) -describe('extract', () => { describe('searches through links', () => { it('ignores links without .mention class', () => { const content = '

Something inspirational about @bob-der-baumeister and @jenny-rostock.

' diff --git a/backend/src/middleware/notifications/index.js b/backend/src/middleware/notifications/index.js index 942eb588d..65cebe253 100644 --- a/backend/src/middleware/notifications/index.js +++ b/backend/src/middleware/notifications/index.js @@ -1,4 +1,4 @@ -import extractIds from './extractMentions' +import extractIds from './extractIds' const notify = async (resolve, root, args, context, resolveInfo) => { // extract user ids before xss-middleware removes link classes From 9dd35a887a174fc0f6d7af2731f2da77e93d9179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 21 May 2019 02:07:43 +0200 Subject: [PATCH 4/5] Fix test --- backend/src/resolvers/users.spec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/src/resolvers/users.spec.js b/backend/src/resolvers/users.spec.js index 48e4741d7..a20bf1dac 100644 --- a/backend/src/resolvers/users.spec.js +++ b/backend/src/resolvers/users.spec.js @@ -68,7 +68,8 @@ describe('users', () => { it('with no name', async () => { const variables = { - id: 'u47' + id: 'u47', + name: null } const expected = 'Username must be at least 3 characters long!' await expect(client.request(mutation, variables)) From 3c22a432e694d743aa520957fcb10996c755bd91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 21 May 2019 03:08:32 +0200 Subject: [PATCH 5/5] Fix lint --- backend/src/middleware/validation/index.js | 2 +- backend/src/resolvers/fileUpload/index.js | 18 +++++++++--------- backend/src/resolvers/fileUpload/spec.js | 14 +++++++------- backend/src/resolvers/posts.js | 4 ++-- backend/src/resolvers/users.js | 10 +++++----- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/backend/src/middleware/validation/index.js b/backend/src/middleware/validation/index.js index ded509b44..5bc8fd02d 100644 --- a/backend/src/middleware/validation/index.js +++ b/backend/src/middleware/validation/index.js @@ -3,7 +3,7 @@ import { UserInputError } from 'apollo-server' const USERNAME_MIN_LENGTH = 3 const validateUsername = async (resolve, root, args, context, info) => { - if (!('name' in args) || args.name && args.name.length >= USERNAME_MIN_LENGTH) { + if (!('name' in args) || (args.name && args.name.length >= USERNAME_MIN_LENGTH)) { /* eslint-disable-next-line no-return-await */ return await resolve(root, args, context, info) } else { diff --git a/backend/src/resolvers/fileUpload/index.js b/backend/src/resolvers/fileUpload/index.js index 65a48a34d..85bdf920b 100644 --- a/backend/src/resolvers/fileUpload/index.js +++ b/backend/src/resolvers/fileUpload/index.js @@ -2,22 +2,22 @@ import { createWriteStream } from 'fs' import path from 'path' import slug from 'slug' -const storeUpload = ({ createReadStream, fileLocation}) => +const storeUpload = ({ createReadStream, fileLocation }) => new Promise((resolve, reject) => - createReadStream() - .pipe(createWriteStream(`public${fileLocation}`)) - .on("finish", resolve) - .on("error", reject) - ); + createReadStream() + .pipe(createWriteStream(`public${fileLocation}`)) + .on('finish', resolve) + .on('error', reject) + ) -export default async function fileUpload(params, { file, url}, uploadCallback = storeUpload) { +export default async function fileUpload (params, { file, url }, uploadCallback = storeUpload) { const upload = params[file] if (upload) { - const { createReadStream, filename } = await upload; + const { createReadStream, filename } = await upload const { name } = path.parse(filename) const fileLocation = `/uploads/${Date.now()}-${slug(name)}` - await uploadCallback({ createReadStream, fileLocation }); + await uploadCallback({ createReadStream, fileLocation }) delete params[file] params[url] = fileLocation diff --git a/backend/src/resolvers/fileUpload/spec.js b/backend/src/resolvers/fileUpload/spec.js index 93e98adfc..798e4f9c5 100644 --- a/backend/src/resolvers/fileUpload/spec.js +++ b/backend/src/resolvers/fileUpload/spec.js @@ -17,25 +17,25 @@ describe('fileUpload', () => { }) it('calls uploadCallback', async () => { - await fileUpload(params, { file: 'uploadAttribute', url: 'attribute'}, uploadCallback) + await fileUpload(params, { file: 'uploadAttribute', url: 'attribute' }, uploadCallback) expect(uploadCallback).toHaveBeenCalled() }) describe('file name', () => { it('saves the upload url in params[url]', async () => { - await fileUpload(params, { file: 'uploadAttribute', url: 'attribute'}, uploadCallback) + await fileUpload(params, { file: 'uploadAttribute', url: 'attribute' }, uploadCallback) expect(params.attribute).toMatch(/^\/uploads\/\d+-avatar$/) }) it('uses the name without file ending', async () => { params.uploadAttribute.filename = 'somePng.png' - await fileUpload(params, { file: 'uploadAttribute', url: 'attribute'}, uploadCallback) + await fileUpload(params, { file: 'uploadAttribute', url: 'attribute' }, uploadCallback) expect(params.attribute).toMatch(/^\/uploads\/\d+-somePng/) }) it('creates a url safe name', async () => { params.uploadAttribute.filename = '/path/to/awkward?/ file-location/?foo- bar-avatar.jpg?foo- bar' - await fileUpload(params, { file: 'uploadAttribute', url: 'attribute'}, uploadCallback) + await fileUpload(params, { file: 'uploadAttribute', url: 'attribute' }, uploadCallback) expect(params.attribute).toMatch(/^\/uploads\/\d+-foo-bar-avatar$/) }) @@ -43,12 +43,12 @@ describe('fileUpload', () => { it('creates unique names to avoid overwriting existing files', async () => { const { attribute: first } = await fileUpload({ ...params - }, { file: 'uploadAttribute', url: 'attribute'}, uploadCallback) + }, { file: 'uploadAttribute', url: 'attribute' }, uploadCallback) - await new Promise(resolve => setTimeout(resolve, 1000)); + await new Promise(resolve => setTimeout(resolve, 1000)) const { attribute: second } = await fileUpload({ ...params - }, { file: 'uploadAttribute', url: 'attribute'}, uploadCallback) + }, { file: 'uploadAttribute', url: 'attribute' }, uploadCallback) expect(first).not.toEqual(second) }) }) diff --git a/backend/src/resolvers/posts.js b/backend/src/resolvers/posts.js index 02239eebb..128a83fc3 100644 --- a/backend/src/resolvers/posts.js +++ b/backend/src/resolvers/posts.js @@ -4,12 +4,12 @@ import fileUpload from './fileUpload' export default { Mutation: { UpdatePost: async (object, params, context, resolveInfo) => { - params = await fileUpload(params, { file: 'imageUpload', url: 'image'}) + params = await fileUpload(params, { file: 'imageUpload', url: 'image' }) return neo4jgraphql(object, params, context, resolveInfo, false) }, CreatePost: async (object, params, context, resolveInfo) => { - params = await fileUpload(params, { file: 'imageUpload', url: 'image'}) + params = await fileUpload(params, { file: 'imageUpload', url: 'image' }) const result = await neo4jgraphql(object, params, context, resolveInfo, false) const session = context.driver.session() diff --git a/backend/src/resolvers/users.js b/backend/src/resolvers/users.js index 838f45138..01beae522 100644 --- a/backend/src/resolvers/users.js +++ b/backend/src/resolvers/users.js @@ -4,12 +4,12 @@ import fileUpload from './fileUpload' export default { Mutation: { UpdateUser: async (object, params, context, resolveInfo) => { - params = await fileUpload(params, { file: 'avatarUpload', url: 'avatar'}) - return await neo4jgraphql(object, params, context, resolveInfo, false) + params = await fileUpload(params, { file: 'avatarUpload', url: 'avatar' }) + return neo4jgraphql(object, params, context, resolveInfo, false) }, CreateUser: async (object, params, context, resolveInfo) => { - params = await fileUpload(params, { file: 'avatarUpload', url: 'avatar'}) - return await neo4jgraphql(object, params, context, resolveInfo, false) + params = await fileUpload(params, { file: 'avatarUpload', url: 'avatar' }) + return neo4jgraphql(object, params, context, resolveInfo, false) } } -}; +}