SSR: Cache State, Take Two (#13339)

Previously, controllers of SSR'd sections were responsible for caching API data. But there's a better way: We're already sending Redux state to the client at server-render time, so why not also cache on the server side, and use that as the initial state when creating the Redux store for the next request (given that this is in logged-out mode, i.e. the cache key -- the route -- maps one-to-one to the state, with the exception of errors like 404s, where one error page can be enough for different invalid routes).

Furthermore, this PR removes `renderCacheKey` and just uses `context.pathname` for _markup_ cache. Previously, we were using concatenated path and API data (~state) cache timestamp as key. Creating the markup cache key isn't the section controller's responsibility now.

Author: ockham

client/my-sites/theme/controller.jsx

@@ -4,7 +4,6 @@
 import React from 'react';
 import { Provider as ReduxProvider } from 'react-redux';
 import debugFactory from 'debug';
-import Lru from 'lru';
 import startsWith from 'lodash/startsWith';
 
 /**
@@ -14,32 +13,24 @@ import ThemeSheetComponent from './main';
 import ThemeNotFoundError from './theme-not-found-error';
 import LayoutLoggedOut from 'layout/logged-out';
 import {
-	receiveTheme,
 	requestTheme,
 	setBackPath
 } from 'state/themes/actions';
 import { getTheme, getThemeRequestErrors } from 'state/themes/selectors';
 import config from 'config';
 
 const debug = debugFactory( 'calypso:themes' );
-const HOUR_IN_MS = 3600000;
-const themeDetailsCache = new Lru( {
-	max: 500,
-	maxAge: HOUR_IN_MS
-} );
 
 export function fetchThemeDetailsData( context, next ) {
 	if ( ! config.isEnabled( 'manage/themes/details' ) || ! context.isServerSide ) {
 		return next();
 	}
 
 	const themeSlug = context.params.slug;
-	const theme = themeDetailsCache.get( themeSlug );
+	const theme = getTheme( context.store.getState(), 'wpcom', themeSlug );
 
 	if ( theme ) {
 		debug( 'found theme!', theme.id );
-		context.store.dispatch( receiveTheme( theme, 'wpcom' ) );
-		context.renderCacheKey = context.path + theme.timestamp;
 		return next();
 	}
 
@@ -49,18 +40,13 @@ export function fetchThemeDetailsData( context, next ) {
 			if ( ! themeDetails ) {
 				const error = getThemeRequestErrors( context.store.getState(), themeSlug, 'wpcom' );
 				debug( `Error fetching theme ${ themeSlug } details: `, error.message || error );
-				context.renderCacheKey = 'theme not found';
 				const err = {
 					status: 404,
 					message: 'Theme Not Found',
 					themeSlug
 				};
 				return next( err );
 			}
-			debug( 'caching', themeSlug );
-			themeDetails.timestamp = Date.now();
-			context.renderCacheKey = context.path + themeDetails.timestamp;
-			themeDetailsCache.set( themeSlug, themeDetails );
 			next();
 		} )
 		.catch( next );

client/my-sites/themes/controller.jsx

@@ -3,7 +3,6 @@
  */
 import { compact, includes, isEmpty, startsWith } from 'lodash';
 import debugFactory from 'debug';
-import Lru from 'lru';
 import React from 'react';
 
 /**
@@ -15,22 +14,12 @@ import LoggedOutComponent from './logged-out';
 import Upload from 'my-sites/themes/theme-upload';
 import trackScrollPage from 'lib/track-scroll-page';
 import { DEFAULT_THEME_QUERY } from 'state/themes/constants';
-import { requestThemes, requestThemeFilters, receiveThemes, setBackPath } from 'state/themes/actions';
-import { getThemesForQuery, getThemesFoundForQuery } from 'state/themes/selectors';
+import { requestThemes, requestThemeFilters, setBackPath } from 'state/themes/actions';
+import { getThemesForQuery } from 'state/themes/selectors';
 import { getAnalyticsData } from './helpers';
 import { getThemeFilters } from 'state/selectors';
-import { THEME_FILTERS_ADD } from 'state/action-types';
 
 const debug = debugFactory( 'calypso:themes' );
-const HOUR_IN_MS = 3600000;
-const themesQueryCache = new Lru( {
-	max: 500,
-	maxAge: HOUR_IN_MS
-} );
-
-// The filters cache is straight-forward since requestThemeFilters() always returns the same
-// list of all available filters.
-let filters = {};
 
 function getProps( context ) {
 	const { tier, filter, vertical, site_id: siteId } = context.params;
@@ -110,11 +99,6 @@ export function fetchThemeData( context, next ) {
 		return next();
 	}
 
-	if ( ! isEmpty( context.query ) ) {
-		// Don't server-render URLs with query params
-		return next();
-	}
-
 	const siteId = 'wpcom';
 	const query = {
 		search: context.query.s,
@@ -123,43 +107,26 @@ export function fetchThemeData( context, next ) {
 		page: 1,
 		number: DEFAULT_THEME_QUERY.number,
 	};
-	// context.pathname includes tier, filter, and verticals, but not the query string, so it's a suitable cacheKey
-	// However, we can't guarantee it's normalized -- filters can be in any order, resulting in multiple possible cacheKeys for
-	// the same sets of results.
-	const cacheKey = context.pathname;
-
-	const cachedData = themesQueryCache.get( cacheKey );
-	if ( cachedData ) {
-		debug( `found theme data in cache key=${ cacheKey }` );
-		context.store.dispatch( receiveThemes( cachedData.themes, siteId, query, cachedData.found ) );
-		context.renderCacheKey = cacheKey + cachedData.timestamp;
+
+	const themes = getThemesForQuery( context.store.getState(), siteId, query );
+	if ( themes ) {
+		debug( 'found theme data in cache' );
 		return next();
 	}
 
-	context.store.dispatch( requestThemes( siteId, query ) )
-		.then( () => {
-			const themes = getThemesForQuery( context.store.getState(), siteId, query );
-			const found = getThemesFoundForQuery( context.store.getState(), siteId, query );
-			const timestamp = Date.now();
-			themesQueryCache.set( cacheKey, { themes, found, timestamp } );
-			context.renderCacheKey = cacheKey + timestamp;
-			debug( `caching theme data key=${ cacheKey }` );
-			next();
-		} )
-		.catch( () => next() );
+	context.store.dispatch( requestThemes( siteId, query ) ).then( next ).catch( next );
 }
 
 export function fetchThemeFilters( context, next ) {
 	const { store } = context;
-	if ( ! isEmpty( filters ) ) {
+
+	if ( ! isEmpty( getThemeFilters( store.getState() ) ) ) {
 		debug( 'found theme filters in cache' );
-		store.dispatch( { type: THEME_FILTERS_ADD, filters } );
 		return next();
 	}
 
 	const unsubscribe = store.subscribe( () => {
-		filters = getThemeFilters( store.getState() );
-		if ( ! isEmpty( filters ) ) {
+		if ( ! isEmpty( getThemeFilters( store.getState() ) ) ) {
 			unsubscribe();
 			return next();
 		}

docs/server-side-rendering.md

@@ -22,11 +22,13 @@ React components used on the server will be rendered to HTML by being passed to
 
 ### Caching
 
-Because it is necessary to serve the redux state along with a server-rendered page, we use two levels of cache on the server: one to store raw query data, from which we can generate and serve redux state, and one to store rendered layouts.
+Because it is necessary to serve the redux state along with a server-rendered page, we use two levels of cache on the server: one to store the redux state, and one to store rendered layouts.
 
 ##### Data Cache
 
-Caching data is currently left to the controller for a [given](../client/my-sites/themes/controller.jsx) [section](../client/my-sites/theme/controller.jsx). Request timestamps are used to force expiration.
+At render time, the Redux state is [serialized and cached](../server/render/index.js), using the current path as the cache key, unless there is a query string, in which case we don't cache.
+
+This means that all data that was fetched to render a given page is available the next time the corresponding route is hit. A section controller thus only needs to check if the required data is available (using selectors), and dispatch the corresponding fetching action if it isn't; see the [themes controller](../client/my-sites/themes/controller.jsx) for an example.
 
 ##### Render Cache
 

server/isomorphic-routing/index.js

@@ -17,7 +17,9 @@ export function serverRouter( expressApp, setUpRoute, section ) {
 					route( err, req.context, next );
 				},
 				// We need 4 args so Express knows this is an error-handling middleware
+				// TODO: Ideally, there'd be a dedicated serverRenderError middleware in server/render
 				( err, req, res, next ) => { // eslint-disable-line no-unused-vars
+					req.error = err;
 					serverRender( req, res.status( err.status ) );
 				}
 			);

server/pages/index.js

@@ -8,7 +8,7 @@ import qs from 'qs';
 import { execSync } from 'child_process';
 import cookieParser from 'cookie-parser';
 import debugFactory from 'debug';
-import { get } from 'lodash';
+import { get, isEmpty, pick } from 'lodash';
 
 /**
  * Internal dependencies
@@ -19,7 +19,9 @@ import utils from 'bundler/utils';
 import sectionsModule from '../../client/sections';
 import { serverRouter } from 'isomorphic-routing';
 import { serverRender } from 'render';
-import { createReduxStore } from 'state';
+import stateCache from 'state-cache';
+import { createReduxStore, reducer } from 'state';
+import { DESERIALIZE } from 'state/action-types';
 
 const debug = debugFactory( 'calypso:pages' );
 
@@ -38,6 +40,13 @@ const staticFiles = [
 
 const sections = sectionsModule.get();
 
+// TODO: Re-use (a modified version of) client/state/initial-state#getInitialServerState here
+function getInitialServerState( serializedServerState ) {
+	// Bootstrapped state from a server-render
+	const serverState = reducer( serializedServerState, { type: DESERIALIZE } );
+	return pick( serverState, Object.keys( serializedServerState ) );
+}
+
 /**
  * Generates a hash of a files contents to be used as a version parameter on asset requests.
  * @param {String} path Path to file we want to hash
@@ -111,6 +120,14 @@ function getCurrentCommitShortChecksum() {
 }
 
 function getDefaultContext( request ) {
+	let initialServerState = {};
+	// We don't cache routes with query params
+	if ( isEmpty( request.query ) ) {
+		// context.pathname is set to request.path, see server/isomorphic-routing#getEnhancedContext()
+		const serializeCachedServerState = stateCache.get( request.path ) || {};
+		initialServerState = getInitialServerState( serializeCachedServerState );
+	}
+
 	const context = Object.assign( {}, request.context, {
 		compileDebug: config( 'env' ) === 'development' ? true : false,
 		urls: generateStaticUrls( request ),
@@ -126,7 +143,7 @@ function getDefaultContext( request ) {
 		isFluidWidth: !! config.isEnabled( 'fluid-width' ),
 		abTestHelper: !! config.isEnabled( 'dev/test-helper' ),
 		devDocsURL: '/devdocs',
-		store: createReduxStore()
+		store: createReduxStore( initialServerState )
 	} );
 
 	context.app = {

server/render/index.js

@@ -4,7 +4,7 @@
 import ReactDomServer from 'react-dom/server';
 import superagent from 'superagent';
 import Lru from 'lru';
-import pick from 'lodash/pick';
+import { isEmpty, pick } from 'lodash';
 import debugFactory from 'debug';
 
 /**
@@ -17,9 +17,16 @@ import {
 	getDocumentHeadMeta,
 	getDocumentHeadLink
 } from 'state/document-head/selectors';
+import { reducer } from 'state';
+import { SERIALIZE } from 'state/action-types';
+import stateCache from 'state-cache';
 
 const debug = debugFactory( 'calypso:server-render' );
-const markupCache = new Lru( { max: 3000 } );
+const HOUR_IN_MS = 3600000;
+const markupCache = new Lru( {
+	max: 3000,
+	maxAge: HOUR_IN_MS
+} );
 
 function bumpStat( group, name ) {
 	const statUrl = `http://pixel.wp.com/g.gif?v=wpcom-no-pv&x_${ group }=${ name }&t=${ Math.random() }`;
@@ -35,21 +42,19 @@ function bumpStat( group, name ) {
 *
 * @param {object} element - React element to be rendered to html
 * @param {string} key - (optional) custom key
-* @return {object} context object with `renderedLayout` field populated
+* @return {string} The rendered Layout
 */
 export function render( element, key = JSON.stringify( element ) ) {
 	try {
 		const startTime = Date.now();
 		debug( 'cache access for key', key );
 
-		let context = markupCache.get( key );
-		if ( ! context ) {
+		let renderedLayout = markupCache.get( key );
+		if ( ! renderedLayout ) {
 			bumpStat( 'calypso-ssr', 'loggedout-design-cache-miss' );
 			debug( 'cache miss for key', key );
-			const renderedLayout = ReactDomServer.renderToString( element );
-			context = { renderedLayout };
-
-			markupCache.set( key, context );
+			renderedLayout = ReactDomServer.renderToString( element );
+			markupCache.set( key, renderedLayout );
 		}
 		const rtsTimeMs = Date.now() - startTime;
 		debug( 'Server render time (ms)', rtsTimeMs );
@@ -59,7 +64,7 @@ export function render( element, key = JSON.stringify( element ) ) {
 			bumpStat( 'calypso-ssr', 'over-100ms-rendertostring' );
 		}
 
-		return context;
+		return renderedLayout;
 	} catch ( ex ) {
 		if ( config( 'env' ) === 'development' ) {
 			throw ex;
@@ -76,9 +81,13 @@ export function serverRender( req, res ) {
 		context.i18nLocaleScript = '//widgets.wp.com/languages/calypso/' + context.lang + '.js';
 	}
 
-	if ( config.isEnabled( 'server-side-rendering' ) && context.layout && ! context.user ) {
-		const key = context.renderCacheKey || JSON.stringify( context.layout );
-		Object.assign( context, render( context.layout, key ) );
+	if ( config.isEnabled( 'server-side-rendering' ) && context.layout && ! context.user && isEmpty( context.query ) ) {
+		// context.pathname doesn't include querystring, so it's a suitable cache key.
+		let key = context.pathname;
+		if ( req.error ) {
+			key = req.error.message;
+		}
+		context.renderedLayout = render( context.layout, key );
 	}
 
 	if ( context.store ) {
@@ -91,7 +100,14 @@ export function serverRender( req, res ) {
 			reduxSubtrees = reduxSubtrees.concat( [ 'ui', 'themes' ] );
 		}
 
+		// Send state to client
 		context.initialReduxState = pick( context.store.getState(), reduxSubtrees );
+		// And cache on the server, too
+		if ( isEmpty( context.query ) ) {
+			// Don't cache if we have query params
+			const serverState = reducer( context.initialReduxState, { type: SERIALIZE } );
+			stateCache.set( context.pathname, serverState );
+		}
 	}
 
 	context.head = { title, metas, links };

server/state-cache/index.js

@@ -0,0 +1,12 @@
+/**
+ * External Dependencies
+ */
+import Lru from 'lru';
+
+const HOUR_IN_MS = 3600000;
+const stateCache = new Lru( {
+	max: 500,
+	maxAge: HOUR_IN_MS
+} );
+
+export default stateCache;