Merge pull request #211 from shortlist-digital/technical-debt

Technical debt
shortlist-digital · Jul 11, 2017 · a636541 · a636541
2 parents 8633e85 + 673a386
commit a636541
Show file tree

Hide file tree

Showing 18 changed files with 325 additions and 208 deletions.
diff --git a/package.json b/package.json
@@ -63,6 +63,7 @@
     "glob-fs": "^0.1.6",
     "h2o2": "^5.4.0",
     "hapi": "~16.4.3",
+    "http-status": "^1.0.1",
     "idx": "^1.5.0",
     "inert": "^4.2.0",
     "isomorphic-fetch": "^2.2.1",

diff --git a/src/server/handle-api.js b/src/server/handle-api.js
@@ -13,10 +13,10 @@ export default ({ server, config }) => {
   server.route({
     method: 'GET',
     path: '/api/v1/{query*}',
-    handler: (req, reply) => {
+    handler: (request, reply) => {
 
       const base = `${stripLeadingTrailingSlashes(config.siteUrl)}/wp-json/wp/v2`
-      const path = `${req.params.query}${req.url.search}`
+      const path = `${request.params.query}${request.url.search}`
       const remote = `${base}/${path}`
       const cacheKey = stripLeadingTrailingSlashes(path)
 

diff --git a/src/server/handle-dynamic.js b/src/server/handle-dynamic.js
@@ -1,11 +1,12 @@
 import { match } from 'react-router'
 import { loadPropsOnServer } from 'async-props'
-import has from 'lodash/has'
 import idx from 'idx'
 import chalk from 'chalk'
+import HTTPStatus from 'http-status'
 
 import RouteWrapper from '../shared/route-wrapper'
-import { renderHtml } from './render'
+import handleApiResponse from '../shared/handle-api-response'
+import renderHtml from './render'
 import log from '../utilities/logger'
 import CacheManager, { stripLeadingTrailingSlashes } from '../utilities/cache-manager'
 let cacheManager = new CacheManager()
@@ -28,7 +29,7 @@ export default ({ server, config, assets }) => {
         // 500 if error from Router
         if (err) {
           log.error(err)
-          return reply(err.message).code(500)
+          return reply(err.message).code(HTTPStatus.INTERNAL_SERVER_ERROR)
         }
 
         // define global deets for nested components
@@ -41,7 +42,7 @@ export default ({ server, config, assets }) => {
               loadContext,
               assets
             })
-          ).code(404)
+          ).code(HTTPStatus.NOT_FOUND)
         }
 
         // 301/2 if redirect
@@ -55,17 +56,19 @@ export default ({ server, config, assets }) => {
           // 500 if error from AsyncProps
           if (err) {
             log.error(err)
-            return reply(err).code(500)
+            return reply(err).code(HTTPStatus.INTERNAL_SERVER_ERROR)
           }
 
-          let status = 200
+          const response = handleApiResponse(
+            idx(asyncProps, _ => _.propsArray[0].data),
+            renderProps.routes[1]
+          )
 
-          const failApi = has(asyncProps.propsArray[0], 'data.data.status')
-          const failRoute = renderProps.routes[1].path === '*'
-          const cacheKey = stripLeadingTrailingSlashes(request.url.path)
+          const status = idx(response, _ => _.code) ?
+            response.code :
+            HTTPStatus.OK
 
-          if (failApi || failRoute)
-            status = 404
+          const cacheKey = stripLeadingTrailingSlashes(request.url.path)
 
           // Find HTML based on path - might be undefined
           const cachedHTML = cache.get(cacheKey)
@@ -79,6 +82,7 @@ export default ({ server, config, assets }) => {
             // No HTML found for this path, or cache expired
             // Regenerate HTML from scratch
             const html = renderHtml({
+              response,
               renderProps,
               loadContext,
               asyncProps,

diff --git a/src/server/handle-preview.js b/src/server/handle-preview.js
@@ -1,24 +1,24 @@
 import chalk from 'chalk'
 import fetch from 'isomorphic-fetch'
 import log from '../utilities/logger'
+import { stripLeadingTrailingSlashes } from '../utilities/cache-manager'
 
 export default ({ server, config }) => {
 
   server.route({
     method: 'GET',
     path: '/api/preview/v1/{query*}',
-    handler: (req, reply) => {
+    handler: (request, reply) => {
 
-      const base = `${config.siteUrl}/wp-json/revision/v1`
-      const path = `${req.params.query}${req.url.search}`
+      const base = `${stripLeadingTrailingSlashes(config.siteUrl)}/wp-json/revision/v1`
+      const path = `${request.params.query}${request.url.search}`
       const remote = `${base}/${path}`
 
-      log.debug(`Preview: Server loading API response over HTTP for ${chalk.green(remote)}`)
-
       fetch(remote)
         .then(resp => resp.json())
         .then(resp => {
-          log.debug(`Preview: Server returned a fresh API response over HTTP for ${chalk.green(remote)}`)
+
+          log.debug(`Preview API response via HTTP for ${chalk.green(remote)}`)
           reply(resp)
         })
         .catch(err => log.error(err))

diff --git a/src/server/handle-purge.js b/src/server/handle-purge.js
@@ -1,43 +1,46 @@
 import chalk from 'chalk'
 import { match } from 'react-router'
+import HTTPStatus from 'http-status'
+import isFunction from 'lodash/isFunction'
+
 import RouteWrapper from '../shared/route-wrapper'
 import log from '../utilities/logger'
 import CacheManager from '../utilities/cache-manager'
+
 const cacheManager = new CacheManager()
 const purgePath = process.env.SECRET_PURGE_PATH || 'purge'
 
 export default ({ server, config }) => {
-  const calculateApiRoute = (path, cb) => {
-    match({
-      routes: RouteWrapper(config),
-      location: path
-    }, (err, redirectLocation, renderProps) => {
-      // The HOC data loader component is always at index [1] while there is progres bar
-      let endpoint = renderProps.components[1].endpoint
-      if (typeof endpoint == 'function') {
-        endpoint = endpoint(renderProps.params)
-      }
-      log.debug(`Purge request for endpoint ${chalk.green(endpoint)}`)
-      cb(endpoint)
-    })
-  }
 
   server.route({
     method: 'GET',
     path: `/${purgePath}/{path*}`,
     handler: (request, reply) => {
-      calculateApiRoute(request.params.path, (apiRoute) => {
 
-        log.debug(`Request: ${chalk.green(request.params.path)} mapped to API: ${apiRoute}`)
-        const remote = `${config.siteUrl}/wp-json/wp/v2/${apiRoute}`
+      match({
+        routes: RouteWrapper(config),
+        location: request.params.path
+      }, (err, redirectLocation, renderProps) => {
 
-        log.debug(`Directly clearing html cache for ${chalk.green(request.params.path)}`)
-        cacheManager.clearCache('html', request.params.path)
+        let endpoint = renderProps.components[1].endpoint
+        let pathToPurge = endpoint
+
+        // resolve function if required
+        if (isFunction(endpoint)) {
+          pathToPurge = endpoint(renderProps.params)
+        }
 
-        log.debug(`Directly clearing api cache for ${chalk.green(remote)}`)
-        cacheManager.clearCache('api', remote)
+        log.debug(`Purge path ${chalk.green(request.params.path)} mapped to ${chalk.green(pathToPurge)}`)
+
+        log.debug(`Cache clear ${chalk.green(pathToPurge)} in API`)
+        cacheManager.clearCache('api', pathToPurge)
+
+        log.debug(`Cache clear ${chalk.green(request.params.path)} in HTML`)
+        cacheManager.clearCache('html', request.params.path)
 
-        reply({status: `Purged ${request.params.path}`}, 200)
+        reply({
+          status: `Purged ${request.params.path}`
+        }, HTTPStatus.OK)
       })
     }
   })

diff --git a/src/server/render/default-html.js b/src/server/render/default-html.js
@@ -1,10 +1,23 @@
 import React from 'react'
-import PropTypes from 'prop-types'
-import has from 'lodash/has'
+import idx from 'idx'
+import propTypes from './prop-types'
+
+// Add a stringify template helper for outputting JSON with forward
+// slashes escaped to prevent '</script>' tag output in JSON within
+// script tags. See http://stackoverflow.com/questions/66837/when-is-a-cdata-section-necessary-within-a-script-tag/1450633#1450633
+const escapeScriptTags = (data) => {
+  return JSON
+    .stringify(data)
+    .replace(/\//g, '\\/')
+    // Escape u2028 and u2029
+    // http://timelessrepo.com/json-isnt-a-javascript-subset
+    // https://github.com/mapbox/tilestream-pro/issues/1638
+    .replace(/\u2028/g, "\\u2028")
+    .replace(/\u2029/g, "\\u2029")
+}
 
 const defaultHtml = ({ markup, head, asyncProps, assets = {} }) => {
   const attr = head.htmlAttributes.toComponent()
-  const hasProps = has(asyncProps, 'propsArray')
   return (
     <html lang="en" {...attr}>
       <head>
@@ -21,41 +34,21 @@ const defaultHtml = ({ markup, head, asyncProps, assets = {} }) => {
       <body>
         <div id="root" dangerouslySetInnerHTML={{ __html: markup.html }} />
         {
-          hasProps &&
-            <script
-              type="text/javascript"
-              dangerouslySetInnerHTML={{
-                __html: `window.__ASYNC_PROPS__ = ${
-                  // Add a stringify template helper for outputting JSON with forward
-                  // slashes escaped to prevent '</script>' tag output in JSON within
-                  // script tags. See http://stackoverflow.com/questions/66837/when-is-a-cdata-section-necessary-within-a-script-tag/1450633#1450633
-                  JSON.stringify(asyncProps.propsArray)
-                    .replace(/\//g, '\\/')
-                    // Escape u2028 and u2029
-                    // http://timelessrepo.com/json-isnt-a-javascript-subset
-                    // https://github.com/mapbox/tilestream-pro/issues/1638
-                    .replace(/\u2028/g, "\\u2028")
-                    .replace(/\u2029/g, "\\u2029")
-                }`
-              }}
-            />
+          idx(asyncProps, _ => _.propsArray) &&
+          <script
+            type="text/javascript"
+            dangerouslySetInnerHTML={{
+              __html: `window.__ASYNC_PROPS__ = ${
+                escapeScriptTags(asyncProps.propsArray)
+              }`
+            }}
+          />
         }
       </body>
     </html>
   )
 }
 
-defaultHtml.propTypes = {
-  markup: PropTypes.shape({
-    html: PropTypes.string.isRequired,
-    ids: PropTypes.array.isRequired,
-    css: PropTypes.string.isRequired
-  }).isRequired,
-  head: PropTypes.object.isRequired,
-  asyncProps: PropTypes.shape({
-    propsArray: PropTypes.array
-  }),
-  assets: PropTypes.object
-}
+defaultHtml.propTypes = propTypes
 
 export default defaultHtml
diff --git a/src/server/render/index.js b/src/server/render/index.js
@@ -3,11 +3,12 @@ import { renderToStaticMarkup, renderToString } from 'react-dom/server'
 import Helmet from 'react-helmet'
 import AsyncProps from 'async-props'
 import { renderStaticOptimized } from 'glamor/server'
+
 import DefaultHTML from './default-html'
 import RenderError from '../../shared/render-error'
 
-
-export const renderHtml = ({
+export default ({
+  response,
   renderProps = false,
   loadContext,
   asyncProps,
@@ -25,6 +26,7 @@ export const renderHtml = ({
             loadContext={loadContext}
           /> :
           <RenderError
+            response={response}
             config={loadContext}
           />
       )

diff --git a/src/server/render/prop-types.js b/src/server/render/prop-types.js
@@ -0,0 +1,14 @@
+import PropTypes from 'prop-types'
+
+export default {
+  markup: PropTypes.shape({
+    html: PropTypes.string.isRequired,
+    ids: PropTypes.array.isRequired,
+    css: PropTypes.string.isRequired
+  }).isRequired,
+  head: PropTypes.object.isRequired,
+  asyncProps: PropTypes.shape({
+    propsArray: PropTypes.array
+  }),
+  assets: PropTypes.object
+}
diff --git a/src/shared/default-error.js b/src/shared/default-error.js
@@ -3,7 +3,7 @@ import PropTypes from 'prop-types'
 
 const DefaultError = ({
   message,
-  status,
+  code,
   children
 }) =>
   <section style={{
@@ -19,16 +19,16 @@ const DefaultError = ({
       <path style={{ fill: '#E5E5E5' }} d="M7.6 20.67h1.93v18.3H7.4c-.9.04-1.75.3-1.75.3V21.6s.86-.95 1.94-.95zm5.46-11.3h3.88V27.7h-3.88V9.38zm14.82.03h3.88v18.3h-3.88V9.4zm14.83 0h3.9v18.3h-3.9V9.4zm7.5 11.3h2.4c.8 0 1.5-.22 1.5-.22v18s-.4.44-1.73.5H50.2v-18.3zM13.1 31.94H17v18.3h-3.9v-18.3zm14.8.06h3.88v18.2H27.9V32zm14.83-.04h3.9v18.3h-3.9v-18.3zm-22.2-11.3h3.9v18.3h-3.9v-18.3zm14.83 0h3.88v18.3h-3.9v-18.3zM7.46 43.24H9.5V57.6c0 1.07-.87 1.95-1.94 1.95-1.08 0-1.95-.88-1.95-1.96V43.9s.5-.62 1.8-.73zM52.2 16.36h-2.1V2.14c0-1.08.87-1.96 1.94-1.96 1.07 0 1.94.88 1.94 1.96V15.9s-.72.47-1.8.47z"/>
     </svg>
     {
-      status &&
-        <p style={{
-          color: '#c0c0c0',
-          fontSize: '28px',
-          fontWeight: 'bold',
-          marginBottom: '10px',
-          marginTop: '20px'
-        }}>
-          {status}
-        </p>
+      code &&
+      <p style={{
+        color: '#c0c0c0',
+        fontSize: '28px',
+        fontWeight: 'bold',
+        marginBottom: '10px',
+        marginTop: '20px'
+      }}>
+        {code}
+      </p>
     }
     <h1 style={{
       color: '#c0c0c0',
@@ -46,7 +46,7 @@ DefaultError.defaultProps = {
 }
 DefaultError.propTypes = {
   message: PropTypes.string,
-  status: PropTypes.string,
+  code: PropTypes.number,
   children: PropTypes.node
 }