* edit email_worker: add Sender and OrcidService interfaces for tests

* add test for successful ProcessPending flow

Author: eltouma

src/email_worker.go

@@ -8,15 +8,25 @@ import (
 	"net/textproto"
 )
 
+type Sender interface {
+	Send(to string, subject string, bodyText string, bodyHTML string) error
+}
+
+type OrcidService interface {
+	GetEmail(ctx context.Context, orcid string) (string, error)
+}
+
 type EmailWorker struct {
 	emailQueueRepo EmailQueueRepository
-	emailSender    *EmailSender
+	emailSender    Sender
+	orcidService   OrcidService
 }
 
-func NewEmailWorker(emailQueueRepo EmailQueueRepository, emailSender *EmailSender) *EmailWorker {
+func NewEmailWorker(emailQueueRepo EmailQueueRepository, emailSender Sender, orcidService OrcidService) *EmailWorker {
 	return &EmailWorker{
 		emailQueueRepo: emailQueueRepo,
 		emailSender:    emailSender,
+		orcidService:   orcidService,
 	}
 }
 
@@ -66,7 +76,7 @@ func (w *EmailWorker) ProcessPending(ctx context.Context, limit int) error {
 	}
 
 	for _, pending := range pendingEmails {
-		recipientEmail, err := GetEmail(ctx, pending.RecipientOrcid)
+		recipientEmail, err := w.orcidService.GetEmail(ctx, pending.RecipientOrcid)
 		if err != nil {
 			if markErr := w.retryOrFail(ctx, pending, "recipient email resolution", err); markErr != nil {
 				return markErr

src/email_worker_test.go

@@ -1,10 +1,10 @@
 package main
 
 import (
+	"context"
 	"fmt"
 	"net/textproto"
 	"testing"
-    "context"
 )
 
 const smtpPermanentErr = 530
@@ -15,37 +15,72 @@ type MockEmailQueueRepository struct {
 	markForRetryCalled bool
 	id                 int64
 	lastError          string
+	getPendingCalled   bool
+	pendingToReturn    []EmailQueue
+	markAsSentCalled   bool
 }
 
+type MockEmailSender struct {
+	sendCalled bool
+	subject    string
+	bodyText   string
+	bodyHTML   string
+	to         string
+}
+
+type MockOrcidService struct {
+	getEmailCalled bool
+	orcidReceived  string
+	emailToReturn  string
+	errToReturn    error
+}
 
 func (m *MockEmailQueueRepository) Enqueue(ctx context.Context, item *EmailQueue) (*EmailQueue, error) {
 	return nil, nil
 }
 
 func (m *MockEmailQueueRepository) GetPending(ctx context.Context, limit int) ([]EmailQueue, error) {
-	return []EmailQueue{}, nil
+	m.getPendingCalled = true
+	return m.pendingToReturn, nil
 }
 
 func (m *MockEmailQueueRepository) MarkAsSent(ctx context.Context, id int64) error {
+	m.markAsSentCalled = true
+	m.id = id
 	return nil
 }
 
 func (m *MockEmailQueueRepository) MarkAsFailed(ctx context.Context, id int64, errMsg string) error {
 	m.markAsFailedCalled = true
-    m.id = id
-    m.lastError = errMsg
+	m.id = id
+	m.lastError = errMsg
 	return nil
 }
 
 func (m *MockEmailQueueRepository) MarkForRetry(ctx context.Context, id int64, errMsg string) error {
 	m.markForRetryCalled = true
-    m.id = id
-    m.lastError = errMsg
+	m.id = id
+	m.lastError = errMsg
 	return nil
 }
 
+func (m *MockEmailSender) Send(to string, subject string, bodyText string, bodyHTML string) error {
+	m.sendCalled = true
+	m.to = to
+	m.subject = subject
+	m.bodyText = bodyText
+	m.bodyHTML = bodyHTML
+	return nil
+}
+
+func (m *MockOrcidService) GetEmail(ctx context.Context, orcid string) (string, error) {
+	m.getEmailCalled = true
+	m.orcidReceived = orcid
+	return m.emailToReturn, m.errToReturn
+}
+
 func TestIsSMTPPermanentErr(t *testing.T) {
-    err := fmt.Errorf("error: %w", &textproto.Error{
+	err := fmt.Errorf("error: %w", &textproto.Error{
 		Code: smtpPermanentErr,
 		Msg:  "authentication required",
 	})
@@ -57,7 +92,7 @@ func TestIsSMTPPermanentErr(t *testing.T) {
 }
 
 func TestIsSMTPTemporaryErr(t *testing.T) {
-    err := fmt.Errorf("error: %w", &textproto.Error{
+	err := fmt.Errorf("error: %w", &textproto.Error{
 		Code: smtpTemporaryErr,
 		Msg:  "domain not found",
 	})
@@ -68,77 +103,154 @@ func TestIsSMTPTemporaryErr(t *testing.T) {
 	}
 }
 
-
 func TestRetryOrFailSMTPPermanentError(t *testing.T) {
-    ctx := context.Background()
+	ctx := context.Background()
 
-    mockRepo := &MockEmailQueueRepository{}
+	mockRepo := &MockEmailQueueRepository{}
 
-    worker := &EmailWorker{
-        emailQueueRepo: mockRepo,
-    }
+	worker := &EmailWorker{
+		emailQueueRepo: mockRepo,
+	}
 
-    pending := EmailQueue{
-        Id: 2,
-        Attempts: 0,
-    }
+	pending := EmailQueue{
+		Id:       2,
+		Attempts: 0,
+	}
 
-    smtpErr := fmt.Errorf("error: %w", &textproto.Error{
+	smtpErr := fmt.Errorf("error: %w", &textproto.Error{
 		Code: smtpPermanentErr,
 		Msg:  "authentication required",
 	})
 
-    err := worker.retryOrFail(ctx, pending, "send", smtpErr)
-    if err != nil {
-        t.Fatalf("expected retryOrFail to succeed, got error: %v", err)
-    }
+	err := worker.retryOrFail(ctx, pending, "send", smtpErr)
+	if err != nil {
+		t.Fatalf("expected retryOrFail to succeed, got error: %v", err)
+	}
 
-    if !mockRepo.markAsFailedCalled {
-        t.Fatal("expected MarkAsFailed to be called")
-    }
+	if !mockRepo.markAsFailedCalled {
+		t.Fatal("expected MarkAsFailed to be called")
+	}
 
-    if mockRepo.markForRetryCalled {
-        t.Fatal("expected MarkForRetry not to be called")
-    }
+	if mockRepo.markForRetryCalled {
+		t.Fatal("expected MarkForRetry not to be called")
+	}
 
-    if mockRepo.id != pending.Id {
-        t.Fatalf("expected queue id %d, got %d", pending.Id, mockRepo.id)
-    }
+	if mockRepo.id != pending.Id {
+		t.Fatalf("expected queue id: %d, got: %d", pending.Id, mockRepo.id)
+	}
 }
 
 func TestRetryOrFailSMTPTemporaryError(t *testing.T) {
-    ctx := context.Background()
+	ctx := context.Background()
 
-    mockRepo := &MockEmailQueueRepository{}
+	mockRepo := &MockEmailQueueRepository{}
 
-    worker := &EmailWorker{
-        emailQueueRepo: mockRepo,
-    }
+	worker := &EmailWorker{
+		emailQueueRepo: mockRepo,
+	}
 
-    pending := EmailQueue{
-        Id: 2,
-        Attempts: 0,
-    }
+	pending := EmailQueue{
+		Id:       2,
+		Attempts: 0,
+	}
 
-    smtpErr := fmt.Errorf("error: %w", &textproto.Error{
+	smtpErr := fmt.Errorf("error: %w", &textproto.Error{
 		Code: smtpTemporaryErr,
 		Msg:  "domain not found",
 	})
 
-    err := worker.retryOrFail(ctx, pending, "send", smtpErr)
-    if err != nil {
-        t.Fatalf("expected retryOrFail to succeed, got error: %v", err)
-    }
+	err := worker.retryOrFail(ctx, pending, "send", smtpErr)
+	if err != nil {
+		t.Fatalf("expected retryOrFail to succeed, got error: %v", err)
+	}
+
+	if !mockRepo.markForRetryCalled {
+		t.Fatal("expected MarkForRetry to be called")
+	}
+
+	if mockRepo.markAsFailedCalled {
+		t.Fatal("expected MarkAsFailed not to be called")
+	}
+
+	if mockRepo.id != pending.Id {
+		t.Fatalf("expected queue id: %d, got: %d", pending.Id, mockRepo.id)
+	}
+}
 
-    if !mockRepo.markForRetryCalled {
-        t.Fatal("expected MarkForRetry to be called")
-    }
+func TestProcessPendingMarkAsSentWhenSuccess(t *testing.T) {
+	ctx := context.Background()
 
-    if mockRepo.markAsFailedCalled {
-        t.Fatal("expected MarkAsFailed not to be called")
-    }
+	mockRepo := &MockEmailQueueRepository{}
+	mockSender := &MockEmailSender{}
+	mockOrcid := &MockOrcidService{}
 
-    if mockRepo.id != pending.Id {
-        t.Fatalf("expected queue id %d, got %d", pending.Id, mockRepo.id)
-    }
+	worker := &EmailWorker{
+		emailQueueRepo: mockRepo,
+		emailSender:    mockSender,
+		orcidService:   mockOrcid,
+	}
+
+	pending := EmailQueue{
+		Id:             2,
+		RecipientOrcid: "0000-0000-0000-0000",
+		Subject:        "Test",
+		BodyText:       "Body test",
+		BodyHTML:       "<p>Body test</p>",
+	}
+
+	mockRepo.pendingToReturn = []EmailQueue{pending}
+	mockOrcid.emailToReturn = "test@test.com"
+
+	err := worker.ProcessPending(ctx, 1)
+	if err != nil {
+		t.Fatalf("expected ProcessPending to succeed, got error: %v", err)
+	}
+
+	if !mockRepo.getPendingCalled {
+		t.Fatal("expected GetPending to be called")
+	}
+
+	if mockRepo.id != pending.Id {
+		t.Fatalf("expected queue id: %d, got: %d", pending.Id, mockRepo.id)
+	}
+
+	if !mockOrcid.getEmailCalled {
+		t.Fatal("expected getEmail to be called")
+	}
+
+	if mockOrcid.orcidReceived != pending.RecipientOrcid {
+		t.Fatalf("expected orcid: %q, got: %q", pending.RecipientOrcid, mockOrcid.orcidReceived)
+	}
+
+	if !mockSender.sendCalled {
+		t.Fatal("expected Send to be called")
+	}
+
+	if mockSender.to != mockOrcid.emailToReturn {
+		t.Fatalf("expected email: %q, got: %q", mockOrcid.emailToReturn, mockSender.to)
+	}
+
+	if mockSender.subject != pending.Subject {
+		t.Fatalf("expected subject: %q, got: %q", pending.Subject, mockSender.subject)
+	}
+
+	if mockSender.bodyText != pending.BodyText {
+		t.Fatalf("expected bodyText: %q, got: %q", pending.BodyText, mockSender.bodyText)
+	}
+
+	if mockSender.bodyHTML != pending.BodyHTML {
+		t.Fatalf("expected bodyHTML: %q, got: %q", pending.BodyHTML, mockSender.bodyHTML)
+	}
+
+	if !mockRepo.markAsSentCalled {
+		t.Fatal("expected MarkAsSent to be called")
+	}
+
+	if mockRepo.markForRetryCalled {
+		t.Fatal("expected MarkForRetry not to be called")
+	}
+
+	if mockRepo.markAsFailedCalled {
+		t.Fatal("expected MarkAsFailed not to be called")
+	}
 }

src/main.go

@@ -474,7 +474,8 @@ func main() {
 	commentRepo := NewPostgresCommentRepository(db)
 	notificationService := NewNotificationService(adminRepo, emailQueueRepo, commentRepo)
 	emailSender := NewEmailSender()
-	emailWorker := NewEmailWorker(emailQueueRepo, emailSender)
+	orcidClient := NewOrcidClient()
+	emailWorker := NewEmailWorker(emailQueueRepo, emailSender, orcidClient)
 	moderationRepo := NewPostgresModerationRepository(db, categoryRepo, rorRepo)
 	moderationHandler := NewModerationHandler(moderationRepo, adminRepo, notificationService, recordRepo)
 	commentHandler := NewCommentHandler(commentRepo, recordRepo, adminRepo, notificationService)

src/orcid_service.go

@@ -9,9 +9,7 @@ import (
 	"time"
 )
 
-type OrcidService interface {
-	GetEmail(ctx context.Context, orcid string) (string, error)
-}
+type OrcidClient struct{}
 
 type HTTPStatusError struct {
 	StatusCode int
@@ -23,6 +21,10 @@ type EmailUnavailable struct {
 	Message string
 }
 
+func NewOrcidClient() *OrcidClient {
+	return &OrcidClient{}
+}
+
 const orcidService = "orcid service"
 
 var orcidHTTPClient = &http.Client{
@@ -38,7 +40,7 @@ func (e *EmailUnavailable) Error() string {
 }
 
 // https://info.orcid.org/documentation/api-tutorials/api-tutorial-read-data-on-a-record
-func GetEmail(ctx context.Context, orcid string) (string, error) {
+func (c *OrcidClient) GetEmail(ctx context.Context, orcid string) (string, error) {
 	if strings.TrimSpace(orcid) == "" {
 		return "", fmt.Errorf("%s: orcid parameter is empty", orcidService)
 	}