FCM client improvements take II

core/hooks/send_messages.go

@@ -35,6 +35,6 @@ func (h *sendMessagesHook) Apply(ctx context.Context, tx *sqlx.Tx, rp *redis.Poo
 		msgs = append(msgs, sceneMsgs...)
 	}
 
-	msgio.SendMessages(ctx, tx, rp, msgs)
+	msgio.SendMessages(ctx, tx, rp, nil, msgs)
 	return nil
 }

core/msgio/android.go

@@ -1,7 +1,6 @@
 package msgio
 
 import (
-	"sync"
 	"time"
 
 	"github.com/nyaruka/mailroom/config"
@@ -12,27 +11,9 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-var clientInit sync.Once
-var fcmClient *fcm.Client
-
-func init() {
-	clientInit.Do(func() {
-		if config.Mailroom.FCMKey == "" {
-			logrus.Error("fcm not configured, no syncing of android channels")
-			return
-		}
-
-		var err error
-		fcmClient, err = fcm.NewClient(config.Mailroom.FCMKey)
-		if err != nil {
-			panic(errors.Wrap(err, "unable to create FCM client"))
-		}
-	})
-}
-
 // SyncAndroidChannels tries to trigger syncs of the given Android channels via FCM
-func SyncAndroidChannels(channels []*models.Channel) {
-	if fcmClient == nil {
+func SyncAndroidChannels(fc *fcm.Client, channels []*models.Channel) {
+	if fc == nil {
 		logrus.Warn("skipping Android sync as instance has not configured FCM")
 		return
 	}
@@ -52,7 +33,7 @@ func SyncAndroidChannels(channels []*models.Channel) {
 		}
 
 		start := time.Now()
-		_, err := fcmClient.Send(sync)
+		_, err := fc.Send(sync)
 
 		if err != nil {
 			// log failures but continue, relayer will sync on its own
@@ -63,7 +44,14 @@ func SyncAndroidChannels(channels []*models.Channel) {
 	}
 }
 
-// SetFCMClient sets the FCM client. Used for testing.
-func SetFCMClient(client *fcm.Client) {
-	fcmClient = client
+// CreateFCMClient creates an FCM client based on the configured FCM API key
+func CreateFCMClient() *fcm.Client {
+	if config.Mailroom.FCMKey == "" {
+		return nil
+	}
+	client, err := fcm.NewClient(config.Mailroom.FCMKey)
+	if err != nil {
+		panic(errors.Wrap(err, "unable to create FCM client"))
+	}
+	return client
 }

core/msgio/android_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/nyaruka/gocommon/jsonx"
 	"github.com/nyaruka/goflow/utils"
+	"github.com/nyaruka/mailroom/config"
 	"github.com/nyaruka/mailroom/core/models"
 	"github.com/nyaruka/mailroom/core/msgio"
 	"github.com/nyaruka/mailroom/testsuite"
@@ -69,7 +70,7 @@ func TestSyncAndroidChannels(t *testing.T) {
 	mockFCM := newMockFCMEndpoint("FCMID3")
 	defer mockFCM.Stop()
 
-	msgio.SetFCMClient(mockFCM.Client("FCMKEY123"))
+	fc := mockFCM.Client("FCMKEY123")
 
 	// create some Android channels
 	channel1ID := testdata.InsertChannel(t, db, models.Org1, "A", "Android 1", []string{"tel"}, "SR", map[string]interface{}{"FCM_ID": ""})       // no FCM ID
@@ -83,7 +84,7 @@ func TestSyncAndroidChannels(t *testing.T) {
 	channel2 := oa.ChannelByID(channel2ID)
 	channel3 := oa.ChannelByID(channel3ID)
 
-	msgio.SyncAndroidChannels([]*models.Channel{channel1, channel2, channel3})
+	msgio.SyncAndroidChannels(fc, []*models.Channel{channel1, channel2, channel3})
 
 	// check that we try to sync the 2 channels with FCM IDs, even tho one fails
 	assert.Equal(t, 2, len(mockFCM.Messages))
@@ -94,3 +95,13 @@ func TestSyncAndroidChannels(t *testing.T) {
 	assert.Equal(t, "sync", mockFCM.Messages[0].CollapseKey)
 	assert.Equal(t, map[string]interface{}{"msg": "sync"}, mockFCM.Messages[0].Data)
 }
+
+func TestCreateFCMClient(t *testing.T) {
+	config.Mailroom.FCMKey = "1234"
+
+	assert.NotNil(t, msgio.CreateFCMClient())
+
+	config.Mailroom.FCMKey = ""
+
+	assert.Nil(t, msgio.CreateFCMClient())
+}

core/msgio/send.go

@@ -3,14 +3,15 @@ package msgio
 import (
 	"context"
 
+	"github.com/edganiukov/fcm"
 	"github.com/nyaruka/mailroom/core/models"
 
 	"github.com/apex/log"
 	"github.com/gomodule/redigo/redis"
 )
 
 // SendMessages tries to send the given messages via Courier or Android syncing
-func SendMessages(ctx context.Context, db models.Queryer, rp *redis.Pool, msgs []*models.Msg) {
+func SendMessages(ctx context.Context, db models.Queryer, rp *redis.Pool, fc *fcm.Client, msgs []*models.Msg) {
 	// messages to be sent by courier, organized by contact
 	courierMsgs := make(map[models.ContactID][]*models.Msg, 100)
 
@@ -61,7 +62,10 @@ func SendMessages(ctx context.Context, db models.Queryer, rp *redis.Pool, msgs [
 
 	// if we have any android messages, trigger syncs for the unique channels
 	if len(androidChannels) > 0 {
-		SyncAndroidChannels(androidChannels)
+		if fc == nil {
+			fc = CreateFCMClient()
+		}
+		SyncAndroidChannels(fc, androidChannels)
 	}
 
 	// any messages that didn't get sent should be moved back to pending (they are queued at creation to save an

core/msgio/send_test.go

@@ -62,7 +62,7 @@ func TestSendMessages(t *testing.T) {
 	mockFCM := newMockFCMEndpoint("FCMID3")
 	defer mockFCM.Stop()
 
-	msgio.SetFCMClient(mockFCM.Client("FCMKEY123"))
+	fc := mockFCM.Client("FCMKEY123")
 
 	// create some Andoid channels
 	androidChannel1ID := testdata.InsertChannel(t, db, models.Org1, "A", "Android 1", []string{"tel"}, "SR", map[string]interface{}{"FCM_ID": "FCMID1"})
@@ -151,7 +151,7 @@ func TestSendMessages(t *testing.T) {
 		rc.Do("FLUSHDB")
 		mockFCM.Messages = nil
 
-		msgio.SendMessages(ctx, db, rp, msgs)
+		msgio.SendMessages(ctx, db, rp, fc, msgs)
 
 		assertCourierQueueSizes(t, rc, tc.QueueSizes, "courier queue sizes mismatch in '%s'", tc.Description)
 

core/tasks/broadcasts/worker.go

@@ -171,6 +171,6 @@ func SendBroadcastBatch(ctx context.Context, db *sqlx.DB, rp *redis.Pool, bcast
 		return errors.Wrapf(err, "error creating broadcast messages")
 	}
 
-	msgio.SendMessages(ctx, db, rp, msgs)
+	msgio.SendMessages(ctx, db, rp, nil, msgs)
 	return nil
 }

mailroom.go

@@ -192,6 +192,11 @@ func (mr *Mailroom) Start() error {
 		log.Info("elastic ok")
 	}
 
+	// warn if we won't be doing FCM syncing
+	if config.Mailroom.FCMKey == "" {
+		logrus.Error("fcm not configured, no syncing of android channels")
+	}
+
 	for _, initFunc := range initFunctions {
 		initFunc(mr)
 	}

services/tickets/utils.go

@@ -110,7 +110,7 @@ func SendReply(ctx context.Context, db *sqlx.DB, rp *redis.Pool, store storage.S
 		return nil, errors.Wrapf(err, "error creating message batch")
 	}
 
-	msgio.SendMessages(ctx, db, rp, msgs)
+	msgio.SendMessages(ctx, db, rp, nil, msgs)
 	return msgs[0], nil
 }