add BatchSender with base mail support and request validation #46
Conversation
WalkthroughThe update introduces batch email sending functionality to the Mailtrap Ruby client. It adds a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant MailtrapAPI
User->>Client: initialize(bulk: true)
User->>Client: send_batch(base, requests)
Client->>Client: Validate inputs
Client->>MailtrapAPI: POST /api/batch (base, requests)
MailtrapAPI-->>Client: Batch response
Client-->>User: Return responses
Possibly related issues
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (16)
lib/mailtrap/errors.rb (1)
19-21: Clarify doc-string wording & usage contractThe comment says “invalid or unexpected structure” but doesn’t mention response (the subject).
Also,InvalidApiResponseErrorinherits fromError, which expectsmessages(an array) during initialization. Make sure every place raising this new error passes an array of messages; otherwise the superclass willNoMethodErroron#join.- # Raised when the API returns invalid or unexpected structure + # Raised when the API returns an invalid or unexpected response structurelib/mailtrap/validators/email_validator.rb (3)
8-10: Email check is overly permissive
/@/allowsfoo@,@bar, or even@. Consider switching to Ruby’s built-in, RFC-5322-aware regex for a low-cost accuracy boost:-require 'uri' - -EMAIL_REGEX = URI::MailTo::EMAIL_REGEXP.freeze - -... - def valid?(email) - email.to_s.match?(EMAIL_REGEX) - end +require 'uri' + +EMAIL_REGEX = URI::MailTo::EMAIL_REGEXP.freeze + +def valid?(email) + email.to_s.match?(EMAIL_REGEX) +endThis keeps the validator lightweight without pulling extra deps.
15-16: Provide actionable feedback in validation errorReturning just “Invalid email” doesn’t help users pinpoint the problem. Including the offending value greatly improves debuggability:
-raise ArgumentError, "Invalid #{field_name}" +raise ArgumentError, "Invalid #{field_name}: '#{email}'"
19-19: Missing final newlineRubocop flagged the absence of a trailing newline. Add one to stay compliant with POSIX editors & linters.
🧰 Tools
🪛 RuboCop (1.75.5)
[convention] 19-19: Final newline missing.
(Layout/TrailingEmptyLines)
README.md (1)
70-100: Example may not match the public API
The sample calls
batch.send_emails, but the service is namedBatchSender.
In code, does the method actually exist, or is it#send/#call? Double-check to avoid broken docs.The client is configured only with
api_host: 'bulk.api.mailtrap.io'.
Everywhere else (tests, specs) you rely onbulk: trueto toggle host selection.
Consider aligning the docs with the canonical approach to avoid user confusion.spec/mailtrap/client_spec.rb (2)
255-263: Trim example size or use helperRuboCop flags the spec block as too long (9 lines). Extract the stub & expectations into a helper to silence the cop and keep specs focused:
- it 'sends payload to /api/batch and returns parsed response' do - stub = stub_request(:post, 'https://bulk.api.mailtrap.io/api/batch') - .with(body: JSON.dump(payload)) - .to_return(status: 200, body: JSON.dump({ responses: [{ status: 202 }] })) - - result = client.batch_send(payload) - expect(result[:responses]).to eq([{ status: 202 }]) - expect(stub).to have_been_requested - end + it 'posts payload and parses response' do + responses_body = { responses: [{ status: 202 }] } + stub_batch_send(status: 200, body: responses_body) + + expect(client.batch_send(payload)[:responses]).to eq responses_body[:responses] + end(Where
stub_batch_sendis a small helper defined in the spec.)🧰 Tools
🪛 RuboCop (1.75.5)
[convention] 255-263: Example has too many lines. [6/5]
(RSpec/ExampleLength)
266-272: Assertion could be tighterThe regex
/bad request/will also match “super bad request”. If you want an exact match:expect { client.batch_send(payload) } .to raise_error(Mailtrap::Error, 'bad request')examples/batch_sender.rb (1)
46-47: Add a trailing newlinePOSIX tools expect files to end with a newline; add one for consistency.
🧰 Tools
🪛 RuboCop (1.75.5)
[convention] 47-47: Final newline missing.
(Layout/TrailingEmptyLines)
lib/mailtrap/batch_sender.rb (2)
54-60: Redundant exception wrappingRescuing
ArgumentErroronly to re-raise the same message adds no value and obscures back-traces. You can drop therescueblock entirely.
63-64: Missing final newlineAdd a newline at EOF.
🧰 Tools
🪛 RuboCop (1.75.5)
[convention] 64-64: Final newline missing.
(Layout/TrailingEmptyLines)
lib/mailtrap/batch/base.rb (2)
20-22: Potential log noise
warnon every unknown key will flood STDERR in production. Consider using a logger with configurable level or raising for truly unexpected keys.
51-53: Missing final newlineAdd a newline at EOF.
🧰 Tools
🪛 RuboCop (1.75.5)
[convention] 53-53: Final newline missing.
(Layout/TrailingEmptyLines)
spec/mailtrap/batch_sender_spec.rb (1)
103-104: Prefer verifying doublesUsing
instance_doubleorclass_doubleensures the test double matches the public API and prevents silent API drift.🧰 Tools
🪛 RuboCop (1.75.5)
[convention] 103-103: Prefer using verifying doubles over normal doubles.
(RSpec/VerifiedDoubles)
lib/mailtrap/client.rb (3)
101-105: Set Net::HTTP timeouts
Net::HTTPdefaults to unlimited timeouts, risking hung threads on network issues. Configure at leastopen_timeoutandread_timeout(and possiblywrite_timeoutin Ruby 3.0+).- Net::HTTP.new(api_host, api_port).tap { |client| client.use_ssl = true } + Net::HTTP.new(api_host, api_port).tap do |client| + client.use_ssl = true + client.open_timeout = 5 + client.read_timeout = 10 + end
120-124: Unuseduriparameter
perform_request(_uri, request)ignoresuri; remove the param or use it (e.g., for logging) to avoid dead code.
152-153: Missing final newlineAdd newline at EOF.
🧰 Tools
🪛 RuboCop (1.75.5)
[convention] 153-153: Final newline missing.
(Layout/TrailingEmptyLines)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
README.md(1 hunks)examples/batch_sender.rb(1 hunks)lib/mailtrap.rb(1 hunks)lib/mailtrap/batch/base.rb(1 hunks)lib/mailtrap/batch_sender.rb(1 hunks)lib/mailtrap/client.rb(3 hunks)lib/mailtrap/errors.rb(1 hunks)lib/mailtrap/validators/email_validator.rb(1 hunks)spec/mailtrap/batch_sender_spec.rb(1 hunks)spec/mailtrap/client_spec.rb(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
spec/mailtrap/client_spec.rb (1)
lib/mailtrap/client.rb (1)
batch_send(55-57)
🪛 RuboCop (1.75.5)
lib/mailtrap/validators/email_validator.rb
[convention] 19-19: Final newline missing.
(Layout/TrailingEmptyLines)
examples/batch_sender.rb
[convention] 47-47: Final newline missing.
(Layout/TrailingEmptyLines)
spec/mailtrap/client_spec.rb
[convention] 255-263: Example has too many lines. [6/5]
(RSpec/ExampleLength)
spec/mailtrap/batch_sender_spec.rb
[convention] 45-54: Example has too many lines. [8/5]
(RSpec/ExampleLength)
[convention] 56-66: Example has too many lines. [7/5]
(RSpec/ExampleLength)
[convention] 76-85: Example has too many lines. [6/5]
(RSpec/ExampleLength)
[convention] 103-103: Prefer using verifying doubles over normal doubles.
(RSpec/VerifiedDoubles)
[convention] 145-145: Final newline missing.
(Layout/TrailingEmptyLines)
lib/mailtrap/batch/base.rb
[convention] 53-53: Final newline missing.
(Layout/TrailingEmptyLines)
lib/mailtrap/batch_sender.rb
[convention] 49-62: Method has too many lines. [11/10]
(Metrics/MethodLength)
[convention] 64-64: Final newline missing.
(Layout/TrailingEmptyLines)
lib/mailtrap/client.rb
[convention] 27-28: Avoid parameter lists longer than 5 parameters. [6/5]
(Metrics/ParameterLists)
[convention] 153-153: Final newline missing.
(Layout/TrailingEmptyLines)
🔇 Additional comments (1)
lib/mailtrap.rb (1)
4-8: 👍 New requires look correctAll newly-introduced components are now eagerly loaded, preventing
NameErrors at runtime. No issues spotted here.
examples/batch_sender.rb
Outdated
| API_KEY = 'your-real-mailtrap-api-key-here' | ||
|
|
||
| client = Mailtrap::Client.new( | ||
| api_key: API_KEY, | ||
| bulk: true | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not commit real API keys
Hard-coding API_KEY is dangerous. Load it from an environment variable instead to avoid accidental leakage.
-API_KEY = 'your-real-mailtrap-api-key-here'
+API_KEY = ENV.fetch('MAILTRAP_API_KEY')[security]
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| API_KEY = 'your-real-mailtrap-api-key-here' | |
| client = Mailtrap::Client.new( | |
| api_key: API_KEY, | |
| bulk: true | |
| ) | |
| API_KEY = ENV.fetch('MAILTRAP_API_KEY') | |
| client = Mailtrap::Client.new( | |
| api_key: API_KEY, | |
| bulk: true | |
| ) |
🤖 Prompt for AI Agents
In examples/batch_sender.rb around lines 6 to 11, the API key is hard-coded
which poses a security risk. Replace the hard-coded API_KEY assignment with code
that loads the API key from an environment variable, such as using
ENV['MAILTRAP_API_KEY']. Ensure the rest of the code uses this variable so the
real key is never stored in the source code.
lib/mailtrap/batch_sender.rb
Outdated
| base_payload = ensure_hash(base).transform_keys(&:to_sym).except(:to, :cc, :bcc) | ||
| validate_requests!(requests) |
There was a problem hiding this comment.
Implicit ActiveSupport dependency & key handling
transform_keys / except are ActiveSupport extensions. If the gem is used in a plain Ruby app these calls will raise NoMethodError.
Either:
require 'active_support/core_ext/hash'and declareactivesupportas a runtime dependency, or- replace with pure-Ruby equivalents:
-base_payload = ensure_hash(base).transform_keys(&:to_sym).except(:to, :cc, :bcc)
+hash = ensure_hash(base).each_with_object({}) { |(k,v),h| h[k.to_sym]=v }
+base_payload = hash.reject { |k,_| %i[to cc bcc].include?(k) }Also consider normalising each request’s keys the same way to accept both string and symbol keys.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| base_payload = ensure_hash(base).transform_keys(&:to_sym).except(:to, :cc, :bcc) | |
| validate_requests!(requests) | |
| hash = ensure_hash(base).each_with_object({}) { |(k, v), h| h[k.to_sym] = v } | |
| base_payload = hash.reject { |k, _| %i[to cc bcc].include?(k) } | |
| validate_requests!(requests) |
🤖 Prompt for AI Agents
In lib/mailtrap/batch_sender.rb around lines 11 to 12, the use of transform_keys
and except relies on ActiveSupport extensions which may cause NoMethodError in
plain Ruby environments. To fix this, either add require
'active_support/core_ext/hash' at the top and declare activesupport as a runtime
dependency, or replace transform_keys(&:to_sym) and except(:to, :cc, :bcc) with
pure Ruby code that converts keys to symbols and excludes those keys manually.
Additionally, normalize each request's keys similarly to handle both string and
symbol keys consistently.
lib/mailtrap/batch_sender.rb
Outdated
| # frozen_string_literal: true | ||
|
|
||
| module Mailtrap | ||
| class BatchSender |
There was a problem hiding this comment.
@mklocek as you are the one who implemented the batch endpoint, I'd appreciate you opinion on the approach with BatchSender?
I think that there is no need for such class and the implementation could live in Mailtrap::Client along with send. Raw but working implementation to demonstrate what I mean:
# @param base [Mailtrap::Base]
# @param requests [Array<Mailtrap::Base>]
# @return [Hash]
def send_batch(base, requests)
body = {
base: base.as_json.except('to', 'cc', 'bcc'),
requests: requests.map(&:as_json)
}.to_json
request = post_request(request_url(batch: true), body)
response = http_client.request(request)
handle_response(response)
endTo be aligned with the existing implementation, I think its ok to rely on our API to validate the input as we already instead of writing validations in ruby. That would make it easier later to change the limits without the need to update all the libraries.
# 501 message
client.send_batch(base, requests)
# => lib/mailtrap/client.rb:109:in `handle_response': a maximum of 500 requests per batch is allowed (Mailtrap::Error)
# large attachment
client.send_batch(base, requests)
# => lib/mailtrap/client.rb:115:in `handle_response': message too large (Mailtrap::MailSizeError)There was a problem hiding this comment.
@i7an Refactored everything according to your feedback — moved the batch logic into the main client. Please check if everything looks good now
lib/mailtrap/client.rb
Outdated
| request = post_request(request_url, mail.to_json) | ||
| response = http_client.request(request) | ||
|
|
||
| response = http_client(cache: true).request(request) | ||
| handle_response(response) | ||
| end | ||
|
|
||
| def post(path, body: {}) | ||
| request(:post, path, body:) | ||
| end | ||
|
|
||
| def batch_send(payload) | ||
| post('/api/batch', body: payload) |
There was a problem hiding this comment.
We don't need 2 different ways to send a basic HTTP POST request. def post probably shouldn't be a public method as well.
There was a problem hiding this comment.
@mklocek Hello,
That’s a fair point — but I’ve introduced post as part of a general-purpose HTTP layer on the client.
We’ll be adding support for other REST APIs like Contacts and Templates:
- [Contacts API] https://api-docs.mailtrap.io/docs/mailtrap-api-docs/0a35b03ff78c5-contacts-api
- [Email Templates API] https://api-docs.mailtrap.io/docs/mailtrap-api-docs/812e3d4cc1e97-email-templates-api
So having get / post / patch / delete methods on Client helps unify and scale all of that going forward.
Happy to revisit this if you still feel it’s unnecessary — let me know what you think!
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
lib/mailtrap/client.rb (2)
27-33: Initializer is already at 6 kwargs – extract config object.The constructor keeps growing (
api_key,api_host,api_port,bulk,sandbox,inbox_id).
Before it turns into the nextNet::HTTP.start, consider a small config struct / options hash that can be validated in one place:Config = Struct.new(:api_key, :api_host, :api_port, :bulk, :sandbox, :inbox_id, keyword_init: true)This keeps the public surface stable while allowing future flags (contacts, templates, etc.) without blowing up the signature.
🧰 Tools
🪛 RuboCop (1.75.5)
[convention] 27-28: Avoid parameter lists longer than 5 parameters. [6/5]
(Metrics/ParameterLists)
111-115: Cached HTTP client is great – butperform_requestbypasses it.
perform_requestinstantiates a new client (cache: false) every call, whereassendcaches and reuses the connection. Decide on one strategy (preferably cached with automatic connection-reuse) to avoid TLS setup overhead.spec/mailtrap/client_spec.rb (1)
235-303: Spec example is verbose & contains trailing whitespace.The happy-path batch spec is ~40 lines long; RuboCop already flags this.
Extract common setup intoletblocks / helper methods and tighten expectations to the essentials to keep the suite readable. Also strip trailing spaces to keep the diff clean.🧰 Tools
🪛 RuboCop (1.75.5)
[convention] 237-237: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 246-246: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 259-259: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 260-279: Example has too many lines. [16/5]
(RSpec/ExampleLength)
[convention] 272-272: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 274-274: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 286-286: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 292-292: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 296-296: Line is too long. [128/120]
(Layout/LineLength)
[convention] 297-297: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 301-301: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 303-303: Trailing whitespace detected.
(Layout/TrailingWhitespace)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(1 hunks)lib/mailtrap/client.rb(3 hunks)lib/mailtrap/mail/base.rb(4 hunks)spec/mailtrap/client_spec.rb(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🧬 Code Graph Analysis (1)
spec/mailtrap/client_spec.rb (2)
lib/mailtrap/mail/base.rb (1)
to_json(67-69)lib/mailtrap/client.rb (1)
send_batch(51-63)
🪛 RuboCop (1.75.5)
lib/mailtrap/client.rb
[convention] 27-28: Avoid parameter lists longer than 5 parameters. [6/5]
(Metrics/ParameterLists)
[convention] 53-53: Add empty line after guard clause.
(Layout/EmptyLineAfterGuardClause)
[convention] 58-58: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 61-61: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 163-163: Final newline missing.
(Layout/TrailingEmptyLines)
spec/mailtrap/client_spec.rb
[convention] 2-2: Add an empty line after magic comments.
(Layout/EmptyLineAfterMagicComment)
[convention] 237-237: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 246-246: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 259-259: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 260-279: Example has too many lines. [16/5]
(RSpec/ExampleLength)
[convention] 272-272: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 274-274: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 286-286: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 292-292: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 296-296: Line is too long. [128/120]
(Layout/LineLength)
[convention] 297-297: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 301-301: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 303-303: Trailing whitespace detected.
(Layout/TrailingWhitespace)
🔇 Additional comments (1)
lib/mailtrap/mail/base.rb (1)
61-64:as_jsonnow leaksnilvalues when serialised.
JSON.generatewill serialiseniltonull, so a missing template field becomes"template_uuid":null.
If the API treats “null” differently from “absent”, switch to:-'template_uuid' => template_uuid, -'template_variables' => template_variables +'template_uuid' => template_uuid, +'template_variables' => template_variables… and keep the existing
.compact– but add:.compact # removes nils so keys disappear entirelyto guarantee the keys are omitted rather than set to
null.
| attr_accessor :from, :to, :reply_to, :cc, :bcc, | ||
| :headers, :custom_variables, :subject, | ||
| :text, :html, :category, | ||
| :template_uuid, :template_variables | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider validating template_uuid & template_variables.
The new attributes are simply accepted and forwarded. A couple of edge-cases worth guarding against:
template_uuid– enforce a UUID format to fail fast on obviously bad data.template_variables– ensure it’s aHash(or at least JSON-serialisable) to avoid runtime errors when the body is serialised.
A light touch validation in the constructor (or a dedicated validator similar to EmailValidator) would make the API more predictable.
🤖 Prompt for AI Agents
In lib/mailtrap/mail/base.rb around lines 8 to 12, the new attributes
template_uuid and template_variables lack validation, which can lead to runtime
errors or bad data usage. Add validation in the constructor or a dedicated
validator method to ensure template_uuid matches a UUID format and
template_variables is a Hash or JSON-serializable object. This will enforce
correct data types early and prevent issues during serialization or API calls.
| def post(path, body: {}) | ||
| request(:post, path, body: body) | ||
| end | ||
|
|
||
| def request(method, path, body: nil, params: nil) | ||
| uri = URI::HTTPS.build( | ||
| host: api_host, | ||
| path: path, | ||
| query: params ? URI.encode_www_form(params) : nil | ||
| ) | ||
|
|
||
| request = build_request(method, uri, body) | ||
| perform_request(uri, request) | ||
| end |
There was a problem hiding this comment.
🛠️ Refactor suggestion
#request duplicates logic already present in send/send_batch.
Both high-level methods manually build a post_request and call http_client.request.
Now that a general-purpose request layer exists, thin those methods down to:
request(:post, '/api/batch', body: body)This reduces code paths and ensures all requests go through the same header / error-handling pipeline.
🤖 Prompt for AI Agents
In lib/mailtrap/client.rb around lines 65 to 78, the request method duplicates
logic already implemented in send and send_batch methods, which manually build
post_request and call http_client.request. Refactor send and send_batch to use
the general-purpose request method by replacing their internal logic with calls
to request(:post, path, body: body). This will consolidate request handling,
reduce code duplication, and ensure consistent header and error handling across
all requests.
| def send_batch(base, requests) | ||
| raise ArgumentError, 'hould be Mailtrap::Mail::Base object' unless base.is_a?(Mailtrap::Mail::Base) | ||
| raise ArgumentError, 'requests must be present' if !requests.is_a?(Array) || requests.empty? | ||
| body = { | ||
| base: base.as_json.except('to', 'cc', 'bcc'), | ||
| requests: requests | ||
| }.to_json | ||
|
|
||
| request = post_request('/api/batch', body) | ||
| response = http_client.request(request) | ||
|
|
||
| handle_response(response) | ||
| end |
There was a problem hiding this comment.
Guard-clause typo & missing bulk-mode enforcement.
- Typo in the error message:
-raise ArgumentError, 'hould be Mailtrap::Mail::Base object'
+raise ArgumentError, 'should be Mailtrap::Mail::Base object'send_batchcan be called on a client initialised withoutbulk: true; in that case the host defaults tosend.api.mailtrap.io, and the request will 404. Fail fast:
+unless bulk || api_host == BULK_SENDING_API_HOST
+ raise ArgumentError, 'send_batch requires the client to be initialised with bulk: true'
+end- RuboCop flagged the missing blank line after the guard clause – worth fixing while you’re here.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def send_batch(base, requests) | |
| raise ArgumentError, 'hould be Mailtrap::Mail::Base object' unless base.is_a?(Mailtrap::Mail::Base) | |
| raise ArgumentError, 'requests must be present' if !requests.is_a?(Array) || requests.empty? | |
| body = { | |
| base: base.as_json.except('to', 'cc', 'bcc'), | |
| requests: requests | |
| }.to_json | |
| request = post_request('/api/batch', body) | |
| response = http_client.request(request) | |
| handle_response(response) | |
| end | |
| def send_batch(base, requests) | |
| raise ArgumentError, 'should be Mailtrap::Mail::Base object' unless base.is_a?(Mailtrap::Mail::Base) | |
| unless bulk || api_host == BULK_SENDING_API_HOST | |
| raise ArgumentError, 'send_batch requires the client to be initialised with bulk: true' | |
| end | |
| raise ArgumentError, 'requests must be present' if !requests.is_a?(Array) || requests.empty? | |
| body = { | |
| base: base.as_json.except('to', 'cc', 'bcc'), | |
| requests: requests | |
| }.to_json | |
| request = post_request('/api/batch', body) | |
| response = http_client.request(request) | |
| handle_response(response) | |
| end |
🧰 Tools
🪛 RuboCop (1.75.5)
[convention] 53-53: Add empty line after guard clause.
(Layout/EmptyLineAfterGuardClause)
[convention] 58-58: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 61-61: Trailing whitespace detected.
(Layout/TrailingWhitespace)
🤖 Prompt for AI Agents
In lib/mailtrap/client.rb around lines 51 to 63, fix the typo in the first
ArgumentError message by correcting 'hould' to 'should'. Add a guard clause at
the start of send_batch to check if the client was initialized with bulk mode
enabled, and raise an error if not, to prevent 404 errors from the wrong host.
Also, insert a blank line after the guard clauses to satisfy RuboCop style
guidelines.
| request.body = JSON.dump(body) if %i[post patch].include?(method.to_sym) && body | ||
|
|
||
| attach_headers(request) | ||
| request |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Double JSON encoding risk in build_request.
post('/foo', body: json_string) will still run JSON.dump, producing a doubly-quoted string.
Safeguard:
-if %i[post patch].include?(method.to_sym) && body
- request.body = JSON.dump(body)
+if %i[post patch].include?(method.to_sym) && body
+ request.body = body.is_a?(String) ? body : JSON.generate(body)
endThis keeps behaviour consistent whether the caller passes a Hash or a pre-encoded String.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lib/mailtrap/client.rb around lines 90 to 93, the method build_request risks
double JSON encoding when the body is already a JSON string. To fix this, check
if the body is a String before calling JSON.dump; only encode if the body is not
a String (e.g., a Hash). This ensures consistent behavior whether the caller
passes a Hash or a pre-encoded JSON string.
Motivation
This PR introduces support for Mailtrap's Batch Send API, allowing the delivery of multiple personalized emails within a single request.
Instead of extending the existing Mailtrap::Client#send method, we designed a dedicated BatchSender service for better separation of concerns and future flexibility. Batch sending has different structural and behavioral expectations compared to single sends, and keeping these paths isolated avoids conditional complexity and ensures correct API usage (e.g. enforcing bulk.api.mailtrap.io).
Changes
**_Added Mailtrap::BatchSender with full support for base + requests payload structure.
Introduced Mailtrap::Batch::Base to allow reuse of common email attributes across batch requests.
Added EmailValidator to ensure recipient email format validity inside to, cc, and bcc.
Added custom InvalidApiResponseError for strict format validation of batch responses.
Extended Mailtrap::Client with batch_send and a general-purpose request layer (get, post, patch, delete) for future REST API usage.
Improved internal structure of the Client to support both send.api and bulk.api hosts._**
How to test
Run bundle exec rspec spec/mailtrap/batch_sender_spec.rb — full test coverage for happy/invalid batch flows
Manually test with valid API key in examples/batch_sender.rb to verify real delivery via bulk API
Why separate BatchSender instead of reusing send?
We intentionally introduced Mailtrap::BatchSender as a dedicated class instead of extending the existing Client#send method, for the following architectural and maintainability reasons:
1. Single Responsibility Principle (SRP)
Mailtrap::Client is responsible for low-level HTTP transport (get, post, etc.).
BatchSender encapsulates batch-specific responsibilities: payload preparation, request validation, and error handling.
This separation keeps both components focused, isolated, and easier to test or evolve independently.
2. Structural divergence from send
The Batch Send API requires a nested { base, requests } payload, which differs entirely from the single-message JSON used in Client#send.
It also enforces a separate endpoint (/api/batch) and a different host (bulk.api.mailtrap.io).
Merging both into one method would have introduced conditional logic, reduced clarity, and increased complexity.
3. Explicit and predictable SDK behavior
Keeping client.send(mail) for single sends and BatchSender.new(client).send_emails(...) for batch operations provides clear, intentional API usage.
This avoids ambiguity in argument formats (e.g. Hash, Array, or Mail object).
4. Validation flexibility
BatchSender supports strict validation (e.g. email formats, base structure, key filtering), which would be inappropriate to bundle into the generic HTTP client.
5. Easier testing and mocking
The sender class can be fully unit-tested with a mocked Client.
There's no need to involve real HTTP requests or fixture payloads when validating logic.
6. Scalable for future use cases
The same pattern can support future senders (e.g. TemplateSender, DelayedSender, PriorityBatchSender) without rewriting or refactoring Client.
Each sender can build upon Mailtrap::Client while enforcing their own rules and expectations.
7. Backward compatibility
This change doesn't affect existing users of Client#send.
It's a non-breaking enhancement — users can opt into batch functionality only if needed.
🔚 In summary: introducing BatchSender allows us to maintain separation of concerns, strict validation, and predictable SDK behavior — while keeping Client clean, HTTP-focused, and reusable for all other API domains
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests