Merge branch 'master' into profile-add-column-sql

cache/sampleStoreInit.js

@@ -162,7 +162,7 @@ function populateAspects() {
  * @returns {Promise} which resolves to the list of redis batch responses.
  */
 function populateSubjects() {
-  return Subject.findAll({ where: { isPublished: true } })
+  return Subject.findAll()
   .then((subjects) => {
     if (infoLoggingEnabled) {
       const msg = `Starting to load ${subjects.length} subjects to cache :|`;

config.js

@@ -15,6 +15,7 @@
 require('./config/toggles'); // Loads the feature toggles
 const featureToggles = require('feature-toggles');
 const configUtil = require('./config/configUtil');
+const redisConfig = require('./config/redisConfig');
 const defaultPort = 3000;
 const defaultPostgresPort = 5432;
 const pe = process.env; // eslint-disable-line no-process-env
@@ -30,7 +31,6 @@ const pghost = pe.PGHOST || 'localhost';
 const pgport = pe.PGPORT || defaultPostgresPort;
 const defaultDbUrl = 'postgres://' + pguser + ':' + pgpass + '@' + pghost +
   ':' + pgport + '/' + pgdatabase;
-const DEFAULT_LOCAL_REDIS_URL = '//127.0.0.1:6379';
 const DEFAULT_DB_CONNECTION_POOL = { // sequelize defaults
   max: 5,
   min: 0,
@@ -109,27 +109,6 @@ const JOB_QUEUE_TTL_SECONDS_SYNC = pe.TTL_KUE_JOBS_SYNC
 // set time interval for enableQueueStatsActivityLogs
 const queueStatsActivityLogsInterval = 60000;
 
-/*
- * Assigns each of the different redis uses cases to a particular redis
- * instance, if configured, or falls back to the primary redis instance.
- */
-const redisUrls = {
-  cache: pe.REDIS_CACHE && pe[pe.REDIS_CACHE] ?
-    pe[pe.REDIS_CACHE] : (pe.REDIS_URL || DEFAULT_LOCAL_REDIS_URL),
-  limiter: pe.REDIS_LIMITER && pe[pe.REDIS_LIMITER] ?
-    pe[pe.REDIS_LIMITER] : (pe.REDIS_URL || DEFAULT_LOCAL_REDIS_URL),
-  pubsub: pe.REDIS_PUBSUB && pe[pe.REDIS_PUBSUB] ?
-    pe[pe.REDIS_PUBSUB] : (pe.REDIS_URL || DEFAULT_LOCAL_REDIS_URL),
-  queue: pe.REDIS_QUEUE && pe[pe.REDIS_QUEUE] ?
-    pe[pe.REDIS_QUEUE] : (pe.REDIS_URL || DEFAULT_LOCAL_REDIS_URL),
-  realtimeLogging: pe.REDIS_REALTIME_LOGGING && pe[pe.REDIS_REALTIME_LOGGING] ?
-    pe[pe.REDIS_REALTIME_LOGGING] : (pe.REDIS_URL || DEFAULT_LOCAL_REDIS_URL),
-  sampleStore: pe.REDIS_SAMPLE_STORE && pe[pe.REDIS_SAMPLE_STORE] ?
-    pe[pe.REDIS_SAMPLE_STORE] : (pe.REDIS_URL || DEFAULT_LOCAL_REDIS_URL),
-  session: pe.REDIS_SESSION && pe[pe.REDIS_SESSION] ?
-    pe[pe.REDIS_SESSION] : (pe.REDIS_URL || DEFAULT_LOCAL_REDIS_URL),
-};
-
 module.exports = {
   api: {
     defaults: {
@@ -175,18 +154,7 @@ module.exports = {
     modelDirName: 'model',
     passwordHashSaltNumRounds: 8,
   },
-  redis: {
-    channelName: 'focus',
-    instanceUrl: {
-      cache: redisUrls.cache,
-      limiter: redisUrls.limiter,
-      pubsub: redisUrls.pubsub,
-      queue: redisUrls.queue,
-      realtimeLogging: redisUrls.realtimeLogging,
-      sampleStore: redisUrls.sampleStore,
-      session: redisUrls.session,
-    },
-  },
+  redis: redisConfig,
 
   // When adding new environment, consider adding it to /config/migrationConfig
   // as well to enable database migraton in the environment.

config/redisConfig.js

@@ -0,0 +1,69 @@
+/**
+ * Copyright (c) 2017, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or
+ * https://opensource.org/licenses/BSD-3-Clause
+ */
+
+/**
+ * ./config/redisConfig.js
+ *
+ * Assigns each of the different redis uses cases to a particular redis
+ * instance, if configured, or falls back to the primary redis instance.
+ */
+'use strict'; // eslint-disable-line strict
+const pe = process.env; // eslint-disable-line no-process-env
+const DEFAULT_LOCAL_REDIS_URL = '//127.0.0.1:6379';
+const PRIMARY_REDIS = (pe.REDIS_URL || DEFAULT_LOCAL_REDIS_URL);
+const channelName = 'focus';
+
+module.exports = {
+  channelName,
+  instanceUrl: {
+    /*
+     * Cache perspectives, lenses, etc.
+     */
+    cache: pe.REDIS_CACHE && pe[pe.REDIS_CACHE] ?
+      pe[pe.REDIS_CACHE] : PRIMARY_REDIS,
+
+    /*
+     * Keys for tracking rate limiter activity
+     */
+    limiter: pe.REDIS_LIMITER && pe[pe.REDIS_LIMITER] ?
+      pe[pe.REDIS_LIMITER] : PRIMARY_REDIS,
+
+    /*
+     * PubSub for real-time events.
+     */
+    pubsub: pe.REDIS_PUBSUB && pe[pe.REDIS_PUBSUB] ?
+      pe[pe.REDIS_PUBSUB] : PRIMARY_REDIS,
+
+    /*
+     * Kue job queue for work being delegated to worker dynos.
+     */
+    queue: pe.REDIS_QUEUE && pe[pe.REDIS_QUEUE] ?
+      pe[pe.REDIS_QUEUE] : PRIMARY_REDIS,
+
+    /*
+     * Transient data about socket connections from browsers with an open
+     * perspective, for logging activity=realtime.
+     */
+    realtimeLogging: pe.REDIS_REALTIME_LOGGING &&
+      pe[pe.REDIS_REALTIME_LOGGING] ? pe[pe.REDIS_REALTIME_LOGGING] :
+      PRIMARY_REDIS,
+
+    /*
+     * SampleStore is the primary persistent store for samples, and an active
+     * cache for aspects and subjects.
+     */
+    sampleStore: pe.REDIS_SAMPLE_STORE && pe[pe.REDIS_SAMPLE_STORE] ?
+      pe[pe.REDIS_SAMPLE_STORE] : PRIMARY_REDIS,
+
+    /*
+     * Active browser sessions.
+     */
+    session: pe.REDIS_SESSION && pe[pe.REDIS_SESSION] ?
+      pe[pe.REDIS_SESSION] : PRIMARY_REDIS,
+  },
+};

db/model/subject.js

@@ -340,36 +340,35 @@ module.exports = function subject(seq, dataTypes) {
 
         /*
          * When the sample store feature is enabled do the following
-         * 1. if subject is changed from published to unpublished -> delete
-         * 2. if subject is changed from unpublished to published -> add
-         * 3. if the asbsolutepath of the subject changes and the subject is
-         * puslished, rename the keys
+         * 1. if subject is changed from unpublished to published -> update subject
+         * 2. if subject is changed from published to unpublished -> update subject,
+         *   delete subject aspect map, remove its samples
+         * 3. if the asbsolutepath of the subject changes: rename the subject key,
+         *   update subject, delete subject aspect map, and remove its samples
         */
         if (featureToggles.isFeatureEnabled(sampleStoreFeature)) {
-          if (inst.changed('absolutePath') && inst.isPublished) {
+          if (inst.changed('absolutePath') ||
+            (inst.changed('isPublished') && !inst.isPublished)) {
             const newAbsPath = inst.absolutePath;
             const oldAbsPath = inst._previousDataValues.absolutePath;
 
-            // rename entry in subject store
-            redisOps.renameKey(subjectType, oldAbsPath, newAbsPath);
-            redisOps.hmSet(subjectType, newAbsPath, inst.get());
+            if (inst.changed('absolutePath')) {
+
+              // rename entry in subject store
+              redisOps.renameKey(subjectType, oldAbsPath, newAbsPath);
+            }
 
             /*
-             * When subject absolutePath changes delete multiple possible
-             * entries in sample master list of index
+             * Delete multiple possible
+             * entries in sample master list of index.
+             * Delete the subject to aspect mapping
              */
             redisOps.deleteKeys(sampleType, subjectType, oldAbsPath);
-
-            // also delete the subject to aspect mapping
             redisOps.deleteKey(subAspMapType, oldAbsPath);
-          } else if (inst.changed('isPublished')) {
-            if (inst.isPublished) {
-              redisOps.addKey(subjectType, inst.absolutePath);
-              redisOps.hmSet(subjectType, inst.absolutePath, inst.get());
-            } else {
-              subjectUtils.removeFromRedis(inst.absolutePath);
-            }
           }
+
+          //  update subject
+          redisOps.hmSet(subjectType, inst.absolutePath, inst.get());
         }
 
         if (inst.changed('parentAbsolutePath') ||

tests/cache/models/aspects/aspectCRUD.js

@@ -153,7 +153,6 @@ describe('redis: aspect: create: ', () => {
   });
 
   it('when aspect is updated, the aspect hash should refelct this', (done) => {
-
     Aspect.findById(aspTempId)
     .then((asp) => {
       return asp.update(

tests/cache/models/subjects/subjectCRUD.js

@@ -25,10 +25,10 @@ const sampleIndexName = redisStore.constants.indexKey.sample;
 const redisOps = rtu.redisOps;
 
 describe('redis: subject: CRUD: ', () => {
-  const par = { name: `${tu.namePrefix}NorthAmerica`, isPublished: true };
+  const parentName = `${tu.namePrefix}NorthAmerica`;
+  const par = { name: parentName, isPublished: true };
   const parUnPub =
         { name: `${tu.namePrefix}SouthAmerica`, isPublished: false };
-
   const aspectTemp = {
     name: 'temperature',
     timeout: '30s',
@@ -80,6 +80,24 @@ describe('redis: subject: CRUD: ', () => {
   afterEach(rtu.flushRedis);
   after(() => tu.toggleOverride('enableRedisSampleStore', false));
 
+  it('on unpublish, a subject should still be found', (done) => {
+    const subjectKey = redisStore.toKey('subject', parentName);
+    Subject.findById(ipar)
+    .then((pubishedSubject) => pubishedSubject.update({ isPublished: false }))
+    .then(() => redisClient.sismemberAsync(redisStore.constants.indexKey.subject, subjectKey))
+    .then((ok) => {
+      expect(ok).to.equal(1);
+      return redisClient.hgetallAsync(subjectKey);
+    })
+    .then((subject) => {
+      expect(subject).to.not.equal(null);
+      expect(subject.name).to.equal(parentName);
+      expect(subject.isPublished).to.equal('false');
+      done();
+    })
+    .catch(done);
+  });
+
   it('created subject should be found', (done) => {
     let absolutePath;
     Subject.findById(ipar)
@@ -100,8 +118,7 @@ describe('redis: subject: CRUD: ', () => {
     .catch(done);
   });
 
-  it('unpublished subject should not be found but should be found ' +
-                  ' after it is published', (done) => {
+  it('unpublished subject should be found', (done) => {
     let subj;
     let key;
     Subject.findById(iparUnPub)
@@ -114,18 +131,6 @@ describe('redis: subject: CRUD: ', () => {
       cmds.push(['hgetall', key]);
       return redisOps.executeBatchCmds(cmds);
     })
-    .then((res) => {
-      expect(res[0]).to.equal(0);
-      expect(res[1]).to.equal(null);
-      return subj.update({ isPublished: true });
-    })
-    .then(() => {
-      const cmds = [];
-      cmds.push(redisOps.keyExistsInIndexCmd(objectType.subject,
-        subj.absolutePath));
-      cmds.push(['hgetall', key]);
-      return redisOps.executeBatchCmds(cmds);
-    })
     .then((res) => {
       expect(res[0]).to.equal(1);
       expect(res[1].absolutePath).to.equal(subj.absolutePath);

tests/cache/sampleStore.js

@@ -83,7 +83,7 @@ describe('sampleStore (feature on):', () => {
     }))
     .then((created) => (a3 = created))
     .then(() => Aspect.create({
-      isPublished: false,
+      isPublished: false, // unpublished subject should still be found
       name: `${tu.namePrefix}Aspect4`,
       timeout: '10m',
       valueType: 'BOOLEAN',
@@ -122,7 +122,7 @@ describe('sampleStore (feature on):', () => {
     }))
     .then((created) => (s2 = created))
     .then(() => Subject.create({
-      isPublished: true,
+      isPublished: false,
       name: `${tu.namePrefix}Subject3`,
       parentId: s1.id,
     }))
@@ -190,7 +190,6 @@ describe('sampleStore (feature on):', () => {
     .catch(done);
   });
 
-
   it('eradicate and populate', (done) => {
     samstoinit.eradicate()
     .then(() => redisClient.keysAsync(sampleStore.constants.prefix + '*'))

tests/db/model/subject/find.js

@@ -40,7 +40,7 @@ describe('db: subject: find: ', () => {
     })
     .then((created3) => {
       idDel = created3.id;
-      created3.destroy();
+      return created3.destroy();
     })
     .then(() => Subject.findAll()) // Buffer added because test is too fast
     .then(() => done()) // created3.destroy() doesnt run without buffer