Improved performance while calculating status by doing one iteration only and memoizing the result when possible

package.json

@@ -37,7 +37,7 @@
     "promise": "^8.0.1",
     "prop-types": "^15.6.2",
     "qs": "^6.5.2",
-    "ramda": "^0.25.0",
+    "ramda": "^0.26.1",
     "react": "^16.7.0",
     "react-autosuggest": "^9.4.0",
     "react-chartjs-2": "^2.7.4",

src/visits/ShortUrlVisits.js

@@ -8,20 +8,13 @@ import DateInput from '../utils/DateInput';
 import MutedMessage from '../utils/MuttedMessage';
 import SortableBarGraph from './SortableBarGraph';
 import { shortUrlVisitsType } from './reducers/shortUrlVisits';
-import { VisitsHeader } from './VisitsHeader';
+import VisitsHeader from './VisitsHeader';
 import GraphCard from './GraphCard';
 import { shortUrlDetailType } from './reducers/shortUrlDetail';
 import './ShortUrlVisits.scss';
 import OpenMapModalBtn from './helpers/OpenMapModalBtn';
 
-const ShortUrlVisits = ({
+const ShortUrlVisits = ({ processStatsFromVisits }) => class ShortUrlVisits extends React.PureComponent {
-  processOsStats,
-  processBrowserStats,
-  processCountriesStats,
-  processCitiesStats,
-  processReferrersStats,
-  processCitiesStatsForMap,
-}) => class ShortUrlVisits extends React.Component {
   static propTypes = {
     match: PropTypes.shape({
       params: PropTypes.object,
@@ -35,18 +28,25 @@ const ShortUrlVisits = ({
   state = { startDate: undefined, endDate: undefined };
   loadVisits = () => {
     const { match: { params }, getShortUrlVisits } = this.props;
-
+    const { shortCode } = params;
-    getShortUrlVisits(params.shortCode, mapObjIndexed(
+    const dates = mapObjIndexed(
       (value) => value && value.format ? value.format('YYYY-MM-DD') : value,
       this.state
-    ));
+    );
+    const { startDate, endDate } = dates;
+
+    // While the "page" is loaded, use the timestamp + filtering dates as memoization IDs for stats calcs
+    this.memoizationId = `${new Date().getTime()}_${shortCode}_${startDate}_${endDate}`;
+
+    getShortUrlVisits(shortCode, dates);
   };
 
   componentDidMount() {
     const { match: { params }, getShortUrlDetail } = this.props;
+    const { shortCode } = params;
 
     this.loadVisits();
-    getShortUrlDetail(params.shortCode);
+    getShortUrlDetail(shortCode);
   }
 
   render() {
@@ -71,17 +71,21 @@ const ShortUrlVisits = ({
         return <MutedMessage>There are no visits matching current filter  :(</MutedMessage>;
       }
 
+      const { os, browsers, referrers, countries, cities, citiesForMap } = processStatsFromVisits(
+        { id: this.memoizationId, visits }
+      );
+
       return (
         <div className="row">
           <div className="col-xl-4 col-lg-6">
-            <GraphCard title="Operating systems" stats={processOsStats(visits)} />
+            <GraphCard title="Operating systems" stats={os} />
           </div>
           <div className="col-xl-4 col-lg-6">
-            <GraphCard title="Browsers" stats={processBrowserStats(visits)} />
+            <GraphCard title="Browsers" stats={browsers} />
           </div>
           <div className="col-xl-4">
             <SortableBarGraph
-              stats={processReferrersStats(visits)}
+              stats={referrers}
               title="Referrers"
               sortingItems={{
                 name: 'Referrer name',
@@ -91,7 +95,7 @@ const ShortUrlVisits = ({
           </div>
           <div className="col-lg-6">
             <SortableBarGraph
-              stats={processCountriesStats(visits)}
+              stats={countries}
               title="Countries"
               sortingItems={{
                 name: 'Country name',
@@ -101,13 +105,13 @@ const ShortUrlVisits = ({
           </div>
           <div className="col-lg-6">
             <SortableBarGraph
-              stats={processCitiesStats(visits)}
+              stats={cities}
               title="Cities"
               extraHeaderContent={[
                 () => (
                   <OpenMapModalBtn
                     modalTitle="Cities"
-                    locations={values(processCitiesStatsForMap(visits))}
+                    locations={values(citiesForMap)}
                   />
                 ),
               ]}

src/visits/VisitsHeader.js

@@ -11,7 +11,7 @@ const propTypes = {
   shortUrlVisits: shortUrlVisitsType.isRequired,
 };
 
-export function VisitsHeader({ shortUrlDetail, shortUrlVisits }) {
+export default function VisitsHeader({ shortUrlDetail, shortUrlVisits }) {
   const { shortUrl, loading } = shortUrlDetail;
   const { visits } = shortUrlVisits;
   const shortLink = shortUrl && shortUrl.shortUrl ? shortUrl.shortUrl : '';

src/visits/services/VisitsParser.js

@@ -1,4 +1,4 @@
-import { assoc, isNil, isEmpty, reduce } from 'ramda';
+import { isNil, isEmpty, memoizeWith, prop, reduce } from 'ramda';
 
 const osFromUserAgent = (userAgent) => {
   const lowerUserAgent = userAgent.toLowerCase();
@@ -42,79 +42,70 @@ const extractDomain = (url) => {
   return domain.split(':')[0];
 };
 
-export const processOsStats = (visits) =>
-  reduce(
-    (stats, { userAgent }) => {
-      const os = isNil(userAgent) ? 'Others' : osFromUserAgent(userAgent);
-
-      return assoc(os, (stats[os] || 0) + 1, stats);
-    },
-    {},
-    visits,
-  );
-
-export const processBrowserStats = (visits) =>
-  reduce(
-    (stats, { userAgent }) => {
-      const browser = isNil(userAgent) ? 'Others' : browserFromUserAgent(userAgent);
-
-      return assoc(browser, (stats[browser] || 0) + 1, stats);
-    },
-    {},
-    visits,
-  );
-
-export const processReferrersStats = (visits) =>
-  reduce(
-    (stats, visit) => {
-      const notHasDomain = isNil(visit.referer) || isEmpty(visit.referer);
-      const domain = notHasDomain ? 'Unknown' : extractDomain(visit.referer);
-
-      return assoc(domain, (stats[domain] || 0) + 1, stats);
-    },
-    {},
-    visits,
-  );
-
 const visitLocationHasProperty = (visitLocation, propertyName) =>
   !isNil(visitLocation)
   && !isNil(visitLocation[propertyName])
   && !isEmpty(visitLocation[propertyName]);
 
-const buildLocationStatsProcessorByProperty = (propertyName) => (visits) =>
+const updateOsStatsForVisit = (osStats, { userAgent }) => {
+  const os = isNil(userAgent) ? 'Others' : osFromUserAgent(userAgent);
+
+  osStats[os] = (osStats[os] || 0) + 1;
+};
+
+const updateBrowsersStatsForVisit = (browsersStats, { userAgent }) => {
+  const browser = isNil(userAgent) ? 'Others' : browserFromUserAgent(userAgent);
+
+  browsersStats[browser] = (browsersStats[browser] || 0) + 1;
+};
+
+const updateReferrersStatsForVisit = (referrersStats, { referer }) => {
+  const notHasDomain = isNil(referer) || isEmpty(referer);
+  const domain = notHasDomain ? 'Unknown' : extractDomain(referer);
+
+  referrersStats[domain] = (referrersStats[domain] || 0) + 1;
+};
+
+const updateLocationsStatsForVisit = (propertyName) => (stats, { visitLocation }) => {
+  const hasLocationProperty = visitLocationHasProperty(visitLocation, propertyName);
+  const value = hasLocationProperty ? visitLocation[propertyName] : 'Unknown';
+
+  stats[value] = (stats[value] || 0) + 1;
+};
+
+const updateCountriesStatsForVisit = updateLocationsStatsForVisit('countryName');
+const updateCitiesStatsForVisit = updateLocationsStatsForVisit('cityName');
+
+const updateCitiesForMapForVisit = (citiesForMapStats, { visitLocation }) => {
+  if (!visitLocationHasProperty(visitLocation, 'cityName')) {
+    return;
+  }
+
+  const { cityName, latitude, longitude } = visitLocation;
+  const currentCity = citiesForMapStats[cityName] || {
+    cityName,
+    count: 0,
+    latLong: [ parseFloat(latitude), parseFloat(longitude) ],
+  };
+
+  currentCity.count++;
+
+  citiesForMapStats[cityName] = currentCity;
+};
+
+export const processStatsFromVisits = memoizeWith(prop('id'), ({ visits }) =>
   reduce(
-    (stats, { visitLocation }) => {
+    (stats, visit) => {
-      const hasLocationProperty = visitLocationHasProperty(visitLocation, propertyName);
+      // We mutate the original object because it has a big side effect when large data sets are processed
-      const value = hasLocationProperty ? visitLocation[propertyName] : 'Unknown';
+      updateOsStatsForVisit(stats.os, visit);
+      updateBrowsersStatsForVisit(stats.browsers, visit);
+      updateReferrersStatsForVisit(stats.referrers, visit);
+      updateCountriesStatsForVisit(stats.countries, visit);
+      updateCitiesStatsForVisit(stats.cities, visit);
+      updateCitiesForMapForVisit(stats.citiesForMap, visit);
 
-      return assoc(value, (stats[value] || 0) + 1, stats);
+      return stats;
     },
-    {},
+    { os: {}, browsers: {}, referrers: {}, countries: {}, cities: {}, citiesForMap: {} },
     visits,
-  );
+  ));
-
-export const processCountriesStats = buildLocationStatsProcessorByProperty('countryName');
-
-export const processCitiesStats = buildLocationStatsProcessorByProperty('cityName');
-
-export const processCitiesStatsForMap = (visits) =>
-  reduce(
-    (stats, { visitLocation }) => {
-      if (!visitLocationHasProperty(visitLocation, 'cityName')) {
-        return stats;
-      }
-
-      const { cityName, latitude, longitude } = visitLocation;
-      const currentCity = stats[cityName] || {
-        cityName,
-        count: 0,
-        latLong: [ parseFloat(latitude), parseFloat(longitude) ],
-      };
-
-      currentCity.count++;
-
-      return assoc(cityName, currentCity, stats);
-    },
-    {},
-    visits,
-  );

test/visits/ShortUrlVisits.test.js

@@ -11,21 +11,16 @@ import SortableBarGraph from '../../src/visits/SortableBarGraph';
 
 describe('<ShortUrlVisits />', () => {
   let wrapper;
-  const statsProcessor = () => ({});
+  const processStatsFromVisits = () => (
+    { os: {}, browsers: {}, referrers: {}, countries: {}, cities: {}, citiesForMap: {} }
+  );
   const getShortUrlVisitsMock = sinon.spy();
   const match = {
     params: { shortCode: 'abc123' },
   };
 
   const createComponent = (shortUrlVisits) => {
-    const ShortUrlVisits = createShortUrlVisits({
+    const ShortUrlVisits = createShortUrlVisits({ processStatsFromVisits });
-      processBrowserStats: statsProcessor,
-      processCountriesStats: statsProcessor,
-      processOsStats: statsProcessor,
-      processReferrersStats: statsProcessor,
-      processCitiesStats: statsProcessor,
-      processCitiesStatsForMap: statsProcessor,
-    });
 
     wrapper = shallow(
       <ShortUrlVisits

test/visits/VisitsHeader.test.js

@@ -1,7 +1,7 @@
 import React from 'react';
 import { shallow } from 'enzyme';
 import Moment from 'react-moment';
-import { VisitsHeader } from '../../src/visits/VisitsHeader';
+import VisitsHeader from '../../src/visits/VisitsHeader';
 import ExternalLink from '../../src/utils/ExternalLink';
 
 describe('<VisitsHeader />', () => {

test/visits/services/VisitsParser.test.js

@@ -1,11 +1,4 @@
-import {
+import { processStatsFromVisits } from '../../../src/visits/services/VisitsParser';
-  processOsStats,
-  processBrowserStats,
-  processReferrersStats,
-  processCountriesStats,
-  processCitiesStats,
-  processCitiesStatsForMap,
-} from '../../../src/visits/services/VisitsParser';
 
 describe('VisitsParser', () => {
   const visits = [
@@ -50,64 +43,71 @@ describe('VisitsParser', () => {
     },
   ];
 
-  describe('processOsStats', () => {
+  describe('processStatsFromVisits', () => {
+    let stats;
+
+    beforeAll(() => {
+      stats = processStatsFromVisits({ id: 'id', visits });
+    });
+
     it('properly parses OS stats', () => {
-      expect(processOsStats(visits)).toEqual({
+      const { os } = stats;
+
+      expect(os).toEqual({
         Linux: 3,
         Windows: 1,
         MacOS: 1,
       });
     });
-  });
 
-  describe('processBrowserStats', () => {
     it('properly parses browser stats', () => {
-      expect(processBrowserStats(visits)).toEqual({
+      const { browsers } = stats;
+
+      expect(browsers).toEqual({
         Firefox: 2,
         Chrome: 2,
         Opera: 1,
       });
     });
-  });
 
-  describe('processReferrersStats', () => {
     it('properly parses referrer stats', () => {
-      expect(processReferrersStats(visits)).toEqual({
+      const { referrers } = stats;
+
+      expect(referrers).toEqual({
         'Unknown': 2,
         'google.com': 2,
         'm.facebook.com': 1,
       });
     });
-  });
 
-  describe('processCountriesStats', () => {
     it('properly parses countries stats', () => {
-      expect(processCountriesStats(visits)).toEqual({
+      const { countries } = stats;
+
+      expect(countries).toEqual({
         'Spain': 3,
         'United States': 1,
         'Unknown': 1,
       });
     });
-  });
 
-  describe('processCitiesStats', () => {
     it('properly parses cities stats', () => {
-      expect(processCitiesStats(visits)).toEqual({
+      const { cities } = stats;
+
+      expect(cities).toEqual({
         'Zaragoza': 2,
         'New York': 1,
         'Unknown': 2,
       });
     });
-  });
 
-  describe('processCitiesStatsForMap', () => {
     it('properly parses cities stats with lat and long', () => {
+      const { citiesForMap } = stats;
       const zaragozaLat = 123.45;
       const zaragozaLong = -543.21;
       const newYorkLat = 1029;
       const newYorkLong = 6758;
 
-      expect(processCitiesStatsForMap(visits)).toEqual({
+      expect(citiesForMap).toEqual({
         'Zaragoza': {
           cityName: 'Zaragoza',
           count: 2,

yarn.lock

@@ -7910,9 +7910,10 @@ railroad-diagrams@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/railroad-diagrams/-/railroad-diagrams-1.0.0.tgz#eb7e6267548ddedfb899c1b90e57374559cddb7e"
 
-ramda@^0.25.0:
+ramda@^0.26.1:
-  version "0.25.0"
+  version "0.26.1"
-  resolved "https://registry.yarnpkg.com/ramda/-/ramda-0.25.0.tgz#8fdf68231cffa90bc2f9460390a0cb74a29b29a9"
+  resolved "https://registry.yarnpkg.com/ramda/-/ramda-0.26.1.tgz#8d41351eb8111c55353617fc3bbffad8e4d35d06"
+  integrity sha512-hLWjpy7EnsDBb0p+Z3B7rPi3GDeRG5ZtiI33kJhTt+ORCd38AbAIjB/9zRIUoeTbE/AVX5ZkU7m6bznsvrf8eQ==
 
 randexp@0.4.6:
   version "0.4.6"