From 240cd746d68069ecc3b284dd9e83627a06f969b7 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Tue, 24 Nov 2020 15:24:00 +0800 Subject: [PATCH 01/18] Feat: add new InputNameConflictError (#78) * Feat: add new InputNameConflictError * Fix: switch to error identification using HTTP error code --- classes/File.js | 7 +++++-- errors/ConflictError.js | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 errors/ConflictError.js diff --git a/classes/File.js b/classes/File.js index ff16048ff..4804a89d9 100644 --- a/classes/File.js +++ b/classes/File.js @@ -3,7 +3,8 @@ const _ = require('lodash') const validateStatus = require('../utils/axios-utils') // Import error -const { NotFoundError } = require('../errors/NotFoundError') +const { NotFoundError } = require('../errors/NotFoundError') +const { ConflictError, inputNameConflictErrorMsg } = require('../errors/ConflictError') const GITHUB_ORG_NAME = process.env.GITHUB_ORG_NAME const BRANCH_REF = process.env.BRANCH_REF @@ -78,7 +79,9 @@ class File { return { sha: resp.data.content.sha } } catch (err) { - throw err + const status = err.response.status + if (status === 422) throw new ConflictError(inputNameConflictErrorMsg(fileName)) + throw err.response } } diff --git a/errors/ConflictError.js b/errors/ConflictError.js new file mode 100644 index 000000000..bfbbc944f --- /dev/null +++ b/errors/ConflictError.js @@ -0,0 +1,17 @@ +// Import base error +const { BaseIsomerError } = require('./BaseError') + +const inputNameConflictErrorMsg = (fileName) => `A file with ${fileName} already exists.` + +class ConflictError extends BaseIsomerError { + constructor (fileName) { + super( + 409, + `A file with ${fileName} already exists.` + ) + } +} +module.exports = { + ConflictError, + inputNameConflictErrorMsg, +} \ No newline at end of file From aec283ad89264db00b2346608b11bea05144e5cd Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Wed, 25 Nov 2020 15:29:43 +0800 Subject: [PATCH 02/18] Feat/update navigation file on creation of new collection and resource room (#80) * Feat: create new Navigation class to load navigation file * Feat: add to navigation on creation of new collection * feat: add to navigation on creation of resource room * Fix: remove redundant permalink field in config file * Fix: use existing File to retrieve navigation instead * Fix: use existing File to retrieve nav file for resourceroom * Fix: remove unused class * Fix: update nav file when deleting and renaming collectionPages This commit also fixes a bug with the existing delete endpoint where the wrong key was being retrieved for the filename. * Feat: handle nav changes on deleting and renaming resource room * Fix: change for loop to filter/map instead --- classes/Collection.js | 66 +++++++++++++++++++++++++++++++++++++---- classes/ResourceRoom.js | 65 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 121 insertions(+), 10 deletions(-) diff --git a/classes/Collection.js b/classes/Collection.js index be5519057..35f880fad 100644 --- a/classes/Collection.js +++ b/classes/Collection.js @@ -4,7 +4,10 @@ const Bluebird = require('bluebird') const _ = require('lodash') const { Config } = require('./Config.js') -const { File, CollectionPageType } = require('./File.js') +const { File, CollectionPageType, DataType } = require('./File.js') +const { deslugifyCollectionName } = require('../utils/utils.js') + +const NAV_FILE_NAME = 'navigation.yml' class Collection { constructor(accessToken, siteName) { @@ -32,14 +35,27 @@ class Collection { // TO-DO: Verify that collection doesn't already exist - contentObject.collections[`${collectionName}`] = { - permalink: '/:collection/:path/:title', + contentObject.collections[`${collectionName}`] = { output: true } const newContent = base64.encode(yaml.safeDump(contentObject)) await config.update(newContent, sha) + const nav = new File(this.accessToken, this.siteName) + const dataType = new DataType() + nav.setFileType(dataType) + const { content:navContent, sha:navSha } = await nav.read(NAV_FILE_NAME) + const navContentObject = yaml.safeLoad(base64.decode(navContent)) + + navContentObject.links.push({ + title: deslugifyCollectionName(collectionName), + collection: collectionName + }) + const newNavContent = base64.encode(yaml.safeDump(navContentObject)) + + await nav.update(NAV_FILE_NAME, newNavContent, navSha) + } catch (err) { throw err } @@ -57,6 +73,21 @@ class Collection { await config.update(newContent, sha) + // Delete collection in nav if it exists + const nav = new File(this.accessToken, this.siteName) + const dataType = new DataType() + nav.setFileType(dataType) + const { content:navContent, sha:navSha } = await nav.read(NAV_FILE_NAME) + const navContentObject = yaml.safeLoad(base64.decode(navContent)) + + const newNavLinks = navContentObject.links.filter(link => link.collection !== collectionName) + const newNavContentObject = { + ...navContentObject, + links: newNavLinks, + } + const newNavContent = base64.encode(yaml.safeDump(newNavContentObject)) + await nav.update(NAV_FILE_NAME, newNavContent, navSha) + // Get all collectionPages const IsomerFile = new File(this.accessToken, this.siteName) const collectionPageType = new CollectionPageType(collectionName) @@ -66,7 +97,7 @@ class Collection { if (!_.isEmpty(collectionPages)) { // Delete all collectionPages await Bluebird.map(collectionPages, async(collectionPage) => { - let pageName = collectionPage.pageName + let pageName = collectionPage.fileName const { sha } = await IsomerFile.read(pageName) return IsomerFile.delete(pageName, sha) }) @@ -83,8 +114,7 @@ class Collection { const { content, sha } = await config.read() const contentObject = yaml.safeLoad(base64.decode(content)) - contentObject.collections[`${newCollectionName}`] = { - permalink: '/:collection/:path/:title', + contentObject.collections[`${newCollectionName}`] = { output: true } delete contentObject.collections[`${oldCollectionName}`] @@ -92,6 +122,30 @@ class Collection { await config.update(newContent, sha) + // Rename collection in nav if it exists + const nav = new File(this.accessToken, this.siteName) + const dataType = new DataType() + nav.setFileType(dataType) + const { content:navContent, sha:navSha } = await nav.read(NAV_FILE_NAME) + const navContentObject = yaml.safeLoad(base64.decode(navContent)) + + const newNavLinks = navContentObject.links.map(link => { + if (link.collection === oldCollectionName) { + return { + title: deslugifyCollectionName(newCollectionName), + collection: newCollectionName + } + } else { + return link + } + }) + const newNavContentObject = { + ...navContentObject, + links: newNavLinks, + } + const newNavContent = base64.encode(yaml.safeDump(newNavContentObject)) + await nav.update(NAV_FILE_NAME, newNavContent, navSha) + // Get all collectionPages const OldIsomerFile = new File(this.accessToken, this.siteName) const oldCollectionPageType = new CollectionPageType(oldCollectionName) diff --git a/classes/ResourceRoom.js b/classes/ResourceRoom.js index 97a59ebe4..4e73310b9 100644 --- a/classes/ResourceRoom.js +++ b/classes/ResourceRoom.js @@ -6,11 +6,13 @@ const _ = require('lodash') // Import Classes const { Config } = require('./Config.js') const { Resource } = require('../classes/Resource.js') -const { File, ResourceType } = require('../classes/File.js') +const { File, ResourceType, DataType } = require('../classes/File.js') +const { deslugifyCollectionName } = require('../utils/utils.js') // Constants const RESOURCE_ROOM_INDEX_PATH = 'index.html' const RESOURCE_ROOM_INDEX_CONTENT = 'LS0tCmxheW91dDogcmVzb3VyY2VzCnRpdGxlOiBSZXNvdXJjZSBSb29tCi0tLQ==' +const NAV_FILE_NAME = 'navigation.yml' class ResourceRoom { constructor(accessToken, siteName) { @@ -48,6 +50,20 @@ class ResourceRoom { await config.update(newContent, sha) + const nav = new File(this.accessToken, this.siteName) + const dataType = new DataType() + nav.setFileType(dataType) + const { content:navContent, sha:navSha } = await nav.read(NAV_FILE_NAME) + const navContentObject = yaml.safeLoad(base64.decode(navContent)) + + navContentObject.links.push({ + title: deslugifyCollectionName(resourceRoom), + resource_room: true + }) + const newNavContent = base64.encode(yaml.safeDump(navContentObject)) + + await nav.update(NAV_FILE_NAME, newNavContent, navSha) + return resourceRoom } catch (err) { throw err @@ -56,6 +72,7 @@ class ResourceRoom { async rename(newResourceRoom) { try { + // Add resource room to config const config = new Config(this.accessToken, this.siteName) const { content, sha } = await config.read() const contentObject = yaml.safeLoad(base64.decode(content)) @@ -63,7 +80,31 @@ class ResourceRoom { // Obtain existing resourceRoomName const resourceRoomName = contentObject.resources_name contentObject.resources_name = newResourceRoom - const newContent = base64.encode(yaml.safeDump(contentObject)) + const newContent = base64.encode(yaml.safeDump(contentObject)) + + // Rename resource room in nav if it exists + const nav = new File(this.accessToken, this.siteName) + const dataType = new DataType() + nav.setFileType(dataType) + const { content:navContent, sha:navSha } = await nav.read(NAV_FILE_NAME) + const navContentObject = yaml.safeLoad(base64.decode(navContent)) + + const newNavLinks = navContentObject.links.map(link => { + if (link.resource_room === true) { + return { + title: deslugifyCollectionName(newResourceRoom), + resource_room: true + } + } else { + return link + } + }) + const newNavContentObject = { + ...navContentObject, + links: newNavLinks, + } + const newNavContent = base64.encode(yaml.safeDump(newNavContentObject)) + await nav.update(NAV_FILE_NAME, newNavContent, navSha) // Delete all resources and resourcePages const IsomerResource = new Resource(this.accessToken, this.siteName) @@ -98,7 +139,7 @@ class ResourceRoom { async delete() { try { - // Delete collection in config + // Delete resource in config const config = new Config(this.accessToken, this.siteName) const { content, sha } = await config.read() const contentObject = yaml.safeLoad(base64.decode(content)) @@ -108,7 +149,23 @@ class ResourceRoom { // Delete resourcses_name from Config delete contentObject.resources_name - const newContent = base64.encode(yaml.safeDump(contentObject)) + const newContent = base64.encode(yaml.safeDump(contentObject)) + + // Delete resource room in nav if it exists + const nav = new File(this.accessToken, this.siteName) + const dataType = new DataType() + nav.setFileType(dataType) + const { content:navContent, sha:navSha } = await nav.read(NAV_FILE_NAME) + const navContentObject = yaml.safeLoad(base64.decode(navContent)) + + // Assumption: only a single resource room exists + const newNavLinks = navContentObject.links.filter(link => link.resource_room !== true) + const newNavContentObject = { + ...navContentObject, + links: newNavLinks, + } + const newNavContent = base64.encode(yaml.safeDump(newNavContentObject)) + await nav.update(NAV_FILE_NAME, newNavContent, navSha) // Delete all resources and resourcePages const IsomerResource = new Resource(this.accessToken, this.siteName) From dada11344ed683bb5468ad67a15718ebaf768d6a Mon Sep 17 00:00:00 2001 From: kwajiehao <31984694+kwajiehao@users.noreply.github.com> Date: Mon, 30 Nov 2020 13:51:04 +0800 Subject: [PATCH 03/18] chore: log error message with distinct string for metric filter (#82) This commit logs the serialized error with a distinct string, "Unrecognized internal server error:" so that we can create a cloudwatch metric filter based on this string to identify and trigger alerts on unrecognized / non-Isomer errors Co-authored-by: Jie Hao Kwa --- middleware/errorHandler.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/middleware/errorHandler.js b/middleware/errorHandler.js index 78dfbc956..d588eab38 100644 --- a/middleware/errorHandler.js +++ b/middleware/errorHandler.js @@ -5,7 +5,7 @@ const { serializeError } = require('serialize-error') const logger = require('../logger/logger'); function errorHandler (err, req, res, next) { - logger.info(`${new Date()}: ${JSON.stringify(serializeError(err))}`) + const errMsg = `${new Date()}: ${JSON.stringify(serializeError(err))}` // set locals, only providing error in development res.locals.message = err.message; @@ -13,6 +13,7 @@ function errorHandler (err, req, res, next) { // Error handling for custom errors if (err.isIsomerError) { + logger.info(errMsg) res.status(err.status).json({ error: { name: err.name, @@ -21,6 +22,7 @@ function errorHandler (err, req, res, next) { }, }) } else { + logger.info(`Unrecognized internal server error: ${errMsg}`) res.status(500).json({ error: { code: 500, From 46d00dec29e89f1205839965a44ba06faa08e828 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Tue, 1 Dec 2020 16:41:51 +0800 Subject: [PATCH 04/18] Feat: throw ConflictError if 409 received (#84) --- classes/File.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/File.js b/classes/File.js index 4804a89d9..aa2b143e6 100644 --- a/classes/File.js +++ b/classes/File.js @@ -80,7 +80,7 @@ class File { return { sha: resp.data.content.sha } } catch (err) { const status = err.response.status - if (status === 422) throw new ConflictError(inputNameConflictErrorMsg(fileName)) + if (status === 422 || status === 409) throw new ConflictError(inputNameConflictErrorMsg(fileName)) throw err.response } } From 530c64faed34edfaa654ba4f88525cd51e60342d Mon Sep 17 00:00:00 2001 From: kwajiehao <31984694+kwajiehao@users.noreply.github.com> Date: Thu, 3 Dec 2020 17:38:19 +0800 Subject: [PATCH 05/18] Feat/add settings fields (#85) * feat: add shareicon and analytics fields for Settings * feat: retrieve and update agency logo in navigation.yml This commit adds additional steps for the Settings route to retrieve and update the navigation.yml file to update agency logo * refactor: Settings class file retrieval and update process This commit refactors the Settings class so that we can reuse a utility function to retrieve the necessary settings files. It also simplifies and standardizes the update process for all settings files involved by putting the settings objects (config, navigation, footer) in an array and looping over them. * feat: update homepage title together with footer title This commit resolves issue #269. If a user edits the title field, they will update both the footer title and the browser title so that the titles are consistent. This commit achieves this by updating the title attribute of the homepage file ONLY IF a change in the `title` field is detected. * feat: send is_government field in response This commit sends an additional is_government field when retrieving settings configurations. The is_government field determines whether the government masthead is displayed on the website. * fix: error when updating settings object * fix: retrieve settings configurations programmatically Currently, we retrive the different settings configuration from various files using an array. This is confusing since the settings files are associated with random array indices. This commit switches to use an object so that the operations to retrieve the files are associated with a key instead. * chore: replace arrays and magic indice numbers with objects Previously, we used magic numbers to reference specific index positions in arrays to retrieve data. This commit replaces such arrays with objects so that data is accessible by key instead. Co-authored-by: Jie Hao Kwa --- classes/Settings.js | 191 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 152 insertions(+), 39 deletions(-) diff --git a/classes/Settings.js b/classes/Settings.js index 16dcea754..1576a58a3 100644 --- a/classes/Settings.js +++ b/classes/Settings.js @@ -1,13 +1,71 @@ const { Base64 } = require('js-base64') +const _ = require('lodash') const yaml = require('js-yaml') const Bluebird = require('bluebird') // import classes const { Config } = require('../classes/Config.js') -const { File, DataType } = require('../classes/File.js') +const { File, DataType, HomepageType } = require('../classes/File.js') // Constants const FOOTER_PATH = 'footer.yml' +const NAVIGATION_PATH = 'navigation.yml' +const HOMEPAGE_INDEX_PATH = 'index.md' // Empty string + +const retrieveSettingsFiles = async (accessToken, siteName, shouldRetrieveHomepage) => { + const configResp = new Config(accessToken, siteName) + + const FooterFile = new File(accessToken, siteName) + const dataType = new DataType() + FooterFile.setFileType(dataType) + + const NavigationFile = new File(accessToken, siteName) + NavigationFile.setFileType(dataType) + + const HomepageFile = new File(accessToken, siteName) + const homepageType = new HomepageType() + HomepageFile.setFileType(homepageType) + + const fileRetrievalObj = { + config: configResp.read(), + footer: FooterFile.read(FOOTER_PATH), + navigation: NavigationFile.read(NAVIGATION_PATH), + } + + // Retrieve homepage only if flag is set to true + if (shouldRetrieveHomepage) { + fileRetrievalObj.homepage = HomepageFile.read(HOMEPAGE_INDEX_PATH); + } + + + const fileContentsArr = await Bluebird.map(Object.keys(fileRetrievalObj), async (fileOpKey) => { + const { content, sha } = await fileRetrievalObj[fileOpKey] + + // homepage requires special extraction as the content is wrapped in front matter + if (fileOpKey === 'homepage') { + const homepageContent = Base64.decode(content) + const homepageFrontMatterObj = yaml.safeLoad(homepageContent.split('---')[1]) + return { type: fileOpKey, content: homepageFrontMatterObj, sha } + } + + return { type: fileOpKey, content: yaml.safeLoad(Base64.decode(content)), sha } + }) + + // Convert to an object so that data is accessible by key + const fileContentsObj = {} + fileContentsArr.forEach((fileObj) => { + const { type, content, sha } = fileObj + fileContentsObj[type] = { content, sha } + }) + + return { + configResp, + FooterFile, + NavigationFile, + HomepageFile, + fileContentsObj, + } +} class Settings { constructor(accessToken, siteName) { @@ -16,66 +74,121 @@ class Settings { } async get() { - // retrieve _config.yml and footer.yml - const configResp = new Config(this.accessToken, this.siteName) - - const IsomerDataFile = new File(this.accessToken, this.siteName) - const dataType = new DataType() - IsomerDataFile.setFileType(dataType) - - const fileRetrievalArr = [configResp.read(), IsomerDataFile.read(FOOTER_PATH)] - - const fileContentsArr = await Bluebird.map(fileRetrievalArr, async (fileOp) => { - const { content, sha } = await fileOp - return { content, sha} - }) + const { fileContentsObj: { + config, + footer, + navigation, + } } = await retrieveSettingsFiles(this.accessToken, this.siteName) // convert data to object form - const configContent = fileContentsArr[0].content - const footerContent = fileContentsArr[1].content - - const configReadableContent = yaml.safeLoad(Base64.decode(configContent)); - const footerReadableContent = yaml.safeLoad(Base64.decode(footerContent)); + const configContent = config.content; + const footerContent = footer.content; + const navigationContent = navigation.content; // retrieve only the relevant config and index fields const configFieldsRequired = { - url: configReadableContent.url, - title: configReadableContent.title, - favicon: configReadableContent.favicon, - resources_name: configReadableContent.resources_name, - colors: configReadableContent.colors, + url: configContent.url, + title: configContent.title, + favicon: configContent.favicon, + shareicon: configContent.shareicon, + is_government: configContent.is_government, + facebook_pixel: configContent['facebook-pixel'], + google_analytics: configContent.google_analytics, + resources_name: configContent.resources_name, + colors: configContent.colors, } // retrieve footer sha since we are sending the footer object wholesale - const footerSha = fileContentsArr[1].sha + const footerSha = footer.sha - return ({ configFieldsRequired, footerContent: footerReadableContent, footerSha }) + return ({ + configFieldsRequired, + footerContent, + navigationContent: { logo: navigationContent.logo }, + footerSha, + }) } async post(payload) { - // setup - const configResp = new Config(this.accessToken, this.siteName) - const config = await configResp.read() - const IsomerDataFile = new File(this.accessToken, this.siteName) - const dataType = new DataType() - IsomerDataFile.setFileType(dataType) + const { + configResp, + FooterFile, + NavigationFile, + HomepageFile, + fileContentsObj: { + config, + footer, + navigation, + homepage, + }, + } = await retrieveSettingsFiles(this.accessToken, this.siteName, true) // extract data const { footerSettings, configSettings, - footerSha, + navigationSettings, } = payload - // update config object - const configContent = yaml.safeLoad(Base64.decode(config.content)); - Object.keys(configSettings).forEach((setting) => (configContent[setting] = configSettings[setting])); + // update settings objects + const configContent = config.content + const footerContent = footer.content + const navigationContent = navigation.content + + const settingsObj = { + config: { + payload: configSettings, + currentData: configContent, + }, + footer: { + payload: footerSettings, + currentData: footerContent, + }, + navigation: { + payload: navigationSettings, + currentData: navigationContent, + }, + } + + const updatedSettingsObjArr = Object.keys(settingsObj).map((settingsObjKey) => { + const { payload, currentData } = settingsObj[settingsObjKey] + const clonedSettingsObj = _.cloneDeep(currentData); + Object.keys(payload).forEach((setting) => (clonedSettingsObj[setting] = payload[setting])); + return { + type: settingsObjKey, + settingsObj: clonedSettingsObj, + } + }) + + const updatedSettingsObj = {} + updatedSettingsObjArr.forEach((setting) => { + const { type, settingsObj } = setting + updatedSettingsObj[`${type}SettingsObj`] = settingsObj + }) + + const { configSettingsObj, footerSettingsObj, navigationSettingsObj } = updatedSettingsObj // update files - const newConfigContent = Base64.encode(yaml.safeDump(configContent)) - const newFooterContent = Base64.encode(yaml.safeDump(footerSettings)) + const newConfigContent = Base64.encode(yaml.safeDump(configSettingsObj)) + const newFooterContent = Base64.encode(yaml.safeDump(footerSettingsObj)) + const newNavigationContent = Base64.encode(yaml.safeDump(navigationSettingsObj)) + + // To-do: use Git Tree to speed up operations await configResp.update(newConfigContent, config.sha) - await IsomerDataFile.update(FOOTER_PATH, newFooterContent, footerSha) + await FooterFile.update(FOOTER_PATH, newFooterContent, footer.sha) + await NavigationFile.update(NAVIGATION_PATH, newNavigationContent, navigation.sha) + + // Update title in homepage as well if it's changed + if (configContent.title !== updatedSettingsObjArr[0].title) { + const { content: homepageContentObj, sha } = homepage; + homepageContentObj.title = configSettings.title; + const homepageFrontMatter = yaml.safeDump(homepageContentObj); + + const homepageContent = ['---\n', homepageFrontMatter, '---'].join('') ; + const newHomepageContent = Base64.encode(homepageContent) + + await HomepageFile.update(HOMEPAGE_INDEX_PATH, newHomepageContent, sha) + } return } } From d03d29beaac4c4fa340b5aea122d6db9fb31e27f Mon Sep 17 00:00:00 2001 From: gweiying <39231249+gweiying@users.noreply.github.com> Date: Fri, 4 Dec 2020 10:30:11 +0100 Subject: [PATCH 06/18] fix: update config settings and footer settings separately (#81) --- classes/Settings.js | 62 ++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/classes/Settings.js b/classes/Settings.js index 1576a58a3..84f2da480 100644 --- a/classes/Settings.js +++ b/classes/Settings.js @@ -135,19 +135,27 @@ class Settings { const footerContent = footer.content const navigationContent = navigation.content - const settingsObj = { - config: { + const settingsObj = {} + + if (configSettings) { + settingsObj.config = { payload: configSettings, currentData: configContent, - }, - footer: { + } + } + + if (footerSettings) { + settingsObj.footer = { payload: footerSettings, currentData: footerContent, - }, - navigation: { + } + } + + if (navigationSettings) { + settingsObj.navigation = { payload: navigationSettings, currentData: navigationContent, - }, + } } const updatedSettingsObjArr = Object.keys(settingsObj).map((settingsObjKey) => { @@ -168,27 +176,35 @@ class Settings { const { configSettingsObj, footerSettingsObj, navigationSettingsObj } = updatedSettingsObj - // update files - const newConfigContent = Base64.encode(yaml.safeDump(configSettingsObj)) - const newFooterContent = Base64.encode(yaml.safeDump(footerSettingsObj)) - const newNavigationContent = Base64.encode(yaml.safeDump(navigationSettingsObj)) - // To-do: use Git Tree to speed up operations - await configResp.update(newConfigContent, config.sha) - await FooterFile.update(FOOTER_PATH, newFooterContent, footer.sha) - await NavigationFile.update(NAVIGATION_PATH, newNavigationContent, navigation.sha) + if (configSettings) { + const newConfigContent = Base64.encode(yaml.safeDump(configSettingsObj)) + await configResp.update(newConfigContent, config.sha) + + // Update title in homepage as well if it's changed + if (configContent.title !== configSettingsObj.title) { + const { content: homepageContentObj, sha } = homepage; + + homepageContentObj.title = configSettings.title; + const homepageFrontMatter = yaml.safeDump(homepageContentObj); - // Update title in homepage as well if it's changed - if (configContent.title !== updatedSettingsObjArr[0].title) { - const { content: homepageContentObj, sha } = homepage; - homepageContentObj.title = configSettings.title; - const homepageFrontMatter = yaml.safeDump(homepageContentObj); + const homepageContent = ['---\n', homepageFrontMatter, '---'].join('') ; + const newHomepageContent = Base64.encode(homepageContent) - const homepageContent = ['---\n', homepageFrontMatter, '---'].join('') ; - const newHomepageContent = Base64.encode(homepageContent) + await HomepageFile.update(HOMEPAGE_INDEX_PATH, newHomepageContent, sha) + } + } + + if (footerSettings) { + const newFooterContent = Base64.encode(yaml.safeDump(footerSettingsObj)) + await FooterFile.update(FOOTER_PATH, newFooterContent, footer.sha) + } - await HomepageFile.update(HOMEPAGE_INDEX_PATH, newHomepageContent, sha) + if (navigationSettings) { + const newNavigationContent = Base64.encode(yaml.safeDump(navigationSettingsObj)) + await NavigationFile.update(NAVIGATION_PATH, newNavigationContent, navigation.sha) } + return } } From 81d7bfbcebf821b82e9d046025f8f118f0283cfb Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Tue, 8 Dec 2020 12:59:24 +0800 Subject: [PATCH 07/18] Fix: throw NotFoundError for files (#87) This commit fixes the behaviour for throwing NotFoundErrors in File. Previously, it was not possible to reach the condition to throw the NotFoundError in read, as a file which could not be retrieved would have have a defined endpoint. This resulted in a different error being thrown. This commit fixes that and also adds in the check during deleting and updating files. --- classes/File.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/classes/File.js b/classes/File.js index aa2b143e6..4c806c196 100644 --- a/classes/File.js +++ b/classes/File.js @@ -89,6 +89,7 @@ class File { try { const files = await this.list() const fileToRead = files.filter((file) => file.fileName === fileName)[0] + if (fileToRead === undefined) throw new NotFoundError ('File does not exist') const endpoint = `${this.baseBlobEndpoint}/${fileToRead.sha}` const params = { @@ -103,9 +104,7 @@ class File { "Content-Type": "application/json" } }) - - if (resp.status === 404) throw new NotFoundError ('File does not exist') - + const { content, sha } = resp.data return { content, sha } @@ -134,6 +133,8 @@ class File { return { newSha: resp.data.commit.sha } } catch (err) { + const status = err.response.status + if (status === 404) throw new NotFoundError ('File does not exist') throw err } } @@ -156,6 +157,8 @@ class File { } }) } catch (err) { + const status = err.response.status + if (status === 404) throw new NotFoundError ('File does not exist') throw err } } From f9bdf21a30a3cceca39c99f9c1ccec782ef88ca5 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Tue, 8 Dec 2020 13:47:43 +0800 Subject: [PATCH 08/18] Feat: add endpoint to check if user has access to site (#89) * Feat: add endpoint to check if user has access to site * Fix: remove log statement --- middleware/auth.js | 1 + routes/sites.js | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/middleware/auth.js b/middleware/auth.js index 88855bfe3..576673a06 100644 --- a/middleware/auth.js +++ b/middleware/auth.js @@ -124,6 +124,7 @@ auth.get('/sites/:siteName/netlify-toml', verifyJwt) // Sites auth.get('/sites', verifyJwt) +auth.get('/sites/:siteName', verifyJwt) auth.use((req, res, next) => { if (!req.route) { diff --git a/routes/sites.js b/routes/sites.js index e4c3facf6..4a8a0e8bd 100644 --- a/routes/sites.js +++ b/routes/sites.js @@ -3,6 +3,8 @@ const router = express.Router(); const axios = require('axios'); const _ = require('lodash'); const { attachRouteHandlerWrapper } = require('../middleware/routeHandler'); +// Import error +const { NotFoundError } = require('../errors/NotFoundError') const ISOMER_GITHUB_ORG_NAME = process.env.GITHUB_ORG_NAME const ISOMER_ADMIN_REPOS = [ @@ -90,6 +92,31 @@ async function getSites (req, res, next) { res.status(200).json({ siteNames }) } +/* Checks if a user has access to a repo. */ +async function checkHasAccess (req, res, next) { + try { + const { accessToken, userId } = req + const { siteName } = req.params + + const endpoint = `https://api.github.com/repos/${ISOMER_GITHUB_ORG_NAME}/${siteName}/collaborators/${userId}` + await axios.get(endpoint, { + headers: { + Authorization: `token ${accessToken}`, + "Content-Type": "application/json", + } + }) + + res.status(200).json() + } catch (err) { + const status = err.response.status + // If user is unauthorized or site does not exist, show the same NotFoundError + if (status === 404 || status === 403) throw new NotFoundError('Site does not exist') + console.log(err) + throw err + } +} + router.get('/', attachRouteHandlerWrapper(getSites)); +router.get('/:siteName', attachRouteHandlerWrapper(checkHasAccess)); module.exports = router; From 0a87b0e2404f473179f072a5db56a24b8f2306bc Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Tue, 8 Dec 2020 15:18:49 +0800 Subject: [PATCH 09/18] Fix/handle renaming of folders with multiple files (#88) * Feat: add util functions for git tree * Feat: change collection rename endpoint to use git trees instead * Fix: convert resource and resource room to use git trees instead * Fix: remove unused variable * Fix: move message into variable for readability --- classes/Collection.js | 37 +++++++--------- classes/Resource.js | 63 ++++++++++++-------------- classes/ResourceRoom.js | 34 +++++--------- routes/resources.js | 2 +- utils/utils.js | 98 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 155 insertions(+), 79 deletions(-) diff --git a/classes/Collection.js b/classes/Collection.js index 35f880fad..c0e882802 100644 --- a/classes/Collection.js +++ b/classes/Collection.js @@ -5,7 +5,7 @@ const _ = require('lodash') const { Config } = require('./Config.js') const { File, CollectionPageType, DataType } = require('./File.js') -const { deslugifyCollectionName } = require('../utils/utils.js') +const { getRootTree, sendTree, deslugifyCollectionName } = require('../utils/utils.js') const NAV_FILE_NAME = 'navigation.yml' @@ -109,6 +109,7 @@ class Collection { async rename(oldCollectionName, newCollectionName) { try { + const commitMessage = `Rename collection from ${oldCollectionName} to ${newCollectionName}` // Rename collection in config const config = new Config(this.accessToken, this.siteName) const { content, sha } = await config.read() @@ -146,28 +147,20 @@ class Collection { const newNavContent = base64.encode(yaml.safeDump(newNavContentObject)) await nav.update(NAV_FILE_NAME, newNavContent, navSha) - // Get all collectionPages - const OldIsomerFile = new File(this.accessToken, this.siteName) - const oldCollectionPageType = new CollectionPageType(oldCollectionName) - OldIsomerFile.setFileType(oldCollectionPageType) - const collectionPages = await OldIsomerFile.list() - - // If the object is empty (there are no pages in the collection), do nothing - if (_.isEmpty(collectionPages)) return - - // Set up new collection File instance - const NewIsomerFile = new File(this.accessToken, this.siteName) - const newCollectionPageType = new CollectionPageType(newCollectionName) - NewIsomerFile.setFileType(newCollectionPageType) - - // Rename all collectionPages - await Bluebird.map(collectionPages, async(collectionPage) => { - let pageName = collectionPage.fileName - const { content, sha } = await OldIsomerFile.read(pageName) - await OldIsomerFile.delete(pageName, sha) - return NewIsomerFile.create(pageName, content) + const { gitTree, currentCommitSha } = await getRootTree(this.siteName, this.accessToken); + const oldCollectionDirectoryName = `_${oldCollectionName}` + const newCollectionDirectoryName = `_${newCollectionName}` + const newGitTree = gitTree.map(item => { + if (item.path === oldCollectionDirectoryName) { + return { + ...item, + path: newCollectionDirectoryName + } + } else { + return item + } }) - + await sendTree(newGitTree, currentCommitSha, this.siteName, this.accessToken, commitMessage); } catch (err) { throw err } diff --git a/classes/Resource.js b/classes/Resource.js index 2711d62c9..87309fa17 100644 --- a/classes/Resource.js +++ b/classes/Resource.js @@ -4,6 +4,7 @@ const _ = require('lodash') // Import classes const { File, ResourceCategoryType, ResourcePageType } = require('../classes/File.js') const { Directory, ResourceRoomType } = require('../classes/Directory.js') +const { getRootTree, getTree, sendTree } = require('../utils/utils.js') // Constants const RESOURCE_INDEX_PATH = 'index.html' @@ -38,42 +39,36 @@ class Resource { } } - async rename(resourceRoomName, resourceName, newResourceRoomName, newResourceName) { + async rename(resourceRoomName, resourceName, newResourceName) { try { - // Delete old index file in old resource - const OldIsomerIndexFile = new File(this.accessToken, this.siteName) - const resourceType = new ResourceCategoryType(resourceRoomName, resourceName) - OldIsomerIndexFile.setFileType(resourceType) - const { sha: oldSha } = await OldIsomerIndexFile.read(`${RESOURCE_INDEX_PATH}`) - await OldIsomerIndexFile.delete(`${RESOURCE_INDEX_PATH}`, oldSha) - - // Create new index file in new resource - const NewIsomerIndexFile = new File(this.accessToken, this.siteName) - const newResourceType = new ResourceCategoryType(newResourceRoomName, newResourceName) - NewIsomerIndexFile.setFileType(newResourceType) - await NewIsomerIndexFile.create(`${RESOURCE_INDEX_PATH}`, RESOURCE_INDEX_CONTENT) - - // Rename resourcePages - const OldIsomerFile = new File(this.accessToken, this.siteName) - const resourcePageType = new ResourcePageType(resourceRoomName, resourceName) - OldIsomerFile.setFileType(resourcePageType) - - const NewIsomerFile = new File(this.accessToken, this.siteName) - const newResourcePageType = new ResourcePageType(newResourceRoomName, newResourceName) - NewIsomerFile.setFileType(newResourcePageType) - - // 1. List all resourcePages in old resource - const resourcePages = await OldIsomerFile.list() - - if (_.isEmpty(resourcePages)) return - - await Bluebird.each(resourcePages, async(resourcePage) => { - // 2. Create new resourcePages in newResource - const { content, sha } = await OldIsomerFile.read(resourcePage.fileName) - await NewIsomerFile.create(resourcePage.fileName, content) - // 3. Delete all resourcePages in resource - return OldIsomerFile.delete(resourcePage.fileName, sha) + const commitMessage = `Rename resource category from ${resourceName} to ${newResourceName}` + const { gitTree, currentCommitSha } = await getRootTree(this.siteName, this.accessToken); + let newGitTree = [] + let resourceRoomTreeSha + // Retrieve all git trees of other items + gitTree.forEach((item) => { + if (item.path === resourceRoomName) { + resourceRoomTreeSha = item.sha + } else { + newGitTree.push(item) + } + }) + const { gitTree: resourceRoomTree } = await getTree(this.siteName, this.accessToken, resourceRoomTreeSha) + resourceRoomTree.forEach(item => { + // We need to append resource room to the file path because the path is relative to the subtree + if (item.path === resourceName) { + newGitTree.push({ + ...item, + path: `${resourceRoomName}/${newResourceName}` + }) + } else { + newGitTree.push({ + ...item, + path: `${resourceRoomName}/${item.path}` + }) + } }) + await sendTree(newGitTree, currentCommitSha, this.siteName, this.accessToken, commitMessage); } catch (err) { throw err } diff --git a/classes/ResourceRoom.js b/classes/ResourceRoom.js index 4e73310b9..90c959d01 100644 --- a/classes/ResourceRoom.js +++ b/classes/ResourceRoom.js @@ -106,28 +106,18 @@ class ResourceRoom { const newNavContent = base64.encode(yaml.safeDump(newNavContentObject)) await nav.update(NAV_FILE_NAME, newNavContent, navSha) - // Delete all resources and resourcePages - const IsomerResource = new Resource(this.accessToken, this.siteName) - const resources = await IsomerResource.list(resourceRoomName) - - // Create index file in resourceRoom - const NewIsomerIndexFile = new File(this.accessToken, this.siteName) - const newResourceType = new ResourceType(newResourceRoom) - NewIsomerIndexFile.setFileType(newResourceType) - await NewIsomerIndexFile.create(RESOURCE_ROOM_INDEX_PATH, RESOURCE_ROOM_INDEX_CONTENT) - - // Delete index file in resourceRoom - const IsomerIndexFile = new File(this.accessToken, this.siteName) - const resourceType = new ResourceType(resourceRoomName) - IsomerIndexFile.setFileType(resourceType) - const { sha: deleteSha } = await IsomerIndexFile.read(RESOURCE_ROOM_INDEX_PATH) - await IsomerIndexFile.delete(RESOURCE_ROOM_INDEX_PATH, deleteSha) - - if (!_.isEmpty(resources)) { - await Bluebird.map(resources, async(resource) => { - return IsomerResource.rename(resourceRoomName, resource.dirName, newResourceRoom, resource.dirName) - }) - } + const { gitTree, currentCommitSha } = await getRootTree(this.siteName, this.accessToken); + const newGitTree = gitTree.map(item => { + if (item.path === resourceRoomName) { + return { + ...item, + path: newResourceRoom + } + } else { + return item + } + }) + await sendTree(newGitTree, currentCommitSha, this.siteName, this.accessToken, `Rename collection from ${oldCollectionName} to ${newCollectionName}`); await config.update(newContent, sha) diff --git a/routes/resources.js b/routes/resources.js index d17d74284..3541eeff3 100644 --- a/routes/resources.js +++ b/routes/resources.js @@ -60,7 +60,7 @@ async function renameResource (req, res, next) { const resourceRoomName = await IsomerResourceRoom.get() const IsomerResource = new Resource(accessToken, siteName) - await IsomerResource.rename(resourceRoomName, resourceName, resourceRoomName, newResourceName) + await IsomerResource.rename(resourceRoomName, resourceName, newResourceName) res.status(200).json({ resourceName, newResourceName }) } diff --git a/utils/utils.js b/utils/utils.js index 30c53749f..7c6fcf222 100644 --- a/utils/utils.js +++ b/utils/utils.js @@ -1,3 +1,6 @@ +const axios = require('axios'); +const GITHUB_ORG_NAME = process.env.GITHUB_ORG_NAME + /** * A function to deslugify a collection page's file name, taken from isomercms-frontend src/utils */ @@ -24,6 +27,98 @@ function deslugifyCollectionPage(collectionPageName) { ) } +// retrieve the tree item +async function getRootTree(repo, accessToken, branchRef='staging') { + try { + const headers = { + Authorization: `token ${accessToken}`, + Accept: 'application/json', + }; + // Get the commits of the repo + const { data: commits } = await axios.get(`https://api.github.com/repos/${GITHUB_ORG_NAME}/${repo}/commits`, { + params: { + ref: branchRef, + }, + headers, + }); + // Get the tree sha of the latest commit + const { commit: { tree: { sha: treeSha } } } = commits[0]; + const currentCommitSha = commits[0].sha; + + const { data: { tree: gitTree } } = await axios.get(`https://api.github.com/repos/${GITHUB_ORG_NAME}/${repo}/git/trees/${treeSha}`, { + params: { + ref: branchRef, + }, + headers, + }); + + return { gitTree, currentCommitSha }; + } catch (err) { + console.log(err); + } +} + +// retrieve the tree from given tree sha +async function getTree(repo, accessToken, treeSha, branchRef='staging') { + try { + const headers = { + Authorization: `token ${accessToken}`, + Accept: 'application/json', + }; + + const { data: { tree: gitTree } } = await axios.get(`https://api.github.com/repos/${GITHUB_ORG_NAME}/${repo}/git/trees/${treeSha}`, { + params: { + ref: branchRef, + }, + headers, + }); + + return { gitTree }; + } catch (err) { + console.log(err); + } +} + +// send the new tree object back to Github and point the latest commit on the staging branch to it +async function sendTree(gitTree, currentCommitSha, repo, accessToken, message, branchRef='staging') { + const headers = { + Authorization: `token ${accessToken}`, + Accept: 'application/json', + }; + const resp = await axios.post(`https://api.github.com/repos/${GITHUB_ORG_NAME}/${repo}/git/trees`, { + tree: gitTree, + }, { + headers, + }); + + const { data: { sha: newTreeSha } } = resp; + + const baseRefEndpoint = `https://api.github.com/repos/${GITHUB_ORG_NAME}/${repo}/git/refs`; + const baseCommitEndpoint = `https://api.github.com/repos/${GITHUB_ORG_NAME}/${repo}/git/commits`; + const refEndpoint = `${baseRefEndpoint}/heads/${branchRef}`; + + const newCommitResp = await axios.post(baseCommitEndpoint, { + message: message, + tree: newTreeSha, + parents: [currentCommitSha], + }, { + headers, + }); + + const newCommitSha = newCommitResp.data.sha; + + /** + * The `staging` branch reference will now point + * to `newCommitSha` instead of `currentCommitSha` + */ + await axios.patch(refEndpoint, { + sha: newCommitSha, + force: true, + }, { + headers, + }); +} + /** * A function to deslugify a collection's name */ @@ -37,4 +132,7 @@ function deslugifyCollectionName(collectionName) { module.exports = { deslugifyCollectionPage, deslugifyCollectionName, + getRootTree, + getTree, + sendTree, } From 30f005bdbc1f7a26fb3060baf27cb1b1d9941750 Mon Sep 17 00:00:00 2001 From: kwajiehao <31984694+kwajiehao@users.noreply.github.com> Date: Tue, 8 Dec 2020 17:45:36 +0800 Subject: [PATCH 10/18] feat: improve `/sites` endpoint performance (#90) * feat: speed site retrieval up by making concurrent api calls Previously, site retrieval was slow because the GitHub API for retrieving org repos was paginated, and we retrieved the data sequentially, one page at a time. This meant that it often took up to 7 or even 8 seconds each time this endpoint is accessed (each page took around 3 seconds, perhaps due to the large amount of data being sent). This commit improves performance by making these api calls concurrently, so that it now only takes around 3 seconds for the endpoint to respond. This commit also introduces an optional env var, ISOMERPAGES_REPO_PAGE_COUNT, which determines how many pages of the GitHub API to comb simultaneously. Since we know the number of repos our github org has, we can use this info to speed up our endpoint by making concurrent calls instead of stepping through the API pagination. * refactor: remove unnecessary filter Co-authored-by: Jie Hao Kwa --- routes/sites.js | 84 +++++++++++++++++++++++++------------------------ 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/routes/sites.js b/routes/sites.js index 4a8a0e8bd..6f55824c4 100644 --- a/routes/sites.js +++ b/routes/sites.js @@ -1,11 +1,16 @@ const express = require('express'); const router = express.Router(); const axios = require('axios'); +const Bluebird = require('bluebird'); const _ = require('lodash'); const { attachRouteHandlerWrapper } = require('../middleware/routeHandler'); +const { flatten } = require('lodash'); + // Import error const { NotFoundError } = require('../errors/NotFoundError') +const GH_MAX_REPO_COUNT = 100 +const ISOMERPAGES_REPO_PAGE_COUNT = process.env.ISOMERPAGES_REPO_PAGE_COUNT || 3 const ISOMER_GITHUB_ORG_NAME = process.env.GITHUB_ORG_NAME const ISOMER_ADMIN_REPOS = [ 'isomercms-backend', @@ -46,50 +51,47 @@ const timeDiff = (lastUpdated) => { async function getSites (req, res, next) { const { accessToken } = req - // Variable to store user repos - let siteNames = [] - - // Variables to track pagination of user's repos in case user has more than 100 - let pageCount = 1 - let hasNextPage = true; - const endpoint = `https://api.github.com/orgs/${ISOMER_GITHUB_ORG_NAME}/repos`; - - // Loop through all user repos - while (hasNextPage) { - const resp = await axios.get(endpoint, { - params: { - per_page: 100, - page: pageCount, - sort: "full_name", - }, - headers: { - Authorization: `token ${accessToken}`, - "Content-Type": "application/json", - } - }) - - // Filter for isomer repos - const isomerRepos = resp.data.reduce((acc, repo) => { - const { permissions, updated_at, name } = repo - if (permissions.push === true) { - return acc.concat({ - repoName: name, - lastUpdated: timeDiff(updated_at), - }) + const endpoint = `https://api.github.com/orgs/${ISOMER_GITHUB_ORG_NAME}/repos`; + + const params = { + per_page: GH_MAX_REPO_COUNT, + sort: "full_name", + } + + // Simultaneously retrieve all isomerpages repos + const paramsArr = [] + for (i = 0; i < ISOMERPAGES_REPO_PAGE_COUNT; i++) { + paramsArr.push({ ...params, page: i + 1 }) + } + + const sites = await Bluebird.map(paramsArr, async (params) => { + const resp = await axios.get(endpoint, { + params, + headers: { + Authorization: `token ${accessToken}`, + "Content-Type": "application/json", + } + }) + + return resp.data + .map((repoData) => { + const { + updated_at, + permissions, + name + } = repoData + + return { + lastUpdated: timeDiff(updated_at), + permissions, + repoName: name, } - return acc - }, []) + }).filter((repoData) => repoData.permissions.push === true && !ISOMER_ADMIN_REPOS.includes(repoData.repoName)) + }) + const flattenedSites = _.flatten(sites) - siteNames = siteNames.concat(isomerRepos) - hasNextPage = resp.headers.link ? resp.headers.link.includes('next') : false - ++pageCount - } - - // Remove Isomer admin repositories from this list - siteNames = _.difference(siteNames, ISOMER_ADMIN_REPOS) - - res.status(200).json({ siteNames }) + res.status(200).json({ siteNames: flattenedSites }) } /* Checks if a user has access to a repo. */ From 6aafcfd674cc089be89fb22d957aa5f97bcc05eb Mon Sep 17 00:00:00 2001 From: kwajiehao <31984694+kwajiehao@users.noreply.github.com> Date: Wed, 9 Dec 2020 14:33:12 +0800 Subject: [PATCH 11/18] feat: only update Settings file if field has changed (#92) The Settings class deals with three different types of configuration files: _config.yml, _data/footer.yml, and _data/navigation.yml. Originally, we would write to all three files, even if the only fields that were changed involved only one of the files. For example, even if I had only changed the `title` field (which affects only the _config.yml file), I would end up making API calls to update the remaining two settings files with their original content. This commit allows us to save on wasted resources, and also increase the speed of response by only updating a file if it needs to be updated. Co-authored-by: Jie Hao Kwa --- classes/Settings.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/classes/Settings.js b/classes/Settings.js index 84f2da480..d68619900 100644 --- a/classes/Settings.js +++ b/classes/Settings.js @@ -137,21 +137,21 @@ class Settings { const settingsObj = {} - if (configSettings) { + if (!_.isEmpty(configSettings)) { settingsObj.config = { payload: configSettings, currentData: configContent, } } - if (footerSettings) { + if (!_.isEmpty(footerSettings)) { settingsObj.footer = { payload: footerSettings, currentData: footerContent, } } - if (navigationSettings) { + if (!_.isEmpty(navigationSettings)) { settingsObj.navigation = { payload: navigationSettings, currentData: navigationContent, @@ -177,7 +177,7 @@ class Settings { const { configSettingsObj, footerSettingsObj, navigationSettingsObj } = updatedSettingsObj // To-do: use Git Tree to speed up operations - if (configSettings) { + if (!_.isEmpty(configSettings)) { const newConfigContent = Base64.encode(yaml.safeDump(configSettingsObj)) await configResp.update(newConfigContent, config.sha) @@ -195,12 +195,12 @@ class Settings { } } - if (footerSettings) { + if (!_.isEmpty(footerSettings)) { const newFooterContent = Base64.encode(yaml.safeDump(footerSettingsObj)) await FooterFile.update(FOOTER_PATH, newFooterContent, footer.sha) } - if (navigationSettings) { + if (!_.isEmpty(navigationSettings)) { const newNavigationContent = Base64.encode(yaml.safeDump(navigationSettingsObj)) await NavigationFile.update(NAVIGATION_PATH, newNavigationContent, navigation.sha) } From e7c024518825f60697a5e736a1108fe2bfbe7673 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Wed, 9 Dec 2020 18:13:12 +0800 Subject: [PATCH 12/18] Fix/error handling for large payload (#94) * Feat: add separate handling for payloadTooLarge error in errorHandler * Fix: swap intialization of cors --- middleware/errorHandler.js | 28 ++++++++++++++++++++-------- server.js | 9 ++++----- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/middleware/errorHandler.js b/middleware/errorHandler.js index d588eab38..5891f59d4 100644 --- a/middleware/errorHandler.js +++ b/middleware/errorHandler.js @@ -22,15 +22,27 @@ function errorHandler (err, req, res, next) { }, }) } else { - logger.info(`Unrecognized internal server error: ${errMsg}`) - res.status(500).json({ - error: { - code: 500, - message: 'Something went wrong', - }, - }) + // Error thrown by large payload is done by express + if (err.name === "PayloadTooLargeError") { + logger.info(errMsg) + res.status(413).json({ + error: { + name: err.name, + code: 413, + message: err.message, + }, + }) + } else { + logger.info(`Unrecognized internal server error: ${errMsg}`) + res.status(500).json({ + error: { + code: 500, + message: 'Something went wrong', + }, + }) + } } - } +} module.exports = { errorHandler, diff --git a/server.js b/server.js index 219a99e62..44f4fce12 100644 --- a/server.js +++ b/server.js @@ -34,15 +34,14 @@ const netlifyTomlRouter = require('./routes/netlifyToml') const app = express(); app.use(logger('dev')); -app.use(express.json({ limit: '5mb'})); -app.use(express.urlencoded({ extended: false })); -app.use(cookieParser()); -app.use(express.static(path.join(__dirname, 'public'))); - app.use(cors({ 'origin': FRONTEND_URL, 'credentials': true, })) +app.use(express.json({ limit: '5mb'})); +app.use(express.urlencoded({ extended: false })); +app.use(cookieParser()); +app.use(express.static(path.join(__dirname, 'public'))); // Use auth middleware app.use(auth) From 2f77caf5da552db477528eb5f31b55b95209ce99 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Wed, 9 Dec 2020 18:14:45 +0800 Subject: [PATCH 13/18] Feat/rollback changes if not all successful (#91) * Feat: split getRootTree function and add revertCommit function This commit accomplishes 2 things: it takes the retrieval of commit and tree shas out of getRootTree (which obsoletes the function, since now getTree can be used), and adds a util function allowing a simple rollback of commits. * Fix: update classes to use new implementation of util functions * Feat: add route handler which allows for rollback * Feat: update endpoints which may require rollback to use new handler * Docs: update docs for sites endpoint * Nit: fix revertCommit comment * Fix: update rename pages to use rollback handler * Fix: throw error instead of logging when encountered in getCommitAndTreeSha and getTree * Chore: install new package * Feat: add retry with exponential backoff for commit revertion * Fix: rename error to be distinguish which error is being forwarded --- classes/Collection.js | 5 +++-- classes/Resource.js | 7 ++++--- classes/ResourceRoom.js | 8 +++++--- docs/openapi.yaml | 17 +++++++++++++++ middleware/routeHandler.js | 38 +++++++++++++++++++++++++++------- package-lock.json | 5 +++++ package.json | 1 + routes/collectionPages.js | 8 ++++---- routes/collections.js | 8 ++++---- routes/pages.js | 4 ++-- routes/resourcePages.js | 8 ++++---- routes/resourceRoom.js | 8 ++++---- routes/resources.js | 8 ++++---- routes/settings.js | 4 ++-- utils/utils.js | 42 +++++++++++++++++++++++++------------- 15 files changed, 118 insertions(+), 53 deletions(-) diff --git a/classes/Collection.js b/classes/Collection.js index c0e882802..378a27618 100644 --- a/classes/Collection.js +++ b/classes/Collection.js @@ -5,7 +5,7 @@ const _ = require('lodash') const { Config } = require('./Config.js') const { File, CollectionPageType, DataType } = require('./File.js') -const { getRootTree, sendTree, deslugifyCollectionName } = require('../utils/utils.js') +const { getCommitAndTreeSha, getTree, sendTree, deslugifyCollectionName } = require('../utils/utils.js') const NAV_FILE_NAME = 'navigation.yml' @@ -147,7 +147,8 @@ class Collection { const newNavContent = base64.encode(yaml.safeDump(newNavContentObject)) await nav.update(NAV_FILE_NAME, newNavContent, navSha) - const { gitTree, currentCommitSha } = await getRootTree(this.siteName, this.accessToken); + const { currentCommitSha, treeSha } = await getCommitAndTreeSha(this.siteName, this.accessToken) + const gitTree = await getTree(this.siteName, this.accessToken, treeSha); const oldCollectionDirectoryName = `_${oldCollectionName}` const newCollectionDirectoryName = `_${newCollectionName}` const newGitTree = gitTree.map(item => { diff --git a/classes/Resource.js b/classes/Resource.js index 87309fa17..76d0fcd27 100644 --- a/classes/Resource.js +++ b/classes/Resource.js @@ -4,7 +4,7 @@ const _ = require('lodash') // Import classes const { File, ResourceCategoryType, ResourcePageType } = require('../classes/File.js') const { Directory, ResourceRoomType } = require('../classes/Directory.js') -const { getRootTree, getTree, sendTree } = require('../utils/utils.js') +const { getCommitAndTreeSha, getTree, sendTree } = require('../utils/utils.js') // Constants const RESOURCE_INDEX_PATH = 'index.html' @@ -42,7 +42,8 @@ class Resource { async rename(resourceRoomName, resourceName, newResourceName) { try { const commitMessage = `Rename resource category from ${resourceName} to ${newResourceName}` - const { gitTree, currentCommitSha } = await getRootTree(this.siteName, this.accessToken); + const { currentCommitSha, treeSha } = await getCommitAndTreeSha(this.siteName, this.accessToken) + const gitTree = await getTree(this.siteName, this.accessToken, treeSha); let newGitTree = [] let resourceRoomTreeSha // Retrieve all git trees of other items @@ -53,7 +54,7 @@ class Resource { newGitTree.push(item) } }) - const { gitTree: resourceRoomTree } = await getTree(this.siteName, this.accessToken, resourceRoomTreeSha) + const resourceRoomTree = await getTree(this.siteName, this.accessToken, resourceRoomTreeSha) resourceRoomTree.forEach(item => { // We need to append resource room to the file path because the path is relative to the subtree if (item.path === resourceName) { diff --git a/classes/ResourceRoom.js b/classes/ResourceRoom.js index 90c959d01..e14df3372 100644 --- a/classes/ResourceRoom.js +++ b/classes/ResourceRoom.js @@ -7,7 +7,7 @@ const _ = require('lodash') const { Config } = require('./Config.js') const { Resource } = require('../classes/Resource.js') const { File, ResourceType, DataType } = require('../classes/File.js') -const { deslugifyCollectionName } = require('../utils/utils.js') +const { getCommitAndTreeSha, getTree, sendTree, deslugifyCollectionName } = require('../utils/utils.js') // Constants const RESOURCE_ROOM_INDEX_PATH = 'index.html' @@ -72,6 +72,7 @@ class ResourceRoom { async rename(newResourceRoom) { try { + const commitMessage = `Rename resource room from ${resourceRoomName} to ${newResourceRoom}` // Add resource room to config const config = new Config(this.accessToken, this.siteName) const { content, sha } = await config.read() @@ -106,7 +107,8 @@ class ResourceRoom { const newNavContent = base64.encode(yaml.safeDump(newNavContentObject)) await nav.update(NAV_FILE_NAME, newNavContent, navSha) - const { gitTree, currentCommitSha } = await getRootTree(this.siteName, this.accessToken); + const { currentCommitSha, treeSha } = await getCommitAndTreeSha(this.siteName, this.accessToken) + const gitTree = await getTree(this.siteName, this.accessToken, treeSha); const newGitTree = gitTree.map(item => { if (item.path === resourceRoomName) { return { @@ -117,7 +119,7 @@ class ResourceRoom { return item } }) - await sendTree(newGitTree, currentCommitSha, this.siteName, this.accessToken, `Rename collection from ${oldCollectionName} to ${newCollectionName}`); + await sendTree(newGitTree, currentCommitSha, this.siteName, this.accessToken, commitMessage); await config.update(newContent, sha) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index ca47c58a7..ef441b66e 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -955,6 +955,23 @@ paths: application/json: schema: $ref: "#/components/schemas/SiteListResponse" + /sites/{siteName}: + get: + tags: + - Sites + parameters: + - name: siteName + in: path + required: true + schema: + type: string + description: Checks if site exists and user has write access + responses: + 200: + description: Success + 404: + description: Not found + /sites/{siteName}/resource-room: get: diff --git a/middleware/routeHandler.js b/middleware/routeHandler.js index 5670083f4..d53eccf8c 100644 --- a/middleware/routeHandler.js +++ b/middleware/routeHandler.js @@ -1,10 +1,34 @@ +const { backOff } = require('exponential-backoff') + +const { getCommitAndTreeSha, revertCommit } = require('../utils/utils.js') + const attachRouteHandlerWrapper = (routeHandler) => async (req, res, next) => { - routeHandler(req, res).catch((err) => { - next(err) - }) + routeHandler(req, res).catch((err) => { + next(err) + }) +} + +const attachRollbackRouteHandlerWrapper = (routeHandler) => async (req, res, next) => { + const { accessToken } = req + const { siteName } = req.params + let originalCommitSha + try { + const { currentCommitSha } = await getCommitAndTreeSha(siteName, accessToken) + originalCommitSha = currentCommitSha + } catch (err) { + next(err) } + routeHandler(req, res).catch(async (err) => { + try { + await backOff(() => revertCommit(originalCommitSha, siteName, accessToken)) + } catch (retryErr) { + next(retryErr) + } + next(err) + }) +} - module.exports = { - attachRouteHandlerWrapper, - } - \ No newline at end of file +module.exports = { + attachRouteHandlerWrapper, + attachRollbackRouteHandlerWrapper, +} \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index a894f7e25..a4ccb5252 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1018,6 +1018,11 @@ "homedir-polyfill": "^1.0.1" } }, + "exponential-backoff": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/exponential-backoff/-/exponential-backoff-3.1.0.tgz", + "integrity": "sha512-oBuz5SYz5zzyuHINoe9ooePwSu0xApKWgeNzok4hZ5YKXFh9zrQBEM15CXqoZkJJPuI2ArvqjPQd8UKJA753XA==" + }, "express": { "version": "4.16.4", "resolved": "https://registry.npmjs.org/express/-/express-4.16.4.tgz", diff --git a/package.json b/package.json index 25be48f12..4c3d93079 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ "cors": "^2.8.5", "debug": "~2.6.9", "dotenv": "^8.1.0", + "exponential-backoff": "^3.1.0", "express": "~4.16.1", "http-errors": "~1.6.3", "js-base64": "^2.5.1", diff --git a/routes/collectionPages.js b/routes/collectionPages.js index 7c346373b..512f63bc0 100644 --- a/routes/collectionPages.js +++ b/routes/collectionPages.js @@ -6,7 +6,7 @@ const base64 = require('base-64'); const _ = require('lodash'); // Import middleware -const { attachRouteHandlerWrapper } = require('../middleware/routeHandler') +const { attachRouteHandlerWrapper, attachRollbackRouteHandlerWrapper } = require('../middleware/routeHandler') // Import classes const { Collection } = require('../classes/Collection.js') @@ -203,10 +203,10 @@ async function renameCollectionPage (req, res, next) { router.get('/:siteName/collections/:collectionName', attachRouteHandlerWrapper(listCollectionPages)) router.get('/:siteName/collections/:collectionName/pages', attachRouteHandlerWrapper(listCollectionPagesDetails)) -router.post('/:siteName/collections/:collectionName/pages', attachRouteHandlerWrapper(createNewcollectionPage)) +router.post('/:siteName/collections/:collectionName/pages', attachRollbackRouteHandlerWrapper(createNewcollectionPage)) router.get('/:siteName/collections/:collectionName/pages/:pageName', attachRouteHandlerWrapper(readCollectionPage)) router.post('/:siteName/collections/:collectionName/pages/:pageName', attachRouteHandlerWrapper(updateCollectionPage)) -router.delete('/:siteName/collections/:collectionName/pages/:pageName', attachRouteHandlerWrapper(deleteCollectionPage)) -router.post('/:siteName/collections/:collectionName/pages/:pageName/rename/:newPageName', attachRouteHandlerWrapper(renameCollectionPage)) +router.delete('/:siteName/collections/:collectionName/pages/:pageName', attachRollbackRouteHandlerWrapper(deleteCollectionPage)) +router.post('/:siteName/collections/:collectionName/pages/:pageName/rename/:newPageName', attachRollbackRouteHandlerWrapper(renameCollectionPage)) module.exports = router; \ No newline at end of file diff --git a/routes/collections.js b/routes/collections.js index 6a47af4b7..a993c45bf 100644 --- a/routes/collections.js +++ b/routes/collections.js @@ -2,7 +2,7 @@ const express = require('express'); const router = express.Router(); // Import middleware -const { attachRouteHandlerWrapper } = require('../middleware/routeHandler') +const { attachRouteHandlerWrapper, attachRollbackRouteHandlerWrapper } = require('../middleware/routeHandler') // Import classes const { Collection } = require('../classes/Collection.js') @@ -59,8 +59,8 @@ async function renameCollection (req, res, next) { } router.get('/:siteName/collections', attachRouteHandlerWrapper(listCollections)) -router.post('/:siteName/collections', attachRouteHandlerWrapper(createNewCollection)) -router.delete('/:siteName/collections/:collectionName', attachRouteHandlerWrapper(deleteCollection)) -router.post('/:siteName/collections/:collectionName/rename/:newCollectionName', attachRouteHandlerWrapper(renameCollection)) +router.post('/:siteName/collections', attachRollbackRouteHandlerWrapper(createNewCollection)) +router.delete('/:siteName/collections/:collectionName', attachRollbackRouteHandlerWrapper(deleteCollection)) +router.post('/:siteName/collections/:collectionName/rename/:newCollectionName', attachRollbackRouteHandlerWrapper(renameCollection)) module.exports = router; \ No newline at end of file diff --git a/routes/pages.js b/routes/pages.js index b623232d4..0147d8b77 100644 --- a/routes/pages.js +++ b/routes/pages.js @@ -4,7 +4,7 @@ const Bluebird = require('bluebird') const _ = require('lodash') // Import middleware -const { attachRouteHandlerWrapper } = require('../middleware/routeHandler') +const { attachRouteHandlerWrapper, attachRollbackRouteHandlerWrapper } = require('../middleware/routeHandler') // Import classes const { File, PageType, CollectionPageType } = require('../classes/File.js') @@ -163,6 +163,6 @@ router.post('/:siteName/pages', attachRouteHandlerWrapper(createNewPage)) router.get('/:siteName/pages/:pageName', attachRouteHandlerWrapper(readPage)) router.post('/:siteName/pages/:pageName', attachRouteHandlerWrapper(updatePage)) router.delete('/:siteName/pages/:pageName', attachRouteHandlerWrapper(deletePage)) -router.post('/:siteName/pages/:pageName/rename/:newPageName', attachRouteHandlerWrapper(renamePage)) +router.post('/:siteName/pages/:pageName/rename/:newPageName', attachRollbackRouteHandlerWrapper(renamePage)) module.exports = router; \ No newline at end of file diff --git a/routes/resourcePages.js b/routes/resourcePages.js index 5b75a14e7..16b58c528 100644 --- a/routes/resourcePages.js +++ b/routes/resourcePages.js @@ -2,7 +2,7 @@ const express = require('express'); const router = express.Router(); // Import middleware -const { attachRouteHandlerWrapper } = require('../middleware/routeHandler') +const { attachRouteHandlerWrapper, attachRollbackRouteHandlerWrapper } = require('../middleware/routeHandler') // Import classes const { File, ResourcePageType } = require('../classes/File.js') @@ -140,10 +140,10 @@ async function renameResourcePage (req, res, next) { res.status(200).json({ resourceName, pageName: newPageName, content, sha: newSha }) } router.get('/:siteName/resources/:resourceName', attachRouteHandlerWrapper(listResourcePages)) -router.post('/:siteName/resources/:resourceName/pages', attachRouteHandlerWrapper(createNewResourcePage)) +router.post('/:siteName/resources/:resourceName/pages', attachRollbackRouteHandlerWrapper(createNewResourcePage)) router.get('/:siteName/resources/:resourceName/pages/:pageName', attachRouteHandlerWrapper(readResourcePage)) router.post('/:siteName/resources/:resourceName/pages/:pageName', attachRouteHandlerWrapper(updateResourcePage)) -router.delete('/:siteName/resources/:resourceName/pages/:pageName', attachRouteHandlerWrapper(deleteResourcePage)) -router.post('/:siteName/resources/:resourceName/pages/:pageName/rename/:newPageName', attachRouteHandlerWrapper(renameResourcePage)) +router.delete('/:siteName/resources/:resourceName/pages/:pageName', attachRollbackRouteHandlerWrapper(deleteResourcePage)) +router.post('/:siteName/resources/:resourceName/pages/:pageName/rename/:newPageName', attachRollbackRouteHandlerWrapper(renameResourcePage)) module.exports = router; \ No newline at end of file diff --git a/routes/resourceRoom.js b/routes/resourceRoom.js index dbf3bf7bc..7ba100b71 100644 --- a/routes/resourceRoom.js +++ b/routes/resourceRoom.js @@ -3,7 +3,7 @@ const router = express.Router(); // Import classes const { ResourceRoom } = require('../classes/ResourceRoom.js'); -const { attachRouteHandlerWrapper } = require('../middleware/routeHandler'); +const { attachRouteHandlerWrapper, attachRollbackRouteHandlerWrapper } = require('../middleware/routeHandler'); // Get resource room name async function getResourceRoomName (req, res, next) { @@ -57,8 +57,8 @@ async function deleteResourceRoom(req, res, next) { } router.get('/:siteName/resource-room', attachRouteHandlerWrapper(getResourceRoomName)) -router.post('/:siteName/resource-room', attachRouteHandlerWrapper(createResourceRoom)) -router.post('/:siteName/resource-room/:resourceRoom', attachRouteHandlerWrapper(renameResourceRoom)) -router.delete('/:siteName/resource-room', attachRouteHandlerWrapper(deleteResourceRoom)) +router.post('/:siteName/resource-room', attachRollbackRouteHandlerWrapper(createResourceRoom)) +router.post('/:siteName/resource-room/:resourceRoom', attachRollbackRouteHandlerWrapper(renameResourceRoom)) +router.delete('/:siteName/resource-room', attachRollbackRouteHandlerWrapper(deleteResourceRoom)) module.exports = router; \ No newline at end of file diff --git a/routes/resources.js b/routes/resources.js index 3541eeff3..e94986e05 100644 --- a/routes/resources.js +++ b/routes/resources.js @@ -2,7 +2,7 @@ const express = require('express'); const router = express.Router(); // Import middleware -const { attachRouteHandlerWrapper } = require('../middleware/routeHandler') +const { attachRouteHandlerWrapper, attachRollbackRouteHandlerWrapper } = require('../middleware/routeHandler') // Import classes const { ResourceRoom } = require('../classes/ResourceRoom.js') @@ -66,8 +66,8 @@ async function renameResource (req, res, next) { } router.get('/:siteName/resources', attachRouteHandlerWrapper(listResources)) -router.post('/:siteName/resources', attachRouteHandlerWrapper(createNewResource)) -router.delete('/:siteName/resources/:resourceName', attachRouteHandlerWrapper(deleteResource)) -router.post('/:siteName/resources/:resourceName/rename/:newResourceName', attachRouteHandlerWrapper(renameResource)) +router.post('/:siteName/resources', attachRollbackRouteHandlerWrapper(createNewResource)) +router.delete('/:siteName/resources/:resourceName', attachRollbackRouteHandlerWrapper(deleteResource)) +router.post('/:siteName/resources/:resourceName/rename/:newResourceName', attachRollbackRouteHandlerWrapper(renameResource)) module.exports = router; \ No newline at end of file diff --git a/routes/settings.js b/routes/settings.js index 8e9bf9497..d2d6acd5f 100644 --- a/routes/settings.js +++ b/routes/settings.js @@ -2,7 +2,7 @@ const express = require('express'); const router = express.Router(); // Import middleware -const { attachRouteHandlerWrapper } = require('../middleware/routeHandler') +const { attachRouteHandlerWrapper, attachRollbackRouteHandlerWrapper } = require('../middleware/routeHandler') // Import Classes const { Settings } = require('../classes/Settings.js') @@ -26,6 +26,6 @@ async function updateSettings (req, res, next) { } router.get('/:siteName/settings', attachRouteHandlerWrapper(getSettings)) -router.post('/:siteName/settings', attachRouteHandlerWrapper(updateSettings)) +router.post('/:siteName/settings', attachRollbackRouteHandlerWrapper(updateSettings)) module.exports = router; diff --git a/utils/utils.js b/utils/utils.js index 7c6fcf222..41852dbd3 100644 --- a/utils/utils.js +++ b/utils/utils.js @@ -27,8 +27,7 @@ function deslugifyCollectionPage(collectionPageName) { ) } -// retrieve the tree item -async function getRootTree(repo, accessToken, branchRef='staging') { +async function getCommitAndTreeSha(repo, accessToken, branchRef='staging') { try { const headers = { Authorization: `token ${accessToken}`, @@ -45,16 +44,9 @@ async function getRootTree(repo, accessToken, branchRef='staging') { const { commit: { tree: { sha: treeSha } } } = commits[0]; const currentCommitSha = commits[0].sha; - const { data: { tree: gitTree } } = await axios.get(`https://api.github.com/repos/${GITHUB_ORG_NAME}/${repo}/git/trees/${treeSha}`, { - params: { - ref: branchRef, - }, - headers, - }); - - return { gitTree, currentCommitSha }; + return { treeSha, currentCommitSha }; } catch (err) { - console.log(err); + throw err } } @@ -73,9 +65,9 @@ async function getTree(repo, accessToken, treeSha, branchRef='staging') { headers, }); - return { gitTree }; + return gitTree; } catch (err) { - console.log(err); + throw err } } @@ -119,6 +111,27 @@ async function sendTree(gitTree, currentCommitSha, repo, accessToken, message, b }); } +// Revert the staging branch back to `originalCommitSha` +async function revertCommit(originalCommitSha, repo, accessToken, branchRef='staging') { + const headers = { + Authorization: `token ${accessToken}`, + Accept: 'application/json', + }; + + const baseRefEndpoint = `https://api.github.com/repos/${GITHUB_ORG_NAME}/${repo}/git/refs`; + const refEndpoint = `${baseRefEndpoint}/heads/${branchRef}`; + + /** + * The `staging` branch reference will now point to `currentCommitSha` + */ + await axios.patch(refEndpoint, { + sha: originalCommitSha, + force: true, + }, { + headers, + }); +} + /** * A function to deslugify a collection's name */ @@ -132,7 +145,8 @@ function deslugifyCollectionName(collectionName) { module.exports = { deslugifyCollectionPage, deslugifyCollectionName, - getRootTree, + getCommitAndTreeSha, getTree, sendTree, + revertCommit, } From f3b5f4b773b36f30af77f89fcdd561ae19f368d2 Mon Sep 17 00:00:00 2001 From: kwajiehao <31984694+kwajiehao@users.noreply.github.com> Date: Fri, 11 Dec 2020 11:14:46 +0800 Subject: [PATCH 14/18] chore: add `/v1` to all URI paths (#95) This commit adds a `/v1` prefix to all URI paths to version our API. This will be useful for several reasons: - allows us to gracefully accommodate a v2 api which uses github's graphql API - allows us to craft tight firewall rules (currently, we search for whether a URI path contains `/sites` or `/auth`, which would allow nonsense URI paths like `/authors`) Co-authored-by: Jie Hao Kwa --- docs/openapi.yaml | 54 ++++++++++----------- middleware/auth.js | 116 ++++++++++++++++++++++----------------------- server.js | 32 ++++++------- 3 files changed, 101 insertions(+), 101 deletions(-) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index ef441b66e..44b7600b3 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -213,7 +213,7 @@ components: paths: - /: + /v1: get: tags: - Authentication @@ -225,7 +225,7 @@ paths: application/json: schema: $ref: "#/components/schemas/OAuthParams" - /sites/{siteName}/collections: + /v1/sites/{siteName}/collections: get: tags: - Collections @@ -265,7 +265,7 @@ paths: application/json: schema: $ref: "#/components/schemas/CollectionNameResponse" - /sites/{siteName}/collections/{collectionName}: + /v1/sites/{siteName}/collections/{collectionName}: get: tags: - Collection Pages @@ -310,7 +310,7 @@ paths: application/json: schema: $ref: "#/components/schemas/CollectionNameResponse" - /sites/{siteName}/collections/{collectionName}/rename/{newCollectionName}: + /v1/sites/{siteName}/collections/{collectionName}/rename/{newCollectionName}: post: tags: - Collections @@ -343,7 +343,7 @@ paths: application/json: schema: $ref: "#/components/schemas/RenameCollection" - /sites/{siteName}/collections/{collectionName}/pages: + /v1/sites/{siteName}/collections/{collectionName}/pages: post: tags: - Collection Pages @@ -371,7 +371,7 @@ paths: application/json: schema: $ref: "#/components/schemas/CollectionListResponse" - /sites/{siteName}/collections/{collectionName}/pages/{pageName}: + /v1/sites/{siteName}/collections/{collectionName}/pages/{pageName}: get: tags: - Collection Pages @@ -459,7 +459,7 @@ paths: responses: 200: description: Success - /sites/{siteName}/collections/{collectionName}/pages/{pageName}/rename/{newPageName}: + /v1/sites/{siteName}/collections/{collectionName}/pages/{pageName}/rename/{newPageName}: post: tags: - Collection Pages @@ -498,7 +498,7 @@ paths: schema: $ref: "#/components/schemas/CollectionPageResponse" - /sites/{siteName}/pages: + /v1/sites/{siteName}/pages: get: tags: - Pages @@ -543,7 +543,7 @@ paths: application/json: schema: $ref: "#/components/schemas/PageResponse" - /sites/{siteName}/pages/{pageName}: + /v1/sites/{siteName}/pages/{pageName}: get: tags: - Pages @@ -616,7 +616,7 @@ paths: responses: 200: description: Success - /sites/{siteName}/pages/{pageName}/rename/{newPageName}: + /v1/sites/{siteName}/pages/{pageName}/rename/{newPageName}: post: tags: - Pages @@ -650,7 +650,7 @@ paths: schema: $ref: "#/components/schemas/PageResponse" - /sites/{siteName}/documents: + /v1/sites/{siteName}/documents: get: tags: - Documents @@ -690,7 +690,7 @@ paths: application/json: schema: $ref: "#/components/schemas/DocumentResponse" - /sites/{siteName}/documents/{documentName}: + /v1/sites/{siteName}/documents/{documentName}: get: tags: - Documents @@ -763,7 +763,7 @@ paths: responses: 200: description: Success - /sites/{siteName}/documents/{documentName}/rename/{newDocumentName}: + /v1/sites/{siteName}/documents/{documentName}/rename/{newDocumentName}: post: tags: - Documents @@ -797,7 +797,7 @@ paths: schema: $ref: "#/components/schemas/DocumentResponse" - /sites/{siteName}/images: + /v1/sites/{siteName}/images: get: tags: - Images @@ -837,7 +837,7 @@ paths: application/json: schema: $ref: "#/components/schemas/ImageResponse" - /sites/{siteName}/images/{imageName}: + /v1/sites/{siteName}/images/{imageName}: get: tags: - Images @@ -910,7 +910,7 @@ paths: responses: 200: description: Success - /sites/{siteName}/images/{imageName}/rename/{newImageName}: + /v1/sites/{siteName}/images/{imageName}/rename/{newImageName}: post: tags: - Images @@ -943,7 +943,7 @@ paths: application/json: schema: $ref: "#/components/schemas/ImageResponse" - /sites: + /v1/sites: get: tags: - Sites @@ -973,7 +973,7 @@ paths: description: Not found - /sites/{siteName}/resource-room: + /v1/sites/{siteName}/resource-room: get: tags: - Resource Room @@ -1026,7 +1026,7 @@ paths: responses: 200: description: Success - /sites/{siteName}/resource-room/{resourceRoom}: + /v1/sites/{siteName}/resource-room/{resourceRoom}: post: tags: - Resource Room @@ -1049,7 +1049,7 @@ paths: application/json: schema: $ref: "#/components/schemas/ResourceRoom" - /sites/{siteName}/resources: + /v1/sites/{siteName}/resources: get: tags: - Resources @@ -1089,7 +1089,7 @@ paths: application/json: schema: $ref: "#/components/schemas/ResourceResponse" - /sites/{siteName}/resources/{resourceName}: + /v1/sites/{siteName}/resources/{resourceName}: get: tags: - Resource Pages @@ -1130,7 +1130,7 @@ paths: responses: 200: description: Success - /sites/{siteName}/resources/{resourceName}/rename/{newResourceName}: + /v1/sites/{siteName}/resources/{resourceName}/rename/{newResourceName}: post: tags: - Resources @@ -1158,7 +1158,7 @@ paths: application/json: schema: $ref: "#/components/schemas/RenameResource" - /sites/{siteName}/resources/{resourceName}/pages: + /v1/sites/{siteName}/resources/{resourceName}/pages: post: tags: - Resource Pages @@ -1186,7 +1186,7 @@ paths: application/json: schema: $ref: "#/components/schemas/ResourcePageResponse" - /sites/{siteName}/resources/{resourceName}/pages/{pageName}: + /v1/sites/{siteName}/resources/{resourceName}/pages/{pageName}: get: tags: - Resource Pages @@ -1274,7 +1274,7 @@ paths: responses: 200: description: Success - /sites/{siteName}/resources/{resourceName}/pages/{pageName}/rename/{newPageName}: + /v1/sites/{siteName}/resources/{resourceName}/pages/{pageName}/rename/{newPageName}: post: tags: - Resource Pages @@ -1312,7 +1312,7 @@ paths: application/json: schema: $ref: "#/components/schemas/ResourcePageResponse" - /sites/{siteName}/menus: + /v1/sites/{siteName}/menus: get: tags: - Menus @@ -1330,7 +1330,7 @@ paths: application/json: schema: $ref: "#/components/schemas/MenuListResponse" - /sites/{siteName}/menus/{menuName}: + /v1/sites/{siteName}/menus/{menuName}: get: tags: - Menus diff --git a/middleware/auth.js b/middleware/auth.js index 576673a06..99dd0b07e 100644 --- a/middleware/auth.js +++ b/middleware/auth.js @@ -37,94 +37,94 @@ const verifyJwt = (req, res, next) => { } // Login and logout -auth.get('/auth', noVerify) -auth.get('/auth/logout', noVerify) +auth.get('/v1/auth', noVerify) +auth.get('/v1/auth/logout', noVerify) // Index -auth.get('/', noVerify) +auth.get('/v1', noVerify) // Homepage -auth.get('/sites/:siteName/homepage', verifyJwt) -auth.post('/sites/:siteName/homepage', verifyJwt) +auth.get('/v1/sites/:siteName/homepage', verifyJwt) +auth.post('/v1/sites/:siteName/homepage', verifyJwt) // Collection pages -auth.get('/sites/:siteName/collections/:collectionName', verifyJwt) -auth.get('/sites/:siteName/collections/:collectionName/pages', verifyJwt) -auth.post('/sites/:siteName/collections/:collectionName/pages', verifyJwt) -auth.get('/sites/:siteName/collections/:collectionName/pages/:pageName', verifyJwt) -auth.post('/sites/:siteName/collections/:collectionName/pages/:pageName', verifyJwt) -auth.delete('/sites/:siteName/collections/:collectionName/pages/:pageName', verifyJwt) -auth.post('/sites/:siteName/collections/:collectionName/pages/:pageName/rename/:newPageName', verifyJwt) +auth.get('/v1/sites/:siteName/collections/:collectionName', verifyJwt) +auth.get('/v1/sites/:siteName/collections/:collectionName/pages', verifyJwt) +auth.post('/v1/sites/:siteName/collections/:collectionName/pages', verifyJwt) +auth.get('/v1/sites/:siteName/collections/:collectionName/pages/:pageName', verifyJwt) +auth.post('/v1/sites/:siteName/collections/:collectionName/pages/:pageName', verifyJwt) +auth.delete('/v1/sites/:siteName/collections/:collectionName/pages/:pageName', verifyJwt) +auth.post('/v1/sites/:siteName/collections/:collectionName/pages/:pageName/rename/:newPageName', verifyJwt) // Collections -auth.get('/sites/:siteName/collections', verifyJwt) -auth.post('/sites/:siteName/collections', verifyJwt) -auth.delete('/sites/:siteName/collections/:collectionName', verifyJwt) -auth.post('/sites/:siteName/collections/:collectionName/rename/:newCollectionName', verifyJwt) +auth.get('/v1/sites/:siteName/collections', verifyJwt) +auth.post('/v1/sites/:siteName/collections', verifyJwt) +auth.delete('/v1/sites/:siteName/collections/:collectionName', verifyJwt) +auth.post('/v1/sites/:siteName/collections/:collectionName/rename/:newCollectionName', verifyJwt) // Documents -auth.get('/sites/:siteName/documents', verifyJwt) -auth.post('/sites/:siteName/documents', verifyJwt) -auth.get('/sites/:siteName/documents/:documentName', verifyJwt) -auth.post('/sites/:siteName/documents/:documentName', verifyJwt) -auth.delete('/sites/:siteName/documents/:documentName', verifyJwt) -auth.post('/sites/:siteName/documents/:documentName/rename/:newDocumentName', verifyJwt) +auth.get('/v1/sites/:siteName/documents', verifyJwt) +auth.post('/v1/sites/:siteName/documents', verifyJwt) +auth.get('/v1/sites/:siteName/documents/:documentName', verifyJwt) +auth.post('/v1/sites/:siteName/documents/:documentName', verifyJwt) +auth.delete('/v1/sites/:siteName/documents/:documentName', verifyJwt) +auth.post('/v1/sites/:siteName/documents/:documentName/rename/:newDocumentName', verifyJwt) // Images -auth.get('/sites/:siteName/images', verifyJwt) -auth.post('/sites/:siteName/images', verifyJwt) -auth.get('/sites/:siteName/images/:imageName', verifyJwt) -auth.post('/sites/:siteName/images/:imageName', verifyJwt) -auth.delete('/sites/:siteName/images/:imageName', verifyJwt) -auth.post('/sites/:siteName/images/:imageName/rename/:newImageName', verifyJwt) +auth.get('/v1/sites/:siteName/images', verifyJwt) +auth.post('v/sites/:siteName/images', verifyJwt) +auth.get('/v1/sites/:siteName/images/:imageName', verifyJwt) +auth.post('/v1/sites/:siteName/images/:imageName', verifyJwt) +auth.delete('/v1/sites/:siteName/images/:imageName', verifyJwt) +auth.post('/v1/sites/:siteName/images/:imageName/rename/:newImageName', verifyJwt) // Menu directory -auth.get('/sites/:siteName/tree', verifyJwt) +auth.get('/v1/sites/:siteName/tree', verifyJwt) // Menu -auth.get('/sites/:siteName/menus', verifyJwt) -auth.get('/sites/:siteName/menus/:menuName', verifyJwt) -auth.post('/sites/:siteName/menus/:menuName', verifyJwt) +auth.get('/v1/sites/:siteName/menus', verifyJwt) +auth.get('/v1/sites/:siteName/menus/:menuName', verifyJwt) +auth.post('/v1/sites/:siteName/menus/:menuName', verifyJwt) // Pages -auth.get('/sites/:siteName/pages', verifyJwt) -auth.get('/sites/:siteName/unlinkedPages', verifyJwt) -auth.post('/sites/:siteName/pages', verifyJwt) -auth.get('/sites/:siteName/pages/:pageName', verifyJwt) -auth.post('/sites/:siteName/pages/:pageName', verifyJwt) -auth.delete('/sites/:siteName/pages/:pageName', verifyJwt) -auth.post('/sites/:siteName/pages/:pageName/rename/:newPageName', verifyJwt) +auth.get('/v1/sites/:siteName/pages', verifyJwt) +auth.get('/v1/sites/:siteName/unlinkedPages', verifyJwt) +auth.post('/v1/sites/:siteName/pages', verifyJwt) +auth.get('/v1/sites/:siteName/pages/:pageName', verifyJwt) +auth.post('/v1/sites/:siteName/pages/:pageName', verifyJwt) +auth.delete('/v1/sites/:siteName/pages/:pageName', verifyJwt) +auth.post('/v1/sites/:siteName/pages/:pageName/rename/:newPageName', verifyJwt) // Resource pages -auth.get('/sites/:siteName/resources/:resourceName', verifyJwt) -auth.post('/sites/:siteName/resources/:resourceName/pages', verifyJwt) -auth.get('/sites/:siteName/resources/:resourceName/pages/:pageName', verifyJwt) -auth.post('/sites/:siteName/resources/:resourceName/pages/:pageName', verifyJwt) -auth.delete('/sites/:siteName/resources/:resourceName/pages/:pageName', verifyJwt) -auth.post('/sites/:siteName/resources/:resourceName/pages/:pageName/rename/:newPageName', verifyJwt) +auth.get('/v1/sites/:siteName/resources/:resourceName', verifyJwt) +auth.post('/v1/sites/:siteName/resources/:resourceName/pages', verifyJwt) +auth.get('/v1/sites/:siteName/resources/:resourceName/pages/:pageName', verifyJwt) +auth.post('/v1/sites/:siteName/resources/:resourceName/pages/:pageName', verifyJwt) +auth.delete('/v1/sites/:siteName/resources/:resourceName/pages/:pageName', verifyJwt) +auth.post('/v1/sites/:siteName/resources/:resourceName/pages/:pageName/rename/:newPageName', verifyJwt) // Resource room -auth.get('/sites/:siteName/resource-room', verifyJwt) -auth.post('/sites/:siteName/resource-room', verifyJwt) -auth.post('/sites/:siteName/resource-room/:resourceRoom', verifyJwt) -auth.delete('/sites/:siteName/resource-room', verifyJwt) +auth.get('/v1/sites/:siteName/resource-room', verifyJwt) +auth.post('/v1/sites/:siteName/resource-room', verifyJwt) +auth.post('/v1/sites/:siteName/resource-room/:resourceRoom', verifyJwt) +auth.delete('/v1/sites/:siteName/resource-room', verifyJwt) // Resources -auth.get('/sites/:siteName/resources', verifyJwt) -auth.post('/sites/:siteName/resources', verifyJwt) -auth.delete('/sites/:siteName/resources/:resourceName', verifyJwt) -auth.post('/sites/:siteName/resources/:resourceName/rename/:newResourceName', verifyJwt) +auth.get('/v1/sites/:siteName/resources', verifyJwt) +auth.post('/v1/sites/:siteName/resources', verifyJwt) +auth.delete('/v1/sites/:siteName/resources/:resourceName', verifyJwt) +auth.post('/v1/sites/:siteName/resources/:resourceName/rename/:newResourceName', verifyJwt) // Settings -auth.get('/sites/:siteName/settings', verifyJwt) -auth.post('/sites/:siteName/settings', verifyJwt) +auth.get('/v1/sites/:siteName/settings', verifyJwt) +auth.post('/v1/sites/:siteName/settings', verifyJwt) // Netlify toml -auth.get('/sites/:siteName/netlify-toml', verifyJwt) +auth.get('/v1/sites/:siteName/netlify-toml', verifyJwt) // Sites -auth.get('/sites', verifyJwt) -auth.get('/sites/:siteName', verifyJwt) +auth.get('/v1/sites', verifyJwt) +auth.get('/v1/sites/:siteName', verifyJwt) auth.use((req, res, next) => { if (!req.route) { diff --git a/server.js b/server.js index 44f4fce12..531f96181 100644 --- a/server.js +++ b/server.js @@ -50,22 +50,22 @@ app.use(auth) app.use(apiLogger) // Routes layer setup -app.use('/', indexRouter); -app.use('/auth', authRouter); -app.use('/sites', sitesRouter) -app.use('/sites', pagesRouter) -app.use('/sites', collectionsRouter) -app.use('/sites', collectionPagesRouter) -app.use('/sites', resourceRoomRouter) -app.use('/sites', resourcesRouter) -app.use('/sites', resourcePagesRouter) -app.use('/sites', imagesRouter) -app.use('/sites', documentsRouter) -app.use('/sites', menuRouter) -app.use('/sites', homepageRouter) -app.use('/sites', menuDirectoryRouter) -app.use('/sites', settingsRouter) -app.use('/sites', netlifyTomlRouter) +app.use('/v1', indexRouter); +app.use('/v1/auth', authRouter); +app.use('/v1/sites', sitesRouter) +app.use('/v1/sites', pagesRouter) +app.use('/v1/sites', collectionsRouter) +app.use('/v1/sites', collectionPagesRouter) +app.use('/v1/sites', resourceRoomRouter) +app.use('/v1/sites', resourcesRouter) +app.use('/v1/sites', resourcePagesRouter) +app.use('/v1/sites', imagesRouter) +app.use('/v1/sites', documentsRouter) +app.use('/v1/sites', menuRouter) +app.use('/v1/sites', homepageRouter) +app.use('/v1/sites', menuDirectoryRouter) +app.use('/v1/sites', settingsRouter) +app.use('/v1/sites', netlifyTomlRouter) // catch 404 and forward to error handler app.use(function(req, res, next) { From 97328af682ef46555a45ce523f921024879e9291 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 14 Dec 2020 06:26:53 +0000 Subject: [PATCH 15/18] build(deps): bump ini from 1.3.5 to 1.3.7 (#96) --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index a4ccb5252..2e8f5f2bf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1649,9 +1649,9 @@ "integrity": "sha1-Yzwsg+PaQqUC9SRmAiSA9CCCYd4=" }, "ini": { - "version": "1.3.5", - "resolved": "https://registry.npmjs.org/ini/-/ini-1.3.5.tgz", - "integrity": "sha512-RZY5huIKCMRWDUqZlEi72f/lmXKMvuszcMBduliQ3nnWbx9X/ZBQO7DijMEYS9EhHBb2qacRUMtC7svLwe0lcw==", + "version": "1.3.7", + "resolved": "https://registry.npmjs.org/ini/-/ini-1.3.7.tgz", + "integrity": "sha512-iKpRpXP+CrP2jyrxvg1kMUpXDyRUFDWurxbnVT1vQPx+Wz9uCYsMIqYuSBLV+PAaZG/d7kRLKRFc9oDMsH+mFQ==", "dev": true }, "inquirer": { From 85dde7a3b11302f7213e7c96b576736e85e368d9 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Tue, 15 Dec 2020 18:34:57 +0800 Subject: [PATCH 16/18] Fix/typo in images endpoint and increase limit of requests (#99) * Fix: typo * Fix: increase limit of requests --- middleware/auth.js | 2 +- server.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/middleware/auth.js b/middleware/auth.js index 99dd0b07e..42714401c 100644 --- a/middleware/auth.js +++ b/middleware/auth.js @@ -72,7 +72,7 @@ auth.post('/v1/sites/:siteName/documents/:documentName/rename/:newDocumentName', // Images auth.get('/v1/sites/:siteName/images', verifyJwt) -auth.post('v/sites/:siteName/images', verifyJwt) +auth.post('/v1/sites/:siteName/images', verifyJwt) auth.get('/v1/sites/:siteName/images/:imageName', verifyJwt) auth.post('/v1/sites/:siteName/images/:imageName', verifyJwt) auth.delete('/v1/sites/:siteName/images/:imageName', verifyJwt) diff --git a/server.js b/server.js index 531f96181..fbb93e43a 100644 --- a/server.js +++ b/server.js @@ -38,7 +38,7 @@ app.use(cors({ 'origin': FRONTEND_URL, 'credentials': true, })) -app.use(express.json({ limit: '5mb'})); +app.use(express.json({ limit: '7mb'})); app.use(express.urlencoded({ extended: false })); app.use(cookieParser()); app.use(express.static(path.join(__dirname, 'public'))); From e26eec4763cbaa3eb01fa9377e29ca0a34efd302 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Wed, 16 Dec 2020 15:45:49 +0800 Subject: [PATCH 17/18] Fix: add check for files in directory when reading file (#97) --- classes/File.js | 1 + 1 file changed, 1 insertion(+) diff --git a/classes/File.js b/classes/File.js index 4c806c196..24ecb7706 100644 --- a/classes/File.js +++ b/classes/File.js @@ -88,6 +88,7 @@ class File { async read(fileName) { try { const files = await this.list() + if (_.isEmpty(files)) throw new NotFoundError ('File does not exist') const fileToRead = files.filter((file) => file.fileName === fileName)[0] if (fileToRead === undefined) throw new NotFoundError ('File does not exist') const endpoint = `${this.baseBlobEndpoint}/${fileToRead.sha}` From 6529512a61e3b450f1423d23a46cd315124233f2 Mon Sep 17 00:00:00 2001 From: kwajiehao <31984694+kwajiehao@users.noreply.github.com> Date: Thu, 17 Dec 2020 17:51:02 +0800 Subject: [PATCH 18/18] Fix/increase nginx max body size (#101) * feat: configure higher limit for nginx client_max_body_size Even though we set a higher file size limit in our express app, we found that uploads exceeding 1mb were being rejected by our server. This was only occurring on the staging website, but not on local dev env, which implies a server config issue. After reading the nginx error logs, we found that nginx was rejecting the requests because the body size of the request was too large. As a result, we are introducing an ebextension which increases the max body size (client_max_body_size) to 10mb (a slight increase over the 7mb limit we set for express, since we want to be conservative and allow for additional overhead). Finally, the ebextension file includes a command to reload the nginx service so that the new configs are used. * chore: update gh workflow file to zip .ebextensions Co-authored-by: Jie Hao Kwa --- .ebextensions/nginx-max-body-size.config | 11 +++++++++++ .github/workflows/ci.yml | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 .ebextensions/nginx-max-body-size.config diff --git a/.ebextensions/nginx-max-body-size.config b/.ebextensions/nginx-max-body-size.config new file mode 100644 index 000000000..fdf7dacca --- /dev/null +++ b/.ebextensions/nginx-max-body-size.config @@ -0,0 +1,11 @@ +files: + "/etc/nginx/conf.d/proxy.conf" : + mode: "000755" + owner: root + group: root + content: | + client_max_body_size 10M; + +commands: + 00_reload_nginx: + command: "service nginx reload" \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c92d71ce0..e0c8ea01b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,7 +55,7 @@ jobs: - name: Install NPM run: npm ci - name: Zip application - run: zip -r "deploy.zip" * -x .env-example .gitignore package-lock.json + run: zip -r "deploy.zip" * .ebextensions -x .env-example .gitignore package-lock.json - name: Get timestamp shell: bash run: echo "##[set-output name=timestamp;]$(env TZ=Asia/Singapore date '+%Y%m%d%H%M%S')"