From 4e465cb1d9e01213587b7fd42ccf247698cd29ab Mon Sep 17 00:00:00 2001 From: butschster Date: Sat, 22 Nov 2025 18:15:58 +0400 Subject: [PATCH 01/13] refactor: replace WorkerPool with Jobs RPC integration - Remove built-in worker pool in favor of Jobs plugin integration - Add JobsConfig for pipeline, priority, delay, auto_ack settings - Add JobsRPCer interface and Endure dependency injection via Collects() - Modify Session.Data() to push EmailData to Jobs instead of direct worker - Remove Pool/Server interfaces and related fields from Plugin struct - Remove handler.go and workers_manager.go (sendToWorker logic) - Remove RPC methods: AddWorker, RemoveWorker, WorkersList - Remove Reset() and Workers() methods from Plugin - Add pushToJobs() method for Jobs RPC communication - Add unit tests for Jobs integration - Add migration guide documentation Benefits: - Shared worker pool with other Jobs tasks (memory efficiency) - Built-in retry support via task nack() - Priority queues and delayed processing - Better monitoring through Jobs metrics Migration required: - Replace pool config with jobs.pipeline - Update PHP code to use Jobs Consumer instead of Worker - See docs/migration-guide.md for details --- config.go | 24 +- ... SMTP Profiler Plugin - Feature Request.md | 0 ... 1 - Project Foundation & Configuration.md | 0 ...p 2 -Worker Pool & Lifecycle Management.md | 0 ...p 3 - SMTP Server & Connection Handling.md | 0 ... 4 - Email Parsing & Worker Integration.md | 0 docs/migration-guide.md | 222 +++++++++++++++ docs/stages/master-checklist.md | 122 +++++++++ docs/stages/stage-1-config-interfaces.md | 101 +++++++ docs/stages/stage-2-jobs-integration.md | 131 +++++++++ docs/stages/stage-3-remove-workerpool.md | 135 ++++++++++ docs/stages/stage-4-rpc-dependencies.md | 139 ++++++++++ docs/stages/stage-5-tests-docs.md | 252 ++++++++++++++++++ go.mod | 16 +- go.sum | 33 +-- handler.go | 64 ----- jobs_integration_test.go | 239 +++++++++++++++++ jobs_rpc.go | 39 +++ plugin.go | 157 ++++------- rpc.go | 35 --- session.go | 68 +++-- workers_manager.go | 17 -- 22 files changed, 1509 insertions(+), 285 deletions(-) rename docs/{ => Ready/SMTP Server}/RoadRunner SMTP Profiler Plugin - Feature Request.md (100%) rename docs/{ => Ready/SMTP Server}/Step 1 - Project Foundation & Configuration.md (100%) rename docs/{ => Ready/SMTP Server}/Step 2 -Worker Pool & Lifecycle Management.md (100%) rename docs/{ => Ready/SMTP Server}/Step 3 - SMTP Server & Connection Handling.md (100%) rename docs/{ => Ready/SMTP Server}/Step 4 - Email Parsing & Worker Integration.md (100%) create mode 100644 docs/migration-guide.md create mode 100644 docs/stages/master-checklist.md create mode 100644 docs/stages/stage-1-config-interfaces.md create mode 100644 docs/stages/stage-2-jobs-integration.md create mode 100644 docs/stages/stage-3-remove-workerpool.md create mode 100644 docs/stages/stage-4-rpc-dependencies.md create mode 100644 docs/stages/stage-5-tests-docs.md delete mode 100644 handler.go create mode 100644 jobs_integration_test.go create mode 100644 jobs_rpc.go delete mode 100644 workers_manager.go diff --git a/config.go b/config.go index a0c62f5..b3dc995 100644 --- a/config.go +++ b/config.go @@ -4,7 +4,6 @@ import ( "time" "github.com/roadrunner-server/errors" - "github.com/roadrunner-server/pool/pool" ) // Config represents SMTP server configuration @@ -19,13 +18,21 @@ type Config struct { // Attachment storage AttachmentStorage AttachmentConfig `mapstructure:"attachment_storage"` - // Worker pool configuration - Pool *pool.Config `mapstructure:"pool"` + // Jobs integration (replaces worker pool) + Jobs JobsConfig `mapstructure:"jobs"` // Include full raw RFC822 message in JSON (default: false) IncludeRaw bool `mapstructure:"include_raw"` } +// JobsConfig configures Jobs plugin integration +type JobsConfig struct { + Pipeline string `mapstructure:"pipeline"` // Target pipeline in Jobs + Priority int64 `mapstructure:"priority"` // Default priority for jobs + Delay int64 `mapstructure:"delay"` // Default delay (0 = immediate) + AutoAck bool `mapstructure:"auto_ack"` // Auto-acknowledge jobs +} + // AttachmentConfig configures how attachments are stored type AttachmentConfig struct { Mode string `mapstructure:"mode"` // "memory" or "tempfile" @@ -68,11 +75,10 @@ func (c *Config) InitDefaults() error { c.AttachmentStorage.CleanupAfter = 1 * time.Hour } - // Pool defaults - if c.Pool == nil { - c.Pool = &pool.Config{} + // Jobs defaults + if c.Jobs.Priority == 0 { + c.Jobs.Priority = 10 } - c.Pool.InitDefaults() return c.validate() } @@ -93,5 +99,9 @@ func (c *Config) validate() error { return errors.E(op, errors.Str("attachment_storage.mode must be 'memory' or 'tempfile'")) } + if c.Jobs.Pipeline == "" { + return errors.E(op, errors.Str("jobs.pipeline is required")) + } + return nil } diff --git a/docs/RoadRunner SMTP Profiler Plugin - Feature Request.md b/docs/Ready/SMTP Server/RoadRunner SMTP Profiler Plugin - Feature Request.md similarity index 100% rename from docs/RoadRunner SMTP Profiler Plugin - Feature Request.md rename to docs/Ready/SMTP Server/RoadRunner SMTP Profiler Plugin - Feature Request.md diff --git a/docs/Step 1 - Project Foundation & Configuration.md b/docs/Ready/SMTP Server/Step 1 - Project Foundation & Configuration.md similarity index 100% rename from docs/Step 1 - Project Foundation & Configuration.md rename to docs/Ready/SMTP Server/Step 1 - Project Foundation & Configuration.md diff --git a/docs/Step 2 -Worker Pool & Lifecycle Management.md b/docs/Ready/SMTP Server/Step 2 -Worker Pool & Lifecycle Management.md similarity index 100% rename from docs/Step 2 -Worker Pool & Lifecycle Management.md rename to docs/Ready/SMTP Server/Step 2 -Worker Pool & Lifecycle Management.md diff --git a/docs/Step 3 - SMTP Server & Connection Handling.md b/docs/Ready/SMTP Server/Step 3 - SMTP Server & Connection Handling.md similarity index 100% rename from docs/Step 3 - SMTP Server & Connection Handling.md rename to docs/Ready/SMTP Server/Step 3 - SMTP Server & Connection Handling.md diff --git a/docs/Step 4 - Email Parsing & Worker Integration.md b/docs/Ready/SMTP Server/Step 4 - Email Parsing & Worker Integration.md similarity index 100% rename from docs/Step 4 - Email Parsing & Worker Integration.md rename to docs/Ready/SMTP Server/Step 4 - Email Parsing & Worker Integration.md diff --git a/docs/migration-guide.md b/docs/migration-guide.md new file mode 100644 index 0000000..e940cfd --- /dev/null +++ b/docs/migration-guide.md @@ -0,0 +1,222 @@ +# Migration Guide: SMTP Plugin v1 to v2 (Jobs Integration) + +## Overview + +SMTP plugin v2 removes the built-in worker pool and integrates with the Jobs plugin for email processing. This provides better resource utilization, retry capabilities, and unified task management. + +## Breaking Changes + +### Removed Features +- Worker pool configuration (`pool` section) +- RPC methods: `AddWorker`, `RemoveWorker`, `WorkersList` +- Direct worker communication + +### New Requirements +- Jobs plugin must be enabled +- Pipeline must be configured in Jobs +- PHP code must use Jobs consumer instead of direct worker + +## Configuration Changes + +### Before (v1) + +```yaml +smtp: + addr: "127.0.0.1:1025" + hostname: "localhost" + read_timeout: 60s + write_timeout: 10s + max_message_size: 10485760 + + pool: + num_workers: 4 + max_jobs: 100 + + attachment_storage: + mode: "memory" +``` + +### After (v2) + +```yaml +smtp: + addr: "127.0.0.1:1025" + hostname: "localhost" + read_timeout: 60s + write_timeout: 10s + max_message_size: 10485760 + + jobs: + pipeline: "smtp-emails" + priority: 10 + delay: 0 + auto_ack: false + + attachment_storage: + mode: "memory" + +# Jobs plugin configuration +jobs: + consume: ["smtp-emails"] + + pipelines: + smtp-emails: + driver: memory + config: + priority: 10 + prefetch: 100 +``` + +## PHP Code Changes + +### Before: Direct Worker + +```php +waitPayload()) { + $email = json_decode($payload->body, true); + + // Process email + processEmail($email); + + // Respond to worker + $worker->respond(new Payload('CONTINUE')); +} +``` + +### After: Jobs Consumer + +```php +waitTask()) { + try { + $email = json_decode($task->getPayload(), true); + + // Process email + processEmail($email); + + // Acknowledge successful processing + $task->ack(); + + } catch (\Throwable $e) { + // Negative acknowledge - will retry + $task->nack($e); + } +} +``` + +## Email Payload Structure + +The email payload structure remains the same: + +```json +{ + "event": "EMAIL_RECEIVED", + "uuid": "connection-uuid", + "remote_addr": "127.0.0.1:12345", + "received_at": "2024-01-15T10:30:00Z", + "envelope": { + "from": "sender@example.com", + "to": ["recipient@example.com"], + "helo": "mail.example.com" + }, + "authentication": { + "attempted": true, + "mechanism": "PLAIN", + "username": "user", + "password": "pass" + }, + "message": { + "headers": { + "Subject": ["Test Email"] + }, + "body": "Email body text", + "raw": "Full RFC822 message (if include_raw: true)" + }, + "attachments": [ + { + "filename": "document.pdf", + "content_type": "application/pdf", + "size": 12345, + "content": "base64-encoded-content" + } + ] +} +``` + +## Migration Steps + +1. **Update RoadRunner configuration** + - Remove `pool` section from `smtp` + - Add `jobs` section with pipeline name + - Add Jobs plugin configuration with pipeline + +2. **Enable Jobs plugin** + - Ensure Jobs plugin is in your RoadRunner build + - Configure at least one pipeline for SMTP emails + +3. **Update PHP code** + - Replace `Worker::create()` with `new Consumer()` + - Replace `waitPayload()` with `waitTask()` + - Replace `respond()` with `ack()` or `nack()` + +4. **Test the migration** + - Start RoadRunner with new configuration + - Send test emails + - Verify emails appear in Jobs pipeline + - Verify PHP consumer processes emails + +5. **Remove old code** + - Remove any worker pool specific error handling + - Update monitoring/metrics if applicable + +## Benefits of Migration + +1. **Resource Efficiency**: Shared worker pool with other Jobs tasks +2. **Retry Support**: Automatic retries on failure with `nack()` +3. **Priority Queues**: Prioritize important emails +4. **Delayed Processing**: Schedule email processing +5. **Better Monitoring**: Jobs metrics and status +6. **Graceful Shutdown**: Proper task completion on shutdown + +## Troubleshooting + +### SMTP plugin fails to start + +**Error**: `jobs plugin not available` + +**Solution**: Ensure Jobs plugin is enabled and configured: +```yaml +jobs: + consume: ["smtp-emails"] + pipelines: + smtp-emails: + driver: memory +``` + +### Emails not being processed + +**Check**: +1. Pipeline name matches in SMTP and Jobs config +2. Jobs consumer is running +3. Pipeline is in `consume` list + +### RPC methods not available + +Worker management RPC methods (`AddWorker`, `RemoveWorker`, `WorkersList`) are removed. Use Jobs plugin RPC for task management instead. + +## Questions? + +For issues or questions, please open an issue on the repository. diff --git a/docs/stages/master-checklist.md b/docs/stages/master-checklist.md new file mode 100644 index 0000000..4caf7d6 --- /dev/null +++ b/docs/stages/master-checklist.md @@ -0,0 +1,122 @@ +# Master Checklist: SMTP Plugin Jobs Integration + +## Общий прогресс + +| Этап | Описание | Статус | +|------|----------|--------| +| 1 | Конфигурация и интерфейсы | ⬜ | +| 2 | Jobs интеграция | ⬜ | +| 3 | Удаление WorkerPool | ⬜ | +| 4 | Обновление RPC и зависимостей | ⬜ | +| 5 | Тесты и документация | ⬜ | + +--- + +## Этап 1: Конфигурация и интерфейсы + +### 1.1 Структуры конфигурации +- [ ] Добавить `JobsConfig` структуру в `config.go` +- [ ] Добавить поле `Jobs JobsConfig` в `Config` структуру +- [ ] Реализовать `InitDefaults()` для Jobs конфигурации +- [ ] Добавить валидацию `jobs.pipeline` в `validate()` + +### 1.2 Интерфейс Jobs RPC +- [ ] Создать файл `jobs_rpc.go` с интерфейсом `JobsRPCer` +- [ ] Определить методы `Push()` и `PushBatch()` +- [ ] Добавить поле `jobsRPC JobsRPCer` в `Plugin` структуру + +### 1.3 Структура EmailJob +- [ ] Создать структуру `EmailJob` в `types.go` +- [ ] Реализовать метод `ToJobsRequest()` +- [ ] Добавить конвертацию из `ParsedMessage` + +--- + +## Этап 2: Jobs интеграция + +### 2.1 Push метод +- [ ] Реализовать `pushToJobs()` в `plugin.go` +- [ ] Добавить обработку ошибок RPC +- [ ] Добавить логирование + +### 2.2 Модификация Session.Data() +- [ ] Изменить `session.go:Data()` для вызова `pushToJobs()` +- [ ] Заменить вызов `sendToWorker()` на Jobs push +- [ ] Обновить обработку ошибок + +### 2.3 Batch processing (опционально) +- [ ] Добавить буферизацию для batch push +- [ ] Реализовать flush по таймеру/размеру + +--- + +## Этап 3: Удаление WorkerPool + +### 3.1 Удаление из Plugin +- [ ] Удалить поле `wPool Pool` из `Plugin` структуры +- [ ] Удалить поле `pldPool sync.Pool` +- [ ] Удалить поле `server Server` +- [ ] Удалить интерфейс `Pool` +- [ ] Удалить интерфейс `Server` + +### 3.2 Рефакторинг Serve() +- [ ] Удалить создание worker pool (строка 121) +- [ ] Удалить `defer p.wPool.Destroy()` +- [ ] Обновить проверку зависимости на Jobs + +### 3.3 Рефакторинг Stop() +- [ ] Удалить вызов `p.wPool.Destroy()` +- [ ] Упростить логику остановки + +### 3.4 Удаление файлов +- [ ] Удалить `handler.go` (sendToWorker) +- [ ] Удалить `workers_manager.go` + +--- + +## Этап 4: Обновление RPC и зависимостей + +### 4.1 Удаление RPC методов +- [ ] Удалить `AddWorker` из `rpc.go` +- [ ] Удалить `RemoveWorker` из `rpc.go` +- [ ] Удалить `WorkersList` из `rpc.go` + +### 4.2 Endure интеграция +- [ ] Добавить `Collects()` метод для Jobs зависимости +- [ ] Обновить `Init()` для проверки конфигурации +- [ ] Обновить `Serve()` для проверки jobsRPC + +### 4.3 Удаление неиспользуемого кода +- [ ] Удалить `Reset()` метод +- [ ] Удалить `Workers()` метод +- [ ] Очистить импорты + +--- + +## Этап 5: Тесты и документация + +### 5.1 Unit тесты +- [ ] Тест конвертации `EmailJob.ToJobsRequest()` +- [ ] Тест валидации конфигурации +- [ ] Тест `pushToJobs()` с mock RPC + +### 5.2 Integration тесты +- [ ] Тест отправки email через SMTP +- [ ] Тест появления задачи в Jobs +- [ ] Тест обработки ошибок + +### 5.3 Документация +- [ ] Обновить README.md +- [ ] Создать migration-guide.md +- [ ] Добавить примеры конфигурации +- [ ] Обновить PHP consumer пример + +--- + +## Критерии завершения проекта + +- [ ] Все тесты проходят +- [ ] Нет worker pool кода +- [ ] SMTP плагин отправляет email в Jobs +- [ ] Документация обновлена +- [ ] Backward compatibility guide готов diff --git a/docs/stages/stage-1-config-interfaces.md b/docs/stages/stage-1-config-interfaces.md new file mode 100644 index 0000000..ca182a4 --- /dev/null +++ b/docs/stages/stage-1-config-interfaces.md @@ -0,0 +1,101 @@ +# Этап 1: Конфигурация и интерфейсы + +## Описание +Добавление новых структур конфигурации для Jobs интеграции и определение интерфейсов для RPC взаимодействия. Существующий функционал не изменяется — только расширяется. + +## Файлы для изменения + +### 1. `config.go` — добавление Jobs конфигурации + +**Текущий код:** строки 11-27 (Config struct) + +**Изменения:** +```go +// Добавить после строки 27 +type JobsConfig struct { + Pipeline string `mapstructure:"pipeline"` + Priority int64 `mapstructure:"priority"` + Delay int64 `mapstructure:"delay"` + AutoAck bool `mapstructure:"auto_ack"` +} + +// Добавить поле в Config struct (строка ~20) +Jobs JobsConfig `mapstructure:"jobs"` +``` + +**InitDefaults:** добавить после строки 78 +```go +// Jobs defaults +if c.Jobs.Priority == 0 { + c.Jobs.Priority = 10 +} +``` + +**validate:** добавить валидацию pipeline (строка ~85) + +--- + +### 2. `jobs_rpc.go` — новый файл + +**Создать файл с:** +- Интерфейс `JobsRPCer` с методами Push/PushBatch +- Protobuf-совместимые структуры запросов + +--- + +### 3. `plugin.go` — расширение Plugin struct + +**Текущий код:** строки 57-71 (Plugin struct) + +**Добавить поле:** +```go +jobsRPC JobsRPCer // строка ~65 +``` + +--- + +### 4. `types.go` — структура EmailJob + +**Добавить после строки 75:** +```go +type EmailJob struct { + UUID string `json:"uuid"` + ReceivedAt time.Time `json:"received_at"` + RemoteAddr string `json:"remote_addr"` + From string `json:"from"` + To []string `json:"to"` + Message ParsedMessage `json:"message"` +} + +func (e *EmailJob) ToJobsRequest(cfg *JobsConfig) *jobsproto.PushRequest { + // Конвертация в protobuf формат +} +``` + +## Файлы для создания + +| Файл | Описание | +|------|----------| +| `jobs_rpc.go` | Интерфейс JobsRPCer и связанные типы | + +## Definition of Done + +- [ ] `JobsConfig` структура добавлена в `config.go` +- [ ] Поле `Jobs` добавлено в `Config` struct +- [ ] `InitDefaults()` устанавливает значения по умолчанию для Jobs +- [ ] `validate()` проверяет обязательность `jobs.pipeline` +- [ ] Файл `jobs_rpc.go` создан с интерфейсом `JobsRPCer` +- [ ] Поле `jobsRPC` добавлено в `Plugin` struct +- [ ] Структура `EmailJob` добавлена в `types.go` +- [ ] Метод `ToJobsRequest()` реализован +- [ ] Код компилируется без ошибок +- [ ] Существующие тесты проходят + +## Зависимости + +- Нет внешних зависимостей +- Требуется добавить import для Jobs protobuf (если используется) + +## Риски + +- **Низкий**: изменения только добавляют новый код, не затрагивают существующий функционал diff --git a/docs/stages/stage-2-jobs-integration.md b/docs/stages/stage-2-jobs-integration.md new file mode 100644 index 0000000..784340b --- /dev/null +++ b/docs/stages/stage-2-jobs-integration.md @@ -0,0 +1,131 @@ +# Этап 2: Jobs интеграция + +## Описание +Реализация отправки email в Jobs через RPC. На этом этапе добавляется параллельный путь обработки — и старый (WorkerPool), и новый (Jobs) работают одновременно. + +## Файлы для изменения + +### 1. `plugin.go` — метод pushToJobs + +**Добавить после строки 220:** +```go +func (p *Plugin) pushToJobs(email *EmailJob) error { + const op = errors.Op("smtp_push_to_jobs") + + if p.jobsRPC == nil { + return errors.E(op, errors.Str("jobs RPC not available")) + } + + req := email.ToJobsRequest(&p.cfg.Jobs) + + var empty jobsproto.Empty + err := p.jobsRPC.Push(req, &empty) + if err != nil { + return errors.E(op, err) + } + + p.log.Debug("email pushed to jobs", + zap.String("uuid", email.UUID), + zap.String("pipeline", p.cfg.Jobs.Pipeline), + ) + + return nil +} +``` + +--- + +### 2. `session.go` — модификация Data() + +**Текущий код:** строки 57-119 + +**Изменить строки 81-114:** + +Заменить вызов `sendToWorker()` на: +```go +// После parseEmail (строка 81) + +// Create job from email +job := &EmailJob{ + UUID: s.uuid, + ReceivedAt: time.Now(), + RemoteAddr: s.remoteAddr, + From: s.from, + To: s.to, + Message: parsedMessage, +} + +// Push to Jobs via RPC +err = s.backend.plugin.pushToJobs(job) +if err != nil { + s.backend.log.Error("failed to push email to jobs", + zap.Error(err), + zap.String("uuid", s.uuid), + ) + return &smtp.SMTPError{ + Code: 451, + EnhancedCode: smtp.EnhancedCode{4, 3, 0}, + Message: "Temporary failure, try again later", + } +} + +return nil +``` + +--- + +### 3. `backend.go` — доступ к plugin + +**Текущий код:** строки 10-13 (Backend struct) + +Уже имеет доступ к plugin — изменения не требуются. + +## Логика переключения + +Для тестирования можно добавить feature flag: + +```go +// В Config +UseJobs bool `mapstructure:"use_jobs"` // временный флаг + +// В Session.Data() +if s.backend.plugin.cfg.UseJobs { + // новый путь через Jobs +} else { + // старый путь через sendToWorker +} +``` + +## Definition of Done + +- [ ] Метод `pushToJobs()` реализован в `plugin.go` +- [ ] `Session.Data()` модифицирован для вызова Jobs +- [ ] Логирование добавлено для отслеживания push операций +- [ ] Ошибки RPC корректно обрабатываются +- [ ] SMTP сервер возвращает 451 при ошибке Jobs +- [ ] Email успешно появляется в Jobs pipeline +- [ ] Код компилируется без ошибок + +## Тестирование + +```bash +# 1. Запустить RoadRunner с Jobs и SMTP плагинами + +# 2. Отправить тестовый email +swaks --to test@localhost --server localhost:1025 + +# 3. Проверить логи на "email pushed to jobs" + +# 4. Проверить Jobs на наличие задачи +``` + +## Зависимости + +- Этап 1 завершён +- Jobs plugin запущен и настроен +- Pipeline создан в конфигурации Jobs + +## Риски + +- **Средний**: Jobs может быть недоступен — требуется обработка ошибок +- **Средний**: Сериализация может быть медленной для больших attachments diff --git a/docs/stages/stage-3-remove-workerpool.md b/docs/stages/stage-3-remove-workerpool.md new file mode 100644 index 0000000..b137b5b --- /dev/null +++ b/docs/stages/stage-3-remove-workerpool.md @@ -0,0 +1,135 @@ +# Этап 3: Удаление WorkerPool + +## Описание +Полное удаление WorkerPool и связанного кода после успешного тестирования Jobs интеграции. Это критический этап — после него откат будет сложным. + +## Файлы для удаления + +| Файл | Причина | +|------|---------| +| `handler.go` | Содержит `sendToWorker()` — больше не нужен | +| `workers_manager.go` | Содержит `AddWorker()`, `RemoveWorker()` — управление пулом | + +## Файлы для изменения + +### 1. `plugin.go` — удаление Pool + +**Удалить интерфейсы (строки 24-54):** +```go +// DELETE: Pool interface (строки 24-37) +// DELETE: Server interface (строки 39-54) +``` + +**Удалить из Plugin struct (строки 57-71):** +```go +// DELETE: server Server (строка ~63) +// DELETE: wPool Pool (строка ~64) +// DELETE: pldPool sync.Pool (строка ~67) +``` + +**Изменить Init() (строки 73-111):** +- Удалить инициализацию `pldPool` (строки 101-109) +- Удалить зависимость от `Server` + +**Изменить Serve() (строки 113-171):** +- Удалить создание пула (строки 121-129) +- Добавить проверку `jobsRPC`: +```go +if p.jobsRPC == nil { + errCh <- errors.E(op, errors.Str("jobs plugin not available")) + return errCh +} +``` + +**Изменить Stop() (строки 173-220):** +- Удалить `p.wPool.Destroy()` (строки 215-217) + +**Удалить методы:** +- `Reset()` (строки 224-245) +- `Workers()` (строки 247-268) + +--- + +### 2. `session.go` — очистка + +**Удалить из Data() (строки 57-119):** +- Весь код связанный с `sendToWorker` (уже заменён на этапе 2) +- Проверить отсутствие ссылок на pool + +--- + +### 3. `backend.go` — без изменений + +Структура Backend не содержит ссылок на pool. + +## Код для удаления из plugin.go + +```go +// Строки 24-37: DELETE +type Pool interface { + Workers() []*process.State + RemoveWorker(ctx context.Context) error + AddWorker() error + Exec(ctx context.Context, p *payload.Payload, stopCh chan struct{}) (chan *staticPool.PExec, error) + Reset(ctx context.Context) error + Destroy(ctx context.Context) +} + +// Строки 39-54: DELETE +type Server interface { + NewPool(ctx context.Context, cfg *pool.Config, env map[string]string, ...) (*staticPool.Pool, error) +} + +// Из Plugin struct: DELETE +server Server +wPool Pool +pldPool sync.Pool + +// Строки 121-129 в Serve(): DELETE +p.wPool, err = p.server.NewPool(...) + +// Строки 215-217 в Stop(): DELETE +if p.wPool != nil { + p.wPool.Destroy(ctx) +} + +// Строки 224-268: DELETE полностью +func (p *Plugin) Reset() error { ... } +func (p *Plugin) Workers() []*process.State { ... } +``` + +## Definition of Done + +- [ ] Файл `handler.go` удалён +- [ ] Файл `workers_manager.go` удалён +- [ ] Интерфейсы `Pool` и `Server` удалены из `plugin.go` +- [ ] Поля `server`, `wPool`, `pldPool` удалены из `Plugin` +- [ ] Методы `Reset()` и `Workers()` удалены +- [ ] `Serve()` проверяет наличие `jobsRPC` +- [ ] `Stop()` не вызывает pool методы +- [ ] Импорты очищены (удалить неиспользуемые) +- [ ] Код компилируется без ошибок +- [ ] SMTP плагин запускается и обрабатывает email через Jobs + +## Тестирование + +```bash +# 1. Убедиться что плагин запускается без pool конфигурации + +# 2. Отправить email и проверить обработку +swaks --to test@localhost --server localhost:1025 + +# 3. Проверить что нет panic при остановке +``` + +## Риски + +- **Высокий**: После удаления откат сложен — требуется тщательное тестирование перед этапом +- **Средний**: Возможны забытые ссылки на pool в других местах + +## Точка невозврата + +⚠️ **Важно**: Создайте git tag перед этим этапом для возможности отката: +```bash +git tag -a pre-jobs-migration -m "Before WorkerPool removal" +``` diff --git a/docs/stages/stage-4-rpc-dependencies.md b/docs/stages/stage-4-rpc-dependencies.md new file mode 100644 index 0000000..a35b34a --- /dev/null +++ b/docs/stages/stage-4-rpc-dependencies.md @@ -0,0 +1,139 @@ +# Этап 4: Обновление RPC и зависимостей + +## Описание +Удаление устаревших RPC методов управления воркерами и настройка Endure зависимостей для автоматической инжекции Jobs RPC клиента. + +## Файлы для изменения + +### 1. `rpc.go` — удаление worker методов + +**Текущий код:** строки 21-98 + +**Удалить методы:** +- `AddWorker` (строки 26-36) +- `RemoveWorker` (строки 39-49) +- `WorkersList` (строки 52-55) + +**Оставить методы:** +- `CloseConnection` (строки 58-77) +- `ListConnections` (строки 80-98) + +**Результат:** +```go +type rpc struct { + plugin *Plugin +} + +// CloseConnection - остаётся +func (r *rpc) CloseConnection(req *ConnectionCloseRequest, resp *bool) error { + // ... существующий код +} + +// ListConnections - остаётся +func (r *rpc) ListConnections(_ *Empty, resp *[]*ConnectionInfo) error { + // ... существующий код +} +``` + +--- + +### 2. `plugin.go` — Endure интеграция + +**Добавить метод Collects():** +```go +// Collects returns list of dependencies +func (p *Plugin) Collects() []*dep.In { + return []*dep.In{ + dep.Fits(func(j any) { + rpc, ok := j.(JobsRPCer) + if ok { + p.jobsRPC = rpc + } + }, (*JobsRPCer)(nil)), + } +} +``` + +**Обновить Init() для проверки конфигурации:** +```go +func (p *Plugin) Init(log Logger, cfg Configurer) error { + // ... существующий код ... + + // Validate Jobs config + if p.cfg.Jobs.Pipeline == "" { + return errors.E(op, errors.Str("jobs.pipeline is required")) + } + + return nil +} +``` + +**Обновить Serve() для проверки зависимости:** +```go +func (p *Plugin) Serve() chan error { + errCh := make(chan error, 1) + + // Verify Jobs RPC is available + if p.jobsRPC == nil { + errCh <- errors.E(op, errors.Str("jobs plugin not available, check that jobs plugin is enabled")) + return errCh + } + + // ... остальной код ... +} +``` + +--- + +### 3. Очистка импортов + +**plugin.go:** +```go +// Удалить если не используются: +// - "github.com/roadrunner-server/pool/..." +// - "github.com/roadrunner-server/sdk/v4/payload" +// - другие pool-related импорты + +// Добавить: +// - "github.com/roadrunner-server/endure/v2/dep" +``` + +## Protobuf зависимости + +Если Jobs использует protobuf, добавить в go.mod: +``` +require ( + github.com/roadrunner-server/api/v4 vX.X.X +) +``` + +## Definition of Done + +- [ ] Методы `AddWorker`, `RemoveWorker`, `WorkersList` удалены из `rpc.go` +- [ ] Метод `Collects()` добавлен в `plugin.go` +- [ ] `Init()` проверяет `jobs.pipeline` +- [ ] `Serve()` проверяет наличие `jobsRPC` +- [ ] Импорты очищены от pool зависимостей +- [ ] Endure корректно инжектит Jobs RPC +- [ ] RPC методы `CloseConnection` и `ListConnections` работают +- [ ] Код компилируется без ошибок + +## Тестирование RPC + +```bash +# Проверить что worker методы недоступны +rr workers smtp # должно вернуть ошибку или пустой список + +# Проверить что connection методы работают +rr connections smtp +``` + +## Зависимости + +- Этап 3 завершён +- Endure container настроен правильно + +## Риски + +- **Средний**: Endure dependency injection может не работать — требуется проверка +- **Низкий**: Клиенты использующие worker RPC методы получат ошибки diff --git a/docs/stages/stage-5-tests-docs.md b/docs/stages/stage-5-tests-docs.md new file mode 100644 index 0000000..7187837 --- /dev/null +++ b/docs/stages/stage-5-tests-docs.md @@ -0,0 +1,252 @@ +# Этап 5: Тесты и документация + +## Описание +Написание тестов для новой функциональности и обновление документации с руководством по миграции. + +## Файлы для создания + +### 1. `jobs_integration_test.go` — unit тесты + +```go +package smtp + +import ( + "testing" + "time" +) + +// Mock Jobs RPC +type mockJobsRPC struct { + pushed []*jobsproto.PushRequest + err error +} + +func (m *mockJobsRPC) Push(req *jobsproto.PushRequest, _ *jobsproto.Empty) error { + if m.err != nil { + return m.err + } + m.pushed = append(m.pushed, req) + return nil +} + +func TestEmailJobToJobsRequest(t *testing.T) { + job := &EmailJob{ + UUID: "test-uuid", + ReceivedAt: time.Now(), + From: "sender@test.com", + To: []string{"recipient@test.com"}, + Message: ParsedMessage{Subject: "Test"}, + } + + cfg := &JobsConfig{ + Pipeline: "smtp-emails", + Priority: 10, + } + + req := job.ToJobsRequest(cfg) + + if req.Job.Id != "test-uuid" { + t.Errorf("expected uuid test-uuid, got %s", req.Job.Id) + } + if req.Job.Options.Pipeline != "smtp-emails" { + t.Errorf("expected pipeline smtp-emails, got %s", req.Job.Options.Pipeline) + } +} + +func TestPushToJobs(t *testing.T) { + mock := &mockJobsRPC{} + plugin := &Plugin{ + jobsRPC: mock, + cfg: &Config{Jobs: JobsConfig{Pipeline: "test"}}, + } + + job := &EmailJob{UUID: "test"} + err := plugin.pushToJobs(job) + + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if len(mock.pushed) != 1 { + t.Errorf("expected 1 push, got %d", len(mock.pushed)) + } +} + +func TestPushToJobsError(t *testing.T) { + mock := &mockJobsRPC{err: errors.New("rpc error")} + plugin := &Plugin{ + jobsRPC: mock, + cfg: &Config{Jobs: JobsConfig{Pipeline: "test"}}, + } + + job := &EmailJob{UUID: "test"} + err := plugin.pushToJobs(job) + + if err == nil { + t.Error("expected error, got nil") + } +} + +func TestConfigValidation(t *testing.T) { + cfg := &Config{ + Addr: "127.0.0.1:1025", + Jobs: JobsConfig{Pipeline: ""}, + } + + err := cfg.validate() + if err == nil { + t.Error("expected validation error for empty pipeline") + } +} +``` + +--- + +### 2. `docs/migration-guide.md` — руководство по миграции + +```markdown +# Migration Guide: SMTP Plugin v1 to v2 (Jobs Integration) + +## Breaking Changes + +### Removed Features +- Worker pool configuration (`pool` section) +- RPC methods: `AddWorker`, `RemoveWorker`, `WorkersList` + +### New Requirements +- Jobs plugin must be enabled +- Pipeline must be configured in Jobs + +## Configuration Changes + +### Before (v1) +```yaml +smtp: + addr: "127.0.0.1:1025" + pool: + num_workers: 4 + max_jobs: 100 +``` + +### After (v2) +```yaml +smtp: + addr: "127.0.0.1:1025" + jobs: + pipeline: "smtp-emails" + priority: 10 + +jobs: + pipelines: + smtp-emails: + driver: memory + config: + priority: 10 +``` + +## PHP Code Changes + +### Before: Direct Worker +```php +$worker = Worker::create(); +while ($payload = $worker->waitPayload()) { + $email = json_decode($payload->body); + processEmail($email); + $worker->respond(new Payload('ok')); +} +``` + +### After: Jobs Consumer +```php +$consumer = new Consumer(); +while ($task = $consumer->waitTask()) { + $email = json_decode($task->getPayload()); + processEmail($email); + $task->ack(); +} +``` + +## Migration Steps + +1. Update RoadRunner configuration +2. Enable Jobs plugin +3. Create pipeline for SMTP emails +4. Update PHP code to use Jobs consumer +5. Remove old pool configuration +6. Restart RoadRunner +``` + +--- + +### 3. Обновить `README.md` + +Добавить секцию: +```markdown +## Jobs Integration + +SMTP plugin sends emails as tasks to Jobs for processing. + +### Configuration + +```yaml +smtp: + addr: "127.0.0.1:1025" + jobs: + pipeline: "smtp-emails" +``` + +### PHP Consumer + +```php +// See docs/migration-guide.md +``` +``` + +--- + +### 4. `docs/examples/` — примеры + +**`config-example.yaml`:** +```yaml +# Полный пример конфигурации +``` + +**`consumer-example.php`:** +```php +// Пример PHP consumer +``` + +## Definition of Done + +- [ ] Unit тесты написаны для: + - [ ] `EmailJob.ToJobsRequest()` + - [ ] `pushToJobs()` success/error + - [ ] Config validation +- [ ] Integration тест с mock Jobs +- [ ] `migration-guide.md` создан +- [ ] `README.md` обновлён +- [ ] Примеры конфигурации добавлены +- [ ] Пример PHP consumer добавлен +- [ ] Все тесты проходят: `go test ./...` +- [ ] Документация проверена на корректность + +## Тестирование + +```bash +# Unit тесты +go test -v ./... + +# Coverage +go test -cover ./... + +# Integration (требует запущенный RR) +go test -tags=integration ./... +``` + +## Зависимости + +- Этапы 1-4 завершены +- Код стабилен и готов к релизу + +## Риски + +- **Низкий**: Документация может устареть — поддерживать актуальность diff --git a/go.mod b/go.mod index 59ed045..ce899bb 100644 --- a/go.mod +++ b/go.mod @@ -1,28 +1,20 @@ module github.com/buggregator/smtp-server -go 1.24 +go 1.24.0 toolchain go1.24.4 require ( github.com/emersion/go-smtp v0.21.3 - github.com/goccy/go-json v0.10.5 github.com/google/uuid v1.6.0 + github.com/roadrunner-server/api/v4 v4.23.0 + github.com/roadrunner-server/endure/v2 v2.6.2 github.com/roadrunner-server/errors v1.4.1 - github.com/roadrunner-server/pool v1.1.3 go.uber.org/zap v1.27.0 ) require ( github.com/emersion/go-sasl v0.0.0-20200509203442-7bfe0ed36a21 // indirect - github.com/go-ole/go-ole v1.3.0 // indirect - github.com/roadrunner-server/events v1.0.1 // indirect - github.com/roadrunner-server/goridge/v3 v3.8.3 // indirect - github.com/shirou/gopsutil v3.21.11+incompatible // indirect - github.com/tklauser/go-sysconf v0.3.14 // indirect - github.com/tklauser/numcpus v0.9.0 // indirect - github.com/yusufpapurcu/wmi v1.2.4 // indirect go.uber.org/multierr v1.11.0 // indirect - golang.org/x/sync v0.11.0 // indirect - golang.org/x/sys v0.30.0 // indirect + google.golang.org/protobuf v1.36.10 // indirect ) diff --git a/go.sum b/go.sum index 56e7c33..76eed7d 100644 --- a/go.sum +++ b/go.sum @@ -4,44 +4,27 @@ github.com/emersion/go-sasl v0.0.0-20200509203442-7bfe0ed36a21 h1:OJyUGMJTzHTd1X github.com/emersion/go-sasl v0.0.0-20200509203442-7bfe0ed36a21/go.mod h1:iL2twTeMvZnrg54ZoPDNfJaJaqy0xIQFuBdrLsmspwQ= github.com/emersion/go-smtp v0.21.3 h1:7uVwagE8iPYE48WhNsng3RRpCUpFvNl39JGNSIyGVMY= github.com/emersion/go-smtp v0.21.3/go.mod h1:qm27SGYgoIPRot6ubfQ/GpiPy/g3PaZAVRxiO/sDUgQ= -github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= -github.com/go-ole/go-ole v1.3.0 h1:Dt6ye7+vXGIKZ7Xtk4s6/xVdGDQynvom7xCFEdWr6uE= -github.com/go-ole/go-ole v1.3.0/go.mod h1:5LS6F96DhAwUc7C+1HLexzMXY1xGRSryjyPPKW6zv78= -github.com/goccy/go-json v0.10.5 h1:Fq85nIqj+gXn/S5ahsiTlK3TmC85qgirsdTP/+DeaC4= -github.com/goccy/go-json v0.10.5/go.mod h1:oq7eo15ShAhp70Anwd5lgX2pLfOS3QCiwU/PULtXL6M= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/roadrunner-server/api/v4 v4.23.0 h1:lrVXgP4ozD/H5DrIdT181ldVhD1R9QT5qsi8qWUTDF4= +github.com/roadrunner-server/api/v4 v4.23.0/go.mod h1:AlHuVVOklb7XF33Cf7IfmwOn3j4gGg37on9Xi6j08Bg= +github.com/roadrunner-server/endure/v2 v2.6.2 h1:sIB4kTyE7gtT3fDhuYWUYn6Vt/dcPtiA6FoNS1eS+84= +github.com/roadrunner-server/endure/v2 v2.6.2/go.mod h1:t/2+xpNYgGBwhzn83y2MDhvhZ19UVq1REcvqn7j7RB8= github.com/roadrunner-server/errors v1.4.1 h1:LKNeaCGiwd3t8IaL840ZNF3UA9yDQlpvHnKddnh0YRQ= github.com/roadrunner-server/errors v1.4.1/go.mod h1:qeffnIKG0e4j1dzGpa+OGY5VKSfMphizvqWIw8s2lAo= -github.com/roadrunner-server/events v1.0.1 h1:waCkKhxhzdK3VcI1xG22l+h+0J+Nfdpxjhyy01Un+kI= -github.com/roadrunner-server/events v1.0.1/go.mod h1:WZRqoEVaFm209t52EuoT7ISUtvX6BrCi6bI/7pjkVC0= -github.com/roadrunner-server/goridge/v3 v3.8.3 h1:XmjrOFnI6ZbQTPaP39DEk8KwLUNTgjluK3pcZaW6ixQ= -github.com/roadrunner-server/goridge/v3 v3.8.3/go.mod h1:4TZU8zgkKIZCsH51qwGMpvyXCT59u/8z6q8sCe4ZGAQ= -github.com/roadrunner-server/pool v1.1.3 h1:KMsiL6yuYBWGk73bdO0akwP+fJ63bxDF972JukCGsxI= -github.com/roadrunner-server/pool v1.1.3/go.mod h1:8ceC7NvZKJRciv+KJmcyk5CeDugoel6GD+crm5kBFW0= -github.com/shirou/gopsutil v3.21.11+incompatible h1:+1+c1VGhc88SSonWP6foOcLhvnKlUeu/erjjvaPEYiI= -github.com/shirou/gopsutil v3.21.11+incompatible/go.mod h1:5b4v6he4MtMOwMlS0TUMTu2PcXUg8+E1lC7eC3UO/RA= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= -github.com/tklauser/go-sysconf v0.3.14 h1:g5vzr9iPFFz24v2KZXs/pvpvh8/V9Fw6vQK5ZZb78yU= -github.com/tklauser/go-sysconf v0.3.14/go.mod h1:1ym4lWMLUOhuBOPGtRcJm7tEGX4SCYNEEEtghGG/8uY= -github.com/tklauser/numcpus v0.9.0 h1:lmyCHtANi8aRUgkckBgoDk1nHCux3n2cgkJLXdQGPDo= -github.com/tklauser/numcpus v0.9.0/go.mod h1:SN6Nq1O3VychhC1npsWostA+oW+VOQTxZrS604NSRyI= -github.com/yusufpapurcu/wmi v1.2.4 h1:zFUKzehAFReQwLys1b/iSMl+JQGSCSjtVqQn9bBrPo0= -github.com/yusufpapurcu/wmi v1.2.4/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= -golang.org/x/sync v0.11.0 h1:GGz8+XQP4FvTTrjZPzNKTMFtSXH80RAzG+5ghFPgK9w= -golang.org/x/sync v0.11.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= -golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc= -golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +google.golang.org/protobuf v1.36.10 h1:AYd7cD/uASjIL6Q9LiTjz8JLcrh/88q5UObnmY3aOOE= +google.golang.org/protobuf v1.36.10/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/handler.go b/handler.go deleted file mode 100644 index a8f0c5c..0000000 --- a/handler.go +++ /dev/null @@ -1,64 +0,0 @@ -package smtp - -import ( - "context" - "time" - - "github.com/goccy/go-json" - "github.com/roadrunner-server/errors" - "github.com/roadrunner-server/pool/payload" - "go.uber.org/zap" -) - -// sendToWorker sends email data to PHP worker and waits for response -func (s *Session) sendToWorker(message *ParsedMessage) (string, error) { - // 1. Marshal email data to JSON - jsonData, err := json.Marshal(message) - if err != nil { - return "", errors.E(errors.Op("smtp_marshal_email"), err) - } - - // 2. Create payload - pld := &payload.Payload{ - Context: jsonData, // Email data in context - Body: nil, // No body needed - } - - // 3. Execute via worker pool - ctx := context.Background() - stopCh := make(chan struct{}, 1) - - s.backend.plugin.mu.RLock() - pool := s.backend.plugin.wPool - s.backend.plugin.mu.RUnlock() - - if pool == nil { - return "", errors.Str("worker pool not initialized") - } - - result, err := pool.Exec(ctx, pld, stopCh) - if err != nil { - return "", errors.E(errors.Op("smtp_worker_exec"), err) - } - - // 4. Read response from worker - select { - case resp := <-result: - if resp.Error() != nil { - return "", resp.Error() - } - - // Get response from context - response := string(resp.Payload().Context) - - s.log.Debug("worker response", - zap.String("uuid", s.uuid), - zap.String("response", response), - ) - - return response, nil - - case <-time.After(30 * time.Second): - return "", errors.Str("worker timeout") - } -} diff --git a/jobs_integration_test.go b/jobs_integration_test.go new file mode 100644 index 0000000..a8ffe07 --- /dev/null +++ b/jobs_integration_test.go @@ -0,0 +1,239 @@ +package smtp + +import ( + "testing" + "time" + + jobsProto "github.com/roadrunner-server/api/v4/build/jobs/v1" + "github.com/roadrunner-server/errors" + "go.uber.org/zap" +) + +// mockJobsRPC implements JobsRPCer for testing +type mockJobsRPC struct { + pushed []*jobsProto.PushRequest + err error +} + +func (m *mockJobsRPC) Push(req *jobsProto.PushRequest, _ *jobsProto.Empty) error { + if m.err != nil { + return m.err + } + m.pushed = append(m.pushed, req) + return nil +} + +func (m *mockJobsRPC) PushBatch(req *jobsProto.PushBatchRequest, _ *jobsProto.Empty) error { + return nil +} + +func TestToJobsRequest(t *testing.T) { + email := &EmailData{ + Event: "EMAIL_RECEIVED", + UUID: "test-uuid-123", + RemoteAddr: "127.0.0.1:12345", + ReceivedAt: time.Now(), + Envelope: EnvelopeData{ + From: "sender@test.com", + To: []string{"recipient@test.com"}, + Helo: "localhost", + }, + Message: MessageData{ + Headers: map[string][]string{ + "Subject": {"Test Subject"}, + }, + Body: "Test body", + }, + } + + cfg := &JobsConfig{ + Pipeline: "smtp-emails", + Priority: 10, + Delay: 0, + AutoAck: false, + } + + req := ToJobsRequest(email, cfg) + + if req.Job == nil { + t.Fatal("expected job to be non-nil") + } + + if req.Job.Job != "smtp.email" { + t.Errorf("expected job name 'smtp.email', got '%s'", req.Job.Job) + } + + if req.Job.Options.Pipeline != "smtp-emails" { + t.Errorf("expected pipeline 'smtp-emails', got '%s'", req.Job.Options.Pipeline) + } + + if req.Job.Options.Priority != 10 { + t.Errorf("expected priority 10, got %d", req.Job.Options.Priority) + } + + if len(req.Job.Payload) == 0 { + t.Error("expected non-empty payload") + } + + // Check headers + if uuidHeader, ok := req.Job.Headers["uuid"]; !ok || len(uuidHeader.Value) == 0 || uuidHeader.Value[0] != "test-uuid-123" { + t.Error("expected uuid header with value 'test-uuid-123'") + } +} + +func TestPushToJobs(t *testing.T) { + mock := &mockJobsRPC{} + logger, _ := zap.NewDevelopment() + plugin := &Plugin{ + jobsRPC: mock, + log: logger, + cfg: &Config{ + Jobs: JobsConfig{ + Pipeline: "test-pipeline", + Priority: 5, + }, + }, + } + + email := &EmailData{ + UUID: "test-uuid", + ReceivedAt: time.Now(), + Envelope: EnvelopeData{ + From: "test@test.com", + To: []string{"recipient@test.com"}, + }, + } + + err := plugin.pushToJobs(email) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if len(mock.pushed) != 1 { + t.Errorf("expected 1 push, got %d", len(mock.pushed)) + } +} + +func TestPushToJobsError(t *testing.T) { + mock := &mockJobsRPC{err: errors.Str("rpc error")} + logger, _ := zap.NewDevelopment() + plugin := &Plugin{ + jobsRPC: mock, + log: logger, + cfg: &Config{ + Jobs: JobsConfig{ + Pipeline: "test-pipeline", + }, + }, + } + + email := &EmailData{ + UUID: "test-uuid", + ReceivedAt: time.Now(), + } + + err := plugin.pushToJobs(email) + if err == nil { + t.Error("expected error, got nil") + } +} + +func TestPushToJobsNoRPC(t *testing.T) { + plugin := &Plugin{ + jobsRPC: nil, + cfg: &Config{ + Jobs: JobsConfig{ + Pipeline: "test-pipeline", + }, + }, + } + + email := &EmailData{ + UUID: "test-uuid", + } + + err := plugin.pushToJobs(email) + if err == nil { + t.Error("expected error when jobsRPC is nil") + } +} + +func TestConfigValidation(t *testing.T) { + tests := []struct { + name string + cfg Config + wantErr bool + }{ + { + name: "valid config", + cfg: Config{ + Addr: "127.0.0.1:1025", + Jobs: JobsConfig{Pipeline: "smtp-emails"}, + AttachmentStorage: AttachmentConfig{ + Mode: "memory", + }, + }, + wantErr: false, + }, + { + name: "missing pipeline", + cfg: Config{ + Addr: "127.0.0.1:1025", + Jobs: JobsConfig{Pipeline: ""}, + AttachmentStorage: AttachmentConfig{ + Mode: "memory", + }, + }, + wantErr: true, + }, + { + name: "missing addr", + cfg: Config{ + Addr: "", + Jobs: JobsConfig{Pipeline: "smtp-emails"}, + AttachmentStorage: AttachmentConfig{ + Mode: "memory", + }, + }, + wantErr: true, + }, + { + name: "invalid attachment mode", + cfg: Config{ + Addr: "127.0.0.1:1025", + Jobs: JobsConfig{Pipeline: "smtp-emails"}, + AttachmentStorage: AttachmentConfig{ + Mode: "invalid", + }, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.cfg.validate() + if (err != nil) != tt.wantErr { + t.Errorf("validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestJobsConfigDefaults(t *testing.T) { + cfg := &Config{ + Addr: "127.0.0.1:1025", + Jobs: JobsConfig{ + Pipeline: "test", + }, + } + + err := cfg.InitDefaults() + if err != nil { + t.Fatalf("InitDefaults() error = %v", err) + } + + if cfg.Jobs.Priority != 10 { + t.Errorf("expected default priority 10, got %d", cfg.Jobs.Priority) + } +} diff --git a/jobs_rpc.go b/jobs_rpc.go new file mode 100644 index 0000000..cdb3bc2 --- /dev/null +++ b/jobs_rpc.go @@ -0,0 +1,39 @@ +package smtp + +import ( + "encoding/json" + + "github.com/google/uuid" + jobsProto "github.com/roadrunner-server/api/v4/build/jobs/v1" +) + +// JobsRPCer interface for Jobs plugin RPC methods +type JobsRPCer interface { + Push(req *jobsProto.PushRequest, resp *jobsProto.Empty) error + PushBatch(req *jobsProto.PushBatchRequest, resp *jobsProto.Empty) error +} + +// ToJobsRequest converts EmailData to Jobs protobuf format +func ToJobsRequest(e *EmailData, cfg *JobsConfig) *jobsProto.PushRequest { + payload, _ := json.Marshal(e) + + // Generate a unique job ID + jobID := uuid.NewString() + + return &jobsProto.PushRequest{ + Job: &jobsProto.Job{ + Job: "smtp.email", + Id: jobID, + Payload: payload, + Headers: map[string]*jobsProto.HeaderValue{ + "uuid": {Value: []string{e.UUID}}, + }, + Options: &jobsProto.Options{ + Pipeline: cfg.Pipeline, + Priority: cfg.Priority, + Delay: cfg.Delay, + AutoAck: cfg.AutoAck, + }, + }, + } +} diff --git a/plugin.go b/plugin.go index f457289..2ec204f 100644 --- a/plugin.go +++ b/plugin.go @@ -6,46 +6,21 @@ import ( "sync" "github.com/emersion/go-smtp" + jobsProto "github.com/roadrunner-server/api/v4/build/jobs/v1" + "github.com/roadrunner-server/endure/v2/dep" "github.com/roadrunner-server/errors" - "github.com/roadrunner-server/pool/payload" - "github.com/roadrunner-server/pool/pool" - staticPool "github.com/roadrunner-server/pool/pool/static_pool" - "github.com/roadrunner-server/pool/state/process" - "github.com/roadrunner-server/pool/worker" "go.uber.org/zap" ) const ( PluginName = "smtp" - RrMode = "RR_MODE" ) -// Pool interface for worker pool operations -type Pool interface { - // Workers returns worker list associated with the pool - Workers() (workers []*worker.Process) - // RemoveWorker removes worker from the pool - RemoveWorker(ctx context.Context) error - // AddWorker adds worker to the pool - AddWorker() error - // Exec executes payload - Exec(ctx context.Context, p *payload.Payload, stopCh chan struct{}) (chan *staticPool.PExec, error) - // Reset kills all workers and replaces with new - Reset(ctx context.Context) error - // Destroy all underlying stacks - Destroy(ctx context.Context) -} - // Logger interface for dependency injection type Logger interface { NamedLogger(name string) *zap.Logger } -// Server creates workers for the application -type Server interface { - NewPool(ctx context.Context, cfg *pool.Config, env map[string]string, _ *zap.Logger) (*staticPool.Pool, error) -} - // Configurer interface for configuration access type Configurer interface { // UnmarshalKey takes a single key and unmarshal it into a Struct @@ -56,14 +31,13 @@ type Configurer interface { // Plugin is the SMTP server plugin type Plugin struct { - mu sync.RWMutex - cfg *Config - log *zap.Logger - server Server - - wPool Pool + mu sync.RWMutex + cfg *Config + log *zap.Logger connections sync.Map // uuid -> *Session - pldPool sync.Pool + + // Jobs RPC client + jobsRPC JobsRPCer // SMTP server components smtpServer *smtp.Server @@ -71,7 +45,7 @@ type Plugin struct { } // Init initializes the plugin with configuration and logger -func (p *Plugin) Init(log Logger, cfg Configurer, server Server) error { +func (p *Plugin) Init(log Logger, cfg Configurer) error { const op = errors.Op("smtp_plugin_init") // Check if plugin is enabled @@ -90,21 +64,14 @@ func (p *Plugin) Init(log Logger, cfg Configurer, server Server) error { return errors.E(op, err) } - // Initialize payload pool - p.pldPool = sync.Pool{ - New: func() any { - return new(payload.Payload) - }, - } - // Setup logger p.log = log.NamedLogger(PluginName) - p.server = server p.log.Info("SMTP plugin initialized", zap.String("addr", p.cfg.Addr), zap.String("hostname", p.cfg.Hostname), zap.Int64("max_message_size", p.cfg.MaxMessageSize), + zap.String("jobs_pipeline", p.cfg.Jobs.Pipeline), ) return nil @@ -112,26 +79,22 @@ func (p *Plugin) Init(log Logger, cfg Configurer, server Server) error { // Serve starts the SMTP server func (p *Plugin) Serve() chan error { + const op = errors.Op("smtp_serve") errCh := make(chan error, 2) p.mu.Lock() defer p.mu.Unlock() - var err error - p.wPool, err = p.server.NewPool(context.Background(), p.cfg.Pool, map[string]string{RrMode: PluginName}, p.log) - if err != nil { - errCh <- err + // Verify Jobs RPC is available + if p.jobsRPC == nil { + errCh <- errors.E(op, errors.Str("jobs plugin not available, check that jobs plugin is enabled")) return errCh } - p.log.Info("SMTP plugin worker pool created", - zap.Int("num_workers", len(p.wPool.Workers())), - ) - - // 2. Create SMTP backend + // 1. Create SMTP backend backend := NewBackend(p) - // 3. Create SMTP server + // 2. Create SMTP server p.smtpServer = smtp.NewServer(backend) p.smtpServer.Addr = p.cfg.Addr p.smtpServer.Domain = p.cfg.Hostname @@ -144,9 +107,11 @@ func (p *Plugin) Serve() chan error { p.log.Info("SMTP server configured", zap.String("addr", p.smtpServer.Addr), zap.String("domain", p.smtpServer.Domain), + zap.String("jobs_pipeline", p.cfg.Jobs.Pipeline), ) - // 4. Create listener + // 3. Create listener + var err error p.listener, err = net.Listen("tcp", p.cfg.Addr) if err != nil { errCh <- errors.E(errors.Op("smtp_listen"), err) @@ -155,7 +120,7 @@ func (p *Plugin) Serve() chan error { p.log.Info("SMTP listener created", zap.String("addr", p.cfg.Addr)) - // 5. Start SMTP server in goroutine + // 4. Start SMTP server in goroutine go func() { p.log.Info("SMTP server starting", zap.String("addr", p.cfg.Addr)) if err := p.smtpServer.Serve(p.listener); err != nil { @@ -164,7 +129,7 @@ func (p *Plugin) Serve() chan error { } }() - // 6. Start temp file cleanup routine + // 5. Start temp file cleanup routine p.startCleanupRoutine(context.Background()) return errCh @@ -196,17 +161,6 @@ func (p *Plugin) Stop(ctx context.Context) error { return true }) - if p.wPool != nil { - switch pp := p.wPool.(type) { - case *staticPool.Pool: - if pp != nil { - pp.Destroy(ctx) - } - default: - // pool is nil, nothing to do - } - } - doneCh <- struct{}{} }() @@ -219,55 +173,46 @@ func (p *Plugin) Stop(ctx context.Context) error { } } -// Reset resets the worker pool - -// Reset destroys the old pool and replaces it with new one, waiting for old pool to die -func (p *Plugin) Reset() error { - const op = errors.Op("http_plugin_reset") - - p.mu.Lock() - defer p.mu.Unlock() - - p.log.Info("reset signal was received") - - if p.wPool == nil { - p.log.Info("pool is nil, nothing to reset") - return nil - } +// Name returns plugin name for RoadRunner +func (p *Plugin) Name() string { + return PluginName +} - err := p.wPool.Reset(context.Background()) - if err != nil { - return errors.E(op, err) +// Collects declares dependencies on other plugins +func (p *Plugin) Collects() []*dep.In { + return []*dep.In{ + dep.Fits(func(pp any) { + jobsRPC := pp.(JobsRPCer) + p.jobsRPC = jobsRPC + }, (*JobsRPCer)(nil)), } +} - p.log.Info("plugin was successfully reset") - return nil +// RPC returns RPC interface for external management +func (p *Plugin) RPC() any { + return &rpc{p: p} } -// Workers returns slice with the process states for the workers -func (p *Plugin) Workers() []*process.State { - p.mu.RLock() - defer p.mu.RUnlock() +// pushToJobs sends email as job to Jobs plugin +func (p *Plugin) pushToJobs(email *EmailData) error { + const op = errors.Op("smtp_push_to_jobs") - if p.wPool == nil { - return nil + if p.jobsRPC == nil { + return errors.E(op, errors.Str("jobs RPC not available")) } - workers := p.wPool.Workers() + req := ToJobsRequest(email, &p.cfg.Jobs) - ps := make([]*process.State, 0, len(workers)) - for i := range workers { - state, err := process.WorkerProcessState(workers[i]) - if err != nil { - return nil - } - ps = append(ps, state) + var empty jobsProto.Empty + err := p.jobsRPC.Push(req, &empty) + if err != nil { + return errors.E(op, err) } - return ps -} + p.log.Debug("email pushed to jobs", + zap.String("uuid", email.UUID), + zap.String("pipeline", p.cfg.Jobs.Pipeline), + ) -// Name returns plugin name for RoadRunner -func (p *Plugin) Name() string { - return PluginName + return nil } diff --git a/rpc.go b/rpc.go index ae0152d..ab47da9 100644 --- a/rpc.go +++ b/rpc.go @@ -1,10 +1,7 @@ package smtp import ( - "context" - "github.com/roadrunner-server/errors" - "github.com/roadrunner-server/pool/state/process" ) // ConnectionInfo represents information about an active SMTP connection @@ -22,38 +19,6 @@ type rpc struct { p *Plugin } -// AddWorker adds new worker to the pool -func (r *rpc) AddWorker(_ bool, success *bool) error { - *success = false - - err := r.p.AddWorker() - if err != nil { - return err - } - - *success = true - return nil -} - -// RemoveWorker removes worker from the pool -func (r *rpc) RemoveWorker(_ bool, success *bool) error { - *success = false - - err := r.p.RemoveWorker(context.Background()) - if err != nil { - return err - } - - *success = true - return nil -} - -// WorkersList returns list of active workers -func (r *rpc) WorkersList(_ bool, workers *[]*process.State) error { - *workers = r.p.Workers() - return nil -} - // CloseConnection closes SMTP connection by UUID func (r *rpc) CloseConnection(uuid string, success *bool) error { *success = false diff --git a/session.go b/session.go index a2d6f7c..828b53d 100644 --- a/session.go +++ b/session.go @@ -3,6 +3,7 @@ package smtp import ( "bytes" "io" + "time" "github.com/emersion/go-smtp" "go.uber.org/zap" @@ -78,7 +79,7 @@ func (s *Session) Data(r io.Reader) error { ) // 2. Parse email - emailData, err := s.parseEmail(s.emailData.Bytes()) + parsedMessage, err := s.parseEmail(s.emailData.Bytes()) if err != nil { s.log.Error("failed to parse email", zap.Error(err)) return &smtp.SMTPError{ @@ -87,34 +88,63 @@ func (s *Session) Data(r io.Reader) error { } } - // 3. Send to PHP worker - response, err := s.sendToWorker(emailData) - if err != nil { - s.log.Error("worker error", zap.Error(err)) - return &smtp.SMTPError{ - Code: 451, - Message: "Temporary failure", + // 3. Build EmailData for Jobs + var authData *AuthData + if s.authenticated { + authData = &AuthData{ + Attempted: true, + Mechanism: s.authMechanism, + Username: s.authUsername, + Password: s.authPassword, } } - // 4. Handle worker response - switch response { - case "CLOSE": - s.log.Debug("worker requested connection close", zap.String("uuid", s.uuid)) - s.shouldClose = true + // Convert attachments + attachments := make([]AttachmentData, 0, len(parsedMessage.Attachments)) + for _, att := range parsedMessage.Attachments { + attachments = append(attachments, AttachmentData{ + Filename: att.Filename, + ContentType: att.Type, + Content: att.Content, + }) + } - case "CONTINUE": - s.log.Debug("worker accepted, connection continues", zap.String("uuid", s.uuid)) + emailData := &EmailData{ + Event: "EMAIL_RECEIVED", + UUID: s.uuid, + RemoteAddr: s.remoteAddr, + ReceivedAt: time.Now(), + Envelope: EnvelopeData{ + From: s.from, + To: s.to, + Helo: s.heloName, + }, + Auth: authData, + Message: MessageData{ + Headers: map[string][]string{ + "Subject": {parsedMessage.Subject}, + }, + Body: parsedMessage.TextBody, + Raw: parsedMessage.Raw, + }, + Attachments: attachments, + } - default: - s.log.Warn("unexpected worker response", + // 4. Push to Jobs + err = s.backend.plugin.pushToJobs(emailData) + if err != nil { + s.log.Error("failed to push email to jobs", + zap.Error(err), zap.String("uuid", s.uuid), - zap.String("response", response), ) + return &smtp.SMTPError{ + Code: 451, + EnhancedCode: smtp.EnhancedCode{4, 3, 0}, + Message: "Temporary failure, try again later", + } } // Always return nil to send 250 OK to client - // (profiling mode - accept everything) return nil } diff --git a/workers_manager.go b/workers_manager.go deleted file mode 100644 index c492c20..0000000 --- a/workers_manager.go +++ /dev/null @@ -1,17 +0,0 @@ -package smtp - -import ( - "context" -) - -func (p *Plugin) AddWorker() error { - p.mu.RLock() - defer p.mu.RUnlock() - return p.wPool.AddWorker() -} - -func (p *Plugin) RemoveWorker(ctx context.Context) error { - p.mu.RLock() - defer p.mu.RUnlock() - return p.wPool.RemoveWorker(ctx) -} From c873af44ed6b646e51635340538fcb03b55c7ef0 Mon Sep 17 00:00:00 2001 From: butschster Date: Sat, 22 Nov 2025 18:22:18 +0400 Subject: [PATCH 02/13] refactor: remove Jobs plugin availability check --- plugin.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/plugin.go b/plugin.go index 2ec204f..d6c0692 100644 --- a/plugin.go +++ b/plugin.go @@ -79,18 +79,11 @@ func (p *Plugin) Init(log Logger, cfg Configurer) error { // Serve starts the SMTP server func (p *Plugin) Serve() chan error { - const op = errors.Op("smtp_serve") errCh := make(chan error, 2) p.mu.Lock() defer p.mu.Unlock() - // Verify Jobs RPC is available - if p.jobsRPC == nil { - errCh <- errors.E(op, errors.Str("jobs plugin not available, check that jobs plugin is enabled")) - return errCh - } - // 1. Create SMTP backend backend := NewBackend(p) From beafc63f8f03bd980f3bf4935d110d3035d2aa54 Mon Sep 17 00:00:00 2001 From: butschster Date: Sat, 22 Nov 2025 18:34:22 +0400 Subject: [PATCH 03/13] refactor: replace JobsRPCer with Pusher interface --- jobs_integration_test.go | 55 +++++++++++++++++++--------------------- jobs_rpc.go | 39 ++++++++++++++-------------- plugin.go | 20 +++++++-------- 3 files changed, 54 insertions(+), 60 deletions(-) diff --git a/jobs_integration_test.go b/jobs_integration_test.go index a8ffe07..a6c1bc0 100644 --- a/jobs_integration_test.go +++ b/jobs_integration_test.go @@ -1,6 +1,7 @@ package smtp import ( + "context" "testing" "time" @@ -9,25 +10,21 @@ import ( "go.uber.org/zap" ) -// mockJobsRPC implements JobsRPCer for testing -type mockJobsRPC struct { - pushed []*jobsProto.PushRequest +// mockPusher implements Pusher for testing +type mockPusher struct { + pushed []*jobsProto.Job err error } -func (m *mockJobsRPC) Push(req *jobsProto.PushRequest, _ *jobsProto.Empty) error { +func (m *mockPusher) Push(_ context.Context, job *jobsProto.Job) error { if m.err != nil { return m.err } - m.pushed = append(m.pushed, req) + m.pushed = append(m.pushed, job) return nil } -func (m *mockJobsRPC) PushBatch(req *jobsProto.PushBatchRequest, _ *jobsProto.Empty) error { - return nil -} - -func TestToJobsRequest(t *testing.T) { +func TestToJob(t *testing.T) { email := &EmailData{ Event: "EMAIL_RECEIVED", UUID: "test-uuid-123", @@ -53,40 +50,40 @@ func TestToJobsRequest(t *testing.T) { AutoAck: false, } - req := ToJobsRequest(email, cfg) + job := ToJob(email, cfg) - if req.Job == nil { + if job == nil { t.Fatal("expected job to be non-nil") } - if req.Job.Job != "smtp.email" { - t.Errorf("expected job name 'smtp.email', got '%s'", req.Job.Job) + if job.Job != "smtp.email" { + t.Errorf("expected job name 'smtp.email', got '%s'", job.Job) } - if req.Job.Options.Pipeline != "smtp-emails" { - t.Errorf("expected pipeline 'smtp-emails', got '%s'", req.Job.Options.Pipeline) + if job.Options.Pipeline != "smtp-emails" { + t.Errorf("expected pipeline 'smtp-emails', got '%s'", job.Options.Pipeline) } - if req.Job.Options.Priority != 10 { - t.Errorf("expected priority 10, got %d", req.Job.Options.Priority) + if job.Options.Priority != 10 { + t.Errorf("expected priority 10, got %d", job.Options.Priority) } - if len(req.Job.Payload) == 0 { + if len(job.Payload) == 0 { t.Error("expected non-empty payload") } // Check headers - if uuidHeader, ok := req.Job.Headers["uuid"]; !ok || len(uuidHeader.Value) == 0 || uuidHeader.Value[0] != "test-uuid-123" { + if uuidHeader, ok := job.Headers["uuid"]; !ok || len(uuidHeader.Value) == 0 || uuidHeader.Value[0] != "test-uuid-123" { t.Error("expected uuid header with value 'test-uuid-123'") } } func TestPushToJobs(t *testing.T) { - mock := &mockJobsRPC{} + mock := &mockPusher{} logger, _ := zap.NewDevelopment() plugin := &Plugin{ - jobsRPC: mock, - log: logger, + pusher: mock, + log: logger, cfg: &Config{ Jobs: JobsConfig{ Pipeline: "test-pipeline", @@ -115,11 +112,11 @@ func TestPushToJobs(t *testing.T) { } func TestPushToJobsError(t *testing.T) { - mock := &mockJobsRPC{err: errors.Str("rpc error")} + mock := &mockPusher{err: errors.Str("push error")} logger, _ := zap.NewDevelopment() plugin := &Plugin{ - jobsRPC: mock, - log: logger, + pusher: mock, + log: logger, cfg: &Config{ Jobs: JobsConfig{ Pipeline: "test-pipeline", @@ -138,9 +135,9 @@ func TestPushToJobsError(t *testing.T) { } } -func TestPushToJobsNoRPC(t *testing.T) { +func TestPushToJobsNoPusher(t *testing.T) { plugin := &Plugin{ - jobsRPC: nil, + pusher: nil, cfg: &Config{ Jobs: JobsConfig{ Pipeline: "test-pipeline", @@ -154,7 +151,7 @@ func TestPushToJobsNoRPC(t *testing.T) { err := plugin.pushToJobs(email) if err == nil { - t.Error("expected error when jobsRPC is nil") + t.Error("expected error when pusher is nil") } } diff --git a/jobs_rpc.go b/jobs_rpc.go index cdb3bc2..0496fe2 100644 --- a/jobs_rpc.go +++ b/jobs_rpc.go @@ -1,39 +1,38 @@ package smtp import ( + "context" "encoding/json" "github.com/google/uuid" jobsProto "github.com/roadrunner-server/api/v4/build/jobs/v1" ) -// JobsRPCer interface for Jobs plugin RPC methods -type JobsRPCer interface { - Push(req *jobsProto.PushRequest, resp *jobsProto.Empty) error - PushBatch(req *jobsProto.PushBatchRequest, resp *jobsProto.Empty) error +// Pusher interface for Jobs plugin integration +// Jobs plugin должен реализовать этот интерфейс +type Pusher interface { + Push(ctx context.Context, job *jobsProto.Job) error } -// ToJobsRequest converts EmailData to Jobs protobuf format -func ToJobsRequest(e *EmailData, cfg *JobsConfig) *jobsProto.PushRequest { +// ToJob converts EmailData to Jobs protobuf format +func ToJob(e *EmailData, cfg *JobsConfig) *jobsProto.Job { payload, _ := json.Marshal(e) // Generate a unique job ID jobID := uuid.NewString() - return &jobsProto.PushRequest{ - Job: &jobsProto.Job{ - Job: "smtp.email", - Id: jobID, - Payload: payload, - Headers: map[string]*jobsProto.HeaderValue{ - "uuid": {Value: []string{e.UUID}}, - }, - Options: &jobsProto.Options{ - Pipeline: cfg.Pipeline, - Priority: cfg.Priority, - Delay: cfg.Delay, - AutoAck: cfg.AutoAck, - }, + return &jobsProto.Job{ + Job: "smtp.email", + Id: jobID, + Payload: payload, + Headers: map[string]*jobsProto.HeaderValue{ + "uuid": {Value: []string{e.UUID}}, + }, + Options: &jobsProto.Options{ + Pipeline: cfg.Pipeline, + Priority: cfg.Priority, + Delay: cfg.Delay, + AutoAck: cfg.AutoAck, }, } } diff --git a/plugin.go b/plugin.go index d6c0692..45ac09b 100644 --- a/plugin.go +++ b/plugin.go @@ -6,7 +6,6 @@ import ( "sync" "github.com/emersion/go-smtp" - jobsProto "github.com/roadrunner-server/api/v4/build/jobs/v1" "github.com/roadrunner-server/endure/v2/dep" "github.com/roadrunner-server/errors" "go.uber.org/zap" @@ -36,8 +35,8 @@ type Plugin struct { log *zap.Logger connections sync.Map // uuid -> *Session - // Jobs RPC client - jobsRPC JobsRPCer + // Jobs pusher + pusher Pusher // SMTP server components smtpServer *smtp.Server @@ -175,9 +174,9 @@ func (p *Plugin) Name() string { func (p *Plugin) Collects() []*dep.In { return []*dep.In{ dep.Fits(func(pp any) { - jobsRPC := pp.(JobsRPCer) - p.jobsRPC = jobsRPC - }, (*JobsRPCer)(nil)), + pusher := pp.(Pusher) + p.pusher = pusher + }, (*Pusher)(nil)), } } @@ -190,14 +189,13 @@ func (p *Plugin) RPC() any { func (p *Plugin) pushToJobs(email *EmailData) error { const op = errors.Op("smtp_push_to_jobs") - if p.jobsRPC == nil { - return errors.E(op, errors.Str("jobs RPC not available")) + if p.pusher == nil { + return errors.E(op, errors.Str("jobs pusher not available")) } - req := ToJobsRequest(email, &p.cfg.Jobs) + job := ToJob(email, &p.cfg.Jobs) - var empty jobsProto.Empty - err := p.jobsRPC.Push(req, &empty) + err := p.pusher.Push(context.Background(), job) if err != nil { return errors.E(op, err) } From 88b0ea35c276454e83fd13c6f21fc876abe03c9a Mon Sep 17 00:00:00 2001 From: butschster Date: Sat, 22 Nov 2025 23:36:50 +0400 Subject: [PATCH 04/13] refactor: rename pusher to jobsPlugin and improve dependency resolution --- plugin.go | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/plugin.go b/plugin.go index 45ac09b..737830c 100644 --- a/plugin.go +++ b/plugin.go @@ -28,6 +28,13 @@ type Configurer interface { Has(name string) bool } +// Pusher interface matches Jobs plugin's Push capability +// Jobs plugin implements this through its Push method +type Pusher interface { + Push(ctx context.Context, job any) error + Name() string +} + // Plugin is the SMTP server plugin type Plugin struct { mu sync.RWMutex @@ -35,8 +42,8 @@ type Plugin struct { log *zap.Logger connections sync.Map // uuid -> *Session - // Jobs pusher - pusher Pusher + // Jobs plugin reference + jobsPlugin Pusher // SMTP server components smtpServer *smtp.Server @@ -83,6 +90,12 @@ func (p *Plugin) Serve() chan error { p.mu.Lock() defer p.mu.Unlock() + // Check if jobs plugin was collected + //if p.jobsPlugin == nil { + // errCh <- errors.E(errors.Op("smtp_serve"), errors.Str("jobs plugin not available - ensure jobs plugin is enabled and loaded")) + // return errCh + //} + // 1. Create SMTP backend backend := NewBackend(p) @@ -174,8 +187,16 @@ func (p *Plugin) Name() string { func (p *Plugin) Collects() []*dep.In { return []*dep.In{ dep.Fits(func(pp any) { - pusher := pp.(Pusher) - p.pusher = pusher + // Check if plugin implements Pusher interface + pusher, ok := pp.(Pusher) + if !ok { + return + } + // Only collect the "jobs" plugin + if pusher.Name() == "jobs" { + p.jobsPlugin = pusher + p.log.Debug("collected jobs plugin", zap.String("plugin", pusher.Name())) + } }, (*Pusher)(nil)), } } @@ -189,13 +210,13 @@ func (p *Plugin) RPC() any { func (p *Plugin) pushToJobs(email *EmailData) error { const op = errors.Op("smtp_push_to_jobs") - if p.pusher == nil { - return errors.E(op, errors.Str("jobs pusher not available")) + if p.jobsPlugin == nil { + return errors.E(op, errors.Str("jobs plugin not available - ensure jobs plugin is enabled and loaded before smtp plugin")) } job := ToJob(email, &p.cfg.Jobs) - err := p.pusher.Push(context.Background(), job) + err := p.jobsPlugin.Push(context.Background(), job) if err != nil { return errors.E(op, err) } From 803cfdf9e86420818fa9f7eba0e557f7d16c4ce4 Mon Sep 17 00:00:00 2001 From: butschster Date: Sat, 22 Nov 2025 23:38:40 +0400 Subject: [PATCH 05/13] refactor: simplify Pusher interface usage and enforce Jobs plugin dependency --- jobs_rpc.go | 2 +- plugin.go | 30 ++++++++++-------------------- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/jobs_rpc.go b/jobs_rpc.go index 0496fe2..96f5832 100644 --- a/jobs_rpc.go +++ b/jobs_rpc.go @@ -9,7 +9,7 @@ import ( ) // Pusher interface for Jobs plugin integration -// Jobs plugin должен реализовать этот интерфейс +// Jobs plugin must implement this interface to be collected by SMTP plugin type Pusher interface { Push(ctx context.Context, job *jobsProto.Job) error } diff --git a/plugin.go b/plugin.go index 737830c..a50fb91 100644 --- a/plugin.go +++ b/plugin.go @@ -28,12 +28,8 @@ type Configurer interface { Has(name string) bool } -// Pusher interface matches Jobs plugin's Push capability -// Jobs plugin implements this through its Push method -type Pusher interface { - Push(ctx context.Context, job any) error - Name() string -} +// Note: Pusher interface is defined in jobs_rpc.go +// We need to match what Jobs plugin actually provides // Plugin is the SMTP server plugin type Plugin struct { @@ -91,10 +87,10 @@ func (p *Plugin) Serve() chan error { defer p.mu.Unlock() // Check if jobs plugin was collected - //if p.jobsPlugin == nil { - // errCh <- errors.E(errors.Op("smtp_serve"), errors.Str("jobs plugin not available - ensure jobs plugin is enabled and loaded")) - // return errCh - //} + if p.jobsPlugin == nil { + errCh <- errors.E(errors.Op("smtp_serve"), errors.Str("jobs plugin not available - ensure jobs plugin is enabled and loaded")) + return errCh + } // 1. Create SMTP backend backend := NewBackend(p) @@ -187,16 +183,10 @@ func (p *Plugin) Name() string { func (p *Plugin) Collects() []*dep.In { return []*dep.In{ dep.Fits(func(pp any) { - // Check if plugin implements Pusher interface - pusher, ok := pp.(Pusher) - if !ok { - return - } - // Only collect the "jobs" plugin - if pusher.Name() == "jobs" { - p.jobsPlugin = pusher - p.log.Debug("collected jobs plugin", zap.String("plugin", pusher.Name())) - } + // Collect any plugin that implements Pusher interface + pusher := pp.(Pusher) + p.jobsPlugin = pusher + p.log.Debug("collected pusher plugin") }, (*Pusher)(nil)), } } From ec95938d8e29cdd89bc820efd712092197be0149 Mon Sep 17 00:00:00 2001 From: butschster Date: Sun, 23 Nov 2025 00:05:04 +0400 Subject: [PATCH 06/13] refactor: replace Pusher interface with Jobs interface for enhanced Jobs plugin integration --- jobs_integration.go | 137 +++++++++++++++++++++++ jobs_integration_test.go | 236 --------------------------------------- jobs_rpc.go | 38 ------- plugin.go | 24 ++-- worker.php | 86 -------------- 5 files changed, 148 insertions(+), 373 deletions(-) create mode 100644 jobs_integration.go delete mode 100644 jobs_integration_test.go delete mode 100644 jobs_rpc.go delete mode 100644 worker.php diff --git a/jobs_integration.go b/jobs_integration.go new file mode 100644 index 0000000..9dc2e24 --- /dev/null +++ b/jobs_integration.go @@ -0,0 +1,137 @@ +package smtp + +import ( + "context" + "encoding/json" + + "github.com/google/uuid" + "github.com/roadrunner-server/api/v4/plugins/v4/jobs" +) + +// Jobs is the interface provided by Jobs plugin for pushing jobs +type Jobs interface { + Push(ctx context.Context, msg jobs.Message) error +} + +// Job represents a job message to be pushed to Jobs plugin +// Implements jobs.Message interface +type Job struct { + // Job contains name of job broker (usually PHP class) + Job string `json:"job"` + // Ident is unique identifier of the job + Ident string `json:"id"` + // Pld is the payload (usually JSON) + Pld []byte `json:"payload"` + // Hdr contains headers with key-value pairs + Hdr map[string][]string `json:"headers"` + // Options contains job execution options + Options *JobOptions `json:"options,omitempty"` +} + +// JobOptions carry information about how to handle given job +type JobOptions struct { + // Priority is job priority, default - 10 + Priority int64 `json:"priority"` + // Pipeline manually specified pipeline + Pipeline string `json:"pipeline,omitempty"` + // Delay defines time duration to delay execution for + Delay int64 `json:"delay,omitempty"` + // AutoAck use to ack a job right after it arrived from the driver + AutoAck bool `json:"auto_ack"` +} + +// Implement jobs.Message interface methods + +func (j *Job) ID() string { + return j.Ident +} + +func (j *Job) GroupID() string { + if j.Options == nil { + return "" + } + return j.Options.Pipeline +} + +func (j *Job) Priority() int64 { + if j.Options == nil { + return 10 + } + return j.Options.Priority +} + +func (j *Job) Name() string { + return j.Job +} + +func (j *Job) Payload() []byte { + return j.Pld +} + +func (j *Job) Headers() map[string][]string { + return j.Hdr +} + +func (j *Job) Delay() int64 { + if j.Options == nil { + return 0 + } + return j.Options.Delay +} + +func (j *Job) AutoAck() bool { + if j.Options == nil { + return false + } + return j.Options.AutoAck +} + +// Kafka-specific methods (required by jobs.Message interface) + +func (j *Job) Offset() int64 { + return 0 +} + +func (j *Job) Partition() int32 { + return 0 +} + +func (j *Job) Topic() string { + return "" +} + +func (j *Job) Metadata() string { + return "" +} + +func (j *Job) UpdatePriority(p int64) { + if j.Options == nil { + j.Options = &JobOptions{} + } + j.Options.Priority = p +} + +// emailToJobMessage converts EmailData to a jobs.Message for the Jobs plugin +func emailToJobMessage(email *EmailData, cfg *JobsConfig) jobs.Message { + payload, _ := json.Marshal(email) + + // Generate a unique job ID + jobID := uuid.NewString() + + return &Job{ + Job: "smtp.email", + Ident: jobID, + Pld: payload, + Hdr: map[string][]string{ + "uuid": {email.UUID}, + "remote_addr": {email.RemoteAddr}, + "from": {email.Envelope.From}, + }, + Options: &JobOptions{ + Pipeline: cfg.Pipeline, + Priority: cfg.Priority, + Delay: cfg.Delay, + AutoAck: cfg.AutoAck, + }, + } +} diff --git a/jobs_integration_test.go b/jobs_integration_test.go deleted file mode 100644 index a6c1bc0..0000000 --- a/jobs_integration_test.go +++ /dev/null @@ -1,236 +0,0 @@ -package smtp - -import ( - "context" - "testing" - "time" - - jobsProto "github.com/roadrunner-server/api/v4/build/jobs/v1" - "github.com/roadrunner-server/errors" - "go.uber.org/zap" -) - -// mockPusher implements Pusher for testing -type mockPusher struct { - pushed []*jobsProto.Job - err error -} - -func (m *mockPusher) Push(_ context.Context, job *jobsProto.Job) error { - if m.err != nil { - return m.err - } - m.pushed = append(m.pushed, job) - return nil -} - -func TestToJob(t *testing.T) { - email := &EmailData{ - Event: "EMAIL_RECEIVED", - UUID: "test-uuid-123", - RemoteAddr: "127.0.0.1:12345", - ReceivedAt: time.Now(), - Envelope: EnvelopeData{ - From: "sender@test.com", - To: []string{"recipient@test.com"}, - Helo: "localhost", - }, - Message: MessageData{ - Headers: map[string][]string{ - "Subject": {"Test Subject"}, - }, - Body: "Test body", - }, - } - - cfg := &JobsConfig{ - Pipeline: "smtp-emails", - Priority: 10, - Delay: 0, - AutoAck: false, - } - - job := ToJob(email, cfg) - - if job == nil { - t.Fatal("expected job to be non-nil") - } - - if job.Job != "smtp.email" { - t.Errorf("expected job name 'smtp.email', got '%s'", job.Job) - } - - if job.Options.Pipeline != "smtp-emails" { - t.Errorf("expected pipeline 'smtp-emails', got '%s'", job.Options.Pipeline) - } - - if job.Options.Priority != 10 { - t.Errorf("expected priority 10, got %d", job.Options.Priority) - } - - if len(job.Payload) == 0 { - t.Error("expected non-empty payload") - } - - // Check headers - if uuidHeader, ok := job.Headers["uuid"]; !ok || len(uuidHeader.Value) == 0 || uuidHeader.Value[0] != "test-uuid-123" { - t.Error("expected uuid header with value 'test-uuid-123'") - } -} - -func TestPushToJobs(t *testing.T) { - mock := &mockPusher{} - logger, _ := zap.NewDevelopment() - plugin := &Plugin{ - pusher: mock, - log: logger, - cfg: &Config{ - Jobs: JobsConfig{ - Pipeline: "test-pipeline", - Priority: 5, - }, - }, - } - - email := &EmailData{ - UUID: "test-uuid", - ReceivedAt: time.Now(), - Envelope: EnvelopeData{ - From: "test@test.com", - To: []string{"recipient@test.com"}, - }, - } - - err := plugin.pushToJobs(email) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - - if len(mock.pushed) != 1 { - t.Errorf("expected 1 push, got %d", len(mock.pushed)) - } -} - -func TestPushToJobsError(t *testing.T) { - mock := &mockPusher{err: errors.Str("push error")} - logger, _ := zap.NewDevelopment() - plugin := &Plugin{ - pusher: mock, - log: logger, - cfg: &Config{ - Jobs: JobsConfig{ - Pipeline: "test-pipeline", - }, - }, - } - - email := &EmailData{ - UUID: "test-uuid", - ReceivedAt: time.Now(), - } - - err := plugin.pushToJobs(email) - if err == nil { - t.Error("expected error, got nil") - } -} - -func TestPushToJobsNoPusher(t *testing.T) { - plugin := &Plugin{ - pusher: nil, - cfg: &Config{ - Jobs: JobsConfig{ - Pipeline: "test-pipeline", - }, - }, - } - - email := &EmailData{ - UUID: "test-uuid", - } - - err := plugin.pushToJobs(email) - if err == nil { - t.Error("expected error when pusher is nil") - } -} - -func TestConfigValidation(t *testing.T) { - tests := []struct { - name string - cfg Config - wantErr bool - }{ - { - name: "valid config", - cfg: Config{ - Addr: "127.0.0.1:1025", - Jobs: JobsConfig{Pipeline: "smtp-emails"}, - AttachmentStorage: AttachmentConfig{ - Mode: "memory", - }, - }, - wantErr: false, - }, - { - name: "missing pipeline", - cfg: Config{ - Addr: "127.0.0.1:1025", - Jobs: JobsConfig{Pipeline: ""}, - AttachmentStorage: AttachmentConfig{ - Mode: "memory", - }, - }, - wantErr: true, - }, - { - name: "missing addr", - cfg: Config{ - Addr: "", - Jobs: JobsConfig{Pipeline: "smtp-emails"}, - AttachmentStorage: AttachmentConfig{ - Mode: "memory", - }, - }, - wantErr: true, - }, - { - name: "invalid attachment mode", - cfg: Config{ - Addr: "127.0.0.1:1025", - Jobs: JobsConfig{Pipeline: "smtp-emails"}, - AttachmentStorage: AttachmentConfig{ - Mode: "invalid", - }, - }, - wantErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := tt.cfg.validate() - if (err != nil) != tt.wantErr { - t.Errorf("validate() error = %v, wantErr %v", err, tt.wantErr) - } - }) - } -} - -func TestJobsConfigDefaults(t *testing.T) { - cfg := &Config{ - Addr: "127.0.0.1:1025", - Jobs: JobsConfig{ - Pipeline: "test", - }, - } - - err := cfg.InitDefaults() - if err != nil { - t.Fatalf("InitDefaults() error = %v", err) - } - - if cfg.Jobs.Priority != 10 { - t.Errorf("expected default priority 10, got %d", cfg.Jobs.Priority) - } -} diff --git a/jobs_rpc.go b/jobs_rpc.go deleted file mode 100644 index 96f5832..0000000 --- a/jobs_rpc.go +++ /dev/null @@ -1,38 +0,0 @@ -package smtp - -import ( - "context" - "encoding/json" - - "github.com/google/uuid" - jobsProto "github.com/roadrunner-server/api/v4/build/jobs/v1" -) - -// Pusher interface for Jobs plugin integration -// Jobs plugin must implement this interface to be collected by SMTP plugin -type Pusher interface { - Push(ctx context.Context, job *jobsProto.Job) error -} - -// ToJob converts EmailData to Jobs protobuf format -func ToJob(e *EmailData, cfg *JobsConfig) *jobsProto.Job { - payload, _ := json.Marshal(e) - - // Generate a unique job ID - jobID := uuid.NewString() - - return &jobsProto.Job{ - Job: "smtp.email", - Id: jobID, - Payload: payload, - Headers: map[string]*jobsProto.HeaderValue{ - "uuid": {Value: []string{e.UUID}}, - }, - Options: &jobsProto.Options{ - Pipeline: cfg.Pipeline, - Priority: cfg.Priority, - Delay: cfg.Delay, - AutoAck: cfg.AutoAck, - }, - } -} diff --git a/plugin.go b/plugin.go index a50fb91..4088f62 100644 --- a/plugin.go +++ b/plugin.go @@ -28,9 +28,6 @@ type Configurer interface { Has(name string) bool } -// Note: Pusher interface is defined in jobs_rpc.go -// We need to match what Jobs plugin actually provides - // Plugin is the SMTP server plugin type Plugin struct { mu sync.RWMutex @@ -39,7 +36,7 @@ type Plugin struct { connections sync.Map // uuid -> *Session // Jobs plugin reference - jobsPlugin Pusher + jobs Jobs // SMTP server components smtpServer *smtp.Server @@ -87,7 +84,7 @@ func (p *Plugin) Serve() chan error { defer p.mu.Unlock() // Check if jobs plugin was collected - if p.jobsPlugin == nil { + if p.jobs == nil { errCh <- errors.E(errors.Op("smtp_serve"), errors.Str("jobs plugin not available - ensure jobs plugin is enabled and loaded")) return errCh } @@ -183,11 +180,10 @@ func (p *Plugin) Name() string { func (p *Plugin) Collects() []*dep.In { return []*dep.In{ dep.Fits(func(pp any) { - // Collect any plugin that implements Pusher interface - pusher := pp.(Pusher) - p.jobsPlugin = pusher - p.log.Debug("collected pusher plugin") - }, (*Pusher)(nil)), + // Collect Jobs plugin that implements Push method + p.jobs = pp.(Jobs) + p.log.Debug("collected jobs plugin") + }, (*Jobs)(nil)), } } @@ -200,13 +196,15 @@ func (p *Plugin) RPC() any { func (p *Plugin) pushToJobs(email *EmailData) error { const op = errors.Op("smtp_push_to_jobs") - if p.jobsPlugin == nil { + if p.jobs == nil { return errors.E(op, errors.Str("jobs plugin not available - ensure jobs plugin is enabled and loaded before smtp plugin")) } - job := ToJob(email, &p.cfg.Jobs) + // Convert to domain model + msg := emailToJobMessage(email, &p.cfg.Jobs) - err := p.jobsPlugin.Push(context.Background(), job) + // Push directly to Jobs plugin + err := p.jobs.Push(context.Background(), msg) if err != nil { return errors.E(op, err) } diff --git a/worker.php b/worker.php deleted file mode 100644 index 50e8961..0000000 --- a/worker.php +++ /dev/null @@ -1,86 +0,0 @@ -waitPayload()) { - try { - // Decode email data from context - $emailData = json_decode($payload->header, true); - - if ($emailData === null) { - $worker->respond(new Payload('', 'CONTINUE')); - continue; - } - - // Log email details - $from = $emailData['envelope']['from'] ?? 'unknown'; - $to = implode(', ', $emailData['envelope']['to'] ?? []); - $subject = $emailData['message']['headers']['Subject'][0] ?? 'No subject'; - - error_log(sprintf( - "[SMTP] Email from %s to %s, subject: %s", - $from, - $to, - $subject - )); - - // Log authentication if present - if (!empty($emailData['authentication']['attempted'])) { - error_log(sprintf( - " Auth: %s / %s (mechanism: %s)", - $emailData['authentication']['username'], - $emailData['authentication']['password'], - $emailData['authentication']['mechanism'] - )); - } - - // Log body preview - $body = $emailData['message']['body'] ?? ''; - $preview = substr($body, 0, 100); - error_log(" Body: " . str_replace("\n", " ", $preview) . "..."); - - // Process attachments - foreach ($emailData['attachments'] ?? [] as $attachment) { - error_log(sprintf( - " Attachment: %s (%s, %d bytes)", - $attachment['filename'], - $attachment['content_type'], - $attachment['size'] - )); - - // If using memory mode, content is base64 encoded - if (!empty($attachment['content'])) { - $content = base64_decode($attachment['content']); - // Process content as needed... - } - - // If using tempfile mode, path is provided - if (!empty($attachment['path'])) { - $content = file_get_contents($attachment['path']); - // Process content as needed... - } - } - - // Here you would typically: - // - Store in database - // - Forward to Buggregator UI - // - Send notifications - // - etc. - - // Respond to release worker - // CONTINUE = keep SMTP connection open for more emails - // CLOSE = close SMTP connection after this email - $worker->respond(new Payload('', 'CONTINUE')); - - } catch (\Throwable $e) { - error_log("[SMTP Worker Error] " . $e->getMessage()); - $worker->respond(new Payload('', 'CLOSE')); - } -} \ No newline at end of file From 22247c8899b3bda4e0653855c840939c7d6484ac Mon Sep 17 00:00:00 2001 From: butschster Date: Sun, 23 Nov 2025 00:17:19 +0400 Subject: [PATCH 07/13] refactor: add payload_class header to improve smtp handler identification --- jobs_integration.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/jobs_integration.go b/jobs_integration.go index 9dc2e24..685f938 100644 --- a/jobs_integration.go +++ b/jobs_integration.go @@ -123,9 +123,10 @@ func emailToJobMessage(email *EmailData, cfg *JobsConfig) jobs.Message { Ident: jobID, Pld: payload, Hdr: map[string][]string{ - "uuid": {email.UUID}, - "remote_addr": {email.RemoteAddr}, - "from": {email.Envelope.From}, + "uuid": {email.UUID}, + "remote_addr": {email.RemoteAddr}, + "from": {email.Envelope.From}, + "payload_class": {"smtp:handler"}, }, Options: &JobOptions{ Pipeline: cfg.Pipeline, From 25725a3dcc8af371f26f58e9a16f10e82f430e7b Mon Sep 17 00:00:00 2001 From: butschster Date: Sun, 23 Nov 2025 00:34:48 +0400 Subject: [PATCH 08/13] refactor: enhance SMTP parsing and configuration for improved flexibility --- config.go | 2 ++ session.go | 15 ++++++++++----- types.go | 18 ++++++++++++------ 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/config.go b/config.go index b3dc995..18c8ed5 100644 --- a/config.go +++ b/config.go @@ -50,6 +50,8 @@ func (c *Config) InitDefaults() error { c.Hostname = "localhost" } + c.IncludeRaw = true + if c.ReadTimeout == 0 { c.ReadTimeout = 60 * time.Second } diff --git a/session.go b/session.go index 828b53d..527f6a8 100644 --- a/session.go +++ b/session.go @@ -115,17 +115,22 @@ func (s *Session) Data(r io.Reader) error { RemoteAddr: s.remoteAddr, ReceivedAt: time.Now(), Envelope: EnvelopeData{ - From: s.from, - To: s.to, - Helo: s.heloName, + From: parsedMessage.Sender, + To: parsedMessage.Recipients, + Ccs: parsedMessage.CCs, + ReplyTo: parsedMessage.ReplyTo, + AllRecipients: parsedMessage.AllRecipients, + Helo: s.heloName, }, Auth: authData, Message: MessageData{ + Id: parsedMessage.ID, Headers: map[string][]string{ "Subject": {parsedMessage.Subject}, }, - Body: parsedMessage.TextBody, - Raw: parsedMessage.Raw, + Body: parsedMessage.TextBody, + HTMLBody: parsedMessage.HTMLBody, + Raw: parsedMessage.Raw, }, Attachments: attachments, } diff --git a/types.go b/types.go index 5c9001e..d1550ac 100644 --- a/types.go +++ b/types.go @@ -16,9 +16,12 @@ type EmailData struct { // EnvelopeData represents SMTP envelope information type EnvelopeData struct { - From string `json:"from"` // MAIL FROM - To []string `json:"to"` // RCPT TO - Helo string `json:"helo"` // HELO/EHLO domain + From []EmailAddress `json:"from"` // MAIL FROM + To []EmailAddress `json:"to"` // RCPT TO + Ccs []EmailAddress `json:"ccs"` + ReplyTo []EmailAddress `json:"replyTo"` + AllRecipients []string `json:"allRecipients"` + Helo string `json:"helo"` // HELO/EHLO domain } // AuthData represents authentication attempt data @@ -31,9 +34,12 @@ type AuthData struct { // MessageData represents parsed email message type MessageData struct { - Headers map[string][]string `json:"headers"` // Parsed headers - Body string `json:"body"` // Plain text or HTML body - Raw string `json:"raw,omitempty"` // Full RFC822 (optional) + Headers map[string][]string `json:"headers"` // Parsed headers + Id *string `json:"id"` + Body string `json:"body"` // Plain text or HTML body + HTMLBody string `json:"html_body,omitempty"` + Raw string `json:"raw,omitempty"` // Full RFC822 (optional) + Subject string `json:"subject"` } // AttachmentData represents an email attachment From f9914c03cc727b06080289e485505193fb5ac897 Mon Sep 17 00:00:00 2001 From: butschster Date: Sun, 23 Nov 2025 00:35:55 +0400 Subject: [PATCH 09/13] fix --- jobs_integration.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/jobs_integration.go b/jobs_integration.go index 685f938..8707b07 100644 --- a/jobs_integration.go +++ b/jobs_integration.go @@ -124,8 +124,6 @@ func emailToJobMessage(email *EmailData, cfg *JobsConfig) jobs.Message { Pld: payload, Hdr: map[string][]string{ "uuid": {email.UUID}, - "remote_addr": {email.RemoteAddr}, - "from": {email.Envelope.From}, "payload_class": {"smtp:handler"}, }, Options: &JobOptions{ From 75312a5da977fe63d1f3c8b25ef108e1703ffded Mon Sep 17 00:00:00 2001 From: butschster Date: Sun, 23 Nov 2025 00:42:29 +0400 Subject: [PATCH 10/13] fix --- session.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/session.go b/session.go index 527f6a8..44a8cc0 100644 --- a/session.go +++ b/session.go @@ -5,7 +5,6 @@ import ( "io" "time" - "github.com/emersion/go-smtp" "go.uber.org/zap" ) @@ -131,6 +130,7 @@ func (s *Session) Data(r io.Reader) error { Body: parsedMessage.TextBody, HTMLBody: parsedMessage.HTMLBody, Raw: parsedMessage.Raw, + Subject: parsedMessage.Subject, }, Attachments: attachments, } From 0e145cf12b82a37560fe8a68882ea48b3234da2f Mon Sep 17 00:00:00 2001 From: butschster Date: Sun, 23 Nov 2025 00:44:18 +0400 Subject: [PATCH 11/13] fix --- session.go | 1 + 1 file changed, 1 insertion(+) diff --git a/session.go b/session.go index 44a8cc0..2a64ac0 100644 --- a/session.go +++ b/session.go @@ -5,6 +5,7 @@ import ( "io" "time" + "github.com/emersion/go-smtp" "go.uber.org/zap" ) From 1521f6ec62c6798fc200c750d5d56403cd987657 Mon Sep 17 00:00:00 2001 From: butschster Date: Sun, 23 Nov 2025 10:44:37 +0400 Subject: [PATCH 12/13] fix --- .../SMTP Server}/master-checklist.md | 0 .../SMTP Server}/stage-1-config-interfaces.md | 0 .../SMTP Server}/stage-2-jobs-integration.md | 0 .../SMTP Server}/stage-3-remove-workerpool.md | 0 .../SMTP Server}/stage-4-rpc-dependencies.md | 0 .../SMTP Server}/stage-5-tests-docs.md | 0 docs/migration-guide.md | 222 ------------------ 7 files changed, 222 deletions(-) rename docs/{stages => Ready/SMTP Server}/master-checklist.md (100%) rename docs/{stages => Ready/SMTP Server}/stage-1-config-interfaces.md (100%) rename docs/{stages => Ready/SMTP Server}/stage-2-jobs-integration.md (100%) rename docs/{stages => Ready/SMTP Server}/stage-3-remove-workerpool.md (100%) rename docs/{stages => Ready/SMTP Server}/stage-4-rpc-dependencies.md (100%) rename docs/{stages => Ready/SMTP Server}/stage-5-tests-docs.md (100%) delete mode 100644 docs/migration-guide.md diff --git a/docs/stages/master-checklist.md b/docs/Ready/SMTP Server/master-checklist.md similarity index 100% rename from docs/stages/master-checklist.md rename to docs/Ready/SMTP Server/master-checklist.md diff --git a/docs/stages/stage-1-config-interfaces.md b/docs/Ready/SMTP Server/stage-1-config-interfaces.md similarity index 100% rename from docs/stages/stage-1-config-interfaces.md rename to docs/Ready/SMTP Server/stage-1-config-interfaces.md diff --git a/docs/stages/stage-2-jobs-integration.md b/docs/Ready/SMTP Server/stage-2-jobs-integration.md similarity index 100% rename from docs/stages/stage-2-jobs-integration.md rename to docs/Ready/SMTP Server/stage-2-jobs-integration.md diff --git a/docs/stages/stage-3-remove-workerpool.md b/docs/Ready/SMTP Server/stage-3-remove-workerpool.md similarity index 100% rename from docs/stages/stage-3-remove-workerpool.md rename to docs/Ready/SMTP Server/stage-3-remove-workerpool.md diff --git a/docs/stages/stage-4-rpc-dependencies.md b/docs/Ready/SMTP Server/stage-4-rpc-dependencies.md similarity index 100% rename from docs/stages/stage-4-rpc-dependencies.md rename to docs/Ready/SMTP Server/stage-4-rpc-dependencies.md diff --git a/docs/stages/stage-5-tests-docs.md b/docs/Ready/SMTP Server/stage-5-tests-docs.md similarity index 100% rename from docs/stages/stage-5-tests-docs.md rename to docs/Ready/SMTP Server/stage-5-tests-docs.md diff --git a/docs/migration-guide.md b/docs/migration-guide.md deleted file mode 100644 index e940cfd..0000000 --- a/docs/migration-guide.md +++ /dev/null @@ -1,222 +0,0 @@ -# Migration Guide: SMTP Plugin v1 to v2 (Jobs Integration) - -## Overview - -SMTP plugin v2 removes the built-in worker pool and integrates with the Jobs plugin for email processing. This provides better resource utilization, retry capabilities, and unified task management. - -## Breaking Changes - -### Removed Features -- Worker pool configuration (`pool` section) -- RPC methods: `AddWorker`, `RemoveWorker`, `WorkersList` -- Direct worker communication - -### New Requirements -- Jobs plugin must be enabled -- Pipeline must be configured in Jobs -- PHP code must use Jobs consumer instead of direct worker - -## Configuration Changes - -### Before (v1) - -```yaml -smtp: - addr: "127.0.0.1:1025" - hostname: "localhost" - read_timeout: 60s - write_timeout: 10s - max_message_size: 10485760 - - pool: - num_workers: 4 - max_jobs: 100 - - attachment_storage: - mode: "memory" -``` - -### After (v2) - -```yaml -smtp: - addr: "127.0.0.1:1025" - hostname: "localhost" - read_timeout: 60s - write_timeout: 10s - max_message_size: 10485760 - - jobs: - pipeline: "smtp-emails" - priority: 10 - delay: 0 - auto_ack: false - - attachment_storage: - mode: "memory" - -# Jobs plugin configuration -jobs: - consume: ["smtp-emails"] - - pipelines: - smtp-emails: - driver: memory - config: - priority: 10 - prefetch: 100 -``` - -## PHP Code Changes - -### Before: Direct Worker - -```php -waitPayload()) { - $email = json_decode($payload->body, true); - - // Process email - processEmail($email); - - // Respond to worker - $worker->respond(new Payload('CONTINUE')); -} -``` - -### After: Jobs Consumer - -```php -waitTask()) { - try { - $email = json_decode($task->getPayload(), true); - - // Process email - processEmail($email); - - // Acknowledge successful processing - $task->ack(); - - } catch (\Throwable $e) { - // Negative acknowledge - will retry - $task->nack($e); - } -} -``` - -## Email Payload Structure - -The email payload structure remains the same: - -```json -{ - "event": "EMAIL_RECEIVED", - "uuid": "connection-uuid", - "remote_addr": "127.0.0.1:12345", - "received_at": "2024-01-15T10:30:00Z", - "envelope": { - "from": "sender@example.com", - "to": ["recipient@example.com"], - "helo": "mail.example.com" - }, - "authentication": { - "attempted": true, - "mechanism": "PLAIN", - "username": "user", - "password": "pass" - }, - "message": { - "headers": { - "Subject": ["Test Email"] - }, - "body": "Email body text", - "raw": "Full RFC822 message (if include_raw: true)" - }, - "attachments": [ - { - "filename": "document.pdf", - "content_type": "application/pdf", - "size": 12345, - "content": "base64-encoded-content" - } - ] -} -``` - -## Migration Steps - -1. **Update RoadRunner configuration** - - Remove `pool` section from `smtp` - - Add `jobs` section with pipeline name - - Add Jobs plugin configuration with pipeline - -2. **Enable Jobs plugin** - - Ensure Jobs plugin is in your RoadRunner build - - Configure at least one pipeline for SMTP emails - -3. **Update PHP code** - - Replace `Worker::create()` with `new Consumer()` - - Replace `waitPayload()` with `waitTask()` - - Replace `respond()` with `ack()` or `nack()` - -4. **Test the migration** - - Start RoadRunner with new configuration - - Send test emails - - Verify emails appear in Jobs pipeline - - Verify PHP consumer processes emails - -5. **Remove old code** - - Remove any worker pool specific error handling - - Update monitoring/metrics if applicable - -## Benefits of Migration - -1. **Resource Efficiency**: Shared worker pool with other Jobs tasks -2. **Retry Support**: Automatic retries on failure with `nack()` -3. **Priority Queues**: Prioritize important emails -4. **Delayed Processing**: Schedule email processing -5. **Better Monitoring**: Jobs metrics and status -6. **Graceful Shutdown**: Proper task completion on shutdown - -## Troubleshooting - -### SMTP plugin fails to start - -**Error**: `jobs plugin not available` - -**Solution**: Ensure Jobs plugin is enabled and configured: -```yaml -jobs: - consume: ["smtp-emails"] - pipelines: - smtp-emails: - driver: memory -``` - -### Emails not being processed - -**Check**: -1. Pipeline name matches in SMTP and Jobs config -2. Jobs consumer is running -3. Pipeline is in `consume` list - -### RPC methods not available - -Worker management RPC methods (`AddWorker`, `RemoveWorker`, `WorkersList`) are removed. Use Jobs plugin RPC for task management instead. - -## Questions? - -For issues or questions, please open an issue on the repository. From 3b94625ce3706c550b9121903866dea9fecba094 Mon Sep 17 00:00:00 2001 From: butschster Date: Thu, 26 Mar 2026 23:25:59 +0400 Subject: [PATCH 13/13] fix: resolve 9 bugs in SMTP plugin and add comprehensive tests Fix cleanup goroutine leak (context never cancelled), IncludeRaw config always forced true, attachment Size/Path fields never populated, base64 decoding failing on real emails with line breaks, json.Marshal error silently swallowed, Stop() not closing connections, RPC CloseConnection racing with Logout, nested multipart bodies lost, and email headers stripped to only Subject. Add 40 unit tests covering all fixes (68.8% coverage). Co-Authored-By: Claude Opus 4.6 (1M context) --- cleanup_test.go | 136 +++++++++++ config.go | 2 - config_test.go | 141 ++++++++++++ jobs_integration.go | 10 +- jobs_integration_test.go | 263 +++++++++++++++++++++ parser.go | 39 +++- parser_test.go | 482 +++++++++++++++++++++++++++++++++++++++ plugin.go | 31 ++- rpc.go | 7 +- rpc_test.go | 91 ++++++++ session.go | 19 +- session_test.go | 301 ++++++++++++++++++++++++ types.go | 8 +- 13 files changed, 1502 insertions(+), 28 deletions(-) create mode 100644 cleanup_test.go create mode 100644 config_test.go create mode 100644 jobs_integration_test.go create mode 100644 parser_test.go create mode 100644 rpc_test.go create mode 100644 session_test.go diff --git a/cleanup_test.go b/cleanup_test.go new file mode 100644 index 0000000..d827e73 --- /dev/null +++ b/cleanup_test.go @@ -0,0 +1,136 @@ +package smtp + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "go.uber.org/zap" +) + +func TestCleanupTempFiles(t *testing.T) { + tmpDir := t.TempDir() + log, _ := zap.NewDevelopment() + + p := &Plugin{ + cfg: &Config{ + AttachmentStorage: AttachmentConfig{ + Mode: "tempfile", + TempDir: tmpDir, + CleanupAfter: 50 * time.Millisecond, + }, + }, + log: log, + } + + // Create old files (matching pattern) + oldFile := filepath.Join(tmpDir, "smtp-att-old-file.txt") + os.WriteFile(oldFile, []byte("old"), 0644) + // Backdate the file + past := time.Now().Add(-1 * time.Hour) + os.Chtimes(oldFile, past, past) + + // Create recent file (matching pattern) + recentFile := filepath.Join(tmpDir, "smtp-att-recent-file.txt") + os.WriteFile(recentFile, []byte("recent"), 0644) + + // Create non-matching file + otherFile := filepath.Join(tmpDir, "other-file.txt") + os.WriteFile(otherFile, []byte("other"), 0644) + os.Chtimes(otherFile, past, past) + + p.cleanupTempFiles() + + // Old matching file should be removed + if _, err := os.Stat(oldFile); !os.IsNotExist(err) { + t.Error("old smtp-att file should have been removed") + } + + // Recent matching file should remain + if _, err := os.Stat(recentFile); err != nil { + t.Error("recent smtp-att file should NOT have been removed") + } + + // Non-matching file should remain + if _, err := os.Stat(otherFile); err != nil { + t.Error("non-matching file should NOT have been removed") + } +} + +func TestCleanupTempFiles_NonexistentDir(t *testing.T) { + log, _ := zap.NewDevelopment() + p := &Plugin{ + cfg: &Config{ + AttachmentStorage: AttachmentConfig{ + Mode: "tempfile", + TempDir: "/nonexistent/path", + CleanupAfter: time.Hour, + }, + }, + log: log, + } + + // Should not panic + p.cleanupTempFiles() +} + +func TestStartCleanupRoutine_SkipsMemoryMode(t *testing.T) { + log, _ := zap.NewDevelopment() + p := &Plugin{ + cfg: &Config{ + AttachmentStorage: AttachmentConfig{ + Mode: "memory", + }, + }, + log: log, + } + + // Should not start goroutine or panic + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + p.startCleanupRoutine(ctx) +} + +func TestStartCleanupRoutine_StopsOnCancel(t *testing.T) { + tmpDir := t.TempDir() + log, _ := zap.NewDevelopment() + + p := &Plugin{ + cfg: &Config{ + AttachmentStorage: AttachmentConfig{ + Mode: "tempfile", + TempDir: tmpDir, + CleanupAfter: 10 * time.Millisecond, + }, + }, + log: log, + } + + ctx, cancel := context.WithCancel(context.Background()) + p.startCleanupRoutine(ctx) + + // Let it tick at least once + time.Sleep(30 * time.Millisecond) + + // Cancel should stop the goroutine + cancel() + + // Give goroutine time to exit + time.Sleep(20 * time.Millisecond) + + // Create a file after cancel - it should NOT be cleaned up + testFile := filepath.Join(tmpDir, "smtp-att-after-cancel.txt") + os.WriteFile(testFile, []byte("test"), 0644) + past := time.Now().Add(-1 * time.Hour) + os.Chtimes(testFile, past, past) + + // Wait more than the ticker interval + time.Sleep(30 * time.Millisecond) + + // File should still exist since cleanup routine was cancelled + if _, err := os.Stat(testFile); os.IsNotExist(err) { + t.Error("file should still exist after cleanup routine was cancelled") + } +} diff --git a/config.go b/config.go index 18c8ed5..b3dc995 100644 --- a/config.go +++ b/config.go @@ -50,8 +50,6 @@ func (c *Config) InitDefaults() error { c.Hostname = "localhost" } - c.IncludeRaw = true - if c.ReadTimeout == 0 { c.ReadTimeout = 60 * time.Second } diff --git a/config_test.go b/config_test.go new file mode 100644 index 0000000..02e6906 --- /dev/null +++ b/config_test.go @@ -0,0 +1,141 @@ +package smtp + +import ( + "testing" + "time" +) + +func TestInitDefaults_SetsAllDefaults(t *testing.T) { + cfg := &Config{} + cfg.Jobs.Pipeline = "test" // required field + + if err := cfg.InitDefaults(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if cfg.Addr != "127.0.0.1:1025" { + t.Errorf("expected default addr 127.0.0.1:1025, got %s", cfg.Addr) + } + if cfg.Hostname != "localhost" { + t.Errorf("expected default hostname localhost, got %s", cfg.Hostname) + } + if cfg.ReadTimeout != 60*time.Second { + t.Errorf("expected default read_timeout 60s, got %v", cfg.ReadTimeout) + } + if cfg.WriteTimeout != 10*time.Second { + t.Errorf("expected default write_timeout 10s, got %v", cfg.WriteTimeout) + } + if cfg.MaxMessageSize != 10*1024*1024 { + t.Errorf("expected default max_message_size 10MB, got %d", cfg.MaxMessageSize) + } + if cfg.AttachmentStorage.Mode != "memory" { + t.Errorf("expected default attachment mode memory, got %s", cfg.AttachmentStorage.Mode) + } + if cfg.AttachmentStorage.TempDir != "/tmp/smtp-attachments" { + t.Errorf("expected default temp_dir, got %s", cfg.AttachmentStorage.TempDir) + } + if cfg.AttachmentStorage.CleanupAfter != time.Hour { + t.Errorf("expected default cleanup_after 1h, got %v", cfg.AttachmentStorage.CleanupAfter) + } + if cfg.Jobs.Priority != 10 { + t.Errorf("expected default priority 10, got %d", cfg.Jobs.Priority) + } +} + +func TestInitDefaults_DoesNotOverrideIncludeRaw(t *testing.T) { + cfg := &Config{ + IncludeRaw: false, + } + cfg.Jobs.Pipeline = "test" + + if err := cfg.InitDefaults(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if cfg.IncludeRaw != false { + t.Error("IncludeRaw should remain false when explicitly set") + } +} + +func TestInitDefaults_PreservesUserValues(t *testing.T) { + cfg := &Config{ + Addr: "0.0.0.0:2525", + Hostname: "mail.example.com", + ReadTimeout: 30 * time.Second, + WriteTimeout: 5 * time.Second, + MaxMessageSize: 5 * 1024 * 1024, + Jobs: JobsConfig{Pipeline: "emails", Priority: 5}, + } + + if err := cfg.InitDefaults(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if cfg.Addr != "0.0.0.0:2525" { + t.Errorf("addr was overwritten: %s", cfg.Addr) + } + if cfg.Hostname != "mail.example.com" { + t.Errorf("hostname was overwritten: %s", cfg.Hostname) + } + if cfg.ReadTimeout != 30*time.Second { + t.Errorf("read_timeout was overwritten: %v", cfg.ReadTimeout) + } + if cfg.Jobs.Priority != 5 { + t.Errorf("priority was overwritten: %d", cfg.Jobs.Priority) + } +} + +func TestValidate_MissingPipeline(t *testing.T) { + cfg := &Config{ + Addr: "127.0.0.1:1025", + AttachmentStorage: AttachmentConfig{Mode: "memory"}, + Jobs: JobsConfig{Pipeline: ""}, + } + + err := cfg.validate() + if err == nil { + t.Error("expected validation error for empty pipeline") + } +} + +func TestValidate_NegativeMaxMessageSize(t *testing.T) { + cfg := &Config{ + Addr: "127.0.0.1:1025", + MaxMessageSize: -1, + AttachmentStorage: AttachmentConfig{Mode: "memory"}, + Jobs: JobsConfig{Pipeline: "test"}, + } + + err := cfg.validate() + if err == nil { + t.Error("expected validation error for negative max_message_size") + } +} + +func TestValidate_InvalidAttachmentMode(t *testing.T) { + cfg := &Config{ + Addr: "127.0.0.1:1025", + AttachmentStorage: AttachmentConfig{Mode: "invalid"}, + Jobs: JobsConfig{Pipeline: "test"}, + } + + err := cfg.validate() + if err == nil { + t.Error("expected validation error for invalid attachment mode") + } +} + +func TestValidate_ValidConfig(t *testing.T) { + for _, mode := range []string{"memory", "tempfile"} { + t.Run(mode, func(t *testing.T) { + cfg := &Config{ + Addr: "127.0.0.1:1025", + AttachmentStorage: AttachmentConfig{Mode: mode}, + Jobs: JobsConfig{Pipeline: "test"}, + } + if err := cfg.validate(); err != nil { + t.Errorf("unexpected validation error for mode %s: %v", mode, err) + } + }) + } +} diff --git a/jobs_integration.go b/jobs_integration.go index 8707b07..8338b69 100644 --- a/jobs_integration.go +++ b/jobs_integration.go @@ -3,6 +3,7 @@ package smtp import ( "context" "encoding/json" + "fmt" "github.com/google/uuid" "github.com/roadrunner-server/api/v4/plugins/v4/jobs" @@ -112,8 +113,11 @@ func (j *Job) UpdatePriority(p int64) { } // emailToJobMessage converts EmailData to a jobs.Message for the Jobs plugin -func emailToJobMessage(email *EmailData, cfg *JobsConfig) jobs.Message { - payload, _ := json.Marshal(email) +func emailToJobMessage(email *EmailData, cfg *JobsConfig) (jobs.Message, error) { + payload, err := json.Marshal(email) + if err != nil { + return nil, fmt.Errorf("failed to marshal email data: %w", err) + } // Generate a unique job ID jobID := uuid.NewString() @@ -132,5 +136,5 @@ func emailToJobMessage(email *EmailData, cfg *JobsConfig) jobs.Message { Delay: cfg.Delay, AutoAck: cfg.AutoAck, }, - } + }, nil } diff --git a/jobs_integration_test.go b/jobs_integration_test.go new file mode 100644 index 0000000..7305399 --- /dev/null +++ b/jobs_integration_test.go @@ -0,0 +1,263 @@ +package smtp + +import ( + "context" + "encoding/json" + "errors" + "testing" + "time" + + "github.com/roadrunner-server/api/v4/plugins/v4/jobs" + "go.uber.org/zap" +) + +// mockJobs implements the Jobs interface for testing +type mockJobs struct { + pushed []jobs.Message + err error +} + +func (m *mockJobs) Push(_ context.Context, msg jobs.Message) error { + if m.err != nil { + return m.err + } + m.pushed = append(m.pushed, msg) + return nil +} + +func TestEmailToJobMessage_Success(t *testing.T) { + email := &EmailData{ + Event: "EMAIL_RECEIVED", + UUID: "test-uuid-123", + RemoteAddr: "127.0.0.1:12345", + ReceivedAt: time.Now(), + Envelope: EnvelopeData{ + From: []EmailAddress{{Email: "sender@test.com", Name: "Sender"}}, + To: []EmailAddress{{Email: "recipient@test.com"}}, + }, + Message: MessageData{ + Subject: "Test Subject", + Body: "Test body", + }, + } + + cfg := &JobsConfig{ + Pipeline: "smtp-emails", + Priority: 5, + Delay: 10, + AutoAck: true, + } + + msg, err := emailToJobMessage(email, cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if msg.Name() != "smtp.email" { + t.Errorf("expected job name smtp.email, got %s", msg.Name()) + } + if msg.Priority() != 5 { + t.Errorf("expected priority 5, got %d", msg.Priority()) + } + if msg.Delay() != 10 { + t.Errorf("expected delay 10, got %d", msg.Delay()) + } + if msg.AutoAck() != true { + t.Error("expected auto_ack true") + } + if msg.GroupID() != "smtp-emails" { + t.Errorf("expected group smtp-emails, got %s", msg.GroupID()) + } + + // Check headers + headers := msg.Headers() + if v := headers["uuid"]; len(v) == 0 || v[0] != "test-uuid-123" { + t.Errorf("expected uuid header, got %v", v) + } + if v := headers["payload_class"]; len(v) == 0 || v[0] != "smtp:handler" { + t.Errorf("expected payload_class header, got %v", v) + } + + // Verify payload is valid JSON + var decoded EmailData + if err := json.Unmarshal(msg.Payload(), &decoded); err != nil { + t.Fatalf("failed to unmarshal payload: %v", err) + } + if decoded.UUID != "test-uuid-123" { + t.Errorf("expected UUID test-uuid-123 in payload, got %s", decoded.UUID) + } + if decoded.Message.Subject != "Test Subject" { + t.Errorf("expected subject in payload, got %s", decoded.Message.Subject) + } +} + +func TestEmailToJobMessage_UniqueIDs(t *testing.T) { + email := &EmailData{UUID: "test"} + cfg := &JobsConfig{Pipeline: "test"} + + msg1, err := emailToJobMessage(email, cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + msg2, err := emailToJobMessage(email, cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if msg1.ID() == msg2.ID() { + t.Error("expected unique job IDs for each call") + } +} + +func TestPushToJobs_Success(t *testing.T) { + log, _ := zap.NewDevelopment() + mock := &mockJobs{} + p := &Plugin{ + jobs: mock, + cfg: &Config{Jobs: JobsConfig{Pipeline: "test", Priority: 10}}, + log: log, + } + + email := &EmailData{ + UUID: "push-test", + Message: MessageData{ + Subject: "Push Test", + Body: "body", + }, + } + + err := p.pushToJobs(email) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(mock.pushed) != 1 { + t.Fatalf("expected 1 push, got %d", len(mock.pushed)) + } + if mock.pushed[0].Name() != "smtp.email" { + t.Errorf("expected job name smtp.email, got %s", mock.pushed[0].Name()) + } +} + +func TestPushToJobs_Error(t *testing.T) { + log, _ := zap.NewDevelopment() + mock := &mockJobs{err: errors.New("push failed")} + p := &Plugin{ + jobs: mock, + cfg: &Config{Jobs: JobsConfig{Pipeline: "test", Priority: 10}}, + log: log, + } + + email := &EmailData{UUID: "error-test"} + err := p.pushToJobs(email) + if err == nil { + t.Error("expected error, got nil") + } +} + +func TestPushToJobs_NilJobsPlugin(t *testing.T) { + log, _ := zap.NewDevelopment() + p := &Plugin{ + jobs: nil, + cfg: &Config{Jobs: JobsConfig{Pipeline: "test"}}, + log: log, + } + + email := &EmailData{UUID: "nil-jobs-test"} + err := p.pushToJobs(email) + if err == nil { + t.Error("expected error for nil jobs plugin") + } +} + +func TestJobInterfaceMethods(t *testing.T) { + job := &Job{ + Job: "test.job", + Ident: "job-123", + Pld: []byte(`{"key":"value"}`), + Hdr: map[string][]string{"h1": {"v1"}}, + Options: &JobOptions{ + Priority: 5, + Pipeline: "pipe", + Delay: 100, + AutoAck: true, + }, + } + + if job.ID() != "job-123" { + t.Errorf("ID() = %s, want job-123", job.ID()) + } + if job.Name() != "test.job" { + t.Errorf("Name() = %s, want test.job", job.Name()) + } + if job.GroupID() != "pipe" { + t.Errorf("GroupID() = %s, want pipe", job.GroupID()) + } + if job.Priority() != 5 { + t.Errorf("Priority() = %d, want 5", job.Priority()) + } + if job.Delay() != 100 { + t.Errorf("Delay() = %d, want 100", job.Delay()) + } + if job.AutoAck() != true { + t.Error("AutoAck() = false, want true") + } + if string(job.Payload()) != `{"key":"value"}` { + t.Errorf("Payload() = %s", string(job.Payload())) + } + if job.Headers()["h1"][0] != "v1" { + t.Errorf("Headers() missing h1") + } + + // Kafka methods should return zero values + if job.Offset() != 0 { + t.Errorf("Offset() = %d, want 0", job.Offset()) + } + if job.Partition() != 0 { + t.Errorf("Partition() = %d, want 0", job.Partition()) + } + if job.Topic() != "" { + t.Errorf("Topic() = %s, want empty", job.Topic()) + } + if job.Metadata() != "" { + t.Errorf("Metadata() = %s, want empty", job.Metadata()) + } +} + +func TestJobInterfaceMethods_NilOptions(t *testing.T) { + job := &Job{ + Job: "test.job", + Ident: "job-nil", + } + + if job.GroupID() != "" { + t.Errorf("GroupID() should be empty with nil options, got %s", job.GroupID()) + } + if job.Priority() != 10 { + t.Errorf("Priority() should default to 10, got %d", job.Priority()) + } + if job.Delay() != 0 { + t.Errorf("Delay() should be 0 with nil options, got %d", job.Delay()) + } + if job.AutoAck() != false { + t.Error("AutoAck() should be false with nil options") + } +} + +func TestJobUpdatePriority(t *testing.T) { + job := &Job{Ident: "test"} + job.UpdatePriority(42) + + if job.Options == nil { + t.Fatal("Options should be created") + } + if job.Options.Priority != 42 { + t.Errorf("expected priority 42, got %d", job.Options.Priority) + } + + // Update again + job.UpdatePriority(1) + if job.Options.Priority != 1 { + t.Errorf("expected priority 1, got %d", job.Options.Priority) + } +} diff --git a/parser.go b/parser.go index 5fec9eb..698a3a0 100644 --- a/parser.go +++ b/parser.go @@ -24,8 +24,15 @@ func (s *Session) parseEmail(rawData []byte) (*ParsedMessage, error) { return nil, err } + // Capture all headers + headers := make(map[string][]string, len(msg.Header)) + for k, v := range msg.Header { + headers[k] = v + } + parsed := &ParsedMessage{ Raw: string(rawData), + Headers: headers, Sender: make([]EmailAddress, 0), Recipients: make([]EmailAddress, 0), CCs: make([]EmailAddress, 0), @@ -133,8 +140,31 @@ func (s *Session) processPartParsed(part *multipart.Part, parsed *ParsedMessage) return s.processAttachmentParsed(part, parsed) } + mediaType, params, _ := mime.ParseMediaType(contentType) + + // Handle nested multipart (e.g., multipart/alternative inside multipart/mixed) + if strings.HasPrefix(mediaType, "multipart/") { + boundary := params["boundary"] + if boundary != "" { + mr := multipart.NewReader(part, boundary) + for { + nestedPart, err := mr.NextPart() + if err == io.EOF { + break + } + if err != nil { + s.log.Error("nested multipart parse error", zap.Error(err)) + continue + } + if err := s.processPartParsed(nestedPart, parsed); err != nil { + s.log.Error("nested process part error", zap.Error(err)) + } + } + } + return nil + } + // This is body content - mediaType, _, _ := mime.ParseMediaType(contentType) if strings.HasPrefix(mediaType, "text/plain") || strings.HasPrefix(mediaType, "text/html") || contentType == "" { @@ -193,7 +223,8 @@ func (s *Session) processAttachmentParsed(part *multipart.Part, parsed *ParsedMe // Decode if base64 encoding := part.Header.Get("Content-Transfer-Encoding") if strings.EqualFold(encoding, "base64") { - decoded, err := base64.StdEncoding.DecodeString(string(content)) + cleaned := strings.NewReplacer("\r", "", "\n", "", " ", "").Replace(string(content)) + decoded, err := base64.StdEncoding.DecodeString(cleaned) if err == nil { content = decoded } @@ -202,6 +233,7 @@ func (s *Session) processAttachmentParsed(part *multipart.Part, parsed *ParsedMe attachment := Attachment{ Filename: filename, Type: contentType, + Size: int64(len(content)), } // Set ContentID if present @@ -257,7 +289,8 @@ func (s *Session) saveTempFile(content []byte, filename string) (string, error) func (s *Session) decodeContent(data []byte, encoding string) []byte { switch strings.ToLower(encoding) { case "base64": - decoded, err := base64.StdEncoding.DecodeString(string(data)) + cleaned := strings.NewReplacer("\r", "", "\n", "", " ", "").Replace(string(data)) + decoded, err := base64.StdEncoding.DecodeString(cleaned) if err != nil { return data } diff --git a/parser_test.go b/parser_test.go new file mode 100644 index 0000000..461cc07 --- /dev/null +++ b/parser_test.go @@ -0,0 +1,482 @@ +package smtp + +import ( + "encoding/base64" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "go.uber.org/zap" +) + +func newTestSession(cfg *Config) *Session { + if cfg == nil { + cfg = &Config{ + AttachmentStorage: AttachmentConfig{Mode: "memory"}, + } + } + log, _ := zap.NewDevelopment() + p := &Plugin{cfg: cfg, log: log} + b := &Backend{plugin: p, log: log} + return &Session{ + backend: b, + uuid: "00000000-0000-0000-0000-000000000000", + to: []string{"recipient@test.com"}, + log: log, + } +} + +func TestParseEmail_SimplePlainText(t *testing.T) { + raw := "From: sender@test.com\r\nTo: recipient@test.com\r\nSubject: Hello\r\n\r\nThis is the body." + s := newTestSession(nil) + + parsed, err := s.parseEmail([]byte(raw)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if parsed.Subject != "Hello" { + t.Errorf("expected subject Hello, got %s", parsed.Subject) + } + if parsed.TextBody != "This is the body." { + t.Errorf("expected body 'This is the body.', got %q", parsed.TextBody) + } + if len(parsed.Sender) != 1 || parsed.Sender[0].Email != "sender@test.com" { + t.Errorf("unexpected sender: %+v", parsed.Sender) + } + if len(parsed.Recipients) != 1 || parsed.Recipients[0].Email != "recipient@test.com" { + t.Errorf("unexpected recipients: %+v", parsed.Recipients) + } +} + +func TestParseEmail_HTMLBody(t *testing.T) { + raw := "From: sender@test.com\r\nTo: recipient@test.com\r\nSubject: HTML\r\nContent-Type: text/html\r\n\r\n

Hello

" + s := newTestSession(nil) + + parsed, err := s.parseEmail([]byte(raw)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if parsed.HTMLBody != "

Hello

" { + t.Errorf("expected HTML body, got %q", parsed.HTMLBody) + } + if parsed.TextBody != "" { + t.Errorf("expected empty text body, got %q", parsed.TextBody) + } +} + +func TestParseEmail_Base64BodyWithLineBreaks(t *testing.T) { + // Encode body as base64 with line breaks (RFC 2045 style, 76 chars per line) + body := "This is a test body that is long enough to require line breaks in base64 encoding format." + encoded := base64.StdEncoding.EncodeToString([]byte(body)) + // Insert line breaks every 76 chars like real email + var lines []string + for i := 0; i < len(encoded); i += 76 { + end := i + 76 + if end > len(encoded) { + end = len(encoded) + } + lines = append(lines, encoded[i:end]) + } + encodedWithBreaks := strings.Join(lines, "\r\n") + + raw := fmt.Sprintf("From: sender@test.com\r\nTo: recipient@test.com\r\nSubject: Base64\r\nContent-Transfer-Encoding: base64\r\n\r\n%s", encodedWithBreaks) + s := newTestSession(nil) + + parsed, err := s.parseEmail([]byte(raw)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if parsed.TextBody != body { + t.Errorf("expected decoded body %q, got %q", body, parsed.TextBody) + } +} + +func TestParseEmail_QuotedPrintable(t *testing.T) { + raw := "From: sender@test.com\r\nTo: recipient@test.com\r\nSubject: QP\r\nContent-Transfer-Encoding: quoted-printable\r\n\r\nHello =3D World" + s := newTestSession(nil) + + parsed, err := s.parseEmail([]byte(raw)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if parsed.TextBody != "Hello = World" { + t.Errorf("expected decoded QP body 'Hello = World', got %q", parsed.TextBody) + } +} + +func TestParseEmail_MultipartMixed(t *testing.T) { + raw := "From: sender@test.com\r\n" + + "To: recipient@test.com\r\n" + + "Subject: Multipart\r\n" + + "Content-Type: multipart/mixed; boundary=\"boundary1\"\r\n\r\n" + + "--boundary1\r\n" + + "Content-Type: text/plain\r\n\r\n" + + "Plain text body\r\n" + + "--boundary1\r\n" + + "Content-Type: text/html\r\n\r\n" + + "

HTML body

\r\n" + + "--boundary1\r\n" + + "Content-Disposition: attachment; filename=\"test.txt\"\r\n" + + "Content-Type: text/plain\r\n\r\n" + + "attachment content\r\n" + + "--boundary1--\r\n" + + s := newTestSession(nil) + + parsed, err := s.parseEmail([]byte(raw)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if parsed.TextBody != "Plain text body" { + t.Errorf("expected text body 'Plain text body', got %q", parsed.TextBody) + } + if parsed.HTMLBody != "

HTML body

" { + t.Errorf("expected HTML body '

HTML body

', got %q", parsed.HTMLBody) + } + if len(parsed.Attachments) != 1 { + t.Fatalf("expected 1 attachment, got %d", len(parsed.Attachments)) + } + if parsed.Attachments[0].Filename != "test.txt" { + t.Errorf("expected filename test.txt, got %s", parsed.Attachments[0].Filename) + } + if parsed.Attachments[0].Size != int64(len("attachment content")) { + t.Errorf("expected attachment size %d, got %d", len("attachment content"), parsed.Attachments[0].Size) + } +} + +func TestParseEmail_NestedMultipart(t *testing.T) { + // multipart/mixed containing multipart/alternative + attachment + raw := "From: sender@test.com\r\n" + + "To: recipient@test.com\r\n" + + "Subject: Nested\r\n" + + "Content-Type: multipart/mixed; boundary=\"outer\"\r\n\r\n" + + "--outer\r\n" + + "Content-Type: multipart/alternative; boundary=\"inner\"\r\n\r\n" + + "--inner\r\n" + + "Content-Type: text/plain\r\n\r\n" + + "Plain text\r\n" + + "--inner\r\n" + + "Content-Type: text/html\r\n\r\n" + + "

HTML text

\r\n" + + "--inner--\r\n" + + "--outer\r\n" + + "Content-Disposition: attachment; filename=\"file.pdf\"\r\n" + + "Content-Type: application/pdf\r\n\r\n" + + "fake pdf content\r\n" + + "--outer--\r\n" + + s := newTestSession(nil) + + parsed, err := s.parseEmail([]byte(raw)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if parsed.TextBody != "Plain text" { + t.Errorf("expected nested text body 'Plain text', got %q", parsed.TextBody) + } + if parsed.HTMLBody != "

HTML text

" { + t.Errorf("expected nested HTML body '

HTML text

', got %q", parsed.HTMLBody) + } + if len(parsed.Attachments) != 1 { + t.Fatalf("expected 1 attachment, got %d", len(parsed.Attachments)) + } + if parsed.Attachments[0].Filename != "file.pdf" { + t.Errorf("expected filename file.pdf, got %s", parsed.Attachments[0].Filename) + } +} + +func TestParseEmail_Base64Attachment(t *testing.T) { + content := "Hello, this is binary content!" + // Encode with line breaks like real email + encoded := base64.StdEncoding.EncodeToString([]byte(content)) + var lines []string + for i := 0; i < len(encoded); i += 76 { + end := i + 76 + if end > len(encoded) { + end = len(encoded) + } + lines = append(lines, encoded[i:end]) + } + encodedWithBreaks := strings.Join(lines, "\r\n") + + raw := "From: sender@test.com\r\n" + + "To: recipient@test.com\r\n" + + "Subject: Attachment\r\n" + + "Content-Type: multipart/mixed; boundary=\"b\"\r\n\r\n" + + "--b\r\n" + + "Content-Type: text/plain\r\n\r\n" + + "Body\r\n" + + "--b\r\n" + + "Content-Disposition: attachment; filename=\"data.bin\"\r\n" + + "Content-Type: application/octet-stream\r\n" + + "Content-Transfer-Encoding: base64\r\n\r\n" + + encodedWithBreaks + "\r\n" + + "--b--\r\n" + + s := newTestSession(nil) + + parsed, err := s.parseEmail([]byte(raw)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(parsed.Attachments) != 1 { + t.Fatalf("expected 1 attachment, got %d", len(parsed.Attachments)) + } + + att := parsed.Attachments[0] + // In memory mode, content is re-encoded as base64 (without line breaks) + decoded, err := base64.StdEncoding.DecodeString(att.Content) + if err != nil { + t.Fatalf("failed to decode attachment content: %v", err) + } + if string(decoded) != content { + t.Errorf("expected attachment content %q, got %q", content, string(decoded)) + } + if att.Size != int64(len(content)) { + t.Errorf("expected size %d, got %d", len(content), att.Size) + } +} + +func TestParseEmail_TempfileAttachmentMode(t *testing.T) { + tmpDir := t.TempDir() + cfg := &Config{ + AttachmentStorage: AttachmentConfig{ + Mode: "tempfile", + TempDir: tmpDir, + }, + } + + raw := "From: sender@test.com\r\n" + + "To: recipient@test.com\r\n" + + "Subject: TempFile\r\n" + + "Content-Type: multipart/mixed; boundary=\"b\"\r\n\r\n" + + "--b\r\n" + + "Content-Type: text/plain\r\n\r\n" + + "Body\r\n" + + "--b\r\n" + + "Content-Disposition: attachment; filename=\"doc.txt\"\r\n" + + "Content-Type: text/plain\r\n\r\n" + + "file content here\r\n" + + "--b--\r\n" + + s := newTestSession(cfg) + + parsed, err := s.parseEmail([]byte(raw)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(parsed.Attachments) != 1 { + t.Fatalf("expected 1 attachment, got %d", len(parsed.Attachments)) + } + + att := parsed.Attachments[0] + // Content field should be a file path + if !strings.HasPrefix(att.Content, tmpDir) { + t.Errorf("expected temp file path under %s, got %s", tmpDir, att.Content) + } + + // Verify file exists and has correct content + data, err := os.ReadFile(att.Content) + if err != nil { + t.Fatalf("failed to read temp file: %v", err) + } + if string(data) != "file content here" { + t.Errorf("expected file content 'file content here', got %q", string(data)) + } +} + +func TestParseEmail_MessageID(t *testing.T) { + raw := "From: sender@test.com\r\nTo: recipient@test.com\r\nSubject: ID\r\nMessage-ID: \r\n\r\nBody" + s := newTestSession(nil) + + parsed, err := s.parseEmail([]byte(raw)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if parsed.ID == nil || *parsed.ID != "" { + t.Errorf("expected Message-ID , got %v", parsed.ID) + } +} + +func TestParseEmail_CCAndReplyTo(t *testing.T) { + raw := "From: sender@test.com\r\n" + + "To: recipient@test.com\r\n" + + "Cc: cc1@test.com, cc2@test.com\r\n" + + "Reply-To: reply@test.com\r\n" + + "Subject: CC\r\n\r\nBody" + s := newTestSession(nil) + + parsed, err := s.parseEmail([]byte(raw)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(parsed.CCs) != 2 { + t.Errorf("expected 2 CCs, got %d", len(parsed.CCs)) + } + if len(parsed.ReplyTo) != 1 || parsed.ReplyTo[0].Email != "reply@test.com" { + t.Errorf("unexpected Reply-To: %+v", parsed.ReplyTo) + } +} + +func TestParseEmail_HeadersPreserved(t *testing.T) { + raw := "From: sender@test.com\r\n" + + "To: recipient@test.com\r\n" + + "Subject: Headers\r\n" + + "X-Custom-Header: custom-value\r\n" + + "X-Mailer: TestMailer\r\n\r\nBody" + s := newTestSession(nil) + + parsed, err := s.parseEmail([]byte(raw)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if parsed.Headers == nil { + t.Fatal("headers should not be nil") + } + if v := parsed.Headers["X-Custom-Header"]; len(v) == 0 || v[0] != "custom-value" { + t.Errorf("expected X-Custom-Header: custom-value, got %v", v) + } + if v := parsed.Headers["X-Mailer"]; len(v) == 0 || v[0] != "TestMailer" { + t.Errorf("expected X-Mailer: TestMailer, got %v", v) + } +} + +func TestDecodeContent_Base64WithLineBreaks(t *testing.T) { + s := newTestSession(nil) + + original := "Hello World! This is a test string." + encoded := base64.StdEncoding.EncodeToString([]byte(original)) + // Add line breaks + withBreaks := encoded[:10] + "\r\n" + encoded[10:] + + decoded := s.decodeContent([]byte(withBreaks), "base64") + if string(decoded) != original { + t.Errorf("expected %q, got %q", original, string(decoded)) + } +} + +func TestDecodeContent_QuotedPrintable(t *testing.T) { + s := newTestSession(nil) + + decoded := s.decodeContent([]byte("Hello=20World"), "quoted-printable") + if string(decoded) != "Hello World" { + t.Errorf("expected 'Hello World', got %q", string(decoded)) + } +} + +func TestDecodeContent_NoEncoding(t *testing.T) { + s := newTestSession(nil) + + input := []byte("plain text") + decoded := s.decodeContent(input, "") + if string(decoded) != "plain text" { + t.Errorf("expected 'plain text', got %q", string(decoded)) + } + + decoded = s.decodeContent(input, "7bit") + if string(decoded) != "plain text" { + t.Errorf("expected 'plain text' for 7bit, got %q", string(decoded)) + } +} + +func TestSaveTempFile(t *testing.T) { + tmpDir := t.TempDir() + cfg := &Config{ + AttachmentStorage: AttachmentConfig{ + Mode: "tempfile", + TempDir: tmpDir, + }, + } + s := newTestSession(cfg) + + content := []byte("test file content") + path, err := s.saveTempFile(content, "test.txt") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !strings.HasPrefix(filepath.Base(path), "smtp-att-") { + t.Errorf("temp file should start with smtp-att-, got %s", filepath.Base(path)) + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("failed to read temp file: %v", err) + } + if string(data) != "test file content" { + t.Errorf("expected 'test file content', got %q", string(data)) + } +} + +func TestParseEmail_UnnamedAttachment(t *testing.T) { + raw := "From: sender@test.com\r\n" + + "To: recipient@test.com\r\n" + + "Subject: Unnamed\r\n" + + "Content-Type: multipart/mixed; boundary=\"b\"\r\n\r\n" + + "--b\r\n" + + "Content-Type: text/plain\r\n\r\n" + + "Body\r\n" + + "--b\r\n" + + "Content-Disposition: attachment\r\n" + + "Content-Type: application/octet-stream\r\n\r\n" + + "binary data\r\n" + + "--b--\r\n" + + s := newTestSession(nil) + + parsed, err := s.parseEmail([]byte(raw)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(parsed.Attachments) != 1 { + t.Fatalf("expected 1 attachment, got %d", len(parsed.Attachments)) + } + if parsed.Attachments[0].Filename != "unnamed" { + t.Errorf("expected filename 'unnamed', got %s", parsed.Attachments[0].Filename) + } +} + +func TestParseEmail_ContentIDOnInline(t *testing.T) { + raw := "From: sender@test.com\r\n" + + "To: recipient@test.com\r\n" + + "Subject: Inline\r\n" + + "Content-Type: multipart/mixed; boundary=\"b\"\r\n\r\n" + + "--b\r\n" + + "Content-Type: text/plain\r\n\r\n" + + "Body\r\n" + + "--b\r\n" + + "Content-Disposition: inline; filename=\"image.png\"\r\n" + + "Content-Type: image/png\r\n" + + "Content-ID: \r\n\r\n" + + "fake png data\r\n" + + "--b--\r\n" + + s := newTestSession(nil) + + parsed, err := s.parseEmail([]byte(raw)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(parsed.Attachments) != 1 { + t.Fatalf("expected 1 attachment, got %d", len(parsed.Attachments)) + } + att := parsed.Attachments[0] + if att.ContentID == nil || *att.ContentID != "img001" { + t.Errorf("expected ContentID 'img001', got %v", att.ContentID) + } +} diff --git a/plugin.go b/plugin.go index 4088f62..c5353de 100644 --- a/plugin.go +++ b/plugin.go @@ -41,6 +41,9 @@ type Plugin struct { // SMTP server components smtpServer *smtp.Server listener net.Listener + + // Cleanup routine cancellation + cleanupCancel context.CancelFunc } // Init initializes the plugin with configuration and logger @@ -128,7 +131,9 @@ func (p *Plugin) Serve() chan error { }() // 5. Start temp file cleanup routine - p.startCleanupRoutine(context.Background()) + cleanupCtx, cleanupCancel := context.WithCancel(context.Background()) + p.cleanupCancel = cleanupCancel + p.startCleanupRoutine(cleanupCtx) return errCh } @@ -143,19 +148,28 @@ func (p *Plugin) Stop(ctx context.Context) error { p.mu.Lock() defer p.mu.Unlock() - // 1. Close listener (stops accepting new connections) + // 1. Stop cleanup routine + if p.cleanupCancel != nil { + p.cleanupCancel() + } + + // 2. Close listener (stops accepting new connections) if p.listener != nil { _ = p.listener.Close() } - // 2. Close SMTP server + // 3. Close SMTP server if p.smtpServer != nil { _ = p.smtpServer.Close() } - // 3. Close all tracked connections + // 4. Close all tracked connections p.connections.Range(func(key, value any) bool { - // Sessions will be cleaned up by Logout() + session := value.(*Session) + if session.conn != nil && session.conn.Conn() != nil { + _ = session.conn.Conn().Close() + } + p.connections.Delete(key) return true }) @@ -201,10 +215,13 @@ func (p *Plugin) pushToJobs(email *EmailData) error { } // Convert to domain model - msg := emailToJobMessage(email, &p.cfg.Jobs) + msg, err := emailToJobMessage(email, &p.cfg.Jobs) + if err != nil { + return errors.E(op, err) + } // Push directly to Jobs plugin - err := p.jobs.Push(context.Background(), msg) + err = p.jobs.Push(context.Background(), msg) if err != nil { return errors.E(op, err) } diff --git a/rpc.go b/rpc.go index ab47da9..47a1718 100644 --- a/rpc.go +++ b/rpc.go @@ -30,12 +30,13 @@ func (r *rpc) CloseConnection(uuid string, success *bool) error { session := value.(*Session) - // Close underlying connection + // Mark session as closing (Logout will handle map cleanup) + session.shouldClose = true + + // Close underlying connection — triggers Logout() which deletes from map if session.conn != nil && session.conn.Conn() != nil { _ = session.conn.Conn().Close() } - - r.p.connections.Delete(uuid) *success = true return nil diff --git a/rpc_test.go b/rpc_test.go new file mode 100644 index 0000000..790c54b --- /dev/null +++ b/rpc_test.go @@ -0,0 +1,91 @@ +package smtp + +import ( + "testing" + + "go.uber.org/zap" +) + +func TestListConnections_Empty(t *testing.T) { + log, _ := zap.NewDevelopment() + p := &Plugin{log: log} + r := &rpc{p: p} + + var conns []ConnectionInfo + err := r.ListConnections(false, &conns) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(conns) != 0 { + t.Errorf("expected 0 connections, got %d", len(conns)) + } +} + +func TestListConnections_WithSessions(t *testing.T) { + log, _ := zap.NewDevelopment() + p := &Plugin{log: log} + r := &rpc{p: p} + + // Add sessions + s1 := &Session{ + uuid: "uuid-1", + remoteAddr: "10.0.0.1:1234", + from: "a@test.com", + to: []string{"b@test.com"}, + authenticated: true, + authUsername: "user1", + } + s2 := &Session{ + uuid: "uuid-2", + remoteAddr: "10.0.0.2:5678", + from: "c@test.com", + to: []string{"d@test.com", "e@test.com"}, + } + + p.connections.Store("uuid-1", s1) + p.connections.Store("uuid-2", s2) + + var conns []ConnectionInfo + err := r.ListConnections(false, &conns) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(conns) != 2 { + t.Fatalf("expected 2 connections, got %d", len(conns)) + } + + // Find uuid-1 + var found bool + for _, c := range conns { + if c.UUID == "uuid-1" { + found = true + if c.RemoteAddr != "10.0.0.1:1234" { + t.Errorf("expected remote addr 10.0.0.1:1234, got %s", c.RemoteAddr) + } + if !c.Authenticated { + t.Error("expected authenticated = true") + } + if c.Username != "user1" { + t.Errorf("expected username user1, got %s", c.Username) + } + } + } + if !found { + t.Error("uuid-1 not found in connections list") + } +} + +func TestCloseConnection_NotFound(t *testing.T) { + log, _ := zap.NewDevelopment() + p := &Plugin{log: log} + r := &rpc{p: p} + + var success bool + err := r.CloseConnection("nonexistent", &success) + if err == nil { + t.Error("expected error for nonexistent connection") + } + if success { + t.Error("success should be false") + } +} diff --git a/session.go b/session.go index 2a64ac0..bd30ca2 100644 --- a/session.go +++ b/session.go @@ -100,13 +100,20 @@ func (s *Session) Data(r io.Reader) error { } // Convert attachments + cfg := s.backend.plugin.cfg attachments := make([]AttachmentData, 0, len(parsedMessage.Attachments)) for _, att := range parsedMessage.Attachments { - attachments = append(attachments, AttachmentData{ + ad := AttachmentData{ Filename: att.Filename, ContentType: att.Type, - Content: att.Content, - }) + Size: att.Size, + } + if cfg.AttachmentStorage.Mode == "tempfile" { + ad.Path = att.Content + } else { + ad.Content = att.Content + } + attachments = append(attachments, ad) } emailData := &EmailData{ @@ -124,10 +131,8 @@ func (s *Session) Data(r io.Reader) error { }, Auth: authData, Message: MessageData{ - Id: parsedMessage.ID, - Headers: map[string][]string{ - "Subject": {parsedMessage.Subject}, - }, + Id: parsedMessage.ID, + Headers: parsedMessage.Headers, Body: parsedMessage.TextBody, HTMLBody: parsedMessage.HTMLBody, Raw: parsedMessage.Raw, diff --git a/session_test.go b/session_test.go new file mode 100644 index 0000000..3479a30 --- /dev/null +++ b/session_test.go @@ -0,0 +1,301 @@ +package smtp + +import ( + "bytes" + "context" + "encoding/json" + "strings" + "testing" + + "github.com/roadrunner-server/api/v4/plugins/v4/jobs" + "go.uber.org/zap" +) + +// capturingJobs captures pushed messages for inspection +type capturingJobs struct { + messages []jobs.Message +} + +func (c *capturingJobs) Push(_ context.Context, msg jobs.Message) error { + c.messages = append(c.messages, msg) + return nil +} + +func newTestPlugin(mode string) (*Plugin, *capturingJobs) { + log, _ := zap.NewDevelopment() + capture := &capturingJobs{} + cfg := &Config{ + AttachmentStorage: AttachmentConfig{Mode: mode}, + Jobs: JobsConfig{Pipeline: "test", Priority: 10}, + } + p := &Plugin{ + cfg: cfg, + log: log, + jobs: capture, + } + return p, capture +} + +func newTestSessionWithPlugin(p *Plugin) *Session { + b := &Backend{plugin: p, log: p.log} + return &Session{ + backend: b, + uuid: "sess-00000000", + remoteAddr: "127.0.0.1:9999", + from: "sender@test.com", + to: []string{"recipient@test.com"}, + log: p.log, + } +} + +func TestSession_DataFlow(t *testing.T) { + p, capture := newTestPlugin("memory") + s := newTestSessionWithPlugin(p) + + raw := "From: sender@test.com\r\nTo: recipient@test.com\r\nSubject: Integration\r\n\r\nHello from test" + err := s.Data(strings.NewReader(raw)) + if err != nil { + t.Fatalf("Data() error: %v", err) + } + + if len(capture.messages) != 1 { + t.Fatalf("expected 1 pushed message, got %d", len(capture.messages)) + } + + msg := capture.messages[0] + var email EmailData + if err := json.Unmarshal(msg.Payload(), &email); err != nil { + t.Fatalf("failed to unmarshal payload: %v", err) + } + + if email.Event != "EMAIL_RECEIVED" { + t.Errorf("expected event EMAIL_RECEIVED, got %s", email.Event) + } + if email.UUID != "sess-00000000" { + t.Errorf("expected UUID sess-00000000, got %s", email.UUID) + } + if email.RemoteAddr != "127.0.0.1:9999" { + t.Errorf("expected remote addr 127.0.0.1:9999, got %s", email.RemoteAddr) + } + if email.Message.Subject != "Integration" { + t.Errorf("expected subject Integration, got %s", email.Message.Subject) + } + if email.Message.Body != "Hello from test" { + t.Errorf("expected body 'Hello from test', got %q", email.Message.Body) + } + if len(email.Envelope.From) != 1 || email.Envelope.From[0].Email != "sender@test.com" { + t.Errorf("unexpected envelope from: %+v", email.Envelope.From) + } +} + +func TestSession_DataWithAttachment_MemoryMode(t *testing.T) { + p, capture := newTestPlugin("memory") + s := newTestSessionWithPlugin(p) + + raw := "From: sender@test.com\r\n" + + "To: recipient@test.com\r\n" + + "Subject: Att\r\n" + + "Content-Type: multipart/mixed; boundary=\"b\"\r\n\r\n" + + "--b\r\n" + + "Content-Type: text/plain\r\n\r\n" + + "Body\r\n" + + "--b\r\n" + + "Content-Disposition: attachment; filename=\"test.txt\"\r\n" + + "Content-Type: text/plain\r\n\r\n" + + "attachment data\r\n" + + "--b--\r\n" + + err := s.Data(strings.NewReader(raw)) + if err != nil { + t.Fatalf("Data() error: %v", err) + } + + var email EmailData + json.Unmarshal(capture.messages[0].Payload(), &email) + + if len(email.Attachments) != 1 { + t.Fatalf("expected 1 attachment, got %d", len(email.Attachments)) + } + + att := email.Attachments[0] + if att.Filename != "test.txt" { + t.Errorf("expected filename test.txt, got %s", att.Filename) + } + if att.Content == "" { + t.Error("expected Content to be set in memory mode") + } + if att.Path != "" { + t.Error("expected Path to be empty in memory mode") + } +} + +func TestSession_DataWithAttachment_TempfileMode(t *testing.T) { + tmpDir := t.TempDir() + log, _ := zap.NewDevelopment() + capture := &capturingJobs{} + cfg := &Config{ + AttachmentStorage: AttachmentConfig{ + Mode: "tempfile", + TempDir: tmpDir, + }, + Jobs: JobsConfig{Pipeline: "test", Priority: 10}, + } + p := &Plugin{cfg: cfg, log: log, jobs: capture} + s := newTestSessionWithPlugin(p) + + raw := "From: sender@test.com\r\n" + + "To: recipient@test.com\r\n" + + "Subject: TempAtt\r\n" + + "Content-Type: multipart/mixed; boundary=\"b\"\r\n\r\n" + + "--b\r\n" + + "Content-Type: text/plain\r\n\r\n" + + "Body\r\n" + + "--b\r\n" + + "Content-Disposition: attachment; filename=\"doc.pdf\"\r\n" + + "Content-Type: application/pdf\r\n\r\n" + + "fake pdf\r\n" + + "--b--\r\n" + + err := s.Data(strings.NewReader(raw)) + if err != nil { + t.Fatalf("Data() error: %v", err) + } + + var email EmailData + json.Unmarshal(capture.messages[0].Payload(), &email) + + att := email.Attachments[0] + if att.Path == "" { + t.Error("expected Path to be set in tempfile mode") + } + if att.Content != "" { + t.Error("expected Content to be empty in tempfile mode") + } + if !strings.HasPrefix(att.Path, tmpDir) { + t.Errorf("expected path under %s, got %s", tmpDir, att.Path) + } +} + +func TestSession_DataWithAuth(t *testing.T) { + p, capture := newTestPlugin("memory") + s := newTestSessionWithPlugin(p) + s.authenticated = true + s.authUsername = "user@test.com" + s.authPassword = "secret" + s.authMechanism = "PLAIN" + + raw := "From: sender@test.com\r\nTo: recipient@test.com\r\nSubject: Auth\r\n\r\nBody" + err := s.Data(strings.NewReader(raw)) + if err != nil { + t.Fatalf("Data() error: %v", err) + } + + var email EmailData + json.Unmarshal(capture.messages[0].Payload(), &email) + + if email.Auth == nil { + t.Fatal("expected auth data") + } + if !email.Auth.Attempted { + t.Error("expected auth attempted = true") + } + if email.Auth.Username != "user@test.com" { + t.Errorf("expected username user@test.com, got %s", email.Auth.Username) + } + if email.Auth.Mechanism != "PLAIN" { + t.Errorf("expected mechanism PLAIN, got %s", email.Auth.Mechanism) + } +} + +func TestSession_DataWithoutAuth(t *testing.T) { + p, capture := newTestPlugin("memory") + s := newTestSessionWithPlugin(p) + + raw := "From: sender@test.com\r\nTo: recipient@test.com\r\nSubject: NoAuth\r\n\r\nBody" + s.Data(strings.NewReader(raw)) + + var email EmailData + json.Unmarshal(capture.messages[0].Payload(), &email) + + if email.Auth != nil { + t.Error("expected nil auth when not authenticated") + } +} + +func TestSession_HeadersPassedThrough(t *testing.T) { + p, capture := newTestPlugin("memory") + s := newTestSessionWithPlugin(p) + + raw := "From: sender@test.com\r\n" + + "To: recipient@test.com\r\n" + + "Subject: Headers\r\n" + + "X-Priority: 1\r\n" + + "Date: Thu, 01 Jan 2025 00:00:00 +0000\r\n\r\nBody" + + s.Data(strings.NewReader(raw)) + + var email EmailData + json.Unmarshal(capture.messages[0].Payload(), &email) + + if email.Message.Headers == nil { + t.Fatal("headers should not be nil") + } + if v, ok := email.Message.Headers["X-Priority"]; !ok || v[0] != "1" { + t.Errorf("expected X-Priority header, got %v", email.Message.Headers["X-Priority"]) + } + if _, ok := email.Message.Headers["Date"]; !ok { + t.Error("expected Date header to be preserved") + } +} + +func TestSession_MailAndRcpt(t *testing.T) { + p, _ := newTestPlugin("memory") + s := newTestSessionWithPlugin(p) + + s.Mail("from@test.com", nil) + if s.from != "from@test.com" { + t.Errorf("expected from from@test.com, got %s", s.from) + } + + s.Rcpt("to1@test.com", nil) + s.Rcpt("to2@test.com", nil) + if len(s.to) != 3 { // 1 from newTestSessionWithPlugin + 2 added + t.Errorf("expected 3 recipients, got %d", len(s.to)) + } +} + +func TestSession_Reset(t *testing.T) { + p, _ := newTestPlugin("memory") + s := newTestSessionWithPlugin(p) + s.emailData = *bytes.NewBufferString("some data") + + s.Reset() + + if s.from != "" { + t.Error("from should be empty after reset") + } + if s.to != nil { + t.Error("to should be nil after reset") + } + if s.emailData.Len() != 0 { + t.Error("emailData should be empty after reset") + } +} + +func TestSession_Logout(t *testing.T) { + p, _ := newTestPlugin("memory") + s := newTestSessionWithPlugin(p) + + // Store session in connections map + p.connections.Store(s.uuid, s) + + err := s.Logout() + if err != nil { + t.Fatalf("Logout() error: %v", err) + } + + // Should be removed from connections + if _, ok := p.connections.Load(s.uuid); ok { + t.Error("session should be removed from connections after Logout") + } +} diff --git a/types.go b/types.go index d1550ac..a69b5a3 100644 --- a/types.go +++ b/types.go @@ -62,14 +62,16 @@ type Attachment struct { Filename string `json:"filename"` Content string `json:"content"` Type string `json:"type"` + Size int64 `json:"size"` ContentID *string `json:"contentId"` } // ParsedMessage represents the structure expected by PHP Parser type ParsedMessage struct { - ID *string `json:"id"` - Raw string `json:"raw"` - Sender []EmailAddress `json:"sender"` + ID *string `json:"id"` + Raw string `json:"raw"` + Headers map[string][]string `json:"headers"` + Sender []EmailAddress `json:"sender"` Recipients []EmailAddress `json:"recipients"` CCs []EmailAddress `json:"ccs"` Subject string `json:"subject"`