From 6cbb72f356f458e4a78b3ba1d69c0c39aea71e90 Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Thu, 27 Jul 2017 22:01:18 -0700 Subject: [PATCH 01/24] Refactor `requestHeaders` in `API.js` of github backend --- src/backends/github/API.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index a4157bb80a4d..db36bff9ef81 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -33,17 +33,11 @@ export default class API { } requestHeaders(headers = {}) { - const baseHeader = { - "Content-Type": "application/json", + return { ...headers, + ...(this.token ? { Authorization: `token ${ this.token }` } : {}), + "Content-Type": "application/json", }; - - if (this.token) { - baseHeader.Authorization = `token ${ this.token }`; - return baseHeader; - } - - return baseHeader; } parseJsonResponse(response) { From 51038e5d917aa3118d5df829bc3386bab39db31a Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Thu, 27 Jul 2017 22:02:21 -0700 Subject: [PATCH 02/24] Refactor `urlFor` in `API.js` of github backend --- src/backends/github/API.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index db36bff9ef81..ce535286afea 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -51,17 +51,12 @@ export default class API { } urlFor(path, options) { - const cacheBuster = new Date().getTime(); - const params = [`ts=${cacheBuster}`]; - if (options.params) { - for (const key in options.params) { - params.push(`${ key }=${ encodeURIComponent(options.params[key]) }`); - } - } - if (params.length) { - path += `?${ params.join("&") }`; - } - return this.api_root + path; + const cacheBuster = `ts=${ new Date().getTime() }`; + const encodedParams = options.params + ? Object.entries(options.params).map( + ([key, val]) => `${ key }=${ encodeURIComponent(val) }`) + : []; + return this.api_root + path + `?${ [cacheBuster, ...encodedParams].join("&") }`; } request(path, options = {}) { From ed593761ed99d0744c567eeea6febb77d19a926e Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Wed, 2 Aug 2017 12:25:07 -0700 Subject: [PATCH 03/24] Consolidate filename generation in `storeMetadata` in github backend --- src/backends/github/API.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index ce535286afea..e25ac7465481 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -101,16 +101,14 @@ export default class API { storeMetadata(key, data) { return this.checkMetadataRef() .then((branchData) => { - const fileTree = { - [`${ key }.json`]: { - path: `${ key }.json`, - raw: JSON.stringify(data), - file: true, - }, + const fileName = `${ key }.json`; + const file = { + path: fileName, + raw: JSON.stringify(data), + file: true, }; - - return this.uploadBlob(fileTree[`${ key }.json`]) - .then(item => this.updateTree(branchData.sha, "/", fileTree)) + return this.uploadBlob(file) + .then(uploadedFile => this.updateTree(branchData.sha, "/", { [fileName]: uploadedFile })) .then(changeTree => this.commit(`Updating “${ key }” metadata`, changeTree)) .then(response => this.patchRef("meta", "_netlify_cms", response.sha)) .then(() => { From 9e22a15c0d9d0cfd6501d8b4070c239a1f4414eb Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Thu, 27 Jul 2017 22:04:13 -0700 Subject: [PATCH 04/24] Refactor request error handling in `API.js` of github backend - Removes `parseJsonResponse` - Errors on non-ok responses for text responses as well as JSON responses - Adds an optional `meta` field to `APIError` in `src/valueObjects/errors/APIError.js` - Passes github error responses to `APIErrors` generated by `request` in `API.js` of github backend --- src/backends/github/API.js | 22 ++++++---------------- src/valueObjects/errors/APIError.js | 3 ++- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index e25ac7465481..d0f8647267d6 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -40,16 +40,6 @@ export default class API { }; } - parseJsonResponse(response) { - return response.json().then((json) => { - if (!response.ok) { - return Promise.reject(json); - } - - return json; - }); - } - urlFor(path, options) { const cacheBuster = `ts=${ new Date().getTime() }`; const encodedParams = options.params @@ -62,17 +52,17 @@ export default class API { request(path, options = {}) { const headers = this.requestHeaders(options.headers || {}); const url = this.urlFor(path, options); - let responseStatus; return fetch(url, { ...options, headers }).then((response) => { - responseStatus = response.status; const contentType = response.headers.get("Content-Type"); if (contentType && contentType.match(/json/)) { - return this.parseJsonResponse(response); + return Promise.all([response, response.json()]); } - return response.text(); + return Promise.all([response, response.text()]); }) - .catch((error) => { - throw new APIError(error.message, responseStatus, 'GitHub'); + .then(([response, value]) => (response.ok ? value : Promise.reject([value, response]))) + .catch(([errorValue, response]) => { + const message = _.isString(errorValue) ? errorValue : errorValue.message; + throw new APIError(message, response.status, 'GitHub', { response }); }); } diff --git a/src/valueObjects/errors/APIError.js b/src/valueObjects/errors/APIError.js index 05db8b5cc4c0..fc45bea2d108 100644 --- a/src/valueObjects/errors/APIError.js +++ b/src/valueObjects/errors/APIError.js @@ -1,11 +1,12 @@ export const API_ERROR = 'API_ERROR'; export default class APIError extends Error { - constructor(message, status, api) { + constructor(message, status, api, meta={}) { super(message); this.message = message; this.status = status; this.api = api; this.name = API_ERROR; + this.meta = meta; } } From 732ede1d6d1181a452cbc46812db2dee089fa03e Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Fri, 28 Jul 2017 15:53:26 -0700 Subject: [PATCH 05/24] Refactor mutation and race conditions in github API Functions refactored: - `storeMetadata` - `composeFileTree` - `persistFiles` - `uploadBlob` These refactors had to be done together, since each the methods refactored had implicit dependencies on mutations made by the others. Specifically, `composeFileTree` and `uploadBlob` had to be called in a specific order to mutate their arguments correctly. This led to a race condition in `persistFiles` that was (fortunately) only affecting debugging. This commit removes the mutation and makes the functions more independent, in addition to making general readability and code length improvements to the affected functions. `composeFileTree` has also had its filtering of uploaded files removed, as that was a concern better handled by the caller. --- src/backends/github/API.js | 64 ++++++++++++-------------------------- 1 file changed, 20 insertions(+), 44 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index d0f8647267d6..4a0b13672134 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -1,6 +1,7 @@ import LocalForage from "localforage"; import { Base64 } from "js-base64"; import _ from "lodash"; +import { Map } from 'immutable'; import { filterPromises, resolvePromiseProperties } from "../../lib/promiseHelper"; import AssetProxy from "../../valueObjects/AssetProxy"; import { SIMPLE, EDITORIAL_WORKFLOW, status } from "../../constants/publishModes"; @@ -206,50 +207,26 @@ export default class API { } composeFileTree(files) { - let filename; - let part; - let parts; - let subtree; - const fileTree = {}; - - files.forEach((file) => { - if (file.uploaded) { return; } - parts = file.path.split("/").filter(part => part); - filename = parts.pop(); - subtree = fileTree; - while (part = parts.shift()) { - subtree[part] = subtree[part] || {}; - subtree = subtree[part]; - } - subtree[filename] = file; - file.file = true; - }); - - return fileTree; + return files + .map(file => ({ ...file, file: true })) + .reduce((tree, file) => tree.setIn(file.path.split("/"), file), Map()) + .toJS(); } persistFiles(entry, mediaFiles, options) { - const uploadPromises = []; - const files = mediaFiles.concat(entry); - - files.forEach((file) => { - if (file.uploaded) { return; } - uploadPromises.push(this.uploadBlob(file)); - }); - - const fileTree = this.composeFileTree(files); + const newFiles = [...mediaFiles, entry].filter(file => !file.uploaded); + const uploadsPromise = Promise.all(newFiles.map(file => this.uploadBlob(file))); + const fileTreePromise = uploadsPromise.then(files => this.composeFileTree(files)); + + if (options.mode === EDITORIAL_WORKFLOW) { + const mediaFilesList = mediaFiles.map(file => ({ path: file.path, sha: file.sha })); + return fileTreePromise + .then(fileTree => this.editorialWorkflowGit(fileTree, entry, mediaFilesList, options)); + } - return Promise.all(uploadPromises).then(() => { - if (!options.mode || (options.mode && options.mode === SIMPLE)) { - return this.getBranch() - .then(branchData => this.updateTree(branchData.commit.sha, "/", fileTree)) - .then(changeTree => this.commit(options.commitMessage, changeTree)) - .then(response => this.patchBranch(this.branch, response.sha)); - } else if (options.mode && options.mode === EDITORIAL_WORKFLOW) { - const mediaFilesList = mediaFiles.map(file => ({ path: file.path, sha: file.sha })); - return this.editorialWorkflowGit(fileTree, entry, mediaFilesList, options); - } - }); + return Promise.all([fileTreePromise, this.getBranch()]) + .then(([fileTree, branchData]) => this.updateTree(branchData.commit.sha, "/", fileTree)) + .then(changeTree => this.commit(options.commitMessage, changeTree)); } deleteFile(path, message, options={}) { @@ -484,10 +461,9 @@ export default class API { content: contentBase64, encoding: "base64", }), - }).then((response) => { - item.sha = response.sha; - item.uploaded = true; - return item; + })).then(response => Object.assign({}, item, { + sha: response.sha, + uploaded: true, })); } From 7ee2a2ef9a0c2f9b9447a72c79e56a893c64f052 Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Fri, 28 Jul 2017 16:01:27 -0700 Subject: [PATCH 06/24] Refactor `forceMergePR` in github API --- src/backends/github/API.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index 4a0b13672134..644d00b272fa 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -431,10 +431,12 @@ export default class API { forceMergePR(pullrequest, objects) { const files = objects.files.concat(objects.entry); const fileTree = this.composeFileTree(files); - let commitMessage = "Automatically generated. Merged on Netlify CMS\n\nForce merge of:"; - files.forEach((file) => { - commitMessage += `\n* "${ file.path }"`; - }); + const commitMessage = `\ +Automatically generated. Merged on Netlify CMS + +Force merge of: +${ files.map(file => `* "${ file.path }"`).join("\n") }\ +`; console.log("%c Automatic merge not possible - Forcing merge.", "line-height: 30px;text-align: center;font-weight: bold"); // eslint-disable-line return this.getBranch() .then(branchData => this.updateTree(branchData.commit.sha, "/", fileTree)) From 36c97d292536eec74969e48664dbd7f7f844ccc9 Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Tue, 1 Aug 2017 20:09:29 -0700 Subject: [PATCH 07/24] Refactor `isCollaborator` in github API --- src/backends/github/API.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index 644d00b272fa..0e004020e2d0 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -21,16 +21,11 @@ export default class API { } isCollaborator(user) { - return this.request('/user/repos').then((repos) => { - let contributor = false - for (const repo of repos) { - if (repo.full_name === this.repo && repo.permissions.push) contributor = true; - } - return contributor; - }).catch((error) => { - console.error("Problem with response of /user/repos from GitHub"); - throw error; - }) + return this.request('/user/repos').then(repos => + Object.keys(repos).some( + key => (repos[key].full_name === this.repo) && repos[key].permissions.push + ) + ); } requestHeaders(headers = {}) { From 232854a3066cd205f3cb421cc7650535eeafa2a5 Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Wed, 2 Aug 2017 12:12:03 -0700 Subject: [PATCH 08/24] Refactor `updateTree` in github API --- src/backends/github/API.js | 48 +++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index 0e004020e2d0..5ebd4306503f 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -464,35 +464,29 @@ ${ files.map(file => `* "${ file.path }"`).join("\n") }\ })); } + isFile(obj) { + return obj.file; + } + updateTree(sha, path, fileTree) { return this.getTree(sha) - .then((tree) => { - let obj; - let filename; - let fileOrDir; - const updates = []; - const added = {}; - - for (let i = 0, len = tree.tree.length; i < len; i++) { - obj = tree.tree[i]; - if (fileOrDir = fileTree[obj.path]) { - added[obj.path] = true; - if (fileOrDir.file) { - updates.push({ path: obj.path, mode: obj.mode, type: obj.type, sha: fileOrDir.sha }); - } else { - updates.push(this.updateTree(obj.sha, obj.path, fileOrDir)); - } - } - } - for (filename in fileTree) { - fileOrDir = fileTree[filename]; - if (added[filename]) { continue; } - updates.push( - fileOrDir.file ? - { path: filename, mode: "100644", type: "blob", sha: fileOrDir.sha } : - this.updateTree(null, filename, fileOrDir) - ); - } + .then(({ tree: dirContents }) => { + const updatedItems = dirContents.filter(item => fileTree[item.path]); + const added = _.zipObject(updatedItems.map(item => item.path), Array(updatedItems.length).fill(true)); + const [updatedFiles, updatedDirs] = _.partition(updatedItems, this.isFile); + const updatePromises = [ + ...updatedDirs.map(dir => this.updateTree(dir.sha, dir.path, fileTree[dir.path])), + ...updatedFiles.map(file => ({ ..._.pick(file, ['path', 'mode', 'type']), sha: fileTree[file.path].sha })), + ]; + + const newItems = _.entries(fileTree).filter(([filename]) => !added[filename]); + const [newFiles, newDirs] = _.partition(newItems, ([, file]) => this.isFile(file)); + const newPromises = [ + ...newDirs.map(([dirName, dir]) => this.updateTree(null, dirName, dir)), + ...newFiles.map(([fileName, file]) => ({ path: fileName, mode: "100644", type: "blob", sha: file.sha })), + ]; + + const updates = [...updatePromises, ...newPromises]; return Promise.all(updates) .then(updates => this.request(`${ this.repoURL }/git/trees`, { method: "POST", From ee02ca96a4984138f4b06fdc4bc0be2c5229308d Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Wed, 2 Aug 2017 12:36:36 -0700 Subject: [PATCH 09/24] Import lodash functions individually --- src/backends/github/API.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index 5ebd4306503f..ad936aaebeb9 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -1,6 +1,6 @@ import LocalForage from "localforage"; import { Base64 } from "js-base64"; -import _ from "lodash"; +import { entries, isString, partition, pick, uniq, zipObject } from "lodash"; import { Map } from 'immutable'; import { filterPromises, resolvePromiseProperties } from "../../lib/promiseHelper"; import AssetProxy from "../../valueObjects/AssetProxy"; @@ -57,7 +57,7 @@ export default class API { }) .then(([response, value]) => (response.ok ? value : Promise.reject([value, response]))) .catch(([errorValue, response]) => { - const message = _.isString(errorValue) ? errorValue : errorValue.message; + const message = isString(errorValue) ? errorValue : errorValue.message; throw new APIError(message, response.status, 'GitHub', { response }); }); } @@ -301,7 +301,7 @@ export default class API { path: entry.path, sha: entry.sha, }, - files: _.uniq(files), + files: uniq(files), }, timeStamp: new Date().toISOString(), }; @@ -472,15 +472,15 @@ ${ files.map(file => `* "${ file.path }"`).join("\n") }\ return this.getTree(sha) .then(({ tree: dirContents }) => { const updatedItems = dirContents.filter(item => fileTree[item.path]); - const added = _.zipObject(updatedItems.map(item => item.path), Array(updatedItems.length).fill(true)); - const [updatedFiles, updatedDirs] = _.partition(updatedItems, this.isFile); + const added = zipObject(updatedItems.map(item => item.path), Array(updatedItems.length).fill(true)); + const [updatedFiles, updatedDirs] = partition(updatedItems, this.isFile); const updatePromises = [ ...updatedDirs.map(dir => this.updateTree(dir.sha, dir.path, fileTree[dir.path])), - ...updatedFiles.map(file => ({ ..._.pick(file, ['path', 'mode', 'type']), sha: fileTree[file.path].sha })), + ...updatedFiles.map(file => ({ ...pick(file, ['path', 'mode', 'type']), sha: fileTree[file.path].sha })), ]; - const newItems = _.entries(fileTree).filter(([filename]) => !added[filename]); - const [newFiles, newDirs] = _.partition(newItems, ([, file]) => this.isFile(file)); + const newItems = entries(fileTree).filter(([filename]) => !added[filename]); + const [newFiles, newDirs] = partition(newItems, ([, file]) => this.isFile(file)); const newPromises = [ ...newDirs.map(([dirName, dir]) => this.updateTree(null, dirName, dir)), ...newFiles.map(([fileName, file]) => ({ path: fileName, mode: "100644", type: "blob", sha: file.sha })), From a3930c7de30dd0c2cbf56f5e0758b6fea713d362 Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Wed, 2 Aug 2017 13:40:01 -0700 Subject: [PATCH 10/24] Minor refactors in github API --- src/backends/github/API.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index ad936aaebeb9..f36716259eb3 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -231,11 +231,7 @@ export default class API { return this.request(fileURL) .then(({ sha }) => this.request(fileURL, { method: "DELETE", - params: { - sha, - message, - branch, - }, + params: { sha, message, branch }, })); } @@ -315,10 +311,7 @@ export default class API { updateUnpublishedEntryStatus(collection, slug, status) { const contentKey = slug; return this.retrieveMetadata(contentKey) - .then(metadata => ({ - ...metadata, - status, - })) + .then(metadata => ({ ...metadata, status })) .then(updatedMetadata => this.storeMetadata(contentKey, updatedMetadata)); } @@ -340,7 +333,6 @@ export default class API { publishUnpublishedEntry(collection, slug) { const contentKey = slug; - let prNumber; return this.retrieveMetadata(contentKey) .then(metadata => this.mergePR(metadata.pr, metadata.objects)) .then(() => this.deleteBranch(`cms/${ contentKey }`)); From 2617acc13b6e02ca04abd782c2c6068a9afa5f47 Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Wed, 2 Aug 2017 16:30:40 -0700 Subject: [PATCH 11/24] Refactor `editorialWorkflowGit` in github API --- src/backends/github/API.js | 113 ++++++++++++++++--------------------- 1 file changed, 48 insertions(+), 65 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index f36716259eb3..ad13318d50f6 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -239,73 +239,56 @@ export default class API { const contentKey = entry.slug; const branchName = `cms/${ contentKey }`; const unpublished = options.unpublished || false; - if (!unpublished) { - // Open new editorial review workflow for this entry - Create new metadata and commit to new branch` - const contentKey = entry.slug; - const branchName = `cms/${ contentKey }`; - return this.getBranch() + const commitPromise = this.getBranch() .then(branchData => this.updateTree(branchData.commit.sha, "/", fileTree)) - .then(changeTree => this.commit(options.commitMessage, changeTree)) - .then(commitResponse => this.createBranch(branchName, commitResponse.sha)) - .then(branchResponse => this.createPR(options.commitMessage, branchName)) - .then(prResponse => this.user().then(user => user.name ? user.name : user.login) - .then(username => this.storeMetadata(contentKey, { - type: "PR", - pr: { - number: prResponse.number, - head: prResponse.head && prResponse.head.sha, - }, - user: username, - status: status.first(), - branch: branchName, - collection: options.collectionName, - title: options.parsedData && options.parsedData.title, - description: options.parsedData && options.parsedData.description, - objects: { - entry: { - path: entry.path, - sha: entry.sha, - }, - files: filesList, - }, - timeStamp: new Date().toISOString(), - } - ))); - } else { - // Entry is already on editorial review workflow - just update metadata and commit to existing branch - return this.getBranch(branchName) - .then(branchData => this.updateTree(branchData.commit.sha, "/", fileTree)) - .then(changeTree => this.commit(options.commitMessage, changeTree)) - .then((response) => { - const contentKey = entry.slug; - const branchName = `cms/${ contentKey }`; - return this.user().then(user => user.name ? user.name : user.login) - .then(username => this.retrieveMetadata(contentKey)) - .then((metadata) => { - let files = metadata.objects && metadata.objects.files || []; - files = files.concat(filesList); - const updatedPR = metadata.pr; - updatedPR.head = response.sha; - return { - ...metadata, - pr: updatedPR, - title: options.parsedData && options.parsedData.title, - description: options.parsedData && options.parsedData.description, - objects: { - entry: { - path: entry.path, - sha: entry.sha, - }, - files: uniq(files), - }, - timeStamp: new Date().toISOString(), - }; - }) - .then(updatedMetadata => this.storeMetadata(contentKey, updatedMetadata)) - .then(this.patchBranch(branchName, response.sha)); - }); - } + .then(changeTree => this.commit(options.commitMessage, changeTree)); + + const usernamePromise = this.user().then(user => (user.name ? user.name : user.login)); + + const initialMetadata = { + title: options.parsedData && options.parsedData.title, + description: options.parsedData && options.parsedData.description, + timeStamp: new Date().toISOString(), + }; + + return (unpublished + + // Entry is already on editorial review workflow - just update + // metadata and commit to existing branch + ? this.retrieveMetadata(contentKey).then(existingMetadata => resolvePromiseProperties({ + ...existingMetadata, + ...initialMetadata, + pr: commitPromise.then(commit => ({ ...existingMetadata.pr, head: commit.sha })), + objects: { + entry: pick(entry, ['path', 'sha']), + files: uniq( + ((existingMetadata.objects && existingMetadata.objects.files) || []).concat(filesList) + ), + }, + })) + .then(newMetadata => Promise.all([newMetadata, commitPromise])) + .then(([newMetadata, commit]) => + this.patchBranch(branchName, commit.sha).then(() => newMetadata) + ) + + // Open new editorial review workflow for this entry - Create new + // metadata and commit to new branch + : resolvePromiseProperties({ + ...initialMetadata, + type: "PR", + pr: commitPromise + .then(commit => this.createBranch(branchName, commit.sha)) + .then(() => this.createPR(options.commitMessage, branchName)) + .then(pr => ({ number: pr.number, head: pr.head && pr.head.sha })), + user: usernamePromise, + status: status.first(), + branch: branchName, + collection: options.collectionName, + objects: { entry: pick(entry, ['path', 'sha']), files: filesList }, + }) + + ).then(metadata => this.storeMetadata(contentKey, metadata)); } updateUnpublishedEntryStatus(collection, slug, status) { From 83514199be6dec095bf59bda6580073e559a2258 Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Wed, 2 Aug 2017 16:40:09 -0700 Subject: [PATCH 12/24] Fix resolvePromiseProperties bug when object prop set to undefined --- src/lib/promiseHelper.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/promiseHelper.js b/src/lib/promiseHelper.js index 0d16bd5ec8d4..ee353d36a8fd 100644 --- a/src/lib/promiseHelper.js +++ b/src/lib/promiseHelper.js @@ -7,7 +7,7 @@ export const filterPromises = (arr, filter) => export const resolvePromiseProperties = (obj) => { // Get the keys which represent promises const promiseKeys = Object.keys(obj).filter( - key => typeof obj[key].then === "function"); + key => obj[key] && typeof obj[key].then === "function"); const promises = promiseKeys.map(key => obj[key]); From c49131217809f3e764846e9b56bbe321f1c4e66e Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Thu, 3 Aug 2017 15:36:02 -0700 Subject: [PATCH 13/24] Small refactors to github API --- src/backends/github/API.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index ad13318d50f6..63e746a87d9f 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -70,7 +70,10 @@ export default class API { .catch((error) => { // Meta ref doesn't exist const readme = { - raw: "# Netlify CMS\n\nThis tree is used by the Netlify CMS to store metadata information for specific files and branches.", + raw: `\ +# Netlify CMS + +This tree is used by the Netlify CMS to store metadata information for specific files and branches.`, }; return this.uploadBlob(readme) @@ -143,7 +146,7 @@ export default class API { return this.request(`${ this.repoURL }/contents/${ path }`, { params: { ref: this.branch }, }) - .then(files => { + .then((files) => { if (!Array.isArray(files)) { throw new Error(`Cannot list files, path ${path} is not a directory but a ${files.type}`); } @@ -169,7 +172,7 @@ export default class API { isUnpublishedEntryModification(path, branch) { return this.readFile(path, null, branch) - .then(data => true) + .then(() => true) .catch((err) => { if (err.message && err.message === "Not Found") { return false; @@ -419,9 +422,7 @@ ${ files.map(file => `* "${ file.path }"`).join("\n") }\ } toBase64(str) { - return Promise.resolve( - Base64.encode(str) - ); + return Promise.resolve(Base64.encode(str)); } uploadBlob(item) { From e3c1f60733b6f27271ec8f02e1fc69c505e8431c Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Fri, 4 Aug 2017 14:15:41 -0700 Subject: [PATCH 14/24] Fix request error handling in github API --- src/backends/github/API.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index 63e746a87d9f..e086b9ffd3b7 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -48,17 +48,21 @@ export default class API { request(path, options = {}) { const headers = this.requestHeaders(options.headers || {}); const url = this.urlFor(path, options); - return fetch(url, { ...options, headers }).then((response) => { + return fetch(url, { ...options, headers }) + .then((response) => { const contentType = response.headers.get("Content-Type"); if (contentType && contentType.match(/json/)) { return Promise.all([response, response.json()]); } return Promise.all([response, response.text()]); }) + .catch(err => [err, null]) .then(([response, value]) => (response.ok ? value : Promise.reject([value, response]))) .catch(([errorValue, response]) => { - const message = isString(errorValue) ? errorValue : errorValue.message; - throw new APIError(message, response.status, 'GitHub', { response }); + const message = (errorValue && errorValue.message) + ? errorValue.message + : (isString(errorValue) ? errorValue : ""); + throw new APIError(message, response && response.status, 'GitHub', { response, errorValue }); }); } From c9edfdd5aaeb83575cb274ac461375ff03c29a60 Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Fri, 11 Aug 2017 11:59:01 -0700 Subject: [PATCH 15/24] Fix remaining eslint errors in github API --- src/backends/github/API.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/backends/github/API.js b/src/backends/github/API.js index e086b9ffd3b7..9d248cddf934 100644 --- a/src/backends/github/API.js +++ b/src/backends/github/API.js @@ -4,7 +4,7 @@ import { entries, isString, partition, pick, uniq, zipObject } from "lodash"; import { Map } from 'immutable'; import { filterPromises, resolvePromiseProperties } from "../../lib/promiseHelper"; import AssetProxy from "../../valueObjects/AssetProxy"; -import { SIMPLE, EDITORIAL_WORKFLOW, status } from "../../constants/publishModes"; +import { EDITORIAL_WORKFLOW, status } from "../../constants/publishModes"; import { APIError, EditorialWorkflowError } from "../../valueObjects/errors"; export default class API { @@ -42,7 +42,7 @@ export default class API { ? Object.entries(options.params).map( ([key, val]) => `${ key }=${ encodeURIComponent(val) }`) : []; - return this.api_root + path + `?${ [cacheBuster, ...encodedParams].join("&") }`; + return `${ this.api_root }${ path }?${ [cacheBuster, ...encodedParams].join("&") }`; } request(path, options = {}) { @@ -59,9 +59,8 @@ export default class API { .catch(err => [err, null]) .then(([response, value]) => (response.ok ? value : Promise.reject([value, response]))) .catch(([errorValue, response]) => { - const message = (errorValue && errorValue.message) - ? errorValue.message - : (isString(errorValue) ? errorValue : ""); + const errorMessageProp = (errorValue && errorValue.message) ? errorValue.message : null; + const message = errorMessageProp || (isString(errorValue) ? errorValue : ""); throw new APIError(message, response && response.status, 'GitHub', { response, errorValue }); }); } @@ -152,7 +151,7 @@ This tree is used by the Netlify CMS to store metadata information for specific }) .then((files) => { if (!Array.isArray(files)) { - throw new Error(`Cannot list files, path ${path} is not a directory but a ${files.type}`); + throw new Error(`Cannot list files, path ${ path } is not a directory but a ${ files.type }`); } return files; }) @@ -374,7 +373,6 @@ This tree is used by the Netlify CMS to store metadata information for specific } closePR(pullrequest, objects) { - const headSha = pullrequest.head; const prNumber = pullrequest.number; console.log("%c Deleting PR", "line-height: 30px;text-align: center;font-weight: bold"); // eslint-disable-line return this.request(`${ this.repoURL }/pulls/${ prNumber }`, { @@ -468,7 +466,7 @@ ${ files.map(file => `* "${ file.path }"`).join("\n") }\ const updates = [...updatePromises, ...newPromises]; return Promise.all(updates) - .then(updates => this.request(`${ this.repoURL }/git/trees`, { + .then(resolvedUpdates => this.request(`${ this.repoURL }/git/trees`, { method: "POST", body: JSON.stringify({ base_tree: sha, tree: updates }), })).then(response => ({ path, mode: "040000", type: "tree", sha: response.sha, parentSha: sha })); From 7faf612562fa2b89cae381de2047940beb8d126f Mon Sep 17 00:00:00 2001 From: Benaiah Mischenko Date: Fri, 11 Aug 2017 12:04:36 -0700 Subject: [PATCH 16/24] Small refactors & eslint fixes in github implementation --- src/backends/github/implementation.js | 48 ++++++++++++--------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/src/backends/github/implementation.js b/src/backends/github/implementation.js index 116f678971b4..f929c4b4a98b 100644 --- a/src/backends/github/implementation.js +++ b/src/backends/github/implementation.js @@ -1,7 +1,7 @@ import semaphore from "semaphore"; import AuthenticationPage from "./AuthenticationPage"; import API from "./API"; -import { fileExtension } from '../../lib/pathHelper' +import { fileExtension } from '../../lib/pathHelper'; const MAX_CONCURRENT_DOWNLOADS = 10; @@ -36,8 +36,7 @@ export default class GitHub { // Unauthorized user if (!isCollab) throw new Error("Your GitHub user account does not have access to this repo."); // Authorized user - user.token = state.token; - return user; + return Object.assign({}, user, { token: state.token }); }) ); } @@ -96,31 +95,28 @@ export default class GitHub { unpublishedEntries() { return this.api.listUnpublishedBranches().then((branches) => { const sem = semaphore(MAX_CONCURRENT_DOWNLOADS); - const promises = []; - branches.map((branch) => { - promises.push(new Promise((resolve, reject) => { - const slug = branch.ref.split("refs/heads/cms/").pop(); - return sem.take(() => this.api.readUnpublishedBranchFile(slug).then((data) => { - if (data === null || data === undefined) { - resolve(null); - sem.leave(); - } else { - const path = data.metaData.objects.entry.path; - resolve({ - slug, - file: { path }, - data: data.fileData, - metaData: data.metaData, - isModification: data.isModification, - }); - sem.leave(); - } - }).catch((err) => { - sem.leave(); + const promises = branches.map(branch => new Promise((resolve, reject) => { + const slug = branch.ref.split("refs/heads/cms/").pop(); + return sem.take(() => this.api.readUnpublishedBranchFile(slug).then((data) => { + if (data === null || data === undefined) { resolve(null); - })); + sem.leave(); + } else { + const path = data.metaData.objects.entry.path; + resolve({ + slug, + file: { path }, + data: data.fileData, + metaData: data.metaData, + isModification: data.isModification, + }); + sem.leave(); + } + }).catch((err) => { + sem.leave(); + resolve(null); })); - }); + })); return Promise.all(promises); }) .catch((error) => { From 2c6d3a1115085c86c2b0e5f26630e62a1d390c03 Mon Sep 17 00:00:00 2001 From: Caleb Date: Fri, 4 Aug 2017 16:25:19 -0600 Subject: [PATCH 17/24] Initial GitLab commit. --- src/backends/backend.js | 3 + src/backends/gitlab/API.js | 162 +++++++++++++++++++++ src/backends/gitlab/AuthenticationPage.css | 12 ++ src/backends/gitlab/AuthenticationPage.js | 50 +++++++ src/backends/gitlab/implementation.js | 100 +++++++++++++ 5 files changed, 327 insertions(+) create mode 100644 src/backends/gitlab/API.js create mode 100644 src/backends/gitlab/AuthenticationPage.css create mode 100644 src/backends/gitlab/AuthenticationPage.js create mode 100644 src/backends/gitlab/implementation.js diff --git a/src/backends/backend.js b/src/backends/backend.js index 1f0276288bf8..9ff9e50b8914 100644 --- a/src/backends/backend.js +++ b/src/backends/backend.js @@ -1,6 +1,7 @@ import { attempt, isError } from 'lodash'; import TestRepoBackend from "./test-repo/implementation"; import GitHubBackend from "./github/implementation"; +import GitLabBackend from "./gitlab/implementation"; import NetlifyAuthBackend from "./netlify-auth/implementation"; import { resolveFormat } from "../formats/formats"; import { selectListMethod, selectEntrySlug, selectEntryPath, selectAllowNewEntries, selectFolderEntryExtension } from "../reducers/collections"; @@ -292,6 +293,8 @@ export function resolveBackend(config) { return new Backend(new TestRepoBackend(config), authStore); case "github": return new Backend(new GitHubBackend(config), authStore); + case "gitlab": + return new Backend(new GitLabBackend(config), authStore); case "netlify-auth": return new Backend(new NetlifyAuthBackend(config), authStore); default: diff --git a/src/backends/gitlab/API.js b/src/backends/gitlab/API.js new file mode 100644 index 000000000000..37f2dcad5a24 --- /dev/null +++ b/src/backends/gitlab/API.js @@ -0,0 +1,162 @@ +import LocalForage from "localforage"; +import { Base64 } from "js-base64"; +import { fill, isString } from "lodash"; +import AssetProxy from "../../valueObjects/AssetProxy"; +import { SIMPLE, EDITORIAL_WORKFLOW, status } from "../../constants/publishModes"; +import { APIError, EditorialWorkflowError } from "../../valueObjects/errors"; + +export default class API { + constructor(config) { + this.api_root = config.api_root || "https://gitlab.com/api/v4"; + this.token = config.token || false; + this.branch = config.branch || "master"; + this.repo = config.repo || ""; + this.repoURL = `/projects/${ encodeURIComponent(this.repo) }`; + } + + user() { + return this.request("/user"); + } + + /* TODO */ + isCollaborator(user) { + return this.request(`${ this.repoURL }/members/${ user.id }`) + .then(member =>(member.access_level >= 30)); + } + + requestHeaders(headers = {}) { + return { + ...headers, + ...(this.token ? { Authorization: `Bearer ${ this.token }` } : {}), + }; + } + + urlFor(path, options) { + const cacheBuster = `ts=${ new Date().getTime() }`; + const encodedParams = options.params + ? Object.entries(options.params).map( + ([key, val]) => `${ key }=${ encodeURIComponent(val) }`) + : []; + return this.api_root + path + `?${ [cacheBuster, ...encodedParams].join("&") }`; + } + + request(path, options = {}) { + const headers = this.requestHeaders(options.headers || {}); + const url = this.urlFor(path, options); + + return fetch(url, { ...options, headers }).then((response) => { + const contentType = response.headers.get("Content-Type"); + if (contentType && contentType.match(/json/)) { + return Promise.all([response, response.json()]); + } + return Promise.all([response, response.text()]); + }) + .catch(err => [err, null]) + .then(([response, value]) => (response.ok ? value : Promise.reject([value, response]))) + .catch(([errorValue, response]) => { + const message = (errorValue && errorValue.message) + ? errorValue.message + : (isString(errorValue) ? errorValue : ""); + throw new APIError(message, response && response.status, 'GitLab', { response, errorValue }); + }); + } + + readFile(path, sha, branch = this.branch) { + const cache = sha ? LocalForage.getItem(`gh.${ sha }`) : Promise.resolve(null); + return cache.then((cached) => { + if (cached) { return cached; } + + // Files must be downloaded from GitLab as base64 due to this bug: + // https://gitlab.com/gitlab-org/gitlab-ce/issues/31470 + return this.request(`${ this.repoURL }/repository/files/${ encodeURIComponent(path) }`, { + params: { ref: branch }, + cache: "no-store", + }).then(response => this.fromBase64(response.content)) + .then((result) => { + if (sha) { + LocalForage.setItem(`gh.${ sha }`, result); + } + return result; + }); + }); + } + + /* TODO */ + fileExists(path) { + return this.request(`${ this.repoURL }/repository/files/${ encodeURIComponent(path) }`, { + params: { ref: branch }, + cache: "no-store", + }).then; + } + + listFiles(path) { + return this.request(`${ this.repoURL }/repository/tree`, { + params: { path, ref: this.branch }, + }) + .then((files) => { + if (!Array.isArray(files)) { + throw new Error(`Cannot list files, path ${path} is not a directory but a ${files.type}`); + } + return files; + }) + .then(files => files.filter(file => file.type === "blob")); + } + + persistFiles(entry, mediaFiles, options) { + /* TODO */ + const newMedia = mediaFiles.filter(file => !file.uploaded); + + const mediaUploads = newMedia.map(file => this.uploadAndCommit(file, `${ options.commitMessage }: create ${ file.value }.`)); + + // Wait until media files are uploaded before we commit the main entry. + // This should help avoid inconsistent state. + return Promise.all(mediaUploads) + .then(() => this.uploadAndCommit({ ...entry, update: !options.newEntry }, options.commitMessage)); + } + + deleteFile(path, commit_message, options={}) { + const branch = options.branch || this.branch; + return this.request(`${ this.repoURL }/repository/files/${ encodeURIComponent(path) }`, { + method: "DELETE", + params: { commit_message, branch }, + }); + } + + toBase64(str) { + return Promise.resolve( + Base64.encode(str) + ); + } + + fromBase64(str) { + return Base64.decode(str); + } + + uploadAndCommit(item, commitMessage, branch = this.branch) { + const content = item instanceof AssetProxy ? item.toBase64() : this.toBase64(item.raw); + // Remove leading slash from path if exists. + const file_path = item.path.replace(/^\//, ''); + + // We cannot use the `/repository/files/:file_path` format here because the file content has to go + // in the URI as a parameter. This overloads the OPTIONS pre-request (at least in Chrome 61 beta). + return content.then(contentBase64 => this.request(`${ this.repoURL }/repository/commits`, { + method: "POST", + headers: { + "Content-Type": "application/json", + }, + body: JSON.stringify({ + branch, + commit_message: commitMessage, + actions: [{ + action: (item.update ? "update" : "create"), + file_path, + content: contentBase64, + encoding: "base64", + }] + }), + })).then(response => Object.assign({}, item, { + sha: response.sha, + uploaded: true, + })); + } +} \ No newline at end of file diff --git a/src/backends/gitlab/AuthenticationPage.css b/src/backends/gitlab/AuthenticationPage.css new file mode 100644 index 000000000000..949d9b7dfc53 --- /dev/null +++ b/src/backends/gitlab/AuthenticationPage.css @@ -0,0 +1,12 @@ +.root { + display: flex; + flex-flow: column nowrap; + align-items: center; + justify-content: center; + height: 100vh; +} + +.button { + padding: .25em 1em; + height: auto; +} diff --git a/src/backends/gitlab/AuthenticationPage.js b/src/backends/gitlab/AuthenticationPage.js new file mode 100644 index 000000000000..036cf420fb88 --- /dev/null +++ b/src/backends/gitlab/AuthenticationPage.js @@ -0,0 +1,50 @@ +import React from 'react'; +import Button from 'react-toolbox/lib/button'; +import Authenticator from '../../lib/netlify-auth'; +import { Icon } from '../../components/UI'; +import { Notifs } from 'redux-notifications'; +import { Toast } from '../../components/UI/index'; +import styles from './AuthenticationPage.css'; + +export default class AuthenticationPage extends React.Component { + static propTypes = { + onLogin: React.PropTypes.func.isRequired, + }; + + state = {}; + + handleLogin = (e) => { + e.preventDefault(); + const cfg = { + base_url: this.props.base_url, + site_id: (document.location.host.split(':')[0] === 'localhost') ? 'cms.netlify.com' : this.props.siteId + }; + const auth = new Authenticator(cfg); + + auth.authenticate({ provider: 'gitlab', scope: 'repo' }, (err, data) => { + if (err) { + this.setState({ loginError: err.toString() }); + return; + } + this.props.onLogin(data); + }); + }; + + render() { + const { loginError } = this.state; + + return ( +
+ + {loginError &&

{loginError}

} + +
+ ); + } +} diff --git a/src/backends/gitlab/implementation.js b/src/backends/gitlab/implementation.js new file mode 100644 index 000000000000..4ab75ddea618 --- /dev/null +++ b/src/backends/gitlab/implementation.js @@ -0,0 +1,100 @@ +import semaphore from "semaphore"; +import AuthenticationPage from "./AuthenticationPage"; +import API from "./API"; +import { fileExtension } from '../../lib/pathHelper'; +import { EDITORIAL_WORKFLOW } from "../../constants/publishModes"; + +const MAX_CONCURRENT_DOWNLOADS = 10; + +export default class GitLab { + constructor(config, proxied = false) { + this.config = config; + + if (config.getIn(["publish_mode"]) === EDITORIAL_WORKFLOW) { + throw new Error("The GitLab backend does not support the Editorial Workflow.") + } + + if (!proxied && config.getIn(["backend", "repo"]) == null) { + throw new Error("The GitLab backend needs a \"repo\" in the backend configuration."); + } + + this.repo = config.getIn(["backend", "repo"], ""); + this.branch = config.getIn(["backend", "branch"], "master"); + this.api_root = config.getIn(["backend", "api_root"], "https://gitlab.com/api/v4"); + this.token = ''; + } + + authComponent() { + return AuthenticationPage; + } + + setUser(user) { + this.token = user.token; + this.api = new API({ token: this.token, branch: this.branch, repo: this.repo }); + } + + authenticate(state) { + this.token = state.token; + this.api = new API({ token: this.token, branch: this.branch, repo: this.repo, api_root: this.api_root }); + return this.api.user().then(user => + this.api.isCollaborator(user.login).then((isCollab) => { + // Unauthorized user + if (!isCollab) throw new Error("Your GitLab user account does not have access to this repo."); + // Authorized user + user.token = state.token; + return user; + }) + ); + } + + getToken() { + return Promise.resolve(this.token); + } + + entriesByFolder(collection, extension) { + return this.api.listFiles(collection.get("folder")) + .then(files => files.filter(file => fileExtension(file.name) === extension)) + .then(this.fetchFiles); + } + + entriesByFiles(collection) { + const files = collection.get("files").map(collectionFile => ({ + path: collectionFile.get("file"), + label: collectionFile.get("label"), + })); + return this.fetchFiles(files); + } + + fetchFiles = (files) => { + const sem = semaphore(MAX_CONCURRENT_DOWNLOADS); + const promises = []; + files.forEach((file) => { + promises.push(new Promise((resolve, reject) => ( + sem.take(() => this.api.readFile(file.path, file.sha).then((data) => { + resolve({ file, data }); + sem.leave(); + }).catch((err) => { + sem.leave(); + reject(err); + })) + ))); + }); + return Promise.all(promises); + }; + + // Fetches a single entry. + getEntry(collection, slug, path) { + return this.api.readFile(path).then(data => ({ + file: { path }, + data, + })); + } + + persistEntry(entry, mediaFiles = [], options = {}) { + return this.api.persistFiles(entry, mediaFiles, options); + } + + deleteFile(path, commitMessage, options) { + return this.api.deleteFile(path, commitMessage, options); + } +} From 1069b01688cefbf1d2d869ab33d38fd4a06bf043 Mon Sep 17 00:00:00 2001 From: Caleb Date: Fri, 4 Aug 2017 18:07:11 -0600 Subject: [PATCH 18/24] Fix authentication/permission checking. --- src/backends/gitlab/API.js | 15 +++++++++++++-- src/backends/gitlab/implementation.js | 2 +- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/backends/gitlab/API.js b/src/backends/gitlab/API.js index 37f2dcad5a24..8ff30789e401 100644 --- a/src/backends/gitlab/API.js +++ b/src/backends/gitlab/API.js @@ -18,10 +18,21 @@ export default class API { return this.request("/user"); } - /* TODO */ isCollaborator(user) { + const WRITE_ACCESS = 30; return this.request(`${ this.repoURL }/members/${ user.id }`) - .then(member =>(member.access_level >= 30)); + .then(member => (member.access_level >= WRITE_ACCESS)) + .catch((err) => { + // Member does not have any access. We cannot just check for 404, + // because a 404 is also returned if we have the wrong URI, + // just with an "error" key instead of a "message" key. + if (err.status === 404 && err.meta.errorValue["message"] === "404 Not found") { + return false; + } else { + // Otherwise, it is actually an API error. + throw err; + } + }); } requestHeaders(headers = {}) { diff --git a/src/backends/gitlab/implementation.js b/src/backends/gitlab/implementation.js index 4ab75ddea618..415c378012bf 100644 --- a/src/backends/gitlab/implementation.js +++ b/src/backends/gitlab/implementation.js @@ -37,7 +37,7 @@ export default class GitLab { this.token = state.token; this.api = new API({ token: this.token, branch: this.branch, repo: this.repo, api_root: this.api_root }); return this.api.user().then(user => - this.api.isCollaborator(user.login).then((isCollab) => { + this.api.isCollaborator(user).then((isCollab) => { // Unauthorized user if (!isCollab) throw new Error("Your GitLab user account does not have access to this repo."); // Authorized user From c8012bd546df154b417679e06f6f3c40a161a282 Mon Sep 17 00:00:00 2001 From: Caleb Date: Mon, 7 Aug 2017 15:13:27 -0600 Subject: [PATCH 19/24] Make images with the same filename as already existing images overwrite them. This is not optimal, but it is how the GitHub implementation works and should be fixed later. --- src/backends/gitlab/API.js | 39 ++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/src/backends/gitlab/API.js b/src/backends/gitlab/API.js index 8ff30789e401..3b9b131aed63 100644 --- a/src/backends/gitlab/API.js +++ b/src/backends/gitlab/API.js @@ -57,13 +57,22 @@ export default class API { return fetch(url, { ...options, headers }).then((response) => { const contentType = response.headers.get("Content-Type"); + if (options.method === "HEAD") { + return Promise.all([response]); + } if (contentType && contentType.match(/json/)) { return Promise.all([response, response.json()]); } return Promise.all([response, response.text()]); }) .catch(err => [err, null]) - .then(([response, value]) => (response.ok ? value : Promise.reject([value, response]))) + .then(([response, value]) => { + if (!response.ok) return Promise.reject([value, response]); + /* TODO: remove magic. */ + if (value === undefined) return response; + /* OK */ + return value; + }) .catch(([errorValue, response]) => { const message = (errorValue && errorValue.message) ? errorValue.message @@ -92,12 +101,16 @@ export default class API { }); } - /* TODO */ - fileExists(path) { + fileExists(path, branch = this.branch) { return this.request(`${ this.repoURL }/repository/files/${ encodeURIComponent(path) }`, { + method: "HEAD", params: { ref: branch }, cache: "no-store", - }).then; + }).then(() => true).catch((err) => { + // TODO: 404 can mean either the file does not exist, or if an API + // endpoint doesn't exist. Is there a better way to check for this? + if (err.status === 404) {return false;} else {throw err;} + }); } listFiles(path) { @@ -114,15 +127,21 @@ export default class API { } persistFiles(entry, mediaFiles, options) { - /* TODO */ const newMedia = mediaFiles.filter(file => !file.uploaded); - - const mediaUploads = newMedia.map(file => this.uploadAndCommit(file, `${ options.commitMessage }: create ${ file.value }.`)); + const mediaUploads = newMedia.map(file => this.fileExists(file.path).then(exists => { + return this.uploadAndCommit(file, { + commitMessage: `${ options.commitMessage }: create ${ file.value }.`, + newFile: !exists + }); + })); // Wait until media files are uploaded before we commit the main entry. // This should help avoid inconsistent state. return Promise.all(mediaUploads) - .then(() => this.uploadAndCommit({ ...entry, update: !options.newEntry }, options.commitMessage)); + .then(() => this.uploadAndCommit(entry, { + commitMessage: options.commitMessage, + newFile: options.newEntry + })); } deleteFile(path, commit_message, options={}) { @@ -143,7 +162,7 @@ export default class API { return Base64.decode(str); } - uploadAndCommit(item, commitMessage, branch = this.branch) { + uploadAndCommit(item, {commitMessage, newFile = true, branch = this.branch}) { const content = item instanceof AssetProxy ? item.toBase64() : this.toBase64(item.raw); // Remove leading slash from path if exists. const file_path = item.path.replace(/^\//, ''); @@ -159,7 +178,7 @@ export default class API { branch, commit_message: commitMessage, actions: [{ - action: (item.update ? "update" : "create"), + action: (newFile ? "create" : "update"), file_path, content: contentBase64, encoding: "base64", From a24c1780055ce01e925ef1f6a710dfc72b93a7aa Mon Sep 17 00:00:00 2001 From: Caleb Date: Tue, 8 Aug 2017 09:22:33 -0600 Subject: [PATCH 20/24] Remove unneeded dependencies. --- src/backends/gitlab/API.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/backends/gitlab/API.js b/src/backends/gitlab/API.js index 3b9b131aed63..4c44d453ae14 100644 --- a/src/backends/gitlab/API.js +++ b/src/backends/gitlab/API.js @@ -1,9 +1,8 @@ import LocalForage from "localforage"; import { Base64 } from "js-base64"; -import { fill, isString } from "lodash"; +import { isString } from "lodash"; import AssetProxy from "../../valueObjects/AssetProxy"; -import { SIMPLE, EDITORIAL_WORKFLOW, status } from "../../constants/publishModes"; -import { APIError, EditorialWorkflowError } from "../../valueObjects/errors"; +import { APIError } from "../../valueObjects/errors"; export default class API { constructor(config) { From 243af7334a6669389829d1559f71af88cdd1c46a Mon Sep 17 00:00:00 2001 From: Caleb Date: Tue, 8 Aug 2017 13:34:20 -0600 Subject: [PATCH 21/24] Remove old GitHub code. --- src/backends/gitlab/API.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/backends/gitlab/API.js b/src/backends/gitlab/API.js index 4c44d453ae14..d4583289fba7 100644 --- a/src/backends/gitlab/API.js +++ b/src/backends/gitlab/API.js @@ -183,9 +183,6 @@ export default class API { encoding: "base64", }] }), - })).then(response => Object.assign({}, item, { - sha: response.sha, - uploaded: true, - })); + })).then(() => Object.assign({}, item, {uploaded: true})); } } \ No newline at end of file From 8eb9172509144c916ad1fe54bc092ea11b750124 Mon Sep 17 00:00:00 2001 From: Caleb Date: Fri, 11 Aug 2017 15:02:34 -0600 Subject: [PATCH 22/24] Code cleanup from #508. --- src/backends/gitlab/API.js | 19 ++++++++----------- src/backends/gitlab/implementation.js | 3 +-- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/backends/gitlab/API.js b/src/backends/gitlab/API.js index d4583289fba7..74607e386ae0 100644 --- a/src/backends/gitlab/API.js +++ b/src/backends/gitlab/API.js @@ -47,14 +47,14 @@ export default class API { ? Object.entries(options.params).map( ([key, val]) => `${ key }=${ encodeURIComponent(val) }`) : []; - return this.api_root + path + `?${ [cacheBuster, ...encodedParams].join("&") }`; + return `${ this.api_root }${ path }?${ [cacheBuster, ...encodedParams].join("&") }`; } request(path, options = {}) { const headers = this.requestHeaders(options.headers || {}); const url = this.urlFor(path, options); - - return fetch(url, { ...options, headers }).then((response) => { + return fetch(url, { ...options, headers }) + .then((response) => { const contentType = response.headers.get("Content-Type"); if (options.method === "HEAD") { return Promise.all([response]); @@ -73,10 +73,9 @@ export default class API { return value; }) .catch(([errorValue, response]) => { - const message = (errorValue && errorValue.message) - ? errorValue.message - : (isString(errorValue) ? errorValue : ""); - throw new APIError(message, response && response.status, 'GitLab', { response, errorValue }); + const errorMessageProp = (errorValue && errorValue.message) ? errorValue.message : null; + const message = errorMessageProp || (isString(errorValue) ? errorValue : ""); + throw new APIError(message, response && response.status, 'GitHub', { response, errorValue }); }); } @@ -152,9 +151,7 @@ export default class API { } toBase64(str) { - return Promise.resolve( - Base64.encode(str) - ); + return Promise.resolve(Base64.encode(str)); } fromBase64(str) { @@ -183,6 +180,6 @@ export default class API { encoding: "base64", }] }), - })).then(() => Object.assign({}, item, {uploaded: true})); + })).then(response => Object.assign({}, item, { uploaded: true })); } } \ No newline at end of file diff --git a/src/backends/gitlab/implementation.js b/src/backends/gitlab/implementation.js index 415c378012bf..0dce5eabe3ba 100644 --- a/src/backends/gitlab/implementation.js +++ b/src/backends/gitlab/implementation.js @@ -41,8 +41,7 @@ export default class GitLab { // Unauthorized user if (!isCollab) throw new Error("Your GitLab user account does not have access to this repo."); // Authorized user - user.token = state.token; - return user; + return Object.assign({}, user, { token: state.token }); }) ); } From c3ff76d798b323f64746d50999ecd856b232540f Mon Sep 17 00:00:00 2001 From: Caleb Date: Mon, 14 Aug 2017 10:34:35 -0600 Subject: [PATCH 23/24] Clarify comment. --- src/backends/gitlab/API.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backends/gitlab/API.js b/src/backends/gitlab/API.js index 74607e386ae0..32f580edc0c3 100644 --- a/src/backends/gitlab/API.js +++ b/src/backends/gitlab/API.js @@ -134,7 +134,7 @@ export default class API { })); // Wait until media files are uploaded before we commit the main entry. - // This should help avoid inconsistent state. + // This should help avoid inconsistent repository/website state. return Promise.all(mediaUploads) .then(() => this.uploadAndCommit(entry, { commitMessage: options.commitMessage, From 9ea2cafe8b7658ea279eae3ab5516aaf29363838 Mon Sep 17 00:00:00 2001 From: Caleb Date: Mon, 14 Aug 2017 17:14:23 -0600 Subject: [PATCH 24/24] Add basic tests from #515. --- package.json | 1 + src/backends/gitlab/__tests__/API.spec.js | 100 ++++++++++++++++++++++ yarn.lock | 24 ++++-- 3 files changed, 120 insertions(+), 5 deletions(-) create mode 100644 src/backends/gitlab/__tests__/API.spec.js diff --git a/package.json b/package.json index 4c6763b9ef2e..1e22ba871421 100644 --- a/package.json +++ b/package.json @@ -98,6 +98,7 @@ "classnames": "^2.2.5", "dateformat": "^1.0.12", "deep-equal": "^1.0.1", + "fetch-mock": "^5.12.1", "fuzzy": "^0.1.1", "history": "^2.1.2", "immutability-helper": "^2.0.0", diff --git a/src/backends/gitlab/__tests__/API.spec.js b/src/backends/gitlab/__tests__/API.spec.js new file mode 100644 index 000000000000..acebe2d387c6 --- /dev/null +++ b/src/backends/gitlab/__tests__/API.spec.js @@ -0,0 +1,100 @@ +import fetchMock from 'fetch-mock'; +import { curry, escapeRegExp, isMatch, merge } from 'lodash'; +import { Map } from 'immutable'; + +import API from '../API'; + +const compose = (...fns) => val => fns.reduceRight((newVal, fn) => fn(newVal), val); +const pipe = (...fns) => compose(...fns.reverse()); + +const regExpOrString = rOrS => (rOrS instanceof RegExp ? rOrS.toString() : escapeRegExp(rOrS)); + +const mockForAllParams = url => `${url}(\\?.*)?`; +const prependRoot = urlRoot => url => `${urlRoot}${url}`; +const matchWholeURL = str => `^${str}$`; +const strToRegex = str => new RegExp(str); +const matchURL = curry((urlRoot, forAllParams, url) => pipe( + regExpOrString, + ...(forAllParams ? [mockForAllParams] : []), + pipe(regExpOrString, prependRoot)(urlRoot), + matchWholeURL, + strToRegex, +)(url)); + +// `mock` gives us a few advantages over using the standard +// `fetchMock.mock`: +// - Routes can have a root specified that is prepended to the path +// - By default, routes swallow URL parameters (the GitHub API code +// uses a `ts` parameter on _every_ request) +const mockRequest = curry((urlRoot, url, response, options={}) => { + const mergedOptions = merge({}, { + forAllParams: true, + fetchMockOptions: {}, + }, options); + return fetchMock.mock( + matchURL(urlRoot, mergedOptions.forAllParams, url), + response, + options.fetchMockOptions, + ); +}); + +const defaultResponseHeaders = { "Content-Type": "application/json" }; + +afterEach(() => fetchMock.restore()); + +describe('gitlab API', () => { + it('should correctly detect a contributor', () => { + const api = new API({ branch: 'test-branch', repo: 'test-user/test-repo' }); + const user = { id: 1 }; + mockRequest(api.api_root)(`${ api.repoURL }/members/${ user.id }`, { + headers: defaultResponseHeaders, + body: { + id: 1, + access_level: 30, + }, + }); + return expect(api.isCollaborator(user)).resolves.toBe(true); + }); + + it('should correctly detect a non-contributor', () => { + const api = new API({ branch: 'test-branch', repo: 'test-user/test-repo' }); + const user = { id: 1 }; + mockRequest(api.api_root)(`${ api.repoURL }/members/${ user.id }`, { + headers: defaultResponseHeaders, + body: { + id: 1, + access_level: 10, + }, + }); + return expect(api.isCollaborator(user)).resolves.toBe(false); + }); + + it('should list the files in a directory', () => { + const api = new API({ branch: 'test-branch', repo: 'test-user/test-repo' }); + mockRequest(api.api_root)(`${ api.repoURL }/repository/tree`, { + headers: defaultResponseHeaders, + body: [ + { + id: "fff6fe3a23bf1c8ea0692b4a883af99bee26fd3b", + name: "octokit.rb", + type: "blob", + path: "test-directory/octokit.rb", + }, + { + id: "a84d88e7554fc1fa21bcbc4efae3c782a70d2b9d", + name: "octokit", + type: "tree", + path: "test-directory/octokit", + }, + ], + }); + return expect(api.listFiles('test-directory')).resolves.toMatchObject([ + { + id: "fff6fe3a23bf1c8ea0692b4a883af99bee26fd3b", + name: "octokit.rb", + type: "blob", + path: "test-directory/octokit.rb", + }, + ]); + }); +}); diff --git a/yarn.lock b/yarn.lock index e9e831e8213f..65ccde77ea67 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2940,6 +2940,14 @@ fbjs@^0.8.4, fbjs@^0.8.9: setimmediate "^1.0.5" ua-parser-js "^0.7.9" +fetch-mock@^5.12.1: + version "5.12.1" + resolved "https://registry.yarnpkg.com/fetch-mock/-/fetch-mock-5.12.1.tgz#b1808310f51ab286d1b487a8cb39dfbecb0d097a" + dependencies: + glob-to-regexp "^0.3.0" + node-fetch "^1.3.3" + path-to-regexp "^1.7.0" + figures@^1.3.5, figures@^1.7.0: version "1.7.0" resolved "https://registry.yarnpkg.com/figures/-/figures-1.7.0.tgz#cbe1e3affcf1cd44b80cadfed28dc793a9701d2e" @@ -3217,6 +3225,10 @@ glob-parent@^2.0.0: dependencies: is-glob "^2.0.0" +glob-to-regexp@^0.3.0: + version "0.3.0" + resolved "https://registry.yarnpkg.com/glob-to-regexp/-/glob-to-regexp-0.3.0.tgz#8c5a1494d2066c570cc3bfe4496175acc4d502ab" + glob@^6.0.1: version "6.0.4" resolved "https://registry.yarnpkg.com/glob/-/glob-6.0.4.tgz#0f08860f6a155127b2fadd4f9ce24b1aab6e4d22" @@ -5085,7 +5097,7 @@ node-emoji@^1.0.3: dependencies: string.prototype.codepointat "^0.2.0" -node-fetch@^1.0.1: +node-fetch@^1.0.1, node-fetch@^1.3.3: version "1.6.3" resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-1.6.3.tgz#dc234edd6489982d58e8f0db4f695029abcd8c04" dependencies: @@ -5667,6 +5679,12 @@ path-to-regexp@0.1.7: version "0.1.7" resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-0.1.7.tgz#df604178005f522f15eb4490e7247a1bfaa67f8c" +path-to-regexp@^1.7.0: + version "1.7.0" + resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-1.7.0.tgz#59fde0f435badacba103a84e9d3bc64e96b9937d" + dependencies: + isarray "0.0.1" + path-type@^1.0.0: version "1.1.0" resolved "https://registry.yarnpkg.com/path-type/-/path-type-1.1.0.tgz#59c44f7ee491da704da415da5a4070ba4f8fe441" @@ -5747,10 +5765,6 @@ pluralize@^1.2.1: version "1.2.1" resolved "https://registry.yarnpkg.com/pluralize/-/pluralize-1.2.1.tgz#d1a21483fd22bb41e58a12fa3421823140897c45" -pluralize@^3.0.0: - version "3.1.0" - resolved "https://registry.yarnpkg.com/pluralize/-/pluralize-3.1.0.tgz#84213d0a12356069daa84060c559242633161368" - portfinder@^1.0.9: version "1.0.13" resolved "https://registry.yarnpkg.com/portfinder/-/portfinder-1.0.13.tgz#bb32ecd87c27104ae6ee44b5a3ccbf0ebb1aede9"