Merge pull request #1024 from DoSomething/cleanup/cache

Cache helper
DoSomethingArchive · Mar 30, 2018 · 104f758 · 104f758
2 parents 149cb9e + 4fa18c0
commit 104f758
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 69 deletions.
diff --git a/config/lib/helpers/cache.js b/config/lib/helpers/cache.js
@@ -0,0 +1,8 @@
+'use strict';
+
+module.exports = {
+  campaigns: {
+    name: 'campaigns',
+    ttl: parseInt(process.env.GAMBIT_CAMPAIGNS_CACHE_TTL, 10) || 3600,
+  },
+};
diff --git a/config/lib/phoenix.js b/config/lib/phoenix.js
@@ -17,9 +17,6 @@ if (useAshes()) {
 }
 
 module.exports = {
-  cache: {
-    ttl: parseInt(process.env.GAMBIT_CAMPAIGNS_CACHE_TTL, 10) || 3600,
-  },
   clientOptions: {
     baseUri,
   },

diff --git a/lib/helpers.js b/lib/helpers.js
@@ -5,7 +5,6 @@
  */
 const logger = require('winston');
 const newrelic = require('newrelic');
-const contentful = require('./contentful');
 const stathat = require('./stathat');
 
 /**
@@ -57,30 +56,6 @@ module.exports.replacePhoenixCampaignVars = function (input, phoenixCampaign) {
   return scope;
 };
 
-/**
- * Replaces given input string with variables from given phoenixCampaign.
- */
-module.exports.findAndReplaceKeywordVarForCampaignId = function (input, campaignId) {
-  return new Promise((resolve, reject) => {
-    logger.debug(`helpers.findAndReplaceKeywordVarForCampaignId:${campaignId}`);
-    if (!input) {
-      return resolve('');
-    }
-
-    return contentful.fetchKeywordsForCampaignId(campaignId)
-      .then((keywords) => {
-        if (!keywords.length) {
-          return resolve(input);
-        }
-        const keyword = keywords[0];
-        const result = input.replace(/{{keyword}}/i, keyword);
-
-        return resolve(result);
-      })
-      .catch(err => reject(err));
-  });
-};
-
 /**
  * Formats given Express response and sends an object including given message, with given code.
  * @param {object} res - Express response

diff --git a/lib/helpers/cache.js b/lib/helpers/cache.js
@@ -0,0 +1,23 @@
+'use strict';
+
+const Cacheman = require('cacheman');
+const RedisEngine = require('cacheman-redis');
+const redisClient = require('../../config/redis')();
+const config = require('../../config/lib/helpers/cache');
+
+const redisEngine = new RedisEngine(redisClient);
+const campaignsCache = new Cacheman(config.campaigns.name, {
+  ttl: config.campaigns.ttl,
+  engine: redisEngine,
+});
+
+module.exports = {
+  campaigns: {
+    get: function get(id) {
+      return campaignsCache.get(id);
+    },
+    set: function set(id, data) {
+      return campaignsCache.set(id, data);
+    },
+  },
+};
diff --git a/lib/phoenix.js b/lib/phoenix.js
@@ -5,26 +5,10 @@
  */
 const superagent = require('superagent');
 const logger = require('winston');
-const Cacheman = require('cacheman');
-const RedisEngine = require('cacheman-redis');
 const Promise = require('bluebird');
+const cacheHelper = require('./helpers/cache');
 const campaignHelper = require('./helpers/campaign');
-
 const config = require('../config/lib/phoenix');
-const redisClient = require('../config/redis')();
-
-let campaignsCache;
-
-function getCampaignsCache() {
-  if (!campaignsCache) {
-    const redisEngine = new RedisEngine(redisClient);
-    campaignsCache = new Cacheman('campaigns', {
-      ttl: config.cache.ttl,
-      engine: redisEngine,
-    });
-  }
-  return campaignsCache;
-}
 
 /**
  * @param {string} endpoint
@@ -76,8 +60,10 @@ module.exports.fetchCampaignById = function (campaignId) {
 
   return new Promise((resolve, reject) => {
     executeGet(endpoint)
-      .then(res => getCampaignsCache()
-        .set(`${campaignId}`, campaignHelper.parseCampaign(res.data)))
+      .then((res) => {
+        const campaignData = campaignHelper.parseCampaign(res.data);
+        return cacheHelper.campaigns.set(`${campaignId}`, campaignData);
+      })
       .then(campaign => resolve(JSON.parse(campaign)))
       .catch(err => reject(exports.parsePhoenixError(err, campaignId)));
   });
@@ -93,7 +79,7 @@ module.exports.fetchCampaignById = function (campaignId) {
 module.exports.getCampaignById = function getCampaignById(campaignId) {
   logger.debug(`phoenix.getCampaignById:${campaignId}`);
 
-  return getCampaignsCache().get(`${campaignId}`)
+  return cacheHelper.campaigns.get(`${campaignId}`)
     .then((campaign) => {
       if (campaign) {
         logger.debug(`Campaigns cache hit:${campaignId}`);

diff --git a/test/lib/helpers.test.js b/test/lib/helpers.test.js
@@ -83,27 +83,6 @@ test('replacePhoenixCampaignVars', async () => {
   // TODO: test more messages!
 });
 
-test('findAndReplaceKeywordVarForCampaignId a message that makes a contentful request to get keywords', async () => {
-  const keywords = stubs.contentful.getKeywords();
-  let renderedMessage = '';
-  const phoenixCampaign = stubs.getPhoenixCampaign();
-  const relativeToSignUpMsg = stubs.getDefaultContenfulCampaignMessage('scheduled_relative_to_signup_date');
-  renderedMessage = await helpers
-    .findAndReplaceKeywordVarForCampaignId(relativeToSignUpMsg, phoenixCampaign.id);
-
-  contentful.fetchKeywordsForCampaignId.should.have.been.called;
-  renderedMessage.should.have.string(keywords[0]);
-});
-
-test('findAndReplaceKeywordVarForCampaignId failure to retrieve keywords should throw', async (t) => {
-  // will trigger fetchKeywordsForCampaignIdStubFail stub
-  const campaignId = 'fail';
-  const memberSupportMsg = stubs.getDefaultContenfulCampaignMessage('memberSupport');
-
-  await t.throws(helpers.findAndReplaceKeywordVarForCampaignId(memberSupportMsg, campaignId));
-  contentful.fetchKeywordsForCampaignId.should.have.been.called;
-});
-
 test('replacePhoenixCampaignVars with no message should return empty string', () => {
   const phoenixCampaign = stubs.getPhoenixCampaign();
   const renderedMessage = helpers.replacePhoenixCampaignVars(undefined, phoenixCampaign);