Skip to content

feat: add email adapter DSN parsing#117

Open
TorstenDittmann wants to merge 1 commit intofeat-multiple-adapters-failoverfrom
feat-email-adapter-dsn
Open

feat: add email adapter DSN parsing#117
TorstenDittmann wants to merge 1 commit intofeat-multiple-adapters-failoverfrom
feat-email-adapter-dsn

Conversation

@TorstenDittmann
Copy link
Copy Markdown
Contributor

Summary

  • add Email::fromDsn() to create email adapters from DSN strings
  • support smtp, smtps, resend, sendgrid, and mailgun schemes with strict validation
  • document the format and cover parsing with dedicated unit tests

Testing

  • ./vendor/bin/phpunit tests/Messaging/Adapter/Email/DsnTest.php
  • ./vendor/bin/phpunit tests/Messaging/MessengerTest.php
  • ./vendor/bin/pint --preset psr12 --test

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 14, 2026

Greptile Summary

This PR adds Email::fromDsn() to the abstract email adapter, supporting smtp, smtps, resend, sendgrid, and mailgun schemes with URL-component extraction, per-option type/value parsing, and accompanying unit tests and README documentation. The implementation is clean and well-structured. A few validation gaps noted in prior review rounds (failing test_rejects_malformed_smtp_dsn assertion, absent port-range check, query-string port precedence) remain unaddressed.

Confidence Score: 4/5

Safe to merge after addressing the previously flagged failing test assertion and the minor validation gaps.

No new P0/P1 findings from this pass. Score is held at 4 rather than 5 because open items from prior review rounds — the test_rejects_malformed_smtp_dsn assertion mismatch that will cause a test failure, the missing TCP port-range guard, and the counter-intuitive query-string port precedence — are still present and unresolved.

tests/Messaging/Adapter/Email/DsnTest.php (wrong exception message assertion), src/Utopia/Messaging/Adapter/Email.php (port range and precedence)

Important Files Changed

Filename Overview
src/Utopia/Messaging/Adapter/Email.php Adds fromDsn() factory with helper parse methods; port range validation gap and query-string port precedence noted in prior threads remain unresolved.
tests/Messaging/Adapter/Email/DsnTest.php Adds DSN unit tests; test_rejects_malformed_smtp_dsn asserts the wrong exception message and will fail (noted in prior thread, unresolved).
README.md Adds DSN usage examples for all five supported schemes; examples are correct and consistent with the implementation.

Reviews (2): Last reviewed commit: "feat: add email adapter DSN parsing" | Re-trigger Greptile

Comment thread tests/Messaging/Adapter/Email/DsnTest.php
Comment thread src/Utopia/Messaging/Adapter/Email.php
Comment on lines +78 to +81
$port = self::parseIntOption(
value: $query['port'] ?? ($parts['port'] ?? ($scheme === 'smtps' ? 465 : 25)),
option: 'port'
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Query-string port silently overrides URL port

The precedence $query['port'] > $parts['port'] > scheme-default means smtp://host:587?port=25 resolves to port 25. Most DSN parsers treat the URL authority port as authoritative. This is a subtle gotcha worth a brief inline comment, or consider flipping the precedence to URL port > query port > scheme default.

@TorstenDittmann TorstenDittmann force-pushed the feat-email-adapter-dsn branch from b62347d to 32c3976 Compare April 15, 2026 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant