Fix give-brownie-points/about API DB queries

This should speed up these api calls significantly

common/models/user.js

@@ -1,26 +1,26 @@
-import { Observable } from 'rx';
+import { Disposable, Observable, Scheduler } from 'rx';
 import uuid from 'node-uuid';
 import moment from 'moment';
 import dedent from 'dedent';
-import debugFactory from 'debug';
+import debug from 'debug';
-
-import { saveUser, observeMethod } from '../../server/utils/rx';
 import { blacklistedUsernames } from '../../server/utils/constants';
 
-const debug = debugFactory('fcc:user:remote');
+const log = debug('fcc:user:remote');
 const BROWNIEPOINTS_TIMEOUT = [1, 'hour'];
 
+const aboutFilter = {
+  username: true,
+  bio: true
+};
 function getAboutProfile({
   username,
-  githubProfile: github,
+  bio,
-  progressTimestamps = [],
+  points
-  bio
 }) {
   return {
     username,
-    github,
+    bio,
-    browniePoints: progressTimestamps.length,
+    browniePoints: points
-    bio
   };
 }
 
@@ -54,8 +54,31 @@ module.exports = function(User) {
 
   User.on('dataSourceAttached', () => {
     User.findOne$ = Observable.fromNodeCallback(User.findOne, User);
+    User.findById$ = Observable.fromNodeCallback(User.findById, User);
     User.update$ = Observable.fromNodeCallback(User.updateAll, User);
     User.count$ = Observable.fromNodeCallback(User.count, User);
+    // getPointsById$(_id: String|ObjectId) => Observable[Number]
+    User.getPointsById$ = function getPointsById$(id) {
+      return Observable.create(observer => {
+        let isDisposed = false;
+        // safe ObjectID creation
+        // MongoID(id: ObjectID|String) => ObjectID
+        // MongoDB requires id's to be of type ObjectID
+        const _id = this.app.dataSources.db.connector.getDefaultIdType()(id);
+        this.app.dataSources.db.connector
+          .collection('user')
+          .aggregate([
+            { $match: { _id } },
+            { $project: { points: { $size: '$progressTimestamps' } } }
+          ], (err, [ { points = 1 } = {}]) => {
+            if (isDisposed) { return null; }
+            if (err) { return observer.onError(err); }
+            observer.onNext(points);
+            return observer.onCompleted();
+          });
+        return Disposable.create(() => { isDisposed = true; });
+      });
+    };
   });
 
   User.observe('before save', function({ instance: user }, next) {
@@ -76,7 +99,7 @@ module.exports = function(User) {
     next();
   });
 
-  debug('setting up user hooks');
+  log('setting up user hooks');
   User.afterRemote('confirm', function(ctx) {
     ctx.req.flash('success', {
       msg: [
@@ -128,9 +151,9 @@ module.exports = function(User) {
     }
 
     // the email of the requested user
-    debug(info.email);
+    log(info.email);
     // the temp access token to allow password reset
-    debug(info.accessToken.id);
+    log(info.accessToken.id);
     // requires AccessToken.belongsTo(User)
     var mailOptions = {
       to: info.email,
@@ -150,7 +173,7 @@ module.exports = function(User) {
 
     User.app.models.Email.send(mailOptions, function(err) {
       if (err) { console.error(err); }
-      debug('email reset sent');
+      log('email reset sent');
     });
   });
 
@@ -173,7 +196,7 @@ module.exports = function(User) {
     };
 
     if (accessToken && accessToken.id) {
-      debug('setting cookies');
+      log('setting cookies');
       res.cookie('access_token', accessToken.id, config);
       res.cookie('userId', accessToken.userId, config);
     }
@@ -181,7 +204,7 @@ module.exports = function(User) {
     return req.logIn({ id: accessToken.userId.toString() }, function(err) {
       if (err) { return next(err); }
 
-      debug('user logged in');
+      log('user logged in');
 
       if (req.session && req.session.returnTo) {
         var redirectTo = req.session.returnTo;
@@ -217,7 +240,7 @@ module.exports = function(User) {
     if (!username && !email) {
       return Promise.resolve(false);
     }
-    debug('checking existence');
+    log('checking existence');
 
     // check to see if username is on blacklist
     if (username && blacklistedUsernames.indexOf(username) !== -1) {
@@ -230,7 +253,7 @@ module.exports = function(User) {
     } else {
       where.email = email ? email.toLowerCase() : email;
     }
-    debug('where', where);
+    log('where', where);
     return User.count(where)
     .then(count => count > 0);
   };
@@ -271,16 +294,23 @@ module.exports = function(User) {
         ));
       });
     }
-    return User.findOne({ where: { username } }, (err, user) => {
+    username = username.toLowerCase();
-      if (err) {
+    const filter = {
-        return cb(err);
+      where: { username },
-      }
+      fields: { id: true, ...aboutFilter }
-      if (!user || user.username !== username) {
+    };
-        return cb(new Error(`no user found for ${ username }`));
+    return User.findOne$(filter)
-      }
+      .doOnNext(user => {
-      const aboutUser = getAboutProfile(user);
+        if (!user || user.username !== username) {
-      return cb(null, aboutUser);
+          throw new Error(`no user found for ${ username }`);
-    });
+        }
+      })
+      .flatMap(user => user.getPoints$().map(user))
+      .map(user => getAboutProfile(user))
+      .subscribe(
+        aboutUser => cb(null, aboutUser),
+        cb
+      );
   };
 
   User.remoteMethod(
@@ -308,7 +338,8 @@ module.exports = function(User) {
 
   User.giveBrowniePoints =
     function giveBrowniePoints(receiver, giver, data = {}, dev = false, cb) {
-      const findUser = observeMethod(User, 'findOne');
+      receiver = receiver.toLowerCase();
+      giver = giver.toLowerCase();
       if (!receiver) {
         return nextTick(() => {
           cb(
@@ -321,63 +352,84 @@ module.exports = function(User) {
           cb(new TypeError(`giver should be a string but got ${ giver }`));
         });
       }
+      if (giver === receiver) {
+        return nextTick(() => {
+          cb(new Error('giver and receiver must be different users'));
+        });
+      }
       let temp = moment();
       const browniePoints = temp
         .subtract.apply(temp, BROWNIEPOINTS_TIMEOUT)
         .valueOf();
-      const user$ = findUser({ where: { username: receiver }});
+      const user$ = User.findOne$({
-
+        where: { username: receiver },
-      return user$
+        fields: {
-        .tapOnNext((user) => {
+          ...aboutFilter,
+          progressTimestamps: true
+        }
+      });
+      const giver$ = User.count$({ username: giver });
+      return Observable.combineLatest(
+        user$,
+        giver$,
+        (user, giver) => ({ doesGiverExist: !!giver, user })
+      )
+        .tapOnNext(({ user, doesGiverExist }) => {
           if (!user) {
             throw new Error(`could not find receiver for ${ receiver }`);
           }
+          if (!doesGiverExist) {
+            throw new Error(`no user found for giver '${giver}'`);
+          }
         })
         .flatMap(({ progressTimestamps = [] }) => {
-          return Observable.from(progressTimestamps);
+          return Observable.from(
+            progressTimestamps,
+            null,
+            null,
+            Scheduler.default
+          );
         })
         // filter out non objects
         .filter((timestamp) => !!timestamp || typeof timestamp === 'object')
         // filterout timestamps older then an hour
-        .filter(({ timestamp = 0 }) => {
+        .filter(({ timestamp = 0 }) => timestamp >= browniePoints)
-          return timestamp >= browniePoints;
-        })
         // filter out brownie points given by giver
-        .filter((browniePoint) => {
+        .filter(browniePoint => browniePoint.giver === giver)
-          return browniePoint.giver === giver;
-        })
         // no results means this is the first brownie point given by giver
         // so return -1 to indicate receiver should receive point
         .first({ defaultValue: -1 })
-        .flatMap((browniePointsFromGiver) => {
+        .flatMap(browniePointsFromGiver => {
           if (browniePointsFromGiver === -1) {
-
+            const updateData = {
-            return user$.flatMap((user) => {
+              $push: {
-              user.progressTimestamps.push({
+                progressTimestamps: {
-                giver,
+                  giver,
-                timestamp: Date.now(),
+                  timestamp: Date.now()
-                ...data
+                }
+              }
+            };
+            return user$
+              .flatMap(user => user.update$(updateData).map(user))
+              .doOnNext(user => {
+                user.points = user.progressTimestamps.length + 1;
               });
-              return saveUser(user);
-            });
           }
           return Observable.throw(
             new Error(`${ giver } already gave ${ receiver } points`)
           );
         })
         .subscribe(
-          (user) => {
+          user => cb(
-            return cb(
+            null,
-              null,
+            getAboutProfile(user),
-              getAboutProfile(user),
+            dev ?
-              dev ?
+              { giver, receiver, data } :
-                { giver, receiver, data } :
+              null
-                null
+          ),
-            );
+          e => cb(e, null, dev ? { giver, receiver, data } : null),
-          },
-          (e) => cb(e, null, dev ? { giver, receiver, data } : null),
           () => {
-            debug('brownie points assigned completed');
+            log('brownie points assigned completed');
           }
         );
     };
@@ -423,7 +475,7 @@ module.exports = function(User) {
     }
   );
 
-  // user.updateTo$(updateData: Object) => Observable[Number]
+  // user.update$(updateData: Object) => Observable[Number]
   User.prototype.update$ = function update$(updateData) {
     const id = this.getId();
     const updateOptions = { allowExtendedOperators: true };
@@ -441,7 +493,7 @@ module.exports = function(User) {
     }
     return this.constructor.update$({ id }, updateData, updateOptions);
   };
-  User.prototype.getPoints$ = function getPoints$() {
+  User.prototype.getTimestamps = function getTimestamps() {
     const id = this.getId();
     const filter = {
       where: { id },
@@ -465,4 +517,11 @@ module.exports = function(User) {
         return user.challengeMap;
       });
   };
+  // user.getPoints$() => Observable[Number]
+  User.prototype.getPoints$ = function getPoints$() {
+    const id = this.getId();
+    return this.constructor
+      .getPointsById$(id)
+      .doOnNext(points => { this.points = points; });
+  };
 };

server/component-passport.js

@@ -1,3 +1,4 @@
+import { Observable } from 'rx';
 import passport from 'passport';
 import { PassportConfigurator } from 'loopback-component-passport';
 import passportProviders from './passport-providers';
@@ -70,22 +71,21 @@ PassportConfigurator.prototype.init = function passportInit(noSession) {
   });
 
   passport.deserializeUser((id, done) => {
-
+    Observable.combineLatest(
-    this.userModel.findById(id, { fields }, (err, user) => {
+      this.userModel.findById$(id, { fields }),
-      if (err || !user) {
+      this.userModel.getPointsById$(id),
-        return done(err, user);
+      (user, points) => {
+        if (user) { user.points = points; }
+        return user;
       }
-      return this.app.dataSources.db.connector
+    )
-        .collection('user')
+      .doOnNext(user => {
-        .aggregate([
+        if (!user) { throw new Error('deserialize found no user'); }
-          { $match: { _id: user.id } },
+      })
-          { $project: { points: { $size: '$progressTimestamps' } } }
+      .subscribe(
-        ], function(err, [{ points = 1 } = {}]) {
+        user => done(null, user),
-          if (err) { return done(err); }
+        done
-          user.points = points;
+      );
-          return done(null, user);
-        });
-    });
   });
 };