index on unique-indexes: c454180 Revert "Log objects rather than JSON…

… stringified objects (parse-community#1922)"
drew-gross · Jun 6, 2016 · 8342f3d · 8342f3d
1 parent c454180
commit 8342f3d
Show file tree

Hide file tree

Showing 7 changed files with 149 additions and 89 deletions.
diff --git a/spec/ParseAPI.spec.js b/spec/ParseAPI.spec.js
@@ -11,7 +11,7 @@ let defaultColumns = require('../src/Controllers/SchemaController').defaultColum
 const requiredUserFields = { fields: Object.assign({}, defaultColumns._Default, defaultColumns._User) };
 
 
-describe('miscellaneous', function() {
+fdescribe('miscellaneous', function() {
   it('create a GameScore object', function(done) {
     var obj = new Parse.Object('GameScore');
     obj.set('score', 1337);
@@ -80,44 +80,53 @@ describe('miscellaneous', function() {
     .catch(done);
   });
 
-  it('ensure that email is uniquely indexed', done => {
-    let numCreated = 0;
-    let numFailed = 0;
-
-    let user1 = new Parse.User();
-    user1.setPassword('asdf');
-    user1.setUsername('u1');
-    user1.setEmail('[email protected]');
-    let p1 = user1.signUp();
-    p1.then(user => {
-      numCreated++;
-      expect(numCreated).toEqual(1);
-    }, error => {
-      numFailed++;
-      expect(numFailed).toEqual(1);
-      expect(error.code).toEqual(Parse.Error.EMAIL_TAKEN);
-    });
+  fit('ensure that email is uniquely indexed', done => {
+    DatabaseAdapter._indexBuildsCompleted('test')
+    .then(() => {
+      let numCreated = 0;
+      let numFailed = 0;
+
+      let user1 = new Parse.User();
+      user1.setPassword('asdf');
+      user1.setUsername('u1');
+      user1.setEmail('[email protected]');
+      let p1 = user1.signUp();
+      p1.then(user => {
+        numCreated++;
+        expect(numCreated).toEqual(1);
+      }, error => {
+        numFailed++;
+        console.log(error);
+        expect(numFailed).toEqual(1);
+        expect(error.code).toEqual(Parse.Error.EMAIL_TAKEN);
+      });
 
-    let user2 = new Parse.User();
-    user2.setPassword('asdf');
-    user2.setUsername('u2');
-    user2.setEmail('[email protected]');
-    let p2 = user2.signUp();
-    p2.then(user => {
-      numCreated++;
-      expect(numCreated).toEqual(1);
-    }, error => {
-      numFailed++;
-      expect(numFailed).toEqual(1);
-      expect(error.code).toEqual(Parse.Error.EMAIL_TAKEN);
-    });
+      let user2 = new Parse.User();
+      user2.setPassword('asdf');
+      user2.setUsername('u2');
+      user2.setEmail('[email protected]');
+      let p2 = user2.signUp();
+      p2.then(user => {
+        numCreated++;
+        expect(numCreated).toEqual(1);
+      }, error => {
+        numFailed++;
+        console.log(error);
+        expect(numFailed).toEqual(1);
+        expect(error.code).toEqual(Parse.Error.EMAIL_TAKEN);
+      });
 
-    Parse.Promise.all([p1, p2])
-    .then(() => {
-      fail('one of the users should not have been created');
-      done();
+      Parse.Promise.all([p1, p2])
+      .then(() => {
+        fail('one of the users should not have been created');
+        done();
+      })
+      .catch(done);
     })
-    .catch(done);
+    .catch(error => {
+      fail('index build failed')
+      done();
+    });
   });
 
   it('ensure that if people already have duplicate users, they can still sign up new users', done => {

diff --git a/spec/helper.js b/spec/helper.js
@@ -111,7 +111,7 @@ afterEach(function(done) {
   })
   .then(() => Parse.User.logOut())
   .then(() => {
-    return TestUtils.destroyAllDataPermanently();
+    //return TestUtils.destroyAllDataPermanently();
   }).then(() => {
     done();
   }, (error) => {
@@ -243,14 +243,16 @@ function mockFacebook() {
   facebook.validateAuthData = function(authData) {
     if (authData.id === '8675309' && authData.access_token === 'jenny') {
       return Promise.resolve();
+    } else {
+      throw undefined;
     }
-    return Promise.reject();
   };
   facebook.validateAppId = function(appId, authData) {
     if (authData.access_token === 'jenny') {
       return Promise.resolve();
+    } else {
+      throw undefined;
     }
-    return Promise.reject();
   };
   return facebook;
 }

diff --git a/src/Adapters/Storage/Mongo/MongoCollection.js b/src/Adapters/Storage/Mongo/MongoCollection.js
@@ -88,8 +88,9 @@ export default class MongoCollection {
       this._mongoCollection.ensureIndex(indexRequest, { unique: true, background: true, sparse: true }, (error, indexName) => {
         if (error) {
           reject(error);
+        } else {
+          resolve();
         }
-        resolve();
       });
     });
   }

diff --git a/src/DatabaseAdapter.js b/src/DatabaseAdapter.js
@@ -17,10 +17,16 @@
 
 import DatabaseController  from './Controllers/DatabaseController';
 import MongoStorageAdapter from './Adapters/Storage/Mongo/MongoStorageAdapter';
+import log                 from './logger';
+
+var SchemaController = require('./Controllers/SchemaController');
 
 let dbConnections = {};
 let appDatabaseURIs = {};
 let appDatabaseOptions = {};
+let indexBuildCreationPromises = {};
+
+const requiredUserFields = { fields: { ...SchemaController.defaultColumns._Default, ...SchemaController.defaultColumns._User } };
 
 function setAppDatabaseURI(appId, uri) {
   appDatabaseURIs[appId] = uri;
@@ -49,6 +55,11 @@ function destroyAllDataPermanently() {
   throw 'Only supported in test environment';
 }
 
+//Super janky. Will be removed in a later PR.
+function _indexBuildsCompleted(appId) {
+  return indexBuildCreationPromises[appId];
+}
+
 function getDatabaseConnection(appId: string, collectionPrefix: string) {
   if (dbConnections[appId]) {
     return dbConnections[appId];
@@ -62,6 +73,57 @@ function getDatabaseConnection(appId: string, collectionPrefix: string) {
 
   dbConnections[appId] = new DatabaseController(new MongoStorageAdapter(mongoAdapterOptions), {appId: appId});
 
+  // Kick off unique index build in the background (or ensure the unique index already exists)
+  // A bit janky, will be fixed in a later PR.
+  let p1 = dbConnections[appId].adapter.ensureUniqueness('_User', ['username'], requiredUserFields)
+  .catch(error => {
+    log.warn('Unable to ensure uniqueness for usernames: ', error);
+    return Promise.reject();
+  });
+
+  let p2 = dbConnections[appId].adapter.ensureUniqueness('_User', ['email'], requiredUserFields)
+  .catch(error => {
+    log.warn('Unabled to ensure uniqueness for user email addresses: ', error);
+    return Promise.reject();
+  })
+
+  indexBuildCreationPromises[appId] = p1.then(() => p2)
+  .then(() => console.log('index build success'))
+  .then(() => {
+    let numCreated = 0;
+    let numFailed = 0;
+
+    let user1 = new Parse.User();
+    user1.setPassword('asdf');
+    user1.setUsername('u1');
+    user1.setEmail('[email protected]');
+    let p1 = user1.signUp();
+    p1.then(user => {
+      numCreated++;
+      console.log(numCreated)
+    }, error => {
+      numFailed++;
+      console.log(error);
+      console.log(numFailed)
+      console.log(error.code)
+    });
+
+    let user2 = new Parse.User();
+    user2.setPassword('asdf');
+    user2.setUsername('u2');
+    user2.setEmail('[email protected]');
+    let p2 = user2.signUp();
+    p2.then(user => {
+      numCreated++;
+      console.log(numCreated)
+    }, error => {
+      numFailed++;
+      console.log(error);
+      console.log(numFailed)
+      console.log(error.code)
+    });
+  })
+
   return dbConnections[appId];
 }
 
@@ -71,4 +133,5 @@ module.exports = {
   setAppDatabaseURI: setAppDatabaseURI,
   clearDatabaseSettings: clearDatabaseSettings,
   destroyAllDataPermanently: destroyAllDataPermanently,
+  _indexBuildsCompleted,
 };
diff --git a/src/PromiseRouter.js b/src/PromiseRouter.js
@@ -6,8 +6,8 @@
 // components that external developers may be modifying.
 
 import express from 'express';
-import url from 'url';
-import log from './logger';
+import url     from 'url';
+import log     from './logger';
 
 export default class PromiseRouter {
   // Each entry should be an object with:

diff --git a/src/RestWrite.js b/src/RestWrite.js
@@ -14,8 +14,6 @@ var triggers = require('./triggers');
 import RestQuery from './RestQuery';
 import _         from 'lodash';
 
-const requiredUserFields = { fields: { ...SchemaController.defaultColumns._Default, ...SchemaController.defaultColumns._User } };
-
 // query and data are both provided in REST API format. So data
 // types are encoded by plain old objects.
 // If query is null, this is a "create" and the data in data should be
@@ -358,61 +356,47 @@ RestWrite.prototype.transformUser = function() {
       }
       return;
     }
-    // TODO: refactor this so that ensureUniqueness isn't called for every single sign up.
-    return this.config.database.adapter.ensureUniqueness('_User', ['username'], requiredUserFields)
-    .catch(error => {
-      if (error.code === Parse.Error.DUPLICATE_VALUE) {
-        // If they already have duplicate usernames or emails, the ensureUniqueness will fail,
-        // and then nobody will be able to sign up D: so just use the old, janky
-        // method to check for duplicate usernames.
-        return this.config.database.find(
-          this.className,
-          { username: this.data.username, objectId: {'$ne': this.objectId()} },
-          { limit: 1 }
-        )
-        .then(results => {
-          if (results.length > 0) {
-            throw new Parse.Error(Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.');
-          }
-          return;
-        });
+    // We need to a find to check for duplicate username in case they are missing the unique index on usernames
+    // TODO: Check if there is a unique index, and if so, skip this query.
+    return this.config.database.find(
+      this.className,
+      { username: this.data.username, objectId: {'$ne': this.objectId()} },
+      { limit: 1 }
+    )
+    .then(results => {
+      if (results.length > 0) {
+        throw new Parse.Error(Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.');
       }
-      throw error;
-    })
-  }).then(() => {
+      return;
+    });
+  })
+  .then(() => {
     if (!this.data.email || this.data.email.__op === 'Delete') {
       return;
     }
     // Validate basic email address format
     if (!this.data.email.match(/^.+@.+$/)) {
       throw new Parse.Error(Parse.Error.INVALID_EMAIL_ADDRESS, 'Email address format is invalid.');
     }
-    // Check for email uniqueness
-    return this.config.database.adapter.ensureUniqueness('_User', ['email'], requiredUserFields)
-    .catch(error => {
-      // Same problem for email as above for username
-      if (error.code === Parse.Error.DUPLICATE_VALUE) {
-        return this.config.database.find(
-          this.className,
-          { email: this.data.email, objectId: {'$ne': this.objectId()} },
-          { limit: 1 }
-        )
-        .then(results => {
-          if (results.length > 0) {
-            throw new Parse.Error(Parse.Error.EMAIL_TAKEN, 'Account already exists for this email address.');
-          }
-          return;
-        });
+    // Same problem for email as above for username
+    return this.config.database.find(
+      this.className,
+      { email: this.data.email, objectId: {'$ne': this.objectId()} },
+      { limit: 1 }
+    )
+    .then(results => {
+      if (results.length > 0) {
+        throw new Parse.Error(Parse.Error.EMAIL_TAKEN, 'Account already exists for this email address.');
       }
-      throw error;
-    })
-    .then(() => {
-      // We updated the email, send a new validation
-      this.storage['sendVerificationEmail'] = true;
-      this.config.userController.setEmailVerifyToken(this.data);
       return;
-    })
-  });
+    });
+  })
+  .then(() => {
+    // We updated the email, send a new validation
+    this.storage['sendVerificationEmail'] = true;
+    this.config.userController.setEmailVerifyToken(this.data);
+    return;
+  })
 };
 
 RestWrite.prototype.createSessionTokenIfNeeded = function() {

diff --git a/src/middlewares.js b/src/middlewares.js
@@ -235,6 +235,7 @@ var handleParseErrors = function(err, req, res, next) {
     }
 
     res.status(httpStatus);
+    console.log(err);
     res.json({code: err.code, error: err.message});
   } else if (err.status && err.message) {
     res.status(err.status);