From 917696a543d533ad75165e1cb01c8680ccc96338 Mon Sep 17 00:00:00 2001
From: Kailash Nadh <kailash@nadh.in>
Date: Mon, 8 May 2023 22:43:25 +0530
Subject: [PATCH] Refactor `Messenger` interface.

Remove `messenger.go` and move the interface definition to `manager`
and the `Message` struct to the `models` package, removing superflous
and redundant message structs used in multiple places.
---
 cmd/init.go                             |  7 ++--
 cmd/main.go                             |  5 +--
 cmd/notifications.go                    |  4 +-
 cmd/public.go                           |  8 ++--
 cmd/settings.go                         |  3 +-
 cmd/tx.go                               |  9 ++--
 internal/manager/manager.go             | 48 +++++++++++++--------
 internal/messenger/email/email.go       |  4 +-
 internal/messenger/messenger.go         | 55 -------------------------
 internal/messenger/postback/postback.go | 12 +++---
 models/models.go                        | 37 +++++++++++++----
 11 files changed, 84 insertions(+), 108 deletions(-)
 delete mode 100644 internal/messenger/messenger.go

diff --git a/cmd/init.go b/cmd/init.go
index b5778a26..830804c2 100644
--- a/cmd/init.go
+++ b/cmd/init.go
@@ -31,7 +31,6 @@ import (
 	"github.com/knadh/listmonk/internal/media"
 	"github.com/knadh/listmonk/internal/media/providers/filesystem"
 	"github.com/knadh/listmonk/internal/media/providers/s3"
-	"github.com/knadh/listmonk/internal/messenger"
 	"github.com/knadh/listmonk/internal/messenger/email"
 	"github.com/knadh/listmonk/internal/messenger/postback"
 	"github.com/knadh/listmonk/internal/subimporter"
@@ -484,7 +483,7 @@ func initImporter(q *models.Queries, db *sqlx.DB, app *App) *subimporter.Importe
 }
 
 // initSMTPMessenger initializes the SMTP messenger.
-func initSMTPMessenger(m *manager.Manager) messenger.Messenger {
+func initSMTPMessenger(m *manager.Manager) manager.Messenger {
 	var (
 		mapKeys = ko.MapKeys("smtp")
 		servers = make([]email.Server, 0, len(mapKeys))
@@ -526,13 +525,13 @@ func initSMTPMessenger(m *manager.Manager) messenger.Messenger {
 
 // initPostbackMessengers initializes and returns all the enabled
 // HTTP postback messenger backends.
-func initPostbackMessengers(m *manager.Manager) []messenger.Messenger {
+func initPostbackMessengers(m *manager.Manager) []manager.Messenger {
 	items := ko.Slices("messengers")
 	if len(items) == 0 {
 		return nil
 	}
 
-	var out []messenger.Messenger
+	var out []manager.Messenger
 	for _, item := range items {
 		if !item.Bool("enabled") {
 			continue
diff --git a/cmd/main.go b/cmd/main.go
index e3f4d623..bcaec935 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -22,7 +22,6 @@ import (
 	"github.com/knadh/listmonk/internal/i18n"
 	"github.com/knadh/listmonk/internal/manager"
 	"github.com/knadh/listmonk/internal/media"
-	"github.com/knadh/listmonk/internal/messenger"
 	"github.com/knadh/listmonk/internal/subimporter"
 	"github.com/knadh/listmonk/models"
 	"github.com/knadh/paginator"
@@ -43,7 +42,7 @@ type App struct {
 	constants  *constants
 	manager    *manager.Manager
 	importer   *subimporter.Importer
-	messengers map[string]messenger.Messenger
+	messengers map[string]manager.Messenger
 	media      media.Store
 	i18n       *i18n.I18n
 	bounce     *bounce.Manager
@@ -167,7 +166,7 @@ func main() {
 		db:         db,
 		constants:  initConstants(),
 		media:      initMediaStore(),
-		messengers: make(map[string]messenger.Messenger),
+		messengers: make(map[string]manager.Messenger),
 		log:        lo,
 		bufLog:     bufLog,
 		captcha:    initCaptcha(),
diff --git a/cmd/notifications.go b/cmd/notifications.go
index b60a137c..0b777e07 100644
--- a/cmd/notifications.go
+++ b/cmd/notifications.go
@@ -3,7 +3,7 @@ package main
 import (
 	"bytes"
 
-	"github.com/knadh/listmonk/internal/manager"
+	"github.com/knadh/listmonk/models"
 )
 
 const (
@@ -32,7 +32,7 @@ func (app *App) sendNotification(toEmails []string, subject, tplName string, dat
 		return err
 	}
 
-	m := manager.Message{}
+	m := models.Message{}
 	m.ContentType = app.notifTpls.contentType
 	m.From = app.constants.FromEmail
 	m.To = toEmails
diff --git a/cmd/public.go b/cmd/public.go
index 3c5e59be..746d182c 100644
--- a/cmd/public.go
+++ b/cmd/public.go
@@ -13,7 +13,7 @@ import (
 	"strings"
 
 	"github.com/knadh/listmonk/internal/i18n"
-	"github.com/knadh/listmonk/internal/messenger"
+	"github.com/knadh/listmonk/internal/manager"
 	"github.com/knadh/listmonk/models"
 	"github.com/labstack/echo/v4"
 	"github.com/lib/pq"
@@ -566,17 +566,17 @@ func handleSelfExportSubscriberData(c echo.Context) error {
 
 	// Send the data as a JSON attachment to the subscriber.
 	const fname = "data.json"
-	if err := app.messengers[emailMsgr].Push(messenger.Message{
+	if err := app.messengers[emailMsgr].Push(models.Message{
 		ContentType: app.notifTpls.contentType,
 		From:        app.constants.FromEmail,
 		To:          []string{data.Email},
 		Subject:     app.i18n.Ts("email.data.title"),
 		Body:        msg.Bytes(),
-		Attachments: []messenger.Attachment{
+		Attachments: []models.Attachment{
 			{
 				Name:    fname,
 				Content: b,
-				Header:  messenger.MakeAttachmentHeader(fname, "base64"),
+				Header:  manager.MakeAttachmentHeader(fname, "base64"),
 			},
 		},
 	}); err != nil {
diff --git a/cmd/settings.go b/cmd/settings.go
index d8bf7f5f..563038ee 100644
--- a/cmd/settings.go
+++ b/cmd/settings.go
@@ -13,7 +13,6 @@ import (
 	"github.com/knadh/koanf/parsers/json"
 	"github.com/knadh/koanf/providers/rawbytes"
 	"github.com/knadh/koanf/v2"
-	"github.com/knadh/listmonk/internal/messenger"
 	"github.com/knadh/listmonk/internal/messenger/email"
 	"github.com/knadh/listmonk/models"
 	"github.com/labstack/echo/v4"
@@ -250,7 +249,7 @@ func handleTestSMTPSettings(c echo.Context) error {
 		return err
 	}
 
-	m := messenger.Message{}
+	m := models.Message{}
 	m.ContentType = app.notifTpls.contentType
 	m.From = app.constants.FromEmail
 	m.To = []string{to}
diff --git a/cmd/tx.go b/cmd/tx.go
index cb27900f..a54b9467 100644
--- a/cmd/tx.go
+++ b/cmd/tx.go
@@ -9,7 +9,6 @@ import (
 	"strings"
 
 	"github.com/knadh/listmonk/internal/manager"
-	"github.com/knadh/listmonk/internal/messenger"
 	"github.com/knadh/listmonk/models"
 	"github.com/labstack/echo/v4"
 )
@@ -56,9 +55,9 @@ func handleSendTxMessage(c echo.Context) error {
 					app.i18n.Ts("globals.messages.invalidFields", "name", fmt.Sprintf("file: %s", err.Error())))
 			}
 
-			m.Attachments = append(m.Attachments, models.TxAttachment{
+			m.Attachments = append(m.Attachments, models.Attachment{
 				Name:    f.Filename,
-				Header:  messenger.MakeAttachmentHeader(f.Filename, "base64"),
+				Header:  manager.MakeAttachmentHeader(f.Filename, "base64"),
 				Content: b,
 			})
 		}
@@ -121,7 +120,7 @@ func handleSendTxMessage(c echo.Context) error {
 		}
 
 		// Prepare the final message.
-		msg := manager.Message{}
+		msg := models.Message{}
 		msg.Subscriber = sub
 		msg.To = []string{sub.Email}
 		msg.From = m.FromEmail
@@ -130,7 +129,7 @@ func handleSendTxMessage(c echo.Context) error {
 		msg.Messenger = m.Messenger
 		msg.Body = m.Body
 		for _, a := range m.Attachments {
-			msg.Attachments = append(msg.Attachments, messenger.Attachment{
+			msg.Attachments = append(msg.Attachments, models.Attachment{
 				Name:    a.Name,
 				Header:  a.Header,
 				Content: a.Content,
diff --git a/internal/manager/manager.go b/internal/manager/manager.go
index 945a5dc4..12a7d4a6 100644
--- a/internal/manager/manager.go
+++ b/internal/manager/manager.go
@@ -13,7 +13,6 @@ import (
 
 	"github.com/Masterminds/sprig/v3"
 	"github.com/knadh/listmonk/internal/i18n"
-	"github.com/knadh/listmonk/internal/messenger"
 	"github.com/knadh/listmonk/models"
 	"github.com/paulbellamy/ratecounter"
 )
@@ -40,6 +39,15 @@ type Store interface {
 	DeleteSubscriber(id int64) error
 }
 
+// Messenger is an interface for a generic messaging backend,
+// for instance, e-mail, SMS etc.
+type Messenger interface {
+	Name() string
+	Push(models.Message) error
+	Flush() error
+	Close() error
+}
+
 // CampStats contains campaign stats like per minute send rate.
 type CampStats struct {
 	SendRate int
@@ -51,7 +59,7 @@ type Manager struct {
 	cfg        Config
 	store      Store
 	i18n       *i18n.I18n
-	messengers map[string]messenger.Messenger
+	messengers map[string]Messenger
 	notifCB    models.AdminNotifCallback
 	logger     *log.Logger
 
@@ -73,7 +81,7 @@ type Manager struct {
 	campMsgQueue       chan CampaignMessage
 	campMsgErrorQueue  chan msgError
 	campMsgErrorCounts map[int]int
-	msgQueue           chan Message
+	msgQueue           chan models.Message
 
 	// Sliding window keeps track of the total number of messages sent in a period
 	// and on reaching the specified limit, waits until the window is over before
@@ -98,14 +106,6 @@ type CampaignMessage struct {
 	unsubURL string
 }
 
-// Message represents a generic message to be pushed to a messenger.
-type Message struct {
-	messenger.Message
-
-	// Messenger is the messenger backend to use: email|postback.
-	Messenger string
-}
-
 // Config has parameters for configuring the manager.
 type Config struct {
 	// Number of subscribers to pull from the DB in a single iteration.
@@ -163,14 +163,14 @@ func New(cfg Config, store Store, notifCB models.AdminNotifCallback, i *i18n.I18
 		i18n:               i,
 		notifCB:            notifCB,
 		logger:             l,
-		messengers:         make(map[string]messenger.Messenger),
+		messengers:         make(map[string]Messenger),
 		camps:              make(map[int]*models.Campaign),
 		campRates:          make(map[int]*ratecounter.RateCounter),
 		tpls:               make(map[int]*models.Template),
 		links:              make(map[string]string),
 		subFetchQueue:      make(chan *models.Campaign, cfg.Concurrency),
 		campMsgQueue:       make(chan CampaignMessage, cfg.Concurrency*2),
-		msgQueue:           make(chan Message, cfg.Concurrency),
+		msgQueue:           make(chan models.Message, cfg.Concurrency),
 		campMsgErrorQueue:  make(chan msgError, cfg.MaxSendErrors),
 		campMsgErrorCounts: make(map[int]int),
 		slidingWindowStart: time.Now(),
@@ -202,7 +202,7 @@ func (m *Manager) NewCampaignMessage(c *models.Campaign, s models.Subscriber) (C
 }
 
 // AddMessenger adds a Messenger messaging backend to the manager.
-func (m *Manager) AddMessenger(msg messenger.Messenger) error {
+func (m *Manager) AddMessenger(msg Messenger) error {
 	id := msg.Name()
 	if _, ok := m.messengers[id]; ok {
 		return fmt.Errorf("messenger '%s' is already loaded", id)
@@ -213,7 +213,7 @@ func (m *Manager) AddMessenger(msg messenger.Messenger) error {
 
 // PushMessage pushes an arbitrary non-campaign Message to be sent out by the workers.
 // It times out if the queue is busy.
-func (m *Manager) PushMessage(msg Message) error {
+func (m *Manager) PushMessage(msg models.Message) error {
 	t := time.NewTicker(pushTimeout)
 	defer t.Stop()
 
@@ -355,7 +355,7 @@ func (m *Manager) worker() {
 			numMsg++
 
 			// Outgoing message.
-			out := messenger.Message{
+			out := models.Message{
 				From:        msg.from,
 				To:          []string{msg.to},
 				Subject:     msg.subject,
@@ -410,7 +410,7 @@ func (m *Manager) worker() {
 				return
 			}
 
-			err := m.messengers[msg.Messenger].Push(msg.Message)
+			err := m.messengers[msg.Messenger].Push(msg)
 			if err != nil {
 				m.logger.Printf("error sending message '%s': %v", msg.Subject, err)
 			}
@@ -801,3 +801,17 @@ func (m *Manager) makeGnericFuncMap() template.FuncMap {
 
 	return f
 }
+
+// MakeAttachmentHeader is a helper function that returns a
+// textproto.MIMEHeader tailored for attachments, primarily
+// email. If no encoding is given, base64 is assumed.
+func MakeAttachmentHeader(filename, encoding string) textproto.MIMEHeader {
+	if encoding == "" {
+		encoding = "base64"
+	}
+	h := textproto.MIMEHeader{}
+	h.Set("Content-Disposition", "attachment; filename="+filename)
+	h.Set("Content-Type", "application/json; name=\""+filename+"\"")
+	h.Set("Content-Transfer-Encoding", encoding)
+	return h
+}
diff --git a/internal/messenger/email/email.go b/internal/messenger/email/email.go
index 90a41f80..ecb56b40 100644
--- a/internal/messenger/email/email.go
+++ b/internal/messenger/email/email.go
@@ -7,7 +7,7 @@ import (
 	"net/smtp"
 	"net/textproto"
 
-	"github.com/knadh/listmonk/internal/messenger"
+	"github.com/knadh/listmonk/models"
 	"github.com/knadh/smtppool"
 )
 
@@ -92,7 +92,7 @@ func (e *Emailer) Name() string {
 }
 
 // Push pushes a message to the server.
-func (e *Emailer) Push(m messenger.Message) error {
+func (e *Emailer) Push(m models.Message) error {
 	// If there are more than one SMTP servers, send to a random
 	// one from the list.
 	var (
diff --git a/internal/messenger/messenger.go b/internal/messenger/messenger.go
deleted file mode 100644
index 03350bd0..00000000
--- a/internal/messenger/messenger.go
+++ /dev/null
@@ -1,55 +0,0 @@
-package messenger
-
-import (
-	"net/textproto"
-
-	"github.com/knadh/listmonk/models"
-)
-
-// Messenger is an interface for a generic messaging backend,
-// for instance, e-mail, SMS etc.
-type Messenger interface {
-	Name() string
-	Push(Message) error
-	Flush() error
-	Close() error
-}
-
-// Message is the message pushed to a Messenger.
-type Message struct {
-	From        string
-	To          []string
-	Subject     string
-	ContentType string
-	Body        []byte
-	AltBody     []byte
-	Headers     textproto.MIMEHeader
-	Attachments []Attachment
-
-	Subscriber models.Subscriber
-
-	// Campaign is generally the same instance for a large number of subscribers.
-	Campaign *models.Campaign
-}
-
-// Attachment represents a file or blob attachment that can be
-// sent along with a message by a Messenger.
-type Attachment struct {
-	Name    string
-	Header  textproto.MIMEHeader
-	Content []byte
-}
-
-// MakeAttachmentHeader is a helper function that returns a
-// textproto.MIMEHeader tailored for attachments, primarily
-// email. If no encoding is given, base64 is assumed.
-func MakeAttachmentHeader(filename, encoding string) textproto.MIMEHeader {
-	if encoding == "" {
-		encoding = "base64"
-	}
-	h := textproto.MIMEHeader{}
-	h.Set("Content-Disposition", "attachment; filename="+filename)
-	h.Set("Content-Type", "application/json; name=\""+filename+"\"")
-	h.Set("Content-Transfer-Encoding", encoding)
-	return h
-}
diff --git a/internal/messenger/postback/postback.go b/internal/messenger/postback/postback.go
index b6ad26bf..01c6f756 100644
--- a/internal/messenger/postback/postback.go
+++ b/internal/messenger/postback/postback.go
@@ -10,11 +10,11 @@ import (
 	"net/textproto"
 	"time"
 
-	"github.com/knadh/listmonk/internal/messenger"
 	"github.com/knadh/listmonk/models"
 )
 
 // postback is the payload that's posted as JSON to the HTTP Postback server.
+//
 //easyjson:json
 type postback struct {
 	Subject     string       `json:"subject"`
@@ -34,11 +34,11 @@ type campaign struct {
 }
 
 type recipient struct {
-	UUID    string                   `json:"uuid"`
-	Email   string                   `json:"email"`
-	Name    string                   `json:"name"`
+	UUID    string      `json:"uuid"`
+	Email   string      `json:"email"`
+	Name    string      `json:"name"`
 	Attribs models.JSON `json:"attribs"`
-	Status  string                   `json:"status"`
+	Status  string      `json:"status"`
 }
 
 type attachment struct {
@@ -94,7 +94,7 @@ func (p *Postback) Name() string {
 }
 
 // Push pushes a message to the server.
-func (p *Postback) Push(m messenger.Message) error {
+func (p *Postback) Push(m models.Message) error {
 	pb := postback{
 		Subject:     m.Subject,
 		ContentType: m.ContentType,
diff --git a/models/models.go b/models/models.go
index d71f2892..39aad10b 100644
--- a/models/models.go
+++ b/models/models.go
@@ -347,6 +347,34 @@ type Bounce struct {
 	Total int `db:"total" json:"-"`
 }
 
+// Message is the message pushed to a Messenger.
+type Message struct {
+	From        string
+	To          []string
+	Subject     string
+	ContentType string
+	Body        []byte
+	AltBody     []byte
+	Headers     textproto.MIMEHeader
+	Attachments []Attachment
+
+	Subscriber Subscriber
+
+	// Campaign is generally the same instance for a large number of subscribers.
+	Campaign *Campaign
+
+	// Messenger is the messenger backend to use: email|postback.
+	Messenger string
+}
+
+// Attachment represents a file or blob attachment that can be
+// sent along with a message by a Messenger.
+type Attachment struct {
+	Name    string
+	Header  textproto.MIMEHeader
+	Content []byte
+}
+
 // TxMessage represents an e-mail campaign.
 type TxMessage struct {
 	SubscriberEmails []string `json:"subscriber_emails"`
@@ -364,7 +392,7 @@ type TxMessage struct {
 	Messenger   string                 `json:"messenger"`
 
 	// File attachments added from multi-part form data.
-	Attachments []TxAttachment `json:"-"`
+	Attachments []Attachment `json:"-"`
 
 	Subject    string             `json:"-"`
 	Body       []byte             `json:"-"`
@@ -372,13 +400,6 @@ type TxMessage struct {
 	SubjectTpl *txttpl.Template   `json:"-"`
 }
 
-// TxAttachment is used by TxMessage, consists of FileName and file Content in bytes
-type TxAttachment struct {
-	Name    string
-	Header  textproto.MIMEHeader
-	Content []byte
-}
-
 // markdown is a global instance of Markdown parser and renderer.
 var markdown = goldmark.New(
 	goldmark.WithParserOptions(