From 724eaa43ce2406161df6ca9427f2d7e0fa6f9cef Mon Sep 17 00:00:00 2001 From: Ben Lowery Date: Thu, 26 May 2016 10:38:32 -0500 Subject: [PATCH 1/3] FeedPostStore: Use the normalization rules directly --- client/lib/feed-post-store/index.js | 22 ++-- .../feed-post-store/normalization-rules.js | 117 +++++++++++------- client/lib/feed-post-store/test/index.js | 2 +- client/lib/post-normalizer/index.js | 1 + .../rule-first-pass-canonical-image.js | 1 + 5 files changed, 83 insertions(+), 60 deletions(-) diff --git a/client/lib/feed-post-store/index.js b/client/lib/feed-post-store/index.js index 625cbe0b12ec29..a554db52313210 100644 --- a/client/lib/feed-post-store/index.js +++ b/client/lib/feed-post-store/index.js @@ -14,8 +14,7 @@ var assign = require( 'lodash/assign' ), */ var Dispatcher = require( 'dispatcher' ), emitter = require( 'lib/mixins/emitter' ), - normalizer = require( 'lib/post-normalizer' ), - normalizationRules = require( './normalization-rules' ), + { runFastRules, runSlowRules } = require( './normalization-rules' ), FeedPostActionType = require( './constants' ).action, FeedStreamActionType = require( 'lib/feed-stream-store/constants' ).action, ReaderSiteBlockActionType = require( 'lib/reader-site-blocks/constants' ).action, @@ -263,18 +262,11 @@ function normalizePost( feedId, postId, post ) { return; } - normalizer( post, normalizationRules.fastRules, function( err, normalizedPost ) { - if ( ! err ) { - setPost( postId, normalizedPost ); - - process.nextTick( function() { - normalizer( normalizedPost, normalizationRules.slowRules, function( fullErr, fullyNormalizedPost ) { - if ( ! fullErr ) { - setPost( postId, fullyNormalizedPost ); - } - } ); - } ); - } + const normalizedPost = runFastRules( post ); + setPost( postId, normalizedPost ); + + process.nextTick( () => { + runSlowRules( normalizedPost ).then( setPost.bind( null, postId ) ); } ); } @@ -291,7 +283,7 @@ function markPostSeen( post ) { if ( post.site_ID ) { // they have a site ID, let's try to push a page view const site = SiteStore.get( post.site_ID ); - const isNotAdmin = ! ( site && site.getIn( [ 'capabilities', 'manage_options' ], false ) ) + const isNotAdmin = ! ( site && site.getIn( [ 'capabilities', 'manage_options' ], false ) ); if ( site && site.get( 'state' ) === SiteState.COMPLETE ) { if ( site.get( 'is_private' ) || isNotAdmin ) { stats.pageViewForPost( site.get( 'ID' ), site.get( 'URL' ), post.ID, site.get( 'is_private' ) ); diff --git a/client/lib/feed-post-store/normalization-rules.js b/client/lib/feed-post-store/normalization-rules.js index bcba1e106c6a9a..87a68e3fe93f7e 100644 --- a/client/lib/feed-post-store/normalization-rules.js +++ b/client/lib/feed-post-store/normalization-rules.js @@ -1,18 +1,41 @@ /** * External Dependencies */ -const find = require( 'lodash/find' ), - forEach = require( 'lodash/forEach' ), - url = require( 'url' ), - matches = require( 'lodash/matches' ), - toArray = require( 'lodash/toArray' ); +import find from 'lodash/find'; +import flow from 'lodash/flow'; +import forEach from 'lodash/forEach'; +import url from 'url'; +import matches from 'lodash/matches'; +import cloneDeep from 'lodash/cloneDeep'; /** * Internal Dependencies */ -const postNormalizer = require( 'lib/post-normalizer' ), - resizeImageUrl = require( 'lib/resize-image-url' ), - DISPLAY_TYPES = require( './display-types' ); +import resizeImageUrl from 'lib/resize-image-url'; +import DISPLAY_TYPES from './display-types'; + +/** + * Rules + */ +import createBetterExcerpt from 'lib/post-normalizer/rule-create-better-excerpt'; +import detectEmbeds from 'lib/post-normalizer/rule-content-detect-embeds'; +import detectPolls from 'lib/post-normalizer/rule-content-detect-polls'; +import makeEmbedsSecure from 'lib/post-normalizer/rule-content-make-embeds-secure'; +import removeStyles from 'lib/post-normalizer/rule-content-remove-styles'; +import safeImages from 'lib/post-normalizer/rule-content-safe-images'; +import wordCount from 'lib/post-normalizer/rule-content-word-count'; +import { disableAutoPlayOnMedia, disableAutoPlayOnEmbeds} from 'lib/post-normalizer/rule-content-disable-autoplay'; +import decodeEntities from 'lib/post-normalizer/rule-decode-entities'; +import firstPassCanonicalImage from 'lib/post-normalizer/rule-first-pass-canonical-image'; +import makeSiteIdSafeForApi from 'lib/post-normalizer/rule-make-site-id-safe-for-api'; +import pickPrimaryTag from 'lib/post-normalizer/rule-pick-primary-tag'; +import preventWidows from 'lib/post-normalizer/rule-prevent-widows'; +import safeImageProperties from 'lib/post-normalizer/rule-safe-image-properties'; +import stripHtml from 'lib/post-normalizer/rule-strip-html'; +import withContentDom from 'lib/post-normalizer/rule-with-content-dom'; +import keepValidImages from 'lib/post-normalizer/rule-keep-valid-images'; +import pickCanonicalImage from 'lib/post-normalizer/rule-pick-canonical-image'; +import waitForImagesToLoad from 'lib/post-normalizer/rule-wait-for-images-to-load'; /** * Module vars @@ -23,35 +46,6 @@ const READER_CONTENT_WIDTH = 720, ONE_LINER_THRESHOLD = ( 20 * 10 ), // roughly 10 lines of words DISCOVER_BLOG_ID = 53424024; -const fastPostNormalizationRules = [ - postNormalizer.decodeEntities, - postNormalizer.stripHTML, - postNormalizer.preventWidows, - postNormalizer.makeSiteIDSafeForAPI, - postNormalizer.pickPrimaryTag, - postNormalizer.safeImageProperties( READER_CONTENT_WIDTH ), - postNormalizer.firstPassCanonicalImage, - postNormalizer.withContentDOM( [ - postNormalizer.content.removeStyles, - postNormalizer.content.safeContentImages( READER_CONTENT_WIDTH ), - discoverFullBleedImages, - postNormalizer.content.makeEmbedsSecure, - postNormalizer.content.disableAutoPlayOnEmbeds, - postNormalizer.content.disableAutoPlayOnMedia, - postNormalizer.content.detectEmbeds, - postNormalizer.content.detectPolls, - postNormalizer.content.wordCountAndReadingTime - ] ), - postNormalizer.createBetterExcerpt, - classifyPost - ], - slowPostNormalizationRules = [ - postNormalizer.waitForImagesToLoad, - postNormalizer.keepValidImages( 144, 72 ), - postNormalizer.pickCanonicalImage, - classifyPost - ]; - function discoverFullBleedImages( post, dom ) { if ( post.site_ID === DISCOVER_BLOG_ID ) { const images = dom.querySelectorAll( '.fullbleed img, img.fullbleed' ); @@ -68,9 +62,9 @@ function discoverFullBleedImages( post, dom ) { /** * Attempt to classify the post into a display type * @param {object} post A post to classify - * @param {Function} callback A callback to invoke when we're done + * @return {object} The classified post */ -function classifyPost( post, callback ) { +function classifyPost( post ) { var displayType = DISPLAY_TYPES.UNCLASSIFIED, canonicalImage = post.canonical_image, canonicalAspect; @@ -133,10 +127,45 @@ function classifyPost( post, callback ) { post.display_type = displayType; - callback(); + return post; +} + +const fastPostNormalizationRules = flow( [ + decodeEntities, + stripHtml, + preventWidows, + makeSiteIdSafeForApi, + pickPrimaryTag, + safeImageProperties( READER_CONTENT_WIDTH ), + firstPassCanonicalImage, + withContentDom( [ + removeStyles, + safeImages( READER_CONTENT_WIDTH ), + discoverFullBleedImages, + makeEmbedsSecure, + disableAutoPlayOnEmbeds, + disableAutoPlayOnMedia, + detectEmbeds, + detectPolls, + wordCount + ] ), + createBetterExcerpt, + classifyPost +] ); + +export function runFastRules( post ) { + post = cloneDeep( post ); + fastPostNormalizationRules( post ); + return post; } -module.exports = { - fastRules: fastPostNormalizationRules, - slowRules: slowPostNormalizationRules -}; +const slowSyncRules = flow( [ + keepValidImages( 144, 72 ), + pickCanonicalImage, + classifyPost +] ); + +export function runSlowRules( post ) { + post = cloneDeep( post ); + return waitForImagesToLoad( post ).then( slowSyncRules ); +} diff --git a/client/lib/feed-post-store/test/index.js b/client/lib/feed-post-store/test/index.js index 237d56a45cf458..460525f5a1c408 100644 --- a/client/lib/feed-post-store/test/index.js +++ b/client/lib/feed-post-store/test/index.js @@ -96,7 +96,7 @@ describe( 'feed-post-store', function() { expect( FeedPostStore.get( { feedId: 1, postId: 2 - } ).title ).to.equal( 'chris & ben' ); + } ).title ).to.equal( 'chris &\xA0ben' ); } ); it( 'should index a post by the site_ID and ID if it is internal', function() { diff --git a/client/lib/post-normalizer/index.js b/client/lib/post-normalizer/index.js index 7b7f276088e564..32ec8f607e625d 100644 --- a/client/lib/post-normalizer/index.js +++ b/client/lib/post-normalizer/index.js @@ -23,6 +23,7 @@ function debugForPost( post ) { * If successful, the callback is invoked with `(null, theMutatedPost)` */ function normalizePost( post, transforms, callback ) { + console && console.warn( '[DEPRECATED]: Please run the rules you need by hand' ); if ( ! callback ) { throw new Error( 'must supply a callback' ); } diff --git a/client/lib/post-normalizer/rule-first-pass-canonical-image.js b/client/lib/post-normalizer/rule-first-pass-canonical-image.js index d1e20cb64846b4..189afc38620493 100644 --- a/client/lib/post-normalizer/rule-first-pass-canonical-image.js +++ b/client/lib/post-normalizer/rule-first-pass-canonical-image.js @@ -31,4 +31,5 @@ export default function firstPassCanonicalImage( post ) { }; } } + return post; } From 544bb86166a9c76812f965b628d857fa085196f9 Mon Sep 17 00:00:00 2001 From: Ben Lowery Date: Thu, 2 Jun 2016 11:45:16 -0400 Subject: [PATCH 2/3] Don't use deep clone, remove warning --- client/lib/feed-post-store/normalization-rules.js | 4 ++-- client/lib/post-normalizer/index.js | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/client/lib/feed-post-store/normalization-rules.js b/client/lib/feed-post-store/normalization-rules.js index 87a68e3fe93f7e..516bf1d27a6fe5 100644 --- a/client/lib/feed-post-store/normalization-rules.js +++ b/client/lib/feed-post-store/normalization-rules.js @@ -154,7 +154,7 @@ const fastPostNormalizationRules = flow( [ ] ); export function runFastRules( post ) { - post = cloneDeep( post ); + post = Object.assign( {}, post ); fastPostNormalizationRules( post ); return post; } @@ -166,6 +166,6 @@ const slowSyncRules = flow( [ ] ); export function runSlowRules( post ) { - post = cloneDeep( post ); + post = Object.assign( {}, post ); return waitForImagesToLoad( post ).then( slowSyncRules ); } diff --git a/client/lib/post-normalizer/index.js b/client/lib/post-normalizer/index.js index 32ec8f607e625d..7b7f276088e564 100644 --- a/client/lib/post-normalizer/index.js +++ b/client/lib/post-normalizer/index.js @@ -23,7 +23,6 @@ function debugForPost( post ) { * If successful, the callback is invoked with `(null, theMutatedPost)` */ function normalizePost( post, transforms, callback ) { - console && console.warn( '[DEPRECATED]: Please run the rules you need by hand' ); if ( ! callback ) { throw new Error( 'must supply a callback' ); } From c9f446a151749c4a7bd06ade93843b76e5cf42c7 Mon Sep 17 00:00:00 2001 From: Ben Lowery Date: Thu, 2 Jun 2016 11:46:15 -0400 Subject: [PATCH 3/3] remove clonedeep from post norm too --- client/lib/feed-post-store/normalization-rules.js | 1 - client/lib/post-normalizer/index.js | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/client/lib/feed-post-store/normalization-rules.js b/client/lib/feed-post-store/normalization-rules.js index 516bf1d27a6fe5..38ce6a1354bf97 100644 --- a/client/lib/feed-post-store/normalization-rules.js +++ b/client/lib/feed-post-store/normalization-rules.js @@ -6,7 +6,6 @@ import flow from 'lodash/flow'; import forEach from 'lodash/forEach'; import url from 'url'; import matches from 'lodash/matches'; -import cloneDeep from 'lodash/cloneDeep'; /** * Internal Dependencies diff --git a/client/lib/post-normalizer/index.js b/client/lib/post-normalizer/index.js index 7b7f276088e564..68e670159f9ebf 100644 --- a/client/lib/post-normalizer/index.js +++ b/client/lib/post-normalizer/index.js @@ -2,8 +2,7 @@ * External Dependencies */ var async = require( 'async' ), - debug = require( 'debug' )( 'calypso:post-normalizer' ), - cloneDeep = require( 'lodash/cloneDeep' ); + debug = require( 'debug' )( 'calypso:post-normalizer' ); /** * Internal dependencies */ @@ -32,7 +31,7 @@ function normalizePost( post, transforms, callback ) { return; } - let normalizedPost = cloneDeep( post ), + let normalizedPost = Object.assign( {}, post ), postDebug = debugForPost( post ); postDebug( 'running transforms' );