From 24f03f1ed4a1c6282efee3de27e75813fee34d7c Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Mon, 28 Sep 2026 16:37:34 +1000 Subject: [PATCH] fix(template_mailer): never hand templated email to itself The Template Mailer picked the module at index 1 of the system's Mailer implementers. That list comes from a redis hash with no defined order, and after a system edit on HIO UAT the order changed so that index 1 was the Template Mailer itself: every templated send recursed until core was OOM killed, and the visitor mailer's sends went to the Calendar module. The mailer is now the next Mailer after this module in the system, and never this module, with the first other Mailer as the fallback. PPT-2816 --- drivers/place/template_mailer.cr | 10 ++++++++- drivers/place/template_mailer_spec.cr | 30 +++++++++++++-------------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/drivers/place/template_mailer.cr b/drivers/place/template_mailer.cr index ecb01a91fc..cfacee7e52 100644 --- a/drivers/place/template_mailer.cr +++ b/drivers/place/template_mailer.cr @@ -33,8 +33,16 @@ class Place::TemplateMailer < PlaceOS::Driver getter org_zone_id : String { get_local_zone_id(org_zone_ids).not_nil! } getter building_zone_id : String { get_local_zone_id(building_zone_ids).not_nil! } + # The mailer templated email is handed to: the next Mailer in the system, + # never this module itself def mailer - system.implementing(Interface::Mailer)[1] + mailers = system.implementing(Interface::Mailer) + mine = mailers.find { |mod| mod.module_id == module_id } + if mine + following = mailers.find { |mod| mod.module_name == mine.module_name && mod.index == mine.index + 1 } + return following if following + end + mailers.find { |mod| mod.module_id != module_id } || raise "no other mailer found in the system to send through" end SEPERATOR = "." diff --git a/drivers/place/template_mailer_spec.cr b/drivers/place/template_mailer_spec.cr index 03755e0644..c3ab59c3f4 100644 --- a/drivers/place/template_mailer_spec.cr +++ b/drivers/place/template_mailer_spec.cr @@ -1,9 +1,9 @@ require "placeos-driver/spec" require "placeos-driver/interface/mailer" -# The TemplateMailer under test has `generic_name :Mailer`, so it occupies the -# Mailer_1 slot. The mock declared below becomes Mailer_2 -- the module that -# `system.implementing(Interface::Mailer)[1]` forwards to. +# The TemplateMailer under test hands templated email to the next Mailer in the +# system that is not itself. The spec runner keeps the driver under test out of +# the module map, so the first mock below (Mailer_1) is the one it forwards to. class StaffAPI < DriverSpecs::MockDriver ZONES = [ { @@ -129,9 +129,9 @@ class Mailer < DriverSpecs::MockDriver end DriverSpecs.mock_driver "Place::TemplateMailer" do - # The TemplateMailer under test forwards to `system.implementing(Mailer)[1]`. - # In production that index 1 is the next mailer in the chain (e.g. SMTP); here - # we declare two mock mailers so index 1 is the recording mock (Mailer_2). + # The TemplateMailer under test forwards to the first Mailer that is not + # itself. Two mocks are declared so the choice between them is visible: the + # recording mock is Mailer_1 and Mailer_2 must stay untouched. system({ StaffAPI: {StaffAPI}, Mailer: {Mailer, Mailer}, @@ -146,7 +146,7 @@ DriverSpecs.mock_driver "Place::TemplateMailer" do args: {name: "Bob"}, reply_to: "host@org.com", ).get - system(:Mailer_2)[:reply_to].should eq "template-reply@org.com" + system(:Mailer_1)[:reply_to].should eq "template-reply@org.com" # 2. With no template match and no configured reply_to, the host reply_to is # forwarded through to the downstream mailer. @@ -157,7 +157,7 @@ DriverSpecs.mock_driver "Place::TemplateMailer" do args: {name: "Bob"}, reply_to: "host@org.com", ).get - system(:Mailer_2)[:reply_to].should eq "host@org.com" + system(:Mailer_1)[:reply_to].should eq "host@org.com" # 3. A reply_to configured on the TemplateMailer overrides BOTH the # per-template reply_to and the host reply_to passed in by the caller. @@ -172,7 +172,7 @@ DriverSpecs.mock_driver "Place::TemplateMailer" do args: {name: "Bob"}, reply_to: "host@org.com", ).get - system(:Mailer_2)[:reply_to].should eq "tenant@org.com" + system(:Mailer_1)[:reply_to].should eq "tenant@org.com" # 4. A template with no recipients of its own leaves the caller's To, CC and # BCC untouched. @@ -183,9 +183,9 @@ DriverSpecs.mock_driver "Place::TemplateMailer" do args: {name: "Bob"}, cc: ["assistant@org.com"], ).get - system(:Mailer_2)[:to].should eq "steve@org.com" - system(:Mailer_2)[:cc].should eq ["assistant@org.com"] - system(:Mailer_2)[:bcc].should eq [] of String + system(:Mailer_1)[:to].should eq "steve@org.com" + system(:Mailer_1)[:cc].should eq ["assistant@org.com"] + system(:Mailer_1)[:bcc].should eq [] of String # 5. A template's `to` replaces the recipient the driver chose, and its `cc` # and `bcc` are added to the caller's, without duplicates. @@ -197,7 +197,7 @@ DriverSpecs.mock_driver "Place::TemplateMailer" do cc: ["assistant@org.com"], bcc: "reception@org.com", ).get - system(:Mailer_2)[:to].should eq ["reception@org.com", "security@org.com"] - system(:Mailer_2)[:cc].should eq ["assistant@org.com", "manager@org.com"] - system(:Mailer_2)[:bcc].should eq ["reception@org.com", "audit@org.com"] + system(:Mailer_1)[:to].should eq ["reception@org.com", "security@org.com"] + system(:Mailer_1)[:cc].should eq ["assistant@org.com", "manager@org.com"] + system(:Mailer_1)[:bcc].should eq ["reception@org.com", "audit@org.com"] end