Merge pull request #673 from stephentuso/installation-handling-fix

Handle duplicate android device tokens correctly

Author: gfosco

spec/ParseInstallation.spec.js

@@ -446,6 +446,52 @@ describe('Installations', () => {
     });
   });
 
+  it('update android device token with duplicate device token', (done) => {
+    var installId1 = '11111111-abcd-abcd-abcd-123456789abc';
+    var installId2 = '22222222-abcd-abcd-abcd-123456789abc';
+    var t = '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef';
+    var input = {
+      'installationId': installId1,
+      'deviceToken': t,
+      'deviceType': 'android'
+    };
+    var firstObject;
+    var secondObject;
+    rest.create(config, auth.nobody(config), '_Installation', input)
+        .then(() => {
+          input = {
+            'installationId': installId2,
+            'deviceType': 'android'
+          };
+          return rest.create(config, auth.nobody(config), '_Installation', input);
+        }).then(() => {
+      return database.mongoFind('_Installation',
+          {installationId: installId1}, {});
+    }).then((results) => {
+      expect(results.length).toEqual(1);
+      firstObject = results[0];
+      return database.mongoFind('_Installation',
+          {installationId: installId2}, {});
+    }).then((results) => {
+      expect(results.length).toEqual(1);
+      secondObject = results[0];
+      // Update second installation to conflict with first installation
+      input = {
+        'objectId': secondObject._id,
+        'deviceToken': t
+      };
+      return rest.update(config, auth.nobody(config), '_Installation',
+          secondObject._id, input);
+    }).then(() => {
+      // The first object should have been deleted
+      return database.mongoFind('_Installation', {_id: firstObject._id}, {});
+    }).then((results) => {
+      expect(results.length).toEqual(0);
+      done();
+    }).catch((error) => { console.log(error); });
+  });
+
+
   it('update ios device token with duplicate device token', (done) => {
     var installId1 = '11111111-abcd-abcd-abcd-123456789abc';
     var installId2 = '22222222-abcd-abcd-abcd-123456789abc';

src/RestWrite.js

@@ -594,6 +594,9 @@ RestWrite.prototype.handleInstallation = function() {
 
   var promise = Promise.resolve();
 
+  var idMatch; // Will be a match on either objectId or installationId
+  var deviceTokenMatches = [];
+
   if (this.query && this.query.objectId) {
     promise = promise.then(() => {
       return this.config.database.find('_Installation', {
@@ -603,22 +606,22 @@ RestWrite.prototype.handleInstallation = function() {
           throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND,
                                 'Object not found for update.');
         }
-        var existing = results[0];
-        if (this.data.installationId && existing.installationId &&
-          this.data.installationId !== existing.installationId) {
+        idMatch = results[0];
+        if (this.data.installationId && idMatch.installationId &&
+          this.data.installationId !== idMatch.installationId) {
           throw new Parse.Error(136,
                                 'installationId may not be changed in this ' +
                                 'operation');
         }
-        if (this.data.deviceToken && existing.deviceToken &&
-          this.data.deviceToken !== existing.deviceToken &&
-          !this.data.installationId && !existing.installationId) {
+        if (this.data.deviceToken && idMatch.deviceToken &&
+          this.data.deviceToken !== idMatch.deviceToken &&
+          !this.data.installationId && !idMatch.installationId) {
           throw new Parse.Error(136,
                                 'deviceToken may not be changed in this ' +
                                 'operation');
         }
         if (this.data.deviceType && this.data.deviceType &&
-          this.data.deviceType !== existing.deviceType) {
+          this.data.deviceType !== idMatch.deviceType) {
           throw new Parse.Error(136,
                                 'deviceType may not be changed in this ' +
                                 'operation');
@@ -629,8 +632,6 @@ RestWrite.prototype.handleInstallation = function() {
   }
 
   // Check if we already have installations for the installationId/deviceToken
-  var installationMatch;
-  var deviceTokenMatches = [];
   promise = promise.then(() => {
     if (this.data.installationId) {
       return this.config.database.find('_Installation', {
@@ -641,7 +642,7 @@ RestWrite.prototype.handleInstallation = function() {
   }).then((results) => {
     if (results && results.length) {
       // We only take the first match by installationId
-      installationMatch = results[0];
+      idMatch = results[0];
     }
     if (this.data.deviceToken) {
       return this.config.database.find(
@@ -653,7 +654,7 @@ RestWrite.prototype.handleInstallation = function() {
     if (results) {
       deviceTokenMatches = results;
     }
-    if (!installationMatch) {
+    if (!idMatch) {
       if (!deviceTokenMatches.length) {
         return;
       } else if (deviceTokenMatches.length == 1 &&
@@ -691,14 +692,14 @@ RestWrite.prototype.handleInstallation = function() {
         // Exactly one device token match and it doesn't have an installation
         // ID. This is the one case where we want to merge with the existing
         // object.
-        var delQuery = {objectId: installationMatch.objectId};
+        var delQuery = {objectId: idMatch.objectId};
         return this.config.database.destroy('_Installation', delQuery)
           .then(() => {
             return deviceTokenMatches[0]['objectId'];
           });
       } else {
         if (this.data.deviceToken &&
-          installationMatch.deviceToken != this.data.deviceToken) {
+          idMatch.deviceToken != this.data.deviceToken) {
           // We're setting the device token on an existing installation, so
           // we should try cleaning out old installations that match this
           // device token.
@@ -714,7 +715,7 @@ RestWrite.prototype.handleInstallation = function() {
           this.config.database.destroy('_Installation', delQuery);
         }
         // In non-merge scenarios, just return the installation match id
-        return installationMatch.objectId;
+        return idMatch.objectId;
       }
     }
   }).then((objId) => {