Skip to content
Snippets Groups Projects
Commit 3b7a56cb authored by Jonah Husson's avatar Jonah Husson
Browse files

Code cleanup part 1

parent cb0bfcae
No related branches found
No related tags found
2 merge requests!12Release,!7Jonah/alt apns
package apns
import "github.com/sideshow/apns2"
type ApnsComm struct {
*apns2.Client
topic string
}
func NewApnsComm(cl *apns2.Client, topic string) *ApnsComm {
return &ApnsComm{
Client: cl,
topic: topic,
}
}
func (c *ApnsComm) GetTopic() string {
return c.topic
}
...@@ -21,13 +21,13 @@ import ( ...@@ -21,13 +21,13 @@ import (
) )
// function types for use in notificationsbot struct // function types for use in notificationsbot struct
type SetupFunc func(string) (*messaging.Client, context.Context, error)
type SendFunc func(FBSender, string, *mixmessages.NotificationData) (string, error) type SendFunc func(FBSender, string, *mixmessages.NotificationData) (string, error)
// FirebaseComm is a struct which holds the functions to setup the messaging app and sending notifications // FirebaseComm is a struct which holds the functions to setup the messaging app and sending notifications
// Using a struct in this manner allows us to properly unit test the NotifyUser function // Using a struct in this manner allows us to properly unit test the NotifyUser function
type FirebaseComm struct { type FirebaseComm struct {
SendNotification SendFunc SendNotification SendFunc
*messaging.Client
} }
// FBSender is an interface which matches the send function in the messaging app, allowing us to unit test sendNotification // FBSender is an interface which matches the send function in the messaging app, allowing us to unit test sendNotification
...@@ -36,9 +36,10 @@ type FBSender interface { ...@@ -36,9 +36,10 @@ type FBSender interface {
} }
// NewFirebaseComm create a *FirebaseComm object with the proper setup and send functions // NewFirebaseComm create a *FirebaseComm object with the proper setup and send functions
func NewFirebaseComm() *FirebaseComm { func NewFirebaseComm(cl *messaging.Client) *FirebaseComm {
return &FirebaseComm{ return &FirebaseComm{
SendNotification: sendNotification, SendNotification: sendNotification,
Client: cl,
} }
} }
......
...@@ -37,7 +37,7 @@ func TestSendNotification(t *testing.T) { ...@@ -37,7 +37,7 @@ func TestSendNotification(t *testing.T) {
// Unit test the NewFirebaseComm method // Unit test the NewFirebaseComm method
func TestNewFirebaseComm(t *testing.T) { func TestNewFirebaseComm(t *testing.T) {
comm := NewFirebaseComm() comm := NewFirebaseComm(nil)
if comm.SendNotification == nil { if comm.SendNotification == nil {
t.Error("Failed to set functions in comm") t.Error("Failed to set functions in comm")
} }
......
...@@ -10,7 +10,8 @@ package notifications ...@@ -10,7 +10,8 @@ package notifications
import ( import (
"encoding/base64" "encoding/base64"
"firebase.google.com/go/messaging" "gitlab.com/elixxir/notifications-bot/notifications/apns"
// "github.com/jonahh-yeti/apns" // "github.com/jonahh-yeti/apns"
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/sideshow/apns2" "github.com/sideshow/apns2"
...@@ -22,7 +23,7 @@ import ( ...@@ -22,7 +23,7 @@ import (
"gitlab.com/elixxir/comms/notificationBot" "gitlab.com/elixxir/comms/notificationBot"
"gitlab.com/elixxir/crypto/hash" "gitlab.com/elixxir/crypto/hash"
"gitlab.com/elixxir/crypto/registration" "gitlab.com/elixxir/crypto/registration"
"gitlab.com/elixxir/notifications-bot/firebase" "gitlab.com/elixxir/notifications-bot/notifications/firebase"
"gitlab.com/elixxir/notifications-bot/storage" "gitlab.com/elixxir/notifications-bot/storage"
"gitlab.com/xx_network/comms/connect" "gitlab.com/xx_network/comms/connect"
"gitlab.com/xx_network/crypto/signature/rsa" "gitlab.com/xx_network/crypto/signature/rsa"
...@@ -38,7 +39,7 @@ import ( ...@@ -38,7 +39,7 @@ import (
) )
// Function type definitions for the main operations (poll and notify) // Function type definitions for the main operations (poll and notify)
type NotifyFunc func(*pb.NotificationData, ApnsSender, *messaging.Client, *firebase.FirebaseComm, *storage.Storage, string) error type NotifyFunc func(*pb.NotificationData, *apns.ApnsComm, *firebase.FirebaseComm, *storage.Storage) error
type ApnsSender interface { type ApnsSender interface {
//Send(token string, p apns.Payload, opts ...apns.SendOption) (*apns.Response, error) //Send(token string, p apns.Payload, opts ...apns.SendOption) (*apns.Response, error)
Push(n *apns2.Notification) (*apns2.Response, error) Push(n *apns2.Notification) (*apns2.Response, error)
...@@ -66,10 +67,9 @@ type Impl struct { ...@@ -66,10 +67,9 @@ type Impl struct {
Storage *storage.Storage Storage *storage.Storage
inst *network.Instance inst *network.Instance
notifyFunc NotifyFunc notifyFunc NotifyFunc
fcm *messaging.Client fcm *firebase.FirebaseComm
apnsClient *apns2.Client apnsClient *apns.ApnsComm
receivedNdf *uint32 receivedNdf *uint32
params *Params
ndfStopper Stopper ndfStopper Stopper
} }
...@@ -98,20 +98,20 @@ func StartNotifications(params Params, noTLS, noFirebase bool) (*Impl, error) { ...@@ -98,20 +98,20 @@ func StartNotifications(params Params, noTLS, noFirebase bool) (*Impl, error) {
} }
// Set up firebase messaging client // Set up firebase messaging client
var app *messaging.Client var fbComm *firebase.FirebaseComm
if !noFirebase { if !noFirebase {
app, err = firebase.SetupMessagingApp(params.FBCreds) app, err := firebase.SetupMessagingApp(params.FBCreds)
if err != nil { if err != nil {
return nil, errors.Wrap(err, "Failed to setup firebase messaging app") return nil, errors.Wrap(err, "Failed to setup firebase messaging app")
} }
fbComm = firebase.NewFirebaseComm(app)
} }
receivedNdf := uint32(0) receivedNdf := uint32(0)
impl := &Impl{ impl := &Impl{
notifyFunc: notifyUser, notifyFunc: notifyUser,
fcm: app, fcm: fbComm,
receivedNdf: &receivedNdf, receivedNdf: &receivedNdf,
params: &params,
} }
if params.APNS.KeyPath == "" { if params.APNS.KeyPath == "" {
...@@ -140,7 +140,7 @@ func StartNotifications(params Params, noTLS, noFirebase bool) (*Impl, error) { ...@@ -140,7 +140,7 @@ func StartNotifications(params Params, noTLS, noFirebase bool) (*Impl, error) {
apnsClient.Production() apnsClient.Production()
} }
impl.apnsClient = apnsClient impl.apnsClient = apns.NewApnsComm(apnsClient, params.APNS.BundleID)
} }
// Start notification comms server // Start notification comms server
...@@ -178,7 +178,7 @@ func NewImplementation(instance *Impl) *notificationBot.Implementation { ...@@ -178,7 +178,7 @@ func NewImplementation(instance *Impl) *notificationBot.Implementation {
// NotifyUser accepts a UID and service key file path. // NotifyUser accepts a UID and service key file path.
// It handles the logic involved in retrieving a user's token and sending the notification // It handles the logic involved in retrieving a user's token and sending the notification
func notifyUser(data *pb.NotificationData, apnsClient ApnsSender, fcm *messaging.Client, fc *firebase.FirebaseComm, db *storage.Storage, topic string) error { func notifyUser(data *pb.NotificationData, apnsClient *apns.ApnsComm, fc *firebase.FirebaseComm, db *storage.Storage) error {
elist, err := db.GetEphemeral(data.EphemeralID) elist, err := db.GetEphemeral(data.EphemeralID)
if err != nil { if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) { if errors.Is(err, gorm.ErrRecordNotFound) {
...@@ -209,7 +209,7 @@ func notifyUser(data *pb.NotificationData, apnsClient ApnsSender, fcm *messaging ...@@ -209,7 +209,7 @@ func notifyUser(data *pb.NotificationData, apnsClient ApnsSender, fcm *messaging
Priority: apns2.PriorityHigh, Priority: apns2.PriorityHigh,
Payload: notifPayload, Payload: notifPayload,
PushType: apns2.PushTypeAlert, PushType: apns2.PushTypeAlert,
Topic: topic, Topic: apnsClient.GetTopic(),
} }
resp, err := apnsClient.Push(notif) resp, err := apnsClient.Push(notif)
//resp, err := apnsClient.Send(u.Token, apns.Payload{ //resp, err := apnsClient.Send(u.Token, apns.Payload{
...@@ -239,7 +239,7 @@ func notifyUser(data *pb.NotificationData, apnsClient ApnsSender, fcm *messaging ...@@ -239,7 +239,7 @@ func notifyUser(data *pb.NotificationData, apnsClient ApnsSender, fcm *messaging
jww.INFO.Printf("Notified ephemeral ID %+v [%+v] and received response %+v", data.EphemeralID, u.Token, resp) jww.INFO.Printf("Notified ephemeral ID %+v [%+v] and received response %+v", data.EphemeralID, u.Token, resp)
} }
} else { } else {
resp, err := fc.SendNotification(fcm, u.Token, data) resp, err := fc.SendNotification(fc.Client, u.Token, data)
if err != nil { if err != nil {
// Catch two firebase errors that we don't want to crash on // Catch two firebase errors that we don't want to crash on
// 400 and 404 indicate that the token stored is incorrect // 400 and 404 indicate that the token stored is incorrect
...@@ -356,9 +356,8 @@ func (nb *Impl) ReceiveNotificationBatch(notifBatch *pb.NotificationBatch, auth ...@@ -356,9 +356,8 @@ func (nb *Impl) ReceiveNotificationBatch(notifBatch *pb.NotificationBatch, auth
jww.INFO.Printf("Received notification batch for round %+v", notifBatch.RoundID) jww.INFO.Printf("Received notification batch for round %+v", notifBatch.RoundID)
fbComm := firebase.NewFirebaseComm()
for _, notifData := range notifBatch.GetNotifications() { for _, notifData := range notifBatch.GetNotifications() {
err := nb.notifyFunc(notifData, nb.apnsClient, nb.fcm, fbComm, nb.Storage, nb.params.APNS.BundleID) err := nb.notifyFunc(notifData, nb.apnsClient, nb.fcm, nb.Storage)
if err != nil { if err != nil {
return err return err
} }
......
...@@ -6,15 +6,14 @@ ...@@ -6,15 +6,14 @@
package notifications package notifications
import ( import (
"firebase.google.com/go/messaging"
"fmt" "fmt"
"github.com/jonahh-yeti/apns"
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/sideshow/apns2" "github.com/sideshow/apns2"
pb "gitlab.com/elixxir/comms/mixmessages" pb "gitlab.com/elixxir/comms/mixmessages"
"gitlab.com/elixxir/crypto/hash" "gitlab.com/elixxir/crypto/hash"
"gitlab.com/elixxir/crypto/registration" "gitlab.com/elixxir/crypto/registration"
"gitlab.com/elixxir/notifications-bot/firebase" "gitlab.com/elixxir/notifications-bot/notifications/apns"
"gitlab.com/elixxir/notifications-bot/notifications/firebase"
"gitlab.com/elixxir/notifications-bot/storage" "gitlab.com/elixxir/notifications-bot/storage"
"gitlab.com/xx_network/comms/connect" "gitlab.com/xx_network/comms/connect"
"gitlab.com/xx_network/crypto/csprng" "gitlab.com/xx_network/crypto/csprng"
...@@ -30,16 +29,6 @@ import ( ...@@ -30,16 +29,6 @@ import (
var port = 4200 var port = 4200
type MockApns struct{}
func (m *MockApns) Send(token string, p apns.Payload, opts ...apns.SendOption) (*apns.Response, error) {
return nil, nil
}
func (m *MockApns) Push(n *apns2.Notification) (*apns2.Response, error) {
return nil, nil
}
// Test notificationbot's notifyuser function // Test notificationbot's notifyuser function
// this mocks the setup and send functions, and only tests the core logic of this function // this mocks the setup and send functions, and only tests the core logic of this function
func TestNotifyUser(t *testing.T) { func TestNotifyUser(t *testing.T) {
...@@ -73,11 +62,13 @@ func TestNotifyUser(t *testing.T) { ...@@ -73,11 +62,13 @@ func TestNotifyUser(t *testing.T) {
if err != nil { if err != nil {
t.Errorf("Failed to add latest ephemeral: %+v", err) t.Errorf("Failed to add latest ephemeral: %+v", err)
} }
ac := apns.NewApnsComm(apns2.NewTokenClient(nil), "")
err = notifyUser(&pb.NotificationData{ err = notifyUser(&pb.NotificationData{
EphemeralID: eph.EphemeralId, EphemeralID: eph.EphemeralId,
IdentityFP: nil, IdentityFP: nil,
MessageHash: nil, MessageHash: nil,
}, &MockApns{}, nil, fcBadSend, s, "") }, ac, fcBadSend, s)
if err == nil { if err == nil {
t.Errorf("Should have returned an error") t.Errorf("Should have returned an error")
} }
...@@ -86,7 +77,7 @@ func TestNotifyUser(t *testing.T) { ...@@ -86,7 +77,7 @@ func TestNotifyUser(t *testing.T) {
EphemeralID: eph.EphemeralId, EphemeralID: eph.EphemeralId,
IdentityFP: nil, IdentityFP: nil,
MessageHash: nil, MessageHash: nil,
}, &MockApns{}, nil, fc, s, "") }, ac, fc, s)
if err != nil { if err != nil {
t.Errorf("Failed to notify user properly") t.Errorf("Failed to notify user properly")
} }
...@@ -297,7 +288,7 @@ func TestImpl_UnregisterForNotifications(t *testing.T) { ...@@ -297,7 +288,7 @@ func TestImpl_UnregisterForNotifications(t *testing.T) {
func TestImpl_ReceiveNotificationBatch(t *testing.T) { func TestImpl_ReceiveNotificationBatch(t *testing.T) {
impl := getNewImpl() impl := getNewImpl()
dataChan := make(chan *pb.NotificationData) dataChan := make(chan *pb.NotificationData)
impl.notifyFunc = func(data *pb.NotificationData, apns ApnsSender, f *messaging.Client, fc *firebase.FirebaseComm, s *storage.Storage, topic string) error { impl.notifyFunc = func(data *pb.NotificationData, apns *apns.ApnsComm, fc *firebase.FirebaseComm, s *storage.Storage) error {
go func() { dataChan <- data }() go func() { dataChan <- data }()
return nil return nil
} }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment