From a3aad6c5b873b5fe5f41b6c6ddf2cc3d474d4c51 Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Fri, 16 Feb 2024 16:18:19 +0200 Subject: [PATCH 01/12] Use MockResponses and ExpectedRequests in RC tests --- handlers/rocketchat/handler.go | 16 +++--- handlers/rocketchat/handler_test.go | 88 ++++++++++++++++------------- 2 files changed, 56 insertions(+), 48 deletions(-) diff --git a/handlers/rocketchat/handler.go b/handlers/rocketchat/handler.go index 0f25b9769..b0aa6da41 100644 --- a/handlers/rocketchat/handler.go +++ b/handlers/rocketchat/handler.go @@ -41,7 +41,7 @@ func (h *handler) Initialize(s courier.Server) error { return nil } -type Attachment struct { +type RCAttachment struct { Type string `json:"type"` URL string `json:"url"` } @@ -52,8 +52,8 @@ type moPayload struct { Username string `json:"username"` FullName string `json:"full_name"` } `json:"user" validate:"required"` - Text string `json:"text"` - Attachments []Attachment `json:"attachments"` + Text string `json:"text"` + Attachments []RCAttachment `json:"attachments"` } // receiveMessage is our HTTP handler function for incoming messages @@ -99,10 +99,10 @@ func (h *handler) BuildAttachmentRequest(ctx context.Context, b courier.Backend, var _ courier.AttachmentRequestBuilder = (*handler)(nil) type mtPayload struct { - UserURN string `json:"user"` - BotUsername string `json:"bot"` - Text string `json:"text,omitempty"` - Attachments []Attachment `json:"attachments,omitempty"` + UserURN string `json:"user"` + BotUsername string `json:"bot"` + Text string `json:"text,omitempty"` + Attachments []RCAttachment `json:"attachments,omitempty"` } func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.SendResult, clog *courier.ChannelLog) error { @@ -125,7 +125,7 @@ func (h *handler) SendLegacy(ctx context.Context, msg courier.MsgOut, clog *cour } for _, attachment := range msg.Attachments() { mimeType, url := handlers.SplitAttachment(attachment) - payload.Attachments = append(payload.Attachments, Attachment{mimeType, url}) + payload.Attachments = append(payload.Attachments, RCAttachment{mimeType, url}) } body := jsonx.MustMarshal(payload) diff --git a/handlers/rocketchat/handler_test.go b/handlers/rocketchat/handler_test.go index 9f2ae5fa6..90fea98a5 100644 --- a/handlers/rocketchat/handler_test.go +++ b/handlers/rocketchat/handler_test.go @@ -1,12 +1,12 @@ package rocketchat import ( - "net/http/httptest" "testing" "github.com/nyaruka/courier" - "github.com/nyaruka/courier/handlers" + . "github.com/nyaruka/courier/handlers" "github.com/nyaruka/courier/test" + "github.com/nyaruka/gocommon/httpx" ) const ( @@ -49,7 +49,7 @@ const attachmentMsg = `{ "attachments": [{"type": "image/jpg", "url": "https://link.to/image.jpg"}] }` -var testCases = []handlers.IncomingTestCase{ +var testCases = []IncomingTestCase{ { Label: "Receive Hello Msg", URL: receiveURL, @@ -58,7 +58,7 @@ var testCases = []handlers.IncomingTestCase{ }, Data: helloMsg, ExpectedURN: "rocketchat:direct:john.doe#john.doe", - ExpectedMsgText: handlers.Sp("Hello World"), + ExpectedMsgText: Sp("Hello World"), ExpectedRespStatus: 200, ExpectedBodyContains: "Accepted", }, @@ -97,54 +97,62 @@ var testCases = []handlers.IncomingTestCase{ } func TestIncoming(t *testing.T) { - handlers.RunIncomingTestCases(t, testChannels, newHandler(), testCases) + RunIncomingTestCases(t, testChannels, newHandler(), testCases) } func BenchmarkHandler(b *testing.B) { - handlers.RunChannelBenchmarks(b, testChannels, newHandler(), testCases) + RunChannelBenchmarks(b, testChannels, newHandler(), testCases) } -func setSendURL(s *httptest.Server, h courier.ChannelHandler, c courier.Channel, m courier.MsgOut) { - c.(*test.MockChannel).SetConfig(configBaseURL, s.URL) -} - -var sendTestCases = []handlers.OutgoingTestCase{ +var sendTestCases = []OutgoingTestCase{ { - Label: "Plain Send", - MsgText: "Simple Message", - MsgURN: "rocketchat:direct:john.doe#john.doe", - ExpectedMsgStatus: "S", - ExpectedRequestBody: `{"user":"direct:john.doe","bot":"rocket.cat","text":"Simple Message"}`, - MockResponseStatus: 201, - MockResponseBody: `{"id":"iNKE8a6k6cjbqWhWd"}`, - ExpectedExtIDs: []string{"iNKE8a6k6cjbqWhWd"}, - SendPrep: setSendURL, + Label: "Plain Send", + MsgText: "Simple Message", + MsgURN: "rocketchat:direct:john.doe#john.doe", + MockResponses: map[string][]*httpx.MockResponse{ + "https://my.rocket.chat/api/apps/public/684202ed-1461-4983-9ea7-fde74b15026c/message": { + httpx.NewMockResponse(201, nil, []byte(`{"id":"iNKE8a6k6cjbqWhWd"}`)), + }, + }, + ExpectedMsgStatus: "S", + ExpectedRequests: []ExpectedRequest{{ + Body: `{"user":"direct:john.doe","bot":"rocket.cat","text":"Simple Message"}`, + }}, + ExpectedExtIDs: []string{"iNKE8a6k6cjbqWhWd"}, }, { - Label: "Send Attachment", - MsgURN: "rocketchat:livechat:onrMgdKbpX9Qqtvoi", - MsgAttachments: []string{"application/pdf:https://link.to/attachment.pdf"}, - ExpectedMsgStatus: "S", - ExpectedRequestBody: `{"user":"livechat:onrMgdKbpX9Qqtvoi","bot":"rocket.cat","attachments":[{"type":"application/pdf","url":"https://link.to/attachment.pdf"}]}`, - MockResponseStatus: 201, - MockResponseBody: `{"id":"iNKE8a6k6cjbqWhWd"}`, - ExpectedExtIDs: []string{"iNKE8a6k6cjbqWhWd"}, - SendPrep: setSendURL, + Label: "Send Attachment", + MsgURN: "rocketchat:livechat:onrMgdKbpX9Qqtvoi", + MsgAttachments: []string{"application/pdf:https://link.to/attachment.pdf"}, + MockResponses: map[string][]*httpx.MockResponse{ + "https://my.rocket.chat/api/apps/public/684202ed-1461-4983-9ea7-fde74b15026c/message": { + httpx.NewMockResponse(201, nil, []byte(`{"id":"iNKE8a6k6cjbqWhWd"}`)), + }, + }, + ExpectedMsgStatus: "S", + ExpectedRequests: []ExpectedRequest{{ + Body: `{"user":"livechat:onrMgdKbpX9Qqtvoi","bot":"rocket.cat","attachments":[{"type":"application/pdf","url":"https://link.to/attachment.pdf"}]}`, + }}, + ExpectedExtIDs: []string{"iNKE8a6k6cjbqWhWd"}, }, { - Label: "Send Text And Attachment", - MsgURN: "rocketchat:direct:john.doe", - MsgText: "Simple Message", - MsgAttachments: []string{"application/pdf:https://link.to/attachment.pdf"}, - ExpectedMsgStatus: "S", - ExpectedRequestBody: `{"user":"direct:john.doe","bot":"rocket.cat","text":"Simple Message","attachments":[{"type":"application/pdf","url":"https://link.to/attachment.pdf"}]}`, - MockResponseStatus: 201, - MockResponseBody: `{"id":"iNKE8a6k6cjbqWhWd"}`, - ExpectedExtIDs: []string{"iNKE8a6k6cjbqWhWd"}, - SendPrep: setSendURL, + Label: "Send Text And Attachment", + MsgURN: "rocketchat:direct:john.doe", + MsgText: "Simple Message", + MsgAttachments: []string{"application/pdf:https://link.to/attachment.pdf"}, + MockResponses: map[string][]*httpx.MockResponse{ + "https://my.rocket.chat/api/apps/public/684202ed-1461-4983-9ea7-fde74b15026c/message": { + httpx.NewMockResponse(201, nil, []byte(`{"id":"iNKE8a6k6cjbqWhWd"}`)), + }, + }, + ExpectedMsgStatus: "S", + ExpectedRequests: []ExpectedRequest{{ + Body: `{"user":"direct:john.doe","bot":"rocket.cat","text":"Simple Message","attachments":[{"type":"application/pdf","url":"https://link.to/attachment.pdf"}]}`, + }}, + ExpectedExtIDs: []string{"iNKE8a6k6cjbqWhWd"}, }, } func TestOutgoing(t *testing.T) { - handlers.RunOutgoingTestCases(t, testChannels[0], newHandler(), sendTestCases, []string{"123456789"}, nil) + RunOutgoingTestCases(t, testChannels[0], newHandler(), sendTestCases, []string{"123456789"}, nil) } From ecc2bba37ba2214218212ca45ed348bffdede16c Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Fri, 16 Feb 2024 16:25:48 +0200 Subject: [PATCH 02/12] Use MockResponses and ExpectedRequests in RR tests --- handlers/redrabbit/handler_test.go | 149 +++++++++++++++++------------ 1 file changed, 87 insertions(+), 62 deletions(-) diff --git a/handlers/redrabbit/handler_test.go b/handlers/redrabbit/handler_test.go index 93593bca0..515f5a274 100644 --- a/handlers/redrabbit/handler_test.go +++ b/handlers/redrabbit/handler_test.go @@ -1,100 +1,125 @@ package redrabbit import ( - "net/http/httptest" + "net/url" "testing" - "github.com/nyaruka/courier" . "github.com/nyaruka/courier/handlers" "github.com/nyaruka/courier/test" + "github.com/nyaruka/gocommon/httpx" ) -// setSendURL takes care of setting the send_url to our test server host -func setSendURL(s *httptest.Server, h courier.ChannelHandler, c courier.Channel, m courier.MsgOut) { - sendURL = s.URL -} - var defaultSendTestCases = []OutgoingTestCase{ {Label: "Plain Send", MsgText: "Simple Message", MsgURN: "tel:+250788383383", ExpectedMsgStatus: "W", - MockResponseBody: "SENT", MockResponseStatus: 200, - ExpectedURLParams: map[string]string{ - "LoginName": "Username", - "Password": "Password", - "Tracking": "1", - "Mobtyp": "1", - "MessageRecipients": "250788383383", - "MessageBody": "Simple Message", - "SenderName": "2020", + MockResponses: map[string][]*httpx.MockResponse{ + "http://http1.javna.com/epicenter/GatewaySendG.asp*": { + httpx.NewMockResponse(200, nil, []byte(`SENT`)), + }, }, - SendPrep: setSendURL}, + ExpectedRequests: []ExpectedRequest{{ + Params: url.Values{ + "LoginName": {"Username"}, + "Password": {"Password"}, + "Tracking": {"1"}, + "Mobtyp": {"1"}, + "MessageRecipients": {"250788383383"}, + "MessageBody": {"Simple Message"}, + "SenderName": {"2020"}, + }}}, + }, {Label: "Unicode Send", MsgText: "☺", MsgURN: "tel:+250788383383", ExpectedMsgStatus: "W", - MockResponseBody: "SENT", MockResponseStatus: 200, - ExpectedURLParams: map[string]string{ - "LoginName": "Username", - "Password": "Password", - "Tracking": "1", - "Mobtyp": "1", - "MessageRecipients": "250788383383", - "MessageBody": "☺", - "SenderName": "2020", - "MsgTyp": "9", + MockResponses: map[string][]*httpx.MockResponse{ + "http://http1.javna.com/epicenter/GatewaySendG.asp*": { + httpx.NewMockResponse(200, nil, []byte(`SENT`)), + }, }, - SendPrep: setSendURL}, + ExpectedRequests: []ExpectedRequest{{ + Params: url.Values{ + "LoginName": {"Username"}, + "Password": {"Password"}, + "Tracking": {"1"}, + "Mobtyp": {"1"}, + "MessageRecipients": {"250788383383"}, + "MessageBody": {"☺"}, + "SenderName": {"2020"}, + "MsgTyp": {"9"}, + }, + }}, + }, {Label: "Longer Unicode Send", MsgText: "This is a message more than seventy characters with some unicode ☺ in them", MsgURN: "tel:+250788383383", ExpectedMsgStatus: "W", - MockResponseBody: "SENT", MockResponseStatus: 200, - ExpectedURLParams: map[string]string{ - "LoginName": "Username", - "Password": "Password", - "Tracking": "1", - "Mobtyp": "1", - "MessageRecipients": "250788383383", - "MessageBody": "This is a message more than seventy characters with some unicode ☺ in them", - "SenderName": "2020", - "MsgTyp": "10", + MockResponses: map[string][]*httpx.MockResponse{ + "http://http1.javna.com/epicenter/GatewaySendG.asp*": { + httpx.NewMockResponse(200, nil, []byte(`SENT`)), + }, }, - SendPrep: setSendURL}, + ExpectedRequests: []ExpectedRequest{{ + Params: url.Values{ + "LoginName": {"Username"}, + "Password": {"Password"}, + "Tracking": {"1"}, + "Mobtyp": {"1"}, + "MessageRecipients": {"250788383383"}, + "MessageBody": {"This is a message more than seventy characters with some unicode ☺ in them"}, + "SenderName": {"2020"}, + "MsgTyp": {"10"}, + }}}, + }, {Label: "Long Send", MsgText: "This is a longer message than 160 characters and will cause us to split it into two separate parts, isn't that right but it is even longer than before I say, I need to keep adding more things to make it work", MsgURN: "tel:+250788383383", ExpectedMsgStatus: "W", - MockResponseBody: "SENT", MockResponseStatus: 200, - ExpectedURLParams: map[string]string{ - "LoginName": "Username", - "Password": "Password", - "Tracking": "1", - "Mobtyp": "1", - "MessageRecipients": "250788383383", - "MessageBody": "This is a longer message than 160 characters and will cause us to split it into two separate parts, isn't that right but it is even longer than before I say, I need to keep adding more things to make it work", - "SenderName": "2020", - "MsgTyp": "5", + MockResponses: map[string][]*httpx.MockResponse{ + "http://http1.javna.com/epicenter/GatewaySendG.asp*": { + httpx.NewMockResponse(200, nil, []byte(`SENT`)), + }, }, - SendPrep: setSendURL}, + ExpectedRequests: []ExpectedRequest{{ + Params: url.Values{ + "LoginName": {"Username"}, + "Password": {"Password"}, + "Tracking": {"1"}, + "Mobtyp": {"1"}, + "MessageRecipients": {"250788383383"}, + "MessageBody": {"This is a longer message than 160 characters and will cause us to split it into two separate parts, isn't that right but it is even longer than before I say, I need to keep adding more things to make it work"}, + "SenderName": {"2020"}, + "MsgTyp": {"5"}, + }}}, + }, {Label: "Send Attachment", MsgText: "My pic!", MsgURN: "tel:+250788383383", MsgAttachments: []string{"image/jpeg:https://foo.bar/image.jpg"}, ExpectedMsgStatus: "W", - MockResponseBody: "SENT", MockResponseStatus: 200, - ExpectedURLParams: map[string]string{ - "LoginName": "Username", - "Password": "Password", - "Tracking": "1", - "Mobtyp": "1", - "MessageRecipients": "250788383383", - "MessageBody": "My pic!\nhttps://foo.bar/image.jpg", - "SenderName": "2020", + MockResponses: map[string][]*httpx.MockResponse{ + "http://http1.javna.com/epicenter/GatewaySendG.asp*": { + httpx.NewMockResponse(200, nil, []byte(`SENT`)), + }, }, - SendPrep: setSendURL}, + ExpectedRequests: []ExpectedRequest{{ + Params: url.Values{ + "LoginName": {"Username"}, + "Password": {"Password"}, + "Tracking": {"1"}, + "Mobtyp": {"1"}, + "MessageRecipients": {"250788383383"}, + "MessageBody": {"My pic!\nhttps://foo.bar/image.jpg"}, + "SenderName": {"2020"}, + }}}, + }, {Label: "Error Sending", MsgText: "Error Sending", MsgURN: "tel:+250788383383", ExpectedMsgStatus: "E", - MockResponseBody: "Error", MockResponseStatus: 403, - SendPrep: setSendURL}, + MockResponses: map[string][]*httpx.MockResponse{ + "http://http1.javna.com/epicenter/GatewaySendG.asp*": { + httpx.NewMockResponse(403, nil, []byte(`Error`)), + }, + }, + }, } func TestOutgoing(t *testing.T) { From 7fbf80d867ad36afc6061ddc3b051709b4603038 Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Fri, 16 Feb 2024 16:34:45 +0200 Subject: [PATCH 03/12] Use MockResponses and ExpectedRequests in SQ tests --- handlers/shaqodoon/handler_test.go | 52 ++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/handlers/shaqodoon/handler_test.go b/handlers/shaqodoon/handler_test.go index f2ed3f0bb..89866ae06 100644 --- a/handlers/shaqodoon/handler_test.go +++ b/handlers/shaqodoon/handler_test.go @@ -2,12 +2,14 @@ package shaqodoon import ( "net/http/httptest" + "net/url" "testing" "time" "github.com/nyaruka/courier" . "github.com/nyaruka/courier/handlers" "github.com/nyaruka/courier/test" + "github.com/nyaruka/gocommon/httpx" ) var ( @@ -59,33 +61,57 @@ var getSendTestCases = []OutgoingTestCase{ {Label: "Plain Send", MsgText: "Simple Message", MsgURN: "tel:+250788383383", ExpectedMsgStatus: "W", - MockResponseBody: "0: Accepted for delivery", MockResponseStatus: 200, - ExpectedURLParams: map[string]string{"msg": "Simple Message", "to": "250788383383", "from": "2020"}, - SendPrep: setSendURL}, + MockResponses: map[string][]*httpx.MockResponse{ + "http://example.com/send*": { + httpx.NewMockResponse(200, nil, []byte(`0: Accepted for delivery`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Params: url.Values{"msg": {"Simple Message"}, "to": {"250788383383"}, "from": {"2020"}, "username": {"Username"}, "password": {"Password"}}, + }}, + SendPrep: setSendURL}, {Label: "Unicode Send", MsgText: "☺", MsgURN: "tel:+250788383383", ExpectedMsgStatus: "W", - MockResponseBody: "0: Accepted for delivery", MockResponseStatus: 200, - ExpectedURLParams: map[string]string{"msg": "☺", "to": "250788383383", "from": "2020"}, - SendPrep: setSendURL}, + MockResponses: map[string][]*httpx.MockResponse{ + "http://example.com/send*": { + httpx.NewMockResponse(200, nil, []byte(`0: Accepted for delivery`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Params: url.Values{"msg": {"☺"}, "to": {"250788383383"}, "from": {"2020"}, "username": {"Username"}, "password": {"Password"}}, + }}, + SendPrep: setSendURL}, {Label: "Error Sending", MsgText: "Error Message", MsgURN: "tel:+250788383383", ExpectedMsgStatus: "E", - MockResponseBody: "1: Unknown channel", MockResponseStatus: 401, - ExpectedURLParams: map[string]string{"msg": `Error Message`, "to": "250788383383"}, - SendPrep: setSendURL}, + MockResponses: map[string][]*httpx.MockResponse{ + "http://example.com/send*": { + httpx.NewMockResponse(401, nil, []byte(`1: Unknown channel`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Params: url.Values{"msg": {"Error Message"}, "to": {"250788383383"}, "from": []string{"2020"}, "username": {"Username"}, "password": {"Password"}}, + }}, + SendPrep: setSendURL}, {Label: "Send Attachment", MsgText: "My pic!", MsgURN: "tel:+250788383383", MsgAttachments: []string{"image/jpeg:https://foo.bar/image.jpg"}, ExpectedMsgStatus: "W", - MockResponseBody: `0: Accepted for delivery`, MockResponseStatus: 200, - ExpectedURLParams: map[string]string{"msg": "My pic!\nhttps://foo.bar/image.jpg", "to": "250788383383", "from": "2020"}, - SendPrep: setSendURL}, + MockResponses: map[string][]*httpx.MockResponse{ + "http://example.com/send*": { + httpx.NewMockResponse(200, nil, []byte(`0: Accepted for delivery`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Params: url.Values{"msg": {"My pic!\nhttps://foo.bar/image.jpg"}, "to": {"250788383383"}, "from": {"2020"}, "username": {"Username"}, "password": {"Password"}}, + }}, + SendPrep: setSendURL}, } func TestOutgoing(t *testing.T) { var getChannel = test.NewMockChannel("8eb23e93-5ecb-45ba-b726-3b064e0c56ab", "SQ", "2020", "US", map[string]any{ - courier.ConfigSendURL: "SendURL", + courier.ConfigSendURL: "http://example.com/send", courier.ConfigPassword: "Password", courier.ConfigUsername: "Username"}) From 4f196f350f99e6d287b1e2f3beb3422c11281eb0 Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Fri, 16 Feb 2024 16:39:14 +0200 Subject: [PATCH 04/12] Use MockResponses and ExpectedRequests in SL tests --- handlers/slack/handler_test.go | 67 +++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/handlers/slack/handler_test.go b/handlers/slack/handler_test.go index f631ff189..0942ec335 100644 --- a/handlers/slack/handler_test.go +++ b/handlers/slack/handler_test.go @@ -123,10 +123,6 @@ const videoFileMsg = `{ "event_time": 1653427243 }` -func setSendURL(s *httptest.Server, h courier.ChannelHandler, c courier.Channel, m courier.MsgOut) { - apiURL = s.URL -} - var handleTestCases = []IncomingTestCase{ { Label: "Receive Hello Msg", @@ -178,35 +174,47 @@ var handleTestCases = []IncomingTestCase{ var defaultSendTestCases = []OutgoingTestCase{ { - Label: "Plain Send", - MsgText: "Simple Message", - MsgURN: "slack:U0123ABCDEF", - MockResponseBody: `{"ok":true,"channel":"U0123ABCDEF"}`, - MockResponseStatus: 200, - ExpectedRequestBody: `{"channel":"U0123ABCDEF","text":"Simple Message"}`, - ExpectedMsgStatus: "W", - SendPrep: setSendURL, + Label: "Plain Send", + MsgText: "Simple Message", + MsgURN: "slack:U0123ABCDEF", + MockResponses: map[string][]*httpx.MockResponse{ + "https://slack.com/api/chat.postMessage": { + httpx.NewMockResponse(200, nil, []byte(`{"ok":true,"channel":"U0123ABCDEF"}`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Body: `{"channel":"U0123ABCDEF","text":"Simple Message"}`, + }}, + ExpectedMsgStatus: "W", }, { - Label: "Unicode Send", - MsgText: "☺", - MsgURN: "slack:U0123ABCDEF", - MockResponseBody: `{"ok":true,"channel":"U0123ABCDEF"}`, - MockResponseStatus: 200, - ExpectedRequestBody: `{"channel":"U0123ABCDEF","text":"☺"}`, - ExpectedMsgStatus: "W", - SendPrep: setSendURL, + Label: "Unicode Send", + MsgText: "☺", + MsgURN: "slack:U0123ABCDEF", + MockResponses: map[string][]*httpx.MockResponse{ + "https://slack.com/api/chat.postMessage": { + httpx.NewMockResponse(200, nil, []byte(`{"ok":true,"channel":"U0123ABCDEF"}`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Body: `{"channel":"U0123ABCDEF","text":"☺"}`, + }}, + ExpectedMsgStatus: "W", }, { - Label: "Send Text Auth Error", - MsgText: "Hello", - MsgURN: "slack:U0123ABCDEF", - MockResponseBody: `{"ok":false,"error":"invalid_auth"}`, - MockResponseStatus: 200, - ExpectedRequestBody: `{"channel":"U0123ABCDEF","text":"Hello"}`, - ExpectedMsgStatus: "E", - ExpectedLogErrors: []*courier.ChannelError{courier.NewChannelError("", "", "invalid_auth")}, - SendPrep: setSendURL, + Label: "Send Text Auth Error", + MsgText: "Hello", + MsgURN: "slack:U0123ABCDEF", + MockResponses: map[string][]*httpx.MockResponse{ + "https://slack.com/api/chat.postMessage": { + httpx.NewMockResponse(200, nil, []byte(`{"ok":false,"error":"invalid_auth"}`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Body: `{"channel":"U0123ABCDEF","text":"Hello"}`, + }}, + ExpectedMsgStatus: "E", + ExpectedLogErrors: []*courier.ChannelError{courier.NewChannelError("", "", "invalid_auth")}, }, } @@ -229,7 +237,6 @@ var fileSendTestCases = []OutgoingTestCase{ {BodyContains: "image.png"}, }, ExpectedMsgStatus: "W", - SendPrep: setSendURL, }, } From ce8d1f0819e382a26379afafb7003ac98ac3599a Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Fri, 16 Feb 2024 16:44:14 +0200 Subject: [PATCH 05/12] Use MockResponses and ExpectedRequests in SC tests --- handlers/smscentral/handler_test.go | 56 +++++++++++++++++++---------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/handlers/smscentral/handler_test.go b/handlers/smscentral/handler_test.go index 7a91cabb5..a731209c1 100644 --- a/handlers/smscentral/handler_test.go +++ b/handlers/smscentral/handler_test.go @@ -1,12 +1,13 @@ package smscentral import ( - "net/http/httptest" + "net/url" "testing" "github.com/nyaruka/courier" . "github.com/nyaruka/courier/handlers" "github.com/nyaruka/courier/test" + "github.com/nyaruka/gocommon/httpx" ) const ( @@ -67,36 +68,55 @@ func BenchmarkHandler(b *testing.B) { RunChannelBenchmarks(b, testChannels, newHandler(), handleTestCases) } -// setSend takes care of setting the sendURL to call -func setSendURL(s *httptest.Server, h courier.ChannelHandler, c courier.Channel, m courier.MsgOut) { - sendURL = s.URL -} - var defaultSendTestCases = []OutgoingTestCase{ {Label: "Plain Send", MsgText: "Simple Message", MsgURN: "tel:+250788383383", ExpectedMsgStatus: "W", - MockResponseBody: `[{"id": "1002"}]`, MockResponseStatus: 200, - ExpectedPostParams: map[string]string{"content": "Simple Message", "mobile": "250788383383", "pass": "Password", "user": "Username"}, - SendPrep: setSendURL}, + MockResponses: map[string][]*httpx.MockResponse{ + "http://smail.smscentral.com.np/bp/ApiSms.php": { + httpx.NewMockResponse(200, nil, []byte(`[{"id": "1002"}]`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Form: url.Values{"content": {"Simple Message"}, "mobile": {"250788383383"}, "pass": {"Password"}, "user": {"Username"}}, + }}, + }, {Label: "Unicode Send", MsgText: "☺", MsgURN: "tel:+250788383383", ExpectedMsgStatus: "W", - MockResponseBody: `[{"id": "1002"}]`, MockResponseStatus: 200, - ExpectedPostParams: map[string]string{"content": "☺", "mobile": "250788383383", "pass": "Password", "user": "Username"}, - SendPrep: setSendURL}, + MockResponses: map[string][]*httpx.MockResponse{ + "http://smail.smscentral.com.np/bp/ApiSms.php": { + httpx.NewMockResponse(200, nil, []byte(`[{"id": "1002"}]`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Form: url.Values{"content": {"☺"}, "mobile": {"250788383383"}, "pass": {"Password"}, "user": {"Username"}}, + }}, + }, {Label: "Send Attachment", MsgText: "My pic!", MsgURN: "tel:+250788383383", MsgAttachments: []string{"image/jpeg:https://foo.bar/image.jpg"}, ExpectedMsgStatus: "W", - MockResponseBody: `[{ "id": "1002" }]`, MockResponseStatus: 200, - ExpectedPostParams: map[string]string{"content": "My pic!\nhttps://foo.bar/image.jpg", "mobile": "250788383383", "pass": "Password", "user": "Username"}, - SendPrep: setSendURL}, + MockResponses: map[string][]*httpx.MockResponse{ + "http://smail.smscentral.com.np/bp/ApiSms.php": { + httpx.NewMockResponse(200, nil, []byte(`[{"id": "1002"}]`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Form: url.Values{"content": {"My pic!\nhttps://foo.bar/image.jpg"}, "mobile": {"250788383383"}, "pass": {"Password"}, "user": {"Username"}}, + }}, + }, {Label: "Error Sending", MsgText: "Error Message", MsgURN: "tel:+250788383383", ExpectedMsgStatus: "E", - MockResponseBody: `{ "error": "failed" }`, MockResponseStatus: 401, - ExpectedPostParams: map[string]string{"content": `Error Message`, "mobile": "250788383383", "pass": "Password", "user": "Username"}, - SendPrep: setSendURL}, + MockResponses: map[string][]*httpx.MockResponse{ + "http://smail.smscentral.com.np/bp/ApiSms.php": { + httpx.NewMockResponse(401, nil, []byte(`{ "error": "failed" }`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Form: url.Values{"content": {`Error Message`}, "mobile": {"250788383383"}, "pass": {"Password"}, "user": {"Username"}}, + }}, + }, } func TestOutgoing(t *testing.T) { From ebc7cd0ac9b978e7402c3c9dc08131f7ad0b8b83 Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Fri, 16 Feb 2024 16:50:46 +0200 Subject: [PATCH 06/12] Use MockResponses and ExpectedRequests in ST tests --- handlers/start/handler_test.go | 154 +++++++++++++++++++-------------- 1 file changed, 89 insertions(+), 65 deletions(-) diff --git a/handlers/start/handler_test.go b/handlers/start/handler_test.go index d1df07cbd..a3427b2ec 100644 --- a/handlers/start/handler_test.go +++ b/handlers/start/handler_test.go @@ -1,7 +1,6 @@ package start import ( - "net/http/httptest" "testing" "time" @@ -159,85 +158,110 @@ func BenchmarkHandler(b *testing.B) { RunChannelBenchmarks(b, testChannels, newHandler(), testCases) } -// setSendURL takes care of setting the sendURL to call -func setSendURL(s *httptest.Server, h courier.ChannelHandler, c courier.Channel, m courier.MsgOut) { - sendURL = s.URL -} - var defaultSendTestCases = []OutgoingTestCase{ { - Label: "Plain Send", - MsgText: "Simple Message ☺", - MsgURN: "tel:+250788383383", - MockResponseBody: `380502535130309161501Accepted`, - MockResponseStatus: 200, - ExpectedHeaders: map[string]string{ - "Content-Type": "application/xml; charset=utf8", - "Authorization": "Basic VXNlcm5hbWU6UGFzc3dvcmQ=", + Label: "Plain Send", + MsgText: "Simple Message ☺", + MsgURN: "tel:+250788383383", + MockResponses: map[string][]*httpx.MockResponse{ + "https://bulk.startmobile.ua/clients.php": { + httpx.NewMockResponse(200, nil, []byte(`380502535130309161501Accepted`)), + }, }, - ExpectedRequestBody: `+250788383383Simple Message ☺`, - ExpectedMsgStatus: "W", - ExpectedExtIDs: []string{"380502535130309161501"}, - SendPrep: setSendURL, + ExpectedRequests: []ExpectedRequest{{ + Headers: map[string]string{ + "Content-Type": "application/xml; charset=utf8", + "Authorization": "Basic VXNlcm5hbWU6UGFzc3dvcmQ=", + }, + Body: `+250788383383Simple Message ☺`, + }}, + ExpectedMsgStatus: "W", + ExpectedExtIDs: []string{"380502535130309161501"}, }, { - Label: "Long Send", - MsgText: "This is a longer message than 160 characters and will cause us to split it into two separate parts, isn't that right but it is even longer than before I say, I need to keep adding more things to make it work", - MsgURN: "tel:+250788383383", - MockResponseBody: `380502535130309161501Accepted`, - MockResponseStatus: 200, - ExpectedHeaders: map[string]string{ - "Content-Type": "application/xml; charset=utf8", - "Authorization": "Basic VXNlcm5hbWU6UGFzc3dvcmQ=", + Label: "Long Send", + MsgText: "This is a longer message than 160 characters and will cause us to split it into two separate parts, isn't that right but it is even longer than before I say, I need to keep adding more things to make it work", + MsgURN: "tel:+250788383383", + MockResponses: map[string][]*httpx.MockResponse{ + "https://bulk.startmobile.ua/clients.php": { + httpx.NewMockResponse(200, nil, []byte(`380502535130309161501Accepted`)), + httpx.NewMockResponse(200, nil, []byte(`380502535130309161501Accepted`)), + }, }, - ExpectedRequestBody: `+250788383383I need to keep adding more things to make it work`, - ExpectedMsgStatus: "W", - ExpectedExtIDs: []string{"380502535130309161501"}, - SendPrep: setSendURL, + ExpectedRequests: []ExpectedRequest{ + { + Headers: map[string]string{ + "Content-Type": "application/xml; charset=utf8", + "Authorization": "Basic VXNlcm5hbWU6UGFzc3dvcmQ=", + }, + Body: `+250788383383This is a longer message than 160 characters and will cause us to split it into two separate parts, isn't that right but it is even longer than before I say,`, + }, + { + Headers: map[string]string{ + "Content-Type": "application/xml; charset=utf8", + "Authorization": "Basic VXNlcm5hbWU6UGFzc3dvcmQ=", + }, + Body: `+250788383383I need to keep adding more things to make it work`, + }}, + ExpectedMsgStatus: "W", + ExpectedExtIDs: []string{"380502535130309161501"}, }, { - Label: "Send Attachment", - MsgText: "My pic!", - MsgAttachments: []string{"image/jpeg:https://foo.bar/image.jpg"}, - MsgURN: "tel:+250788383383", - MockResponseBody: `380502535130309161501Accepted`, - MockResponseStatus: 200, - ExpectedHeaders: map[string]string{ - "Content-Type": "application/xml; charset=utf8", - "Authorization": "Basic VXNlcm5hbWU6UGFzc3dvcmQ=", + Label: "Send Attachment", + MsgText: "My pic!", + MsgAttachments: []string{"image/jpeg:https://foo.bar/image.jpg"}, + MsgURN: "tel:+250788383383", + MockResponses: map[string][]*httpx.MockResponse{ + "https://bulk.startmobile.ua/clients.php": { + httpx.NewMockResponse(200, nil, []byte(`380502535130309161501Accepted`)), + }, }, - ExpectedRequestBody: `+250788383383My pic! https://foo.bar/image.jpg`, - ExpectedMsgStatus: "W", - ExpectedExtIDs: []string{"380502535130309161501"}, - SendPrep: setSendURL, + ExpectedRequests: []ExpectedRequest{{ + Headers: map[string]string{ + "Content-Type": "application/xml; charset=utf8", + "Authorization": "Basic VXNlcm5hbWU6UGFzc3dvcmQ=", + }, + Body: `+250788383383My pic! https://foo.bar/image.jpg`, + }}, + ExpectedMsgStatus: "W", + ExpectedExtIDs: []string{"380502535130309161501"}, }, { - Label: "Error Response", - MsgText: "Simple Message ☺", - MsgURN: "tel:+250788383383", - MockResponseBody: `This is an error`, - MockResponseStatus: 200, - ExpectedHeaders: map[string]string{ - "Content-Type": "application/xml; charset=utf8", - "Authorization": "Basic VXNlcm5hbWU6UGFzc3dvcmQ=", + Label: "Error Response", + MsgText: "Simple Message ☺", + MsgURN: "tel:+250788383383", + MockResponses: map[string][]*httpx.MockResponse{ + "https://bulk.startmobile.ua/clients.php": { + httpx.NewMockResponse(200, nil, []byte(`This is an error`)), + }, }, - ExpectedRequestBody: `+250788383383Simple Message ☺`, - ExpectedMsgStatus: "E", - SendPrep: setSendURL, + ExpectedRequests: []ExpectedRequest{{ + Headers: map[string]string{ + "Content-Type": "application/xml; charset=utf8", + "Authorization": "Basic VXNlcm5hbWU6UGFzc3dvcmQ=", + }, + Body: `+250788383383Simple Message ☺`, + }}, + ExpectedMsgStatus: "E", }, { - Label: "Error Sending", - MsgText: "Error Message", - MsgURN: "tel:+250788383383", - MockResponseBody: `Error`, - MockResponseStatus: 401, - ExpectedHeaders: map[string]string{ - "Content-Type": "application/xml; charset=utf8", - "Authorization": "Basic VXNlcm5hbWU6UGFzc3dvcmQ=", + Label: "Error Sending", + MsgText: "Error Message", + MsgURN: "tel:+250788383383", + MockResponses: map[string][]*httpx.MockResponse{ + "https://bulk.startmobile.ua/clients.php": { + httpx.NewMockResponse(401, nil, []byte(`Error`)), + }, }, - ExpectedRequestBody: `+250788383383Error Message`, - ExpectedMsgStatus: "E", - SendPrep: setSendURL, + ExpectedRequests: []ExpectedRequest{{ + Headers: map[string]string{ + "Content-Type": "application/xml; charset=utf8", + "Authorization": "Basic VXNlcm5hbWU6UGFzc3dvcmQ=", + }, + Body: `+250788383383Error Message`, + }}, + + ExpectedMsgStatus: "E", }, } From 50bd05af34bea27bd881694558d9ab6a23f4d260 Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Fri, 16 Feb 2024 16:59:32 +0200 Subject: [PATCH 07/12] Rework ST handler to new send --- handlers/start/handler.go | 41 +++++++++++++++------------------- handlers/start/handler_test.go | 14 +++++------- 2 files changed, 23 insertions(+), 32 deletions(-) diff --git a/handlers/start/handler.go b/handlers/start/handler.go index 116bf0a85..e4f185bb9 100644 --- a/handlers/start/handler.go +++ b/handlers/start/handler.go @@ -122,24 +122,14 @@ type mtResponse struct { } func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.SendResult, clog *courier.ChannelLog) error { - // TODO convert functionality from legacy method below - return nil -} - -func (h *handler) SendLegacy(ctx context.Context, msg courier.MsgOut, clog *courier.ChannelLog) (courier.StatusUpdate, error) { username := msg.Channel().StringConfigForKey(courier.ConfigUsername, "") - if username == "" { - return nil, fmt.Errorf("no username set for ST channel: %s", msg.Channel().UUID()) - } - password := msg.Channel().StringConfigForKey(courier.ConfigPassword, "") - if password == "" { - return nil, fmt.Errorf("no password set for ST channel: %s", msg.Channel().UUID()) + if username == "" || password == "" { + return courier.ErrChannelConfig } - status := h.Backend().NewStatusUpdate(msg.Channel(), msg.ID(), courier.MsgStatusErrored, clog) parts := handlers.SplitMsgByChannel(msg.Channel(), handlers.GetTextAndAttachments(msg), maxMsgLength) - for i, part := range parts { + for _, part := range parts { payload := mtPayload{ Service: mtService{ @@ -158,33 +148,38 @@ func (h *handler) SendLegacy(ctx context.Context, msg courier.MsgOut, clog *cour requestBody := &bytes.Buffer{} err := xml.NewEncoder(requestBody).Encode(payload) if err != nil { - return nil, err + return err } // build our request req, err := http.NewRequest(http.MethodPost, sendURL, requestBody) if err != nil { - return nil, err + return err } req.Header.Set("Content-Type", "application/xml; charset=utf8") req.SetBasicAuth(username, password) resp, respBody, err := h.RequestHTTP(req, clog) - if err != nil || resp.StatusCode/100 != 2 { - return status, nil + if err != nil || resp.StatusCode/100 == 5 { + return courier.ErrConnectionFailed + } else if resp.StatusCode/100 != 2 { + return courier.ErrResponseStatus } response := &mtResponse{} err = xml.Unmarshal(respBody, response) - if err == nil { - status.SetStatus(courier.MsgStatusWired) - if i == 0 { - status.SetExternalID(response.ID) - } + if err != nil { + return courier.ErrResponseUnparseable + } + + if response.ID == "" { + return courier.ErrResponseUnexpected } + + res.AddExternalID(response.ID) } - return status, nil + return nil } func (h *handler) RedactValues(ch courier.Channel) []string { diff --git a/handlers/start/handler_test.go b/handlers/start/handler_test.go index a3427b2ec..2294b924a 100644 --- a/handlers/start/handler_test.go +++ b/handlers/start/handler_test.go @@ -175,8 +175,7 @@ var defaultSendTestCases = []OutgoingTestCase{ }, Body: `+250788383383Simple Message ☺`, }}, - ExpectedMsgStatus: "W", - ExpectedExtIDs: []string{"380502535130309161501"}, + ExpectedExtIDs: []string{"380502535130309161501"}, }, { Label: "Long Send", @@ -203,8 +202,7 @@ var defaultSendTestCases = []OutgoingTestCase{ }, Body: `+250788383383I need to keep adding more things to make it work`, }}, - ExpectedMsgStatus: "W", - ExpectedExtIDs: []string{"380502535130309161501"}, + ExpectedExtIDs: []string{"380502535130309161501", "380502535130309161501"}, }, { Label: "Send Attachment", @@ -223,8 +221,7 @@ var defaultSendTestCases = []OutgoingTestCase{ }, Body: `+250788383383My pic! https://foo.bar/image.jpg`, }}, - ExpectedMsgStatus: "W", - ExpectedExtIDs: []string{"380502535130309161501"}, + ExpectedExtIDs: []string{"380502535130309161501"}, }, { Label: "Error Response", @@ -242,7 +239,7 @@ var defaultSendTestCases = []OutgoingTestCase{ }, Body: `+250788383383Simple Message ☺`, }}, - ExpectedMsgStatus: "E", + ExpectedError: courier.ErrResponseUnparseable, }, { Label: "Error Sending", @@ -260,8 +257,7 @@ var defaultSendTestCases = []OutgoingTestCase{ }, Body: `+250788383383Error Message`, }}, - - ExpectedMsgStatus: "E", + ExpectedError: courier.ErrResponseStatus, }, } From 20fc301d139b70d7d299fa58cdef717c7a5c4e88 Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Fri, 16 Feb 2024 17:02:53 +0200 Subject: [PATCH 08/12] Rework SC handler to new send --- handlers/smscentral/handler.go | 28 ++++++++-------------------- handlers/smscentral/handler_test.go | 17 +++++++++++++---- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/handlers/smscentral/handler.go b/handlers/smscentral/handler.go index 282ce9bbe..0863a41ac 100644 --- a/handlers/smscentral/handler.go +++ b/handlers/smscentral/handler.go @@ -2,7 +2,6 @@ package smscentral import ( "context" - "fmt" "net/http" "net/url" "strings" @@ -63,23 +62,12 @@ func (h *handler) receiveMessage(ctx context.Context, channel courier.Channel, w } func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.SendResult, clog *courier.ChannelLog) error { - // TODO convert functionality from legacy method below - return nil -} - -func (h *handler) SendLegacy(ctx context.Context, msg courier.MsgOut, clog *courier.ChannelLog) (courier.StatusUpdate, error) { username := msg.Channel().StringConfigForKey(courier.ConfigUsername, "") - if username == "" { - return nil, fmt.Errorf("no username set for SC channel") - } - password := msg.Channel().StringConfigForKey(courier.ConfigPassword, "") - if password == "" { - return nil, fmt.Errorf("no password set for SC channel") + if username == "" || password == "" { + return courier.ErrChannelConfig } - status := h.Backend().NewStatusUpdate(msg.Channel(), msg.ID(), courier.MsgStatusErrored, clog) - // build our request form := url.Values{ "user": []string{username}, @@ -90,16 +78,16 @@ func (h *handler) SendLegacy(ctx context.Context, msg courier.MsgOut, clog *cour req, err := http.NewRequest(http.MethodPost, sendURL, strings.NewReader(form.Encode())) if err != nil { - return nil, err + return err } req.Header.Set("Content-Type", "application/x-www-form-urlencoded") resp, _, err := h.RequestHTTP(req, clog) - if err != nil || resp.StatusCode/100 != 2 { - return status, nil + if err != nil || resp.StatusCode/100 == 5 { + return courier.ErrConnectionFailed + } else if resp.StatusCode/100 != 2 { + return courier.ErrResponseStatus } - status.SetStatus(courier.MsgStatusWired) - - return status, nil + return nil } diff --git a/handlers/smscentral/handler_test.go b/handlers/smscentral/handler_test.go index a731209c1..a1b52186d 100644 --- a/handlers/smscentral/handler_test.go +++ b/handlers/smscentral/handler_test.go @@ -71,7 +71,6 @@ func BenchmarkHandler(b *testing.B) { var defaultSendTestCases = []OutgoingTestCase{ {Label: "Plain Send", MsgText: "Simple Message", MsgURN: "tel:+250788383383", - ExpectedMsgStatus: "W", MockResponses: map[string][]*httpx.MockResponse{ "http://smail.smscentral.com.np/bp/ApiSms.php": { httpx.NewMockResponse(200, nil, []byte(`[{"id": "1002"}]`)), @@ -83,7 +82,6 @@ var defaultSendTestCases = []OutgoingTestCase{ }, {Label: "Unicode Send", MsgText: "☺", MsgURN: "tel:+250788383383", - ExpectedMsgStatus: "W", MockResponses: map[string][]*httpx.MockResponse{ "http://smail.smscentral.com.np/bp/ApiSms.php": { httpx.NewMockResponse(200, nil, []byte(`[{"id": "1002"}]`)), @@ -95,7 +93,6 @@ var defaultSendTestCases = []OutgoingTestCase{ }, {Label: "Send Attachment", MsgText: "My pic!", MsgURN: "tel:+250788383383", MsgAttachments: []string{"image/jpeg:https://foo.bar/image.jpg"}, - ExpectedMsgStatus: "W", MockResponses: map[string][]*httpx.MockResponse{ "http://smail.smscentral.com.np/bp/ApiSms.php": { httpx.NewMockResponse(200, nil, []byte(`[{"id": "1002"}]`)), @@ -107,7 +104,6 @@ var defaultSendTestCases = []OutgoingTestCase{ }, {Label: "Error Sending", MsgText: "Error Message", MsgURN: "tel:+250788383383", - ExpectedMsgStatus: "E", MockResponses: map[string][]*httpx.MockResponse{ "http://smail.smscentral.com.np/bp/ApiSms.php": { httpx.NewMockResponse(401, nil, []byte(`{ "error": "failed" }`)), @@ -116,6 +112,19 @@ var defaultSendTestCases = []OutgoingTestCase{ ExpectedRequests: []ExpectedRequest{{ Form: url.Values{"content": {`Error Message`}, "mobile": {"250788383383"}, "pass": {"Password"}, "user": {"Username"}}, }}, + ExpectedError: courier.ErrResponseStatus, + }, + {Label: "Connection Error", + MsgText: "Error Message", MsgURN: "tel:+250788383383", + MockResponses: map[string][]*httpx.MockResponse{ + "http://smail.smscentral.com.np/bp/ApiSms.php": { + httpx.NewMockResponse(500, nil, []byte(`{ "error": "failed" }`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Form: url.Values{"content": {`Error Message`}, "mobile": {"250788383383"}, "pass": {"Password"}, "user": {"Username"}}, + }}, + ExpectedError: courier.ErrConnectionFailed, }, } From 106d769c38df13d7f3d62706c0f69e9b88172c8b Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Fri, 16 Feb 2024 17:29:08 +0200 Subject: [PATCH 09/12] Rework SL handler to new send --- handlers/slack/handler.go | 40 +++++++++++++++------------------- handlers/slack/handler_test.go | 11 ++++------ sender.go | 4 ++-- 3 files changed, 23 insertions(+), 32 deletions(-) diff --git a/handlers/slack/handler.go b/handlers/slack/handler.go index 4d3bee685..61fa0b3d8 100644 --- a/handlers/slack/handler.go +++ b/handlers/slack/handler.go @@ -146,30 +146,21 @@ func (h *handler) resolveFile(ctx context.Context, channel courier.Channel, file } func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.SendResult, clog *courier.ChannelLog) error { - // TODO convert functionality from legacy method below - return nil -} - -func (h *handler) SendLegacy(ctx context.Context, msg courier.MsgOut, clog *courier.ChannelLog) (courier.StatusUpdate, error) { botToken := msg.Channel().StringConfigForKey(configBotToken, "") if botToken == "" { - return nil, fmt.Errorf("missing bot token for SL/slack channel") + return courier.ErrChannelConfig } - status := h.Backend().NewStatusUpdate(msg.Channel(), msg.ID(), courier.MsgStatusErrored, clog) - for _, attachment := range msg.Attachments() { fileAttachment, err := h.parseAttachmentToFileParams(msg, attachment, clog) if err != nil { - clog.RawError(err) - return status, nil + return err } if fileAttachment != nil { err = h.sendFilePart(msg, botToken, fileAttachment, clog) if err != nil { - clog.RawError(err) - return status, nil + return err } } } @@ -177,13 +168,11 @@ func (h *handler) SendLegacy(ctx context.Context, msg courier.MsgOut, clog *cour if msg.Text() != "" { err := h.sendTextMsgPart(msg, botToken, clog) if err != nil { - clog.RawError(err) - return status, nil + return err } } - status.SetStatus(courier.MsgStatusWired) - return status, nil + return nil } func (h *handler) sendTextMsgPart(msg courier.MsgOut, token string, clog *courier.ChannelLog) error { @@ -208,20 +197,23 @@ func (h *handler) sendTextMsgPart(msg courier.MsgOut, token string, clog *courie resp, respBody, err := h.RequestHTTP(req, clog) if err != nil || resp.StatusCode/100 != 2 { - return errors.New("error sending message") + return courier.ErrConnectionFailed + } else if resp.StatusCode/100 != 2 { + return courier.ErrResponseStatus } ok, err := jsonparser.GetBoolean(respBody, "ok") if err != nil { - return err + return courier.ErrResponseUnexpected } if !ok { errDescription, err := jsonparser.GetString(respBody, "error") if err != nil { - return err + return courier.ErrResponseUnexpected } - return errors.New(errDescription) + clog.Error(courier.NewChannelError("", "", errDescription)) + return courier.ErrFailedWithReason("", errDescription) } return nil } @@ -280,16 +272,18 @@ func (h *handler) sendFilePart(msg courier.MsgOut, token string, fileParams *Fil resp, respBody, err := h.RequestHTTP(req, clog) if err != nil || resp.StatusCode/100 != 2 { - return errors.New("error uploading file to slack") + return courier.ErrConnectionFailed + } else if resp.StatusCode/100 != 2 { + return courier.ErrResponseStatus } var fr FileResponse if err := json.Unmarshal(respBody, &fr); err != nil { - return errors.Errorf("couldn't unmarshal file response: %v", err) + return courier.ErrResponseUnparseable } if !fr.OK { - return errors.Errorf("error uploading file to slack: %s.", fr.Error) + return courier.ErrResponseUnexpected } return nil diff --git a/handlers/slack/handler_test.go b/handlers/slack/handler_test.go index 0942ec335..7a758dbdd 100644 --- a/handlers/slack/handler_test.go +++ b/handlers/slack/handler_test.go @@ -178,42 +178,40 @@ var defaultSendTestCases = []OutgoingTestCase{ MsgText: "Simple Message", MsgURN: "slack:U0123ABCDEF", MockResponses: map[string][]*httpx.MockResponse{ - "https://slack.com/api/chat.postMessage": { + "*/chat.postMessage": { httpx.NewMockResponse(200, nil, []byte(`{"ok":true,"channel":"U0123ABCDEF"}`)), }, }, ExpectedRequests: []ExpectedRequest{{ Body: `{"channel":"U0123ABCDEF","text":"Simple Message"}`, }}, - ExpectedMsgStatus: "W", }, { Label: "Unicode Send", MsgText: "☺", MsgURN: "slack:U0123ABCDEF", MockResponses: map[string][]*httpx.MockResponse{ - "https://slack.com/api/chat.postMessage": { + "*/chat.postMessage": { httpx.NewMockResponse(200, nil, []byte(`{"ok":true,"channel":"U0123ABCDEF"}`)), }, }, ExpectedRequests: []ExpectedRequest{{ Body: `{"channel":"U0123ABCDEF","text":"☺"}`, }}, - ExpectedMsgStatus: "W", }, { Label: "Send Text Auth Error", MsgText: "Hello", MsgURN: "slack:U0123ABCDEF", MockResponses: map[string][]*httpx.MockResponse{ - "https://slack.com/api/chat.postMessage": { + "*/chat.postMessage": { httpx.NewMockResponse(200, nil, []byte(`{"ok":false,"error":"invalid_auth"}`)), }, }, ExpectedRequests: []ExpectedRequest{{ Body: `{"channel":"U0123ABCDEF","text":"Hello"}`, }}, - ExpectedMsgStatus: "E", + ExpectedError: courier.ErrFailedWithReason("", "invalid_auth"), ExpectedLogErrors: []*courier.ChannelError{courier.NewChannelError("", "", "invalid_auth")}, }, } @@ -236,7 +234,6 @@ var fileSendTestCases = []OutgoingTestCase{ {}, {BodyContains: "image.png"}, }, - ExpectedMsgStatus: "W", }, } diff --git a/sender.go b/sender.go index 7e6473446..fe45b8526 100644 --- a/sender.go +++ b/sender.go @@ -105,8 +105,8 @@ func ErrFailedWithReason(code, desc string) *SendError { retryable: false, loggable: false, clogCode: "rejected_with_reason", - clogMsg: code, - clogExtCode: desc, + clogMsg: desc, + clogExtCode: code, } } From f461d95221bbb6713dc44efb342c2f3c6a8d3221 Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Fri, 16 Feb 2024 17:33:47 +0200 Subject: [PATCH 10/12] Rework SQ handler to new send --- handlers/shaqodoon/handler.go | 25 ++++++++----------------- handlers/shaqodoon/handler_test.go | 18 +++++------------- 2 files changed, 13 insertions(+), 30 deletions(-) diff --git a/handlers/shaqodoon/handler.go b/handlers/shaqodoon/handler.go index c616c9999..1a4781038 100644 --- a/handlers/shaqodoon/handler.go +++ b/handlers/shaqodoon/handler.go @@ -83,20 +83,12 @@ func (h *handler) receiveMessage(ctx context.Context, channel courier.Channel, w } func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.SendResult, clog *courier.ChannelLog) error { - // TODO convert functionality from legacy method below - return nil -} - -func (h *handler) SendLegacy(ctx context.Context, msg courier.MsgOut, clog *courier.ChannelLog) (courier.StatusUpdate, error) { sendURL := msg.Channel().StringConfigForKey(courier.ConfigSendURL, "") - if sendURL == "" { - return nil, fmt.Errorf("missing send_url for SQ channel") - } username := msg.Channel().StringConfigForKey(courier.ConfigUsername, "") password := msg.Channel().StringConfigForKey(courier.ConfigPassword, "") - if username == "" || password == "" { - return nil, fmt.Errorf("missing username or password for SQ channel") + if username == "" || password == "" || sendURL == "" { + return courier.ErrChannelConfig } // build our request @@ -113,17 +105,16 @@ func (h *handler) SendLegacy(ctx context.Context, msg courier.MsgOut, clog *cour req, err := http.NewRequest(http.MethodGet, sendURL, nil) if err != nil { - return nil, err + return err } req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - status := h.Backend().NewStatusUpdate(msg.Channel(), msg.ID(), courier.MsgStatusErrored, clog) - resp, _, err := h.RequestHTTPInsecure(req, clog) - if err != nil || resp.StatusCode/100 != 2 { - return status, nil + if err != nil || resp.StatusCode/100 == 5 { + return courier.ErrConnectionFailed + } else if resp.StatusCode/100 != 2 { + return courier.ErrResponseStatus } - status.SetStatus(courier.MsgStatusWired) - return status, nil + return nil } diff --git a/handlers/shaqodoon/handler_test.go b/handlers/shaqodoon/handler_test.go index 89866ae06..3d7fd3777 100644 --- a/handlers/shaqodoon/handler_test.go +++ b/handlers/shaqodoon/handler_test.go @@ -1,7 +1,6 @@ package shaqodoon import ( - "net/http/httptest" "net/url" "testing" "time" @@ -53,14 +52,9 @@ func BenchmarkHandler(b *testing.B) { RunChannelBenchmarks(b, testChannels, newHandler(), handleTestCases) } -func setSendURL(s *httptest.Server, h courier.ChannelHandler, c courier.Channel, m courier.MsgOut) { - c.(*test.MockChannel).SetConfig(courier.ConfigSendURL, s.URL) -} - var getSendTestCases = []OutgoingTestCase{ {Label: "Plain Send", MsgText: "Simple Message", MsgURN: "tel:+250788383383", - ExpectedMsgStatus: "W", MockResponses: map[string][]*httpx.MockResponse{ "http://example.com/send*": { httpx.NewMockResponse(200, nil, []byte(`0: Accepted for delivery`)), @@ -69,10 +63,9 @@ var getSendTestCases = []OutgoingTestCase{ ExpectedRequests: []ExpectedRequest{{ Params: url.Values{"msg": {"Simple Message"}, "to": {"250788383383"}, "from": {"2020"}, "username": {"Username"}, "password": {"Password"}}, }}, - SendPrep: setSendURL}, + }, {Label: "Unicode Send", MsgText: "☺", MsgURN: "tel:+250788383383", - ExpectedMsgStatus: "W", MockResponses: map[string][]*httpx.MockResponse{ "http://example.com/send*": { httpx.NewMockResponse(200, nil, []byte(`0: Accepted for delivery`)), @@ -81,10 +74,9 @@ var getSendTestCases = []OutgoingTestCase{ ExpectedRequests: []ExpectedRequest{{ Params: url.Values{"msg": {"☺"}, "to": {"250788383383"}, "from": {"2020"}, "username": {"Username"}, "password": {"Password"}}, }}, - SendPrep: setSendURL}, + }, {Label: "Error Sending", MsgText: "Error Message", MsgURN: "tel:+250788383383", - ExpectedMsgStatus: "E", MockResponses: map[string][]*httpx.MockResponse{ "http://example.com/send*": { httpx.NewMockResponse(401, nil, []byte(`1: Unknown channel`)), @@ -93,10 +85,10 @@ var getSendTestCases = []OutgoingTestCase{ ExpectedRequests: []ExpectedRequest{{ Params: url.Values{"msg": {"Error Message"}, "to": {"250788383383"}, "from": []string{"2020"}, "username": {"Username"}, "password": {"Password"}}, }}, - SendPrep: setSendURL}, + ExpectedError: courier.ErrResponseStatus, + }, {Label: "Send Attachment", MsgText: "My pic!", MsgURN: "tel:+250788383383", MsgAttachments: []string{"image/jpeg:https://foo.bar/image.jpg"}, - ExpectedMsgStatus: "W", MockResponses: map[string][]*httpx.MockResponse{ "http://example.com/send*": { httpx.NewMockResponse(200, nil, []byte(`0: Accepted for delivery`)), @@ -105,7 +97,7 @@ var getSendTestCases = []OutgoingTestCase{ ExpectedRequests: []ExpectedRequest{{ Params: url.Values{"msg": {"My pic!\nhttps://foo.bar/image.jpg"}, "to": {"250788383383"}, "from": {"2020"}, "username": {"Username"}, "password": {"Password"}}, }}, - SendPrep: setSendURL}, + }, } func TestOutgoing(t *testing.T) { From d5c8d2e10a6276f3ecd1d48232397f507427f325 Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Fri, 16 Feb 2024 17:40:04 +0200 Subject: [PATCH 11/12] Rework RC handler to new send --- handlers/rocketchat/handler.go | 28 +++++++++++--------------- handlers/rocketchat/handler_test.go | 31 ++++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/handlers/rocketchat/handler.go b/handlers/rocketchat/handler.go index b0aa6da41..a06b37011 100644 --- a/handlers/rocketchat/handler.go +++ b/handlers/rocketchat/handler.go @@ -106,18 +106,12 @@ type mtPayload struct { } func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.SendResult, clog *courier.ChannelLog) error { - // TODO convert functionality from legacy method below - return nil -} - -func (h *handler) SendLegacy(ctx context.Context, msg courier.MsgOut, clog *courier.ChannelLog) (courier.StatusUpdate, error) { baseURL := msg.Channel().StringConfigForKey(configBaseURL, "") secret := msg.Channel().StringConfigForKey(configSecret, "") botUsername := msg.Channel().StringConfigForKey(configBotUsername, "") - - // the status that will be written for this message - status := h.Backend().NewStatusUpdate(msg.Channel(), msg.ID(), courier.MsgStatusErrored, clog) - + if baseURL == "" || secret == "" || botUsername == "" { + return courier.ErrChannelConfig + } payload := &mtPayload{ UserURN: msg.URN().Path(), BotUsername: botUsername, @@ -132,21 +126,23 @@ func (h *handler) SendLegacy(ctx context.Context, msg courier.MsgOut, clog *cour req, err := http.NewRequest(http.MethodPost, baseURL+"/message", bytes.NewReader(body)) if err != nil { - return status, err + return err } req.Header.Set("Content-Type", "application/json") req.Header.Set("Authorization", fmt.Sprintf("Token %s", secret)) resp, respBody, err := h.RequestHTTP(req, clog) - if err != nil || resp.StatusCode/100 != 2 { - return status, nil + if err != nil || resp.StatusCode/100 == 5 { + return courier.ErrConnectionFailed + } else if resp.StatusCode/100 != 2 { + return courier.ErrResponseStatus } msgID, err := jsonparser.GetString(respBody, "id") - if err == nil { - status.SetExternalID(msgID) + if err != nil { + return courier.ErrResponseUnexpected } + res.AddExternalID(msgID) - status.SetStatus(courier.MsgStatusSent) - return status, nil + return nil } diff --git a/handlers/rocketchat/handler_test.go b/handlers/rocketchat/handler_test.go index 90fea98a5..7410ea0ec 100644 --- a/handlers/rocketchat/handler_test.go +++ b/handlers/rocketchat/handler_test.go @@ -114,7 +114,6 @@ var sendTestCases = []OutgoingTestCase{ httpx.NewMockResponse(201, nil, []byte(`{"id":"iNKE8a6k6cjbqWhWd"}`)), }, }, - ExpectedMsgStatus: "S", ExpectedRequests: []ExpectedRequest{{ Body: `{"user":"direct:john.doe","bot":"rocket.cat","text":"Simple Message"}`, }}, @@ -129,7 +128,6 @@ var sendTestCases = []OutgoingTestCase{ httpx.NewMockResponse(201, nil, []byte(`{"id":"iNKE8a6k6cjbqWhWd"}`)), }, }, - ExpectedMsgStatus: "S", ExpectedRequests: []ExpectedRequest{{ Body: `{"user":"livechat:onrMgdKbpX9Qqtvoi","bot":"rocket.cat","attachments":[{"type":"application/pdf","url":"https://link.to/attachment.pdf"}]}`, }}, @@ -145,12 +143,39 @@ var sendTestCases = []OutgoingTestCase{ httpx.NewMockResponse(201, nil, []byte(`{"id":"iNKE8a6k6cjbqWhWd"}`)), }, }, - ExpectedMsgStatus: "S", ExpectedRequests: []ExpectedRequest{{ Body: `{"user":"direct:john.doe","bot":"rocket.cat","text":"Simple Message","attachments":[{"type":"application/pdf","url":"https://link.to/attachment.pdf"}]}`, }}, ExpectedExtIDs: []string{"iNKE8a6k6cjbqWhWd"}, }, + { + Label: "Unexcepted status response", + MsgText: "Simple Message", + MsgURN: "rocketchat:direct:john.doe#john.doe", + MockResponses: map[string][]*httpx.MockResponse{ + "https://my.rocket.chat/api/apps/public/684202ed-1461-4983-9ea7-fde74b15026c/message": { + httpx.NewMockResponse(400, nil, []byte(`Error`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Body: `{"user":"direct:john.doe","bot":"rocket.cat","text":"Simple Message"}`, + }}, + ExpectedError: courier.ErrResponseStatus, + }, + { + Label: "Connection Error", + MsgText: "Simple Message", + MsgURN: "rocketchat:direct:john.doe#john.doe", + MockResponses: map[string][]*httpx.MockResponse{ + "https://my.rocket.chat/api/apps/public/684202ed-1461-4983-9ea7-fde74b15026c/message": { + httpx.NewMockResponse(500, nil, []byte(`Error`)), + }, + }, + ExpectedRequests: []ExpectedRequest{{ + Body: `{"user":"direct:john.doe","bot":"rocket.cat","text":"Simple Message"}`, + }}, + ExpectedError: courier.ErrConnectionFailed, + }, } func TestOutgoing(t *testing.T) { From c10e554089b9df4b244974f9bfa82afc451e4632 Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Fri, 16 Feb 2024 17:43:59 +0200 Subject: [PATCH 12/12] Rework RR handler to new send --- handlers/redrabbit/handler.go | 22 +++++++--------------- handlers/redrabbit/handler_test.go | 25 +++++++++++++++---------- 2 files changed, 22 insertions(+), 25 deletions(-) diff --git a/handlers/redrabbit/handler.go b/handlers/redrabbit/handler.go index f9df0bd4b..7c1a5f93b 100644 --- a/handlers/redrabbit/handler.go +++ b/handlers/redrabbit/handler.go @@ -2,7 +2,6 @@ package redrabbit import ( "context" - "fmt" "net/http" "net/url" "strings" @@ -36,19 +35,13 @@ func (h *handler) Initialize(s courier.Server) error { } func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.SendResult, clog *courier.ChannelLog) error { - // TODO convert functionality from legacy method below - return nil -} - -func (h *handler) SendLegacy(ctx context.Context, msg courier.MsgOut, clog *courier.ChannelLog) (courier.StatusUpdate, error) { username := msg.Channel().StringConfigForKey(courier.ConfigUsername, "") password := msg.Channel().StringConfigForKey(courier.ConfigPassword, "") if username == "" || password == "" { - return nil, fmt.Errorf("Missing username or password for RR channel") + return courier.ErrChannelConfig } text := handlers.GetTextAndAttachments(msg) - status := h.Backend().NewStatusUpdate(msg.Channel(), msg.ID(), courier.MsgStatusErrored, clog) form := url.Values{ "LoginName": []string{username}, "Password": []string{password}, @@ -73,16 +66,15 @@ func (h *handler) SendLegacy(ctx context.Context, msg courier.MsgOut, clog *cour msgURL.RawQuery = form.Encode() req, err := http.NewRequest(http.MethodGet, msgURL.String(), nil) if err != nil { - return nil, err + return err } resp, _, err := h.RequestHTTP(req, clog) - if err != nil || resp.StatusCode/100 != 2 { - return status, nil + if err != nil || resp.StatusCode/100 == 5 { + return courier.ErrConnectionFailed + } else if resp.StatusCode/100 != 2 { + return courier.ErrResponseStatus } - // all went well, set ourselves to wired - status.SetStatus(courier.MsgStatusWired) - - return status, nil + return nil } diff --git a/handlers/redrabbit/handler_test.go b/handlers/redrabbit/handler_test.go index 515f5a274..58785b8be 100644 --- a/handlers/redrabbit/handler_test.go +++ b/handlers/redrabbit/handler_test.go @@ -4,6 +4,7 @@ import ( "net/url" "testing" + "github.com/nyaruka/courier" . "github.com/nyaruka/courier/handlers" "github.com/nyaruka/courier/test" "github.com/nyaruka/gocommon/httpx" @@ -12,7 +13,6 @@ import ( var defaultSendTestCases = []OutgoingTestCase{ {Label: "Plain Send", MsgText: "Simple Message", MsgURN: "tel:+250788383383", - ExpectedMsgStatus: "W", MockResponses: map[string][]*httpx.MockResponse{ "http://http1.javna.com/epicenter/GatewaySendG.asp*": { httpx.NewMockResponse(200, nil, []byte(`SENT`)), @@ -31,7 +31,6 @@ var defaultSendTestCases = []OutgoingTestCase{ }, {Label: "Unicode Send", MsgText: "☺", MsgURN: "tel:+250788383383", - ExpectedMsgStatus: "W", MockResponses: map[string][]*httpx.MockResponse{ "http://http1.javna.com/epicenter/GatewaySendG.asp*": { httpx.NewMockResponse(200, nil, []byte(`SENT`)), @@ -51,9 +50,8 @@ var defaultSendTestCases = []OutgoingTestCase{ }}, }, {Label: "Longer Unicode Send", - MsgText: "This is a message more than seventy characters with some unicode ☺ in them", - MsgURN: "tel:+250788383383", - ExpectedMsgStatus: "W", + MsgText: "This is a message more than seventy characters with some unicode ☺ in them", + MsgURN: "tel:+250788383383", MockResponses: map[string][]*httpx.MockResponse{ "http://http1.javna.com/epicenter/GatewaySendG.asp*": { httpx.NewMockResponse(200, nil, []byte(`SENT`)), @@ -72,9 +70,8 @@ var defaultSendTestCases = []OutgoingTestCase{ }}}, }, {Label: "Long Send", - MsgText: "This is a longer message than 160 characters and will cause us to split it into two separate parts, isn't that right but it is even longer than before I say, I need to keep adding more things to make it work", - MsgURN: "tel:+250788383383", - ExpectedMsgStatus: "W", + MsgText: "This is a longer message than 160 characters and will cause us to split it into two separate parts, isn't that right but it is even longer than before I say, I need to keep adding more things to make it work", + MsgURN: "tel:+250788383383", MockResponses: map[string][]*httpx.MockResponse{ "http://http1.javna.com/epicenter/GatewaySendG.asp*": { httpx.NewMockResponse(200, nil, []byte(`SENT`)), @@ -94,7 +91,6 @@ var defaultSendTestCases = []OutgoingTestCase{ }, {Label: "Send Attachment", MsgText: "My pic!", MsgURN: "tel:+250788383383", MsgAttachments: []string{"image/jpeg:https://foo.bar/image.jpg"}, - ExpectedMsgStatus: "W", MockResponses: map[string][]*httpx.MockResponse{ "http://http1.javna.com/epicenter/GatewaySendG.asp*": { httpx.NewMockResponse(200, nil, []byte(`SENT`)), @@ -113,12 +109,21 @@ var defaultSendTestCases = []OutgoingTestCase{ }, {Label: "Error Sending", MsgText: "Error Sending", MsgURN: "tel:+250788383383", - ExpectedMsgStatus: "E", MockResponses: map[string][]*httpx.MockResponse{ "http://http1.javna.com/epicenter/GatewaySendG.asp*": { httpx.NewMockResponse(403, nil, []byte(`Error`)), }, }, + ExpectedError: courier.ErrResponseStatus, + }, + {Label: "Connection Error", + MsgText: "Error Sending", MsgURN: "tel:+250788383383", + MockResponses: map[string][]*httpx.MockResponse{ + "http://http1.javna.com/epicenter/GatewaySendG.asp*": { + httpx.NewMockResponse(500, nil, []byte(`Error`)), + }, + }, + ExpectedError: courier.ErrConnectionFailed, }, }