Migrate from GCM to FCM notifications (#7860)

* Fixed logging. Fixed lint issues. Temporarly removed GCM code

* Removed extra packages. Removed lingering gcm code

* Update message structure to send a data notification instead of a regular notification

* Removed excess code

* switch from fcm-push to node-gcm

config.json.example

@@ -65,7 +65,8 @@
     "LOGGLY_ACCOUNT": "account",
     "PUSH_CONFIGS": {
         "GCM_SERVER_API_KEY": "",
-        "APN_ENABLED": "true"
+        "APN_ENABLED": "true",
+        "FCM_SERVER_API_KEY": ""
     },
     "PUSHER": {
         "ENABLED": "false",

package.json

@@ -68,6 +68,7 @@
     "nconf": "~0.8.2",
     "newrelic": "^1.27.2",
     "nib": "^1.1.0",
+    "node-gcm": "^0.14.4",
     "nodemailer": "^2.3.2",
     "object-path": "^0.9.2",
     "pageres": "^4.1.1",

test/api/v3/unit/libs/pushNotifications.js

@@ -2,12 +2,13 @@ import { model as User } from '../../../../../website/server/models/user';
 import requireAgain from 'require-again';
 import pushNotify from 'push-notify';
 import nconf from 'nconf';
+import gcmLib from 'node-gcm'; // works with FCM notifications too
 
 describe('pushNotifications', () => {
   let user;
   let sendPushNotification;
   let pathToPushNotifications = '../../../../../website/server/libs/pushNotifications';
-  let gcmSendSpy;
+  let fcmSendSpy;
   let apnSendSpy;
 
   let identifier = 'identifier';
@@ -16,15 +17,12 @@ describe('pushNotifications', () => {
 
   beforeEach(() => {
     user = new User();
-    gcmSendSpy = sinon.spy();
+    fcmSendSpy = sinon.spy();
     apnSendSpy = sinon.spy();
 
-    sandbox.stub(nconf, 'get').returns('true');
+    sandbox.stub(nconf, 'get').returns('true-key');
 
-    sandbox.stub(pushNotify, 'gcm').returns({
+    sandbox.stub(gcmLib.Sender.prototype, 'send', fcmSendSpy);
-      on: () => null,
-      send: gcmSendSpy,
-    });
 
     sandbox.stub(pushNotify, 'apn').returns({
       on: () => null,
@@ -40,14 +38,14 @@ describe('pushNotifications', () => {
 
   it('throws if user is not supplied', () => {
     expect(sendPushNotification).to.throw;
-    expect(gcmSendSpy).to.not.have.been.called;
+    expect(fcmSendSpy).to.not.have.been.called;
     expect(apnSendSpy).to.not.have.been.called;
   });
 
   it('throws if user.preferences.pushNotifications.unsubscribeFromAll is true', () => {
     user.preferences.pushNotifications.unsubscribeFromAll = true;
     expect(() => sendPushNotification(user)).to.throw;
-    expect(gcmSendSpy).to.not.have.been.called;
+    expect(fcmSendSpy).to.not.have.been.called;
     expect(apnSendSpy).to.not.have.been.called;
   });
 
@@ -56,7 +54,7 @@ describe('pushNotifications', () => {
       title,
       message,
     })).to.throw;
-    expect(gcmSendSpy).to.not.have.been.called;
+    expect(fcmSendSpy).to.not.have.been.called;
     expect(apnSendSpy).to.not.have.been.called;
   });
 
@@ -65,7 +63,7 @@ describe('pushNotifications', () => {
       identifier,
       message,
     })).to.throw;
-    expect(gcmSendSpy).to.not.have.been.called;
+    expect(fcmSendSpy).to.not.have.been.called;
     expect(apnSendSpy).to.not.have.been.called;
   });
 
@@ -74,7 +72,7 @@ describe('pushNotifications', () => {
       identifier,
       title,
     })).to.throw;
-    expect(gcmSendSpy).to.not.have.been.called;
+    expect(fcmSendSpy).to.not.have.been.called;
     expect(apnSendSpy).to.not.have.been.called;
   });
 
@@ -84,68 +82,7 @@ describe('pushNotifications', () => {
       title,
       message,
     });
-    expect(gcmSendSpy).to.not.have.been.called;
+    expect(fcmSendSpy).to.not.have.been.called;
-    expect(apnSendSpy).to.not.have.been.called;
-  });
-
-  it('uses GCM for Android devices', () => {
-    user.pushDevices.push({
-      type: 'android',
-      regId: '123',
-    });
-
-    let details = {
-      identifier,
-      title,
-      message,
-      payload: {
-        a: true,
-        b: true,
-      },
-      timeToLive: 23,
-    };
-
-    sendPushNotification(user, details);
-    expect(gcmSendSpy).to.have.been.calledOnce;
-    expect(gcmSendSpy).to.have.been.calledWithMatch({
-      registrationId: '123',
-      delayWhileIdle: true,
-      timeToLive: 23,
-      data: {
-        identifier,
-        title,
-        message,
-        a: true,
-        b: true,
-      },
-    });
-    expect(apnSendSpy).to.not.have.been.called;
-  });
-
-  it('defaults timeToLive to 15', () => {
-    user.pushDevices.push({
-      type: 'android',
-      regId: '123',
-    });
-
-    let details = {
-      identifier,
-      title,
-      message,
-    };
-
-    sendPushNotification(user, details);
-    expect(gcmSendSpy).to.have.been.calledOnce;
-    expect(gcmSendSpy).to.have.been.calledWithMatch({
-      registrationId: '123',
-      delayWhileIdle: true,
-      timeToLive: 15,
-      data: {
-        identifier,
-        title,
-        message,
-      },
-    });
     expect(apnSendSpy).to.not.have.been.called;
   });
 
@@ -180,6 +117,6 @@ describe('pushNotifications', () => {
         b: true,
       },
     });
-    expect(gcmSendSpy).to.not.have.been.called;
+    expect(fcmSendSpy).to.not.have.been.called;
   });
 });

website/server/libs/pushNotifications.js

@@ -1,5 +1,6 @@
 import _ from 'lodash';
 import nconf from 'nconf';
+// TODO remove this lib and use directly the apn module
 import pushNotify from 'push-notify';
 import apnLib from 'apn';
 import logger from './logger';
@@ -7,19 +8,11 @@ import Bluebird from 'bluebird';
 import {
   S3,
 } from './aws';
+import gcmLib from 'node-gcm'; // works with FCM notifications too
 
-const GCM_API_KEY = nconf.get('PUSH_CONFIGS:GCM_SERVER_API_KEY');
+const FCM_API_KEY = nconf.get('PUSH_CONFIGS:FCM_SERVER_API_KEY');
 
-let gcm = GCM_API_KEY ? pushNotify.gcm({
+const fcmSender = FCM_API_KEY ? new gcmLib.Sender(FCM_API_KEY) : undefined;
-  apiKey: GCM_API_KEY,
-  retries: 3,
-}) : undefined;
-
-if (gcm) {
-  gcm.on('transmissionError', (err, message, registrationId) => {
-    logger.error('GCM Error', err, message, registrationId);
-  });
-}
 
 let apn;
 
@@ -82,15 +75,18 @@ module.exports = function sendNotification (user, details = {}) {
   _.each(pushDevices, pushDevice => {
     switch (pushDevice.type) {
       case 'android':
-        if (gcm) {
+        // Required for fcm to be received in background
-          payload.title = details.title;
+        payload.title = details.title;
-          payload.message = details.message;
+        payload.body = details.message;
-          gcm.send({
+
-            registrationId: pushDevice.regId,
+        if (fcmSender) {
-            delayWhileIdle: true,
+          let message = new gcmLib.Message({
-            timeToLive: details.timeToLive ? details.timeToLive : 15,
             data: payload,
           });
+
+          fcmSender.send(message, {
+            registrationTokens: [pushDevice.regId],
+          }, 10, (err) => logger.error('FCM Error', err));
         }
         break;