Merge branch 'master' of https://github.com/cosminrentea/gobbler into…

… feature/pn-310-ttl-for-messages

client/mocks_client_gen_test.go

@@ -4,7 +4,6 @@
 package client
 
 import (
-
 	protocol "github.com/cosminrentea/gobbler/protocol"
 	gomock "github.com/golang/mock/gomock"
 )

server/apns/apns.go

@@ -3,10 +3,10 @@ package apns
 import (
 	"errors"
 	"fmt"
-	"github.com/sideshow/apns2"
 	"github.com/cosminrentea/gobbler/server/connector"
 	"github.com/cosminrentea/gobbler/server/metrics"
 	"github.com/cosminrentea/gobbler/server/router"
+	"github.com/sideshow/apns2"
 	"time"
 )
 

server/apns/apns_sender.go

@@ -2,9 +2,9 @@ package apns
 
 import (
 	"errors"
+	"github.com/cosminrentea/gobbler/server/connector"
 	"github.com/jpillora/backoff"
 	"github.com/sideshow/apns2"
-	"github.com/cosminrentea/gobbler/server/connector"
 	"net"
 	"time"
 )

server/apns/apns_sender_test.go

@@ -2,10 +2,10 @@ package apns
 
 import (
 	"errors"
-	"github.com/golang/mock/gomock"
 	"github.com/cosminrentea/gobbler/protocol"
 	"github.com/cosminrentea/gobbler/server/router"
 	"github.com/cosminrentea/gobbler/testutil"
+	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 	"testing"
 )

server/apns/apns_test.go

@@ -2,11 +2,11 @@ package apns
 
 import (
 	"errors"
-	"github.com/golang/mock/gomock"
-	"github.com/sideshow/apns2"
 	"github.com/cosminrentea/gobbler/protocol"
 	"github.com/cosminrentea/gobbler/server/connector"
 	"github.com/cosminrentea/gobbler/testutil"
+	"github.com/golang/mock/gomock"
+	"github.com/sideshow/apns2"
 	"github.com/stretchr/testify/assert"
 	"testing"
 )

server/auth/mocks_auth_gen_test.go

@@ -5,7 +5,7 @@ package auth
 
 import (
 	protocol "github.com/cosminrentea/gobbler/protocol"
-	
+
 	gomock "github.com/golang/mock/gomock"
 )
 

server/benchmarking_apns_test.go

@@ -4,14 +4,14 @@ import (
 	"bytes"
 	"fmt"
 	log "github.com/Sirupsen/logrus"
-	"github.com/golang/mock/gomock"
-	"github.com/sideshow/apns2"
 	"github.com/cosminrentea/gobbler/client"
 	"github.com/cosminrentea/gobbler/server/apns"
 	"github.com/cosminrentea/gobbler/server/connector"
 	"github.com/cosminrentea/gobbler/server/router"
 	"github.com/cosminrentea/gobbler/server/websocket"
 	"github.com/cosminrentea/gobbler/testutil"
+	"github.com/golang/mock/gomock"
+	"github.com/sideshow/apns2"
 	"github.com/stretchr/testify/assert"
 	"io/ioutil"
 	"net/http"

server/connector/connector_test.go

@@ -8,10 +8,10 @@ import (
 	"testing"
 	"time"
 
-	"github.com/golang/mock/gomock"
 	"github.com/cosminrentea/gobbler/protocol"
 	"github.com/cosminrentea/gobbler/server/router"
 	"github.com/cosminrentea/gobbler/testutil"
+	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 )
 

server/connector/mocks_connector_gen_test.go

@@ -6,7 +6,7 @@ package connector
 import (
 	context "context"
 	protocol "github.com/cosminrentea/gobbler/protocol"
-	
+
 	router "github.com/cosminrentea/gobbler/server/router"
 	gomock "github.com/golang/mock/gomock"
 	http "net/http"

server/fcm/fcm_test.go

@@ -9,12 +9,12 @@ import (
 	"time"
 
 	"github.com/Bogh/gcm"
-	"github.com/golang/mock/gomock"
 	"github.com/cosminrentea/gobbler/protocol"
 	"github.com/cosminrentea/gobbler/server/connector"
 	"github.com/cosminrentea/gobbler/server/kvstore"
 	"github.com/cosminrentea/gobbler/server/router"
 	"github.com/cosminrentea/gobbler/testutil"
+	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 )
 

server/router/mocks_router_gen_test.go

@@ -8,7 +8,7 @@ import (
 	auth "github.com/cosminrentea/gobbler/server/auth"
 	cluster "github.com/cosminrentea/gobbler/server/cluster"
 	kvstore "github.com/cosminrentea/gobbler/server/kvstore"
-	
+
 	store "github.com/cosminrentea/gobbler/server/store"
 	gomock "github.com/golang/mock/gomock"
 )

server/sms/nexmo_sms_sender.go

@@ -11,12 +11,7 @@ import (
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/cosminrentea/gobbler/protocol"
-)
-
-var (
-	URL                = "https://rest.nexmo.com/sms/json?"
-	MaxIdleConnections = 100
-	RequestTimeout     = 500 * time.Millisecond
+	"github.com/jpillora/backoff"
 )
 
 type ResponseCode int
@@ -44,10 +39,17 @@ const (
 )
 
 var (
-	ErrNoSMSSent                 = errors.New("No sms was sent to Nexmo")
-	ErrIncompleteSMSSent         = errors.New("Nexmo sms was only partial delivered.One or more part returned an error")
-	ErrSMSResponseDecodingFailed = errors.New("Nexmo response decoding failed.")
-	ErrNoRetry                   = errors.New("SMS failed. No retrying.")
+	URL                = "https://rest.nexmo.com/sms/json?"
+	MaxIdleConnections = 100
+	RequestTimeout     = 500 * time.Millisecond
+
+	ErrHTTPClientError           = errors.New("Http client sending to Nexmo Failed.No sms was sent")
+	ErrNexmoResponseStatusNotOk  = errors.New("Nexmo response status not ResponseSuccess")
+	ErrSMSResponseDecodingFailed = errors.New("Nexmo response decoding failed")
+	ErrInvalidSender             = errors.New("Sms destination phoneNumber is invalid")
+	ErrMultipleSmsSent           = errors.New("Multiple  or no sms we're sent.SMS message may be too long")
+	ErrRetryFailed               = errors.New("Failed retrying to send message")
+	ErrEncodeFailed              = errors.New("Encoding of message to be sent to Nexmo  failed")
 )
 
 var nexmoResponseCodeMap = map[ResponseCode]string{
@@ -93,21 +95,19 @@ type NexmoMessageResponse struct {
 }
 
 func (nm NexmoMessageResponse) Check() error {
-	if nm.MessageCount == 0 {
-		return ErrNoSMSSent
+	if nm.MessageCount != 1 {
+		logger.WithField("message_count", nm.MessageCount).Error("Nexmo message count error.")
+		return ErrMultipleSmsSent
 	}
-	for i := 0; i < nm.MessageCount; i++ {
-		if nm.Messages[i].Status != ResponseSuccess {
-			logger.WithField("status", nm.Messages[i].Status).
-				WithField("error", nm.Messages[i].ErrorText).
-				Error("Error received from Nexmo")
-
-			if nm.Messages[i].Status == ResponseInvalidSenderAddress {
-				return nil
-			}
+	if nm.Messages[0].Status != ResponseSuccess {
+		logger.WithField("status", nm.Messages[0].Status).WithField("error", nm.Messages[0].ErrorText).
+			Error("Error received from Nexmo")
 
-			return ErrIncompleteSMSSent
+		if nm.Messages[0].Status == ResponseInvalidSenderAddress {
+			logger.Info("Invalid Sender detected.No retries will be made.")
+			return ErrInvalidSender
 		}
+		return ErrNexmoResponseStatusNotOk
 	}
 	return nil
 }
@@ -134,27 +134,71 @@ func (ns *NexmoSender) Send(msg *protocol.Message) error {
 	nexmoSMS := new(NexmoSms)
 	err := json.Unmarshal(msg.Body, nexmoSMS)
 	if err != nil {
-		logger.WithField("error", err.Error()).Error("Could not decode message body to send to nexmo")
-		return err
+		logger.WithField("msg", msg).WithField("error", err.Error()).Error("Could not decode message body to send to nexmo.No retries will be made for this message.")
+		return ErrRetryFailed
 	}
-	nexmoSMSResponse, err := ns.sendSms(nexmoSMS)
-	if err != nil {
-		logger.WithField("error", err.Error()).Error("Could not decode nexmo response message body")
-		return err
+
+	sendSms := func() (*NexmoMessageResponse, error) {
+		return ns.sendSms(nexmoSMS)
+	}
+	withRetry := &retryable{
+		maxTries: 3,
+		Backoff: backoff.Backoff{
+			Min:    50 * time.Millisecond,
+			Max:    250 * time.Millisecond,
+			Factor: 2,
+			Jitter: true,
+		},
 	}
-	logger.WithField("response", nexmoSMSResponse).Info("Decoded nexmo response")
 
-	return nexmoSMSResponse.Check()
+	err = withRetry.executeAndCheck(sendSms)
+	if err != nil && err == ErrRetryFailed {
+		logger.WithField("msg", msg).Info("Retry failed or not necessary.Moving on")
+	}
+
+	return err
+}
+
+type retryable struct {
+	maxTries int
+	backoff.Backoff
+}
+
+func (r *retryable) executeAndCheck(op func() (*NexmoMessageResponse, error)) error {
+	tryCounter := 0
+
+	for {
+		tryCounter++
+		nexmoSMSResponse, err := op()
+		if err == nil {
+			logger.WithField("response", nexmoSMSResponse).WithField("try", tryCounter).Info("Decoded nexmo response")
+			err = nexmoSMSResponse.Check()
+			if err == nil {
+				return nil
+			}
+
+			if err == ErrInvalidSender {
+				return ErrRetryFailed
+			}
+
+		}
+
+		if tryCounter >= r.maxTries {
+			return ErrRetryFailed
+		}
+		d := r.Duration()
+		logger.WithField("error", err.Error()).WithField("duration", d).Info("Retry in")
+		time.Sleep(d)
+	}
 }
 
 func (ns *NexmoSender) sendSms(sms *NexmoSms) (*NexmoMessageResponse, error) {
-	// log before encoding
 	logger.WithField("sms_details", sms).Info("sendSms")
 
 	smsEncoded, err := sms.EncodeNexmoSms(ns.ApiKey, ns.ApiSecret)
 	if err != nil {
 		logger.WithField("error", err.Error()).Error("Error encoding sms")
-		return nil, err
+		return nil, ErrEncodeFailed
 	}
 
 	req, err := http.NewRequest(http.MethodPost, URL, bytes.NewBuffer(smsEncoded))
@@ -166,7 +210,7 @@ func (ns *NexmoSender) sendSms(sms *NexmoSms) (*NexmoMessageResponse, error) {
 		logger.WithField("error", err.Error()).Error("Error doing the request to nexmo endpoint")
 		ns.createHttpClient()
 		mTotalSendErrors.Add(1)
-		return nil, ErrNoSMSSent
+		return nil, ErrHTTPClientError
 	}
 	defer resp.Body.Close()
 

server/sms/nexmo_sms_sender_test.go

@@ -1,35 +1,25 @@
 package sms
 
 import (
-	"encoding/json"
 	"testing"
 	"time"
 
-	"github.com/cosminrentea/gobbler/protocol"
-	"github.com/cosminrentea/gobbler/testutil"
 	"github.com/stretchr/testify/assert"
 )
 
-const (
-	KEY    = "ce40b46d"
-	SECRET = "153d2b2c72985370"
-)
-
-func TestNexmoSender_Send(t *testing.T) {
+func Test_HttpClientRecreation(t *testing.T) {
 	a := assert.New(t)
-	testutil.SkipIfDisabled(t)
-	sender, err := NewNexmoSender(KEY, SECRET)
-	a.NoError(err)
 
-	sms := new(NexmoSms)
-	sms.To = "+40746278186"
-	sms.From = "REWE Lieferservice"
-	sms.Text = "Lieber Kunde! Ihre Lieferung kommt heute zwischen 12.04 und 12.34 Uhr. Vielen Dank für Ihre Bestellung! Ihr REWE Lieferservice"
+	port := createRandomPort(7000, 8000)
+	URL = "http://127.0.0.1" + port
 
-	response, err := sender.sendSms(sms)
-	a.Equal(1, response.MessageCount)
-	a.Equal(ResponseSuccess, response.Messages[0].Status)
-	a.NoError(err)
+	sender := createNexmoSender(t)
+	msg := encodeProtocolMessage(t, 2)
+
+	err := sender.Send(&msg)
+	time.Sleep(3 * timeInterval)
+
+	a.Equal(ErrRetryFailed, err)
 }
 
 func TestNexmoSender_SendWithError(t *testing.T) {
@@ -38,23 +28,10 @@ func TestNexmoSender_SendWithError(t *testing.T) {
 	sender, err := NewNexmoSender(KEY, SECRET)
 	a.NoError(err)
 
-	sms := NexmoSms{
-		To:   "toNumber",
-		From: "FromNUmber",
-		Text: "body",
-	}
-	d, err := json.Marshal(&sms)
-	a.NoError(err)
-
-	msg := protocol.Message{
-		Path:          protocol.Path(SMSDefaultTopic),
-		UserID:        "samsa",
-		ApplicationID: "sms",
-		ID:            uint64(4),
-		Body:          d,
-	}
+	msg := encodeProtocolMessage(t, 0)
 
 	err = sender.Send(&msg)
+	time.Sleep(3 * timeInterval)
 	a.Error(err)
-	a.Equal(ErrIncompleteSMSSent, err)
+	a.Equal(ErrRetryFailed, err)
 }