From b08e729728bca6aa0098a0715b1188b961b31e36 Mon Sep 17 00:00:00 2001 From: James Maes Date: Wed, 14 Oct 2026 10:20:27 -0500 Subject: [PATCH] refactor(email): use qbit-email's public api package; no G1 allow-list Moves the optional email integration from qbit-email internals to com.kingsrook.qbits.email.api (qbit-email 0.1.0-RC.2) with identical behaviour, and moves to qbit-build-parent 2.1.1, whose default G1 allow-lists include qbit-email's API (QRun-IO/qbit-bom#12), so the per-repo allow-lists are gone. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 +- README.md | 2 +- pom.xml | 9 +--- .../qbits/alerts/email/AlertsEmail.java | 4 +- .../qbits/alerts/email/AlertsEmailSender.java | 50 +++++++++---------- .../alerts/email/EmailQBitSendingTest.java | 33 +++++++----- src/test/resources/email-h2.sql | 4 +- 7 files changed, 55 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d65099d..1f336bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,8 @@ All notable changes to this project are documented here. The format follows Keep ## [0.1.0] - Unreleased ### Changed -- Email goes through qbit-email when it is produced in the instance (optional dependency, `qbit-email.version` 0.1.0-RC.1): alert channel emails (template `alerts_alert`), notification emails (`alerts_notification`) and digests (`alerts_digest`), all NOTIFICATION, one recipient per message, with idempotency keys and the alert or notification as the linked record. `messagingProviderName` stays as the fallback when qbit-email is absent, and is not required when it is present. +- Switch the optional qbit-email integration to its published `com.kingsrook.qbits.email.api` package in `0.1.0-RC.2`, preserving templates, categories, idempotency keys, linked records and outcome mapping. Narrow the architecture allowance to that API package while retaining all guardrails. +- Email goes through qbit-email when it is produced in the instance (optional dependency, `qbit-email.version` 0.1.0-RC.2): alert channel emails (template `alerts_alert`), notification emails (`alerts_notification`) and digests (`alerts_digest`), all NOTIFICATION, one recipient per message, with idempotency keys and the alert or notification as the linked record. `messagingProviderName` stays as the fallback when qbit-email is absent, and is not required when it is present. - Target released QQQ `4.1.0-RC.6`; verify normally with `./mvnw -B verify` (no version override). - Adopt `qbit-build-parent:2.1.0` with architecture guardrail enforcement and strict UI API routes enabled for tests. Dependency, extension contract and stand-in checks run in the normal build. - Add record followers to alert routing through core `RecordFollowers.get(instance, ref, FollowEvent.ALERT)` (#16, QRun-IO/qqq#1164). Deduplicate by user/channel with subscriptions and other record refs; require source-record read access under the follower's session, including record locks. Followers use the existing alert visibility, preferences and suppression pipeline without a dependency on qbit-collaboration. Missing/disabled/failing providers preserve subscription routing. diff --git a/README.md b/README.md index e60f32d..202063f 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ How each one sends: ## Email through qbit-email -When qbit-email is produced in the same instance (and on the classpath; it is an optional dependency), alert channel emails, notification emails and daily digests go through `EmailService` instead of core `SendMessageAction`: +When qbit-email is produced in the same instance (and on the classpath; it is an optional dependency), alert channel emails, notification emails and daily digests go through the published `com.kingsrook.qbits.email.api.EmailApi` in qbit-email `0.1.0-RC.2` instead of core `SendMessageAction`. Template registration, sends and installation detection use only the public API; templates, categories, idempotency keys, linked records and outcome mapping are unchanged: - **Templates**, registered on first use and never overwritten (edit them in the Email app): `alerts_alert` (a channel email for one alert or a batch), `alerts_notification` (one notification) and `alerts_digest` (a list of items, `{{#each items}}`). All are Markdown, category NOTIFICATION, so unsubscribe and suppression apply. - **One recipient per message.** An email list channel sends one message per address. diff --git a/pom.xml b/pom.xml index e096d3b..c534691 100644 --- a/pom.xml +++ b/pom.xml @@ -23,20 +23,15 @@ 21 0.8.15 - 0.1.0-RC.1 + 0.1.0-RC.2 true true - - - - com.kingsrook.qbits:qbit-object-storage,com.kingsrook.qbits:*-api,com.kingsrook.qbits:qbit-email - com.kingsrook.qbits.objectstorage.api,com.kingsrook.qbits.email com.kingsrook qbit-build-parent - 2.1.0 + 2.1.1 com.kingsrook.qbits diff --git a/src/main/java/com/kingsrook/qbits/alerts/email/AlertsEmail.java b/src/main/java/com/kingsrook/qbits/alerts/email/AlertsEmail.java index 3bbb4cc..e48e2a2 100644 --- a/src/main/java/com/kingsrook/qbits/alerts/email/AlertsEmail.java +++ b/src/main/java/com/kingsrook/qbits/alerts/email/AlertsEmail.java @@ -30,7 +30,7 @@ import com.kingsrook.qqq.backend.core.model.metadata.QInstance; *******************************************************************************/ public final class AlertsEmail { - private static final String EMAIL_SERVICE_CLASS = "com.kingsrook.qbits.email.outbox.EmailService"; + private static final String EMAIL_API_CLASS = "com.kingsrook.qbits.email.api.EmailApi"; @@ -64,7 +64,7 @@ public final class AlertsEmail } try { - Class.forName(EMAIL_SERVICE_CLASS); + Class.forName(EMAIL_API_CLASS); } catch(ClassNotFoundException | LinkageError e) { diff --git a/src/main/java/com/kingsrook/qbits/alerts/email/AlertsEmailSender.java b/src/main/java/com/kingsrook/qbits/alerts/email/AlertsEmailSender.java index 7a73c0c..6ab65a5 100644 --- a/src/main/java/com/kingsrook/qbits/alerts/email/AlertsEmailSender.java +++ b/src/main/java/com/kingsrook/qbits/alerts/email/AlertsEmailSender.java @@ -27,13 +27,11 @@ import java.util.Locale; import java.util.Map; import com.kingsrook.qbits.alerts.model.AlertsAlert; import com.kingsrook.qbits.alerts.model.AlertsNotification; -import com.kingsrook.qbits.email.EmailRuntime; -import com.kingsrook.qbits.email.outbox.EmailCategory; -import com.kingsrook.qbits.email.outbox.EmailRequest; -import com.kingsrook.qbits.email.outbox.EmailSendResult; -import com.kingsrook.qbits.email.outbox.EmailService; -import com.kingsrook.qbits.email.template.EmailTemplateDefinition; -import com.kingsrook.qbits.email.template.EmailTemplates; +import com.kingsrook.qbits.email.api.EmailApi; +import com.kingsrook.qbits.email.api.EmailApiCategory; +import com.kingsrook.qbits.email.api.EmailApiRequest; +import com.kingsrook.qbits.email.api.EmailApiResult; +import com.kingsrook.qbits.email.api.EmailApiTemplate; import com.kingsrook.qqq.backend.core.exceptions.QException; import com.kingsrook.qqq.backend.core.model.metadata.QInstance; @@ -42,7 +40,7 @@ import com.kingsrook.qqq.backend.core.model.metadata.QInstance; ** The one place that names qbit-email classes. Callers reach it only after ** {@link AlertsEmail#isActive()}. It registers the alerts templates on first ** use (never overwriting people's edits) and sends one message per recipient - ** through {@code EmailService}, all in the NOTIFICATION category: + ** through {@code EmailApi}, all in the NOTIFICATION category: ** - alerts_alert: an alert channel email (one alert or a batch); ** - alerts_notification: one user notification; ** - alerts_digest: a user's daily digest of notifications. @@ -96,7 +94,7 @@ public final class AlertsEmailSender *******************************************************************************/ static boolean isInstalled(QInstance qInstance) { - return (EmailRuntime.config(qInstance) != null); + return (EmailApi.isInstalled(qInstance)); } @@ -126,7 +124,7 @@ public final class AlertsEmailSender String idPart = ids.isEmpty() ? "test" : String.join("-", ids.stream().map(String::valueOf).toList()); String key = "alerts:alert:" + idPart + ":" + hash(to) + ":" + messageKind + ":" + (now.getEpochSecond() / 60); - EmailRequest request = base(to, process, key) + EmailApiRequest request = base(to, process, key) .withTemplateName(TEMPLATE_ALERT) .withVariable("subject", subject) .withVariable("severity", severity) @@ -138,7 +136,7 @@ public final class AlertsEmailSender } withUrl(request, "link", link); withUrl(request, "settings_url", settingsUrl); - return (result(EmailService.send(request))); + return (result(EmailApi.send(request))); } @@ -155,7 +153,7 @@ public final class AlertsEmailSender public static Result sendNotification(String to, String process, Integer notificationId, Map item, String settingsUrl) throws QException { ensure(TEMPLATE_NOTIFICATION); - EmailRequest request = base(to, process, "alerts:notification:" + notificationId + ":" + hash(to)) + EmailApiRequest request = base(to, process, "alerts:notification:" + notificationId + ":" + hash(to)) .withTemplateName(TEMPLATE_NOTIFICATION) .withLinkedRecord(AlertsNotification.TABLE_NAME, notificationId); for(Map.Entry entry : item.entrySet()) @@ -170,7 +168,7 @@ public final class AlertsEmailSender } } withUrl(request, "settings_url", settingsUrl); - return (result(EmailService.send(request))); + return (result(EmailApi.send(request))); } @@ -190,13 +188,13 @@ public final class AlertsEmailSender String settingsUrl) throws QException { ensure(TEMPLATE_DIGEST); - EmailRequest request = base(to, process, "alerts:digest:" + userId + ":" + windowStart) + EmailApiRequest request = base(to, process, "alerts:digest:" + userId + ":" + windowStart) .withTemplateName(TEMPLATE_DIGEST) .withVariable("count", items.size()) .withVariable("items", (Serializable) items) .withLinkedRecord(AlertsNotification.TABLE_NAME, firstNotificationId); withUrl(request, "settings_url", settingsUrl); - return (result(EmailService.send(request))); + return (result(EmailApi.send(request))); } @@ -204,9 +202,9 @@ public final class AlertsEmailSender /******************************************************************************* ** The parts every message has: one recipient, the sender and the key. *******************************************************************************/ - private static EmailRequest base(String to, String process, String idempotencyKey) + private static EmailApiRequest base(String to, String process, String idempotencyKey) { - return (new EmailRequest().withTo(to).withCategory(EmailCategory.NOTIFICATION).withSender(SENDER_QBIT, process).withIdempotencyKey(idempotencyKey)); + return (new EmailApiRequest().withTo(to).withCategory(EmailApiCategory.NOTIFICATION).withSender(SENDER_QBIT, process).withIdempotencyKey(idempotencyKey)); } @@ -215,7 +213,7 @@ public final class AlertsEmailSender ** Pass a URL only when it is absolute http(s); the template schema refuses ** relative links, and they are useless in a mail anyway. *******************************************************************************/ - private static void withUrl(EmailRequest request, String name, String url) + private static void withUrl(EmailApiRequest request, String name, String url) { if(url != null && (url.startsWith("https://") || url.startsWith("http://"))) { @@ -228,7 +226,7 @@ public final class AlertsEmailSender /******************************************************************************* ** Translate qbit-email's result. *******************************************************************************/ - private static Result result(EmailSendResult sent) + private static Result result(EmailApiResult sent) { String detail = sent.reasons() == null || sent.reasons().isEmpty() ? null : String.join("; ", sent.reasons()); return (new Result(Status.valueOf(sent.status().name()), sent.messageId(), detail)); @@ -266,10 +264,10 @@ public final class AlertsEmailSender *******************************************************************************/ private static void ensure(String name) throws QException { - EmailTemplates.ensure(switch(name) + EmailApi.ensureTemplate(switch(name) { - case TEMPLATE_ALERT -> new EmailTemplateDefinition() - .withName(TEMPLATE_ALERT).withLabel("Alerts: alert").withCategory(EmailCategory.NOTIFICATION).withKind("MARKDOWN").withOwnerQbit(SENDER_QBIT) + case TEMPLATE_ALERT -> new EmailApiTemplate() + .withName(TEMPLATE_ALERT).withLabel("Alerts: alert").withCategory(EmailApiCategory.NOTIFICATION).withKind("MARKDOWN").withOwnerQbit(SENDER_QBIT) .withSubject("{{subject}}") .withBody(""" {{#each items}}**{{this.title}}** @@ -293,8 +291,8 @@ public final class AlertsEmailSender .withVariable("settings_url", "Settings link", "URL", false, null) .withSampleVariables("{\"subject\":\"[ERROR] Export failed\",\"severity\":\"ERROR\",\"kind\":\"RAISED\",\"items\":[{\"title\":\"Export failed\",\"severity\":\"ERROR\"," + "\"typeLabel\":\"Jobs\",\"summary\":\"3 records\"}],\"link\":\"https://example.com/app/alerts_alert/1\",\"settings_url\":\"https://example.com/app/alerts_subscription\"}"); - case TEMPLATE_NOTIFICATION -> new EmailTemplateDefinition() - .withName(TEMPLATE_NOTIFICATION).withLabel("Alerts: notification").withCategory(EmailCategory.NOTIFICATION).withKind("MARKDOWN").withOwnerQbit(SENDER_QBIT) + case TEMPLATE_NOTIFICATION -> new EmailApiTemplate() + .withName(TEMPLATE_NOTIFICATION).withLabel("Alerts: notification").withCategory(EmailApiCategory.NOTIFICATION).withKind("MARKDOWN").withOwnerQbit(SENDER_QBIT) .withSubject("{{title}}") .withBody(""" {{#if body}}{{body}} @@ -309,8 +307,8 @@ public final class AlertsEmailSender .withVariable("settings_url", "Settings link", "URL", false, null) .withSampleVariables("{\"title\":\"Export failed\",\"body\":\"The nightly export did not finish.\",\"link\":\"https://example.com/app/widget\",\"linkLabel\":\"Open\"," + "\"settings_url\":\"https://example.com/app/alerts_notification_preference\"}"); - case TEMPLATE_DIGEST -> new EmailTemplateDefinition() - .withName(TEMPLATE_DIGEST).withLabel("Alerts: daily digest").withCategory(EmailCategory.NOTIFICATION).withKind("MARKDOWN").withOwnerQbit(SENDER_QBIT) + case TEMPLATE_DIGEST -> new EmailApiTemplate() + .withName(TEMPLATE_DIGEST).withLabel("Alerts: daily digest").withCategory(EmailApiCategory.NOTIFICATION).withKind("MARKDOWN").withOwnerQbit(SENDER_QBIT) .withSubject("{{count}} new notifications") .withBody(""" {{#each items}}**{{this.title}}** diff --git a/src/test/java/com/kingsrook/qbits/alerts/email/EmailQBitSendingTest.java b/src/test/java/com/kingsrook/qbits/alerts/email/EmailQBitSendingTest.java index ff90006..974f884 100644 --- a/src/test/java/com/kingsrook/qbits/alerts/email/EmailQBitSendingTest.java +++ b/src/test/java/com/kingsrook/qbits/alerts/email/EmailQBitSendingTest.java @@ -22,6 +22,7 @@ import java.nio.charset.StandardCharsets; import java.sql.Connection; import java.sql.Statement; import java.time.Instant; +import java.util.ArrayList; import java.util.List; import com.kingsrook.qbits.alerts.AlertsPreferences; import com.kingsrook.qbits.alerts.AlertsQBitConfig; @@ -36,8 +37,8 @@ import com.kingsrook.qbits.alerts.model.AlertsUserPreference; import com.kingsrook.qbits.alerts.notifications.NotificationEmailer; import com.kingsrook.qbits.email.EmailQBitConfig; import com.kingsrook.qbits.email.EmailQBitProducer; -import com.kingsrook.qbits.email.EmailRuntime; -import com.kingsrook.qbits.email.metadata.EmailTables; +import com.kingsrook.qbits.email.api.EmailApi; +import com.kingsrook.qbits.email.api.EmailApiMessage; import com.kingsrook.qqq.backend.core.alerts.AlertLink; import com.kingsrook.qqq.backend.core.alerts.NotificationAudience; import com.kingsrook.qqq.backend.core.alerts.NotificationCategory; @@ -64,7 +65,8 @@ class EmailQBitSendingTest extends BaseAlertsTest { /******************************************************************************* ** The email qbit in the same instance, and the alerts config without a - ** messaging provider (qbit-email makes it unnecessary). + ** messaging provider (qbit-email makes it unnecessary). G1 permits the + ** producer and configuration imports in test setup. *******************************************************************************/ @Override protected void customizeInstance(QInstance qInstance) @@ -116,7 +118,7 @@ class EmailQBitSendingTest extends BaseAlertsTest *******************************************************************************/ private void addProvider() throws Exception { - runtime.asSystem(() -> AlertsDb.insert(EmailTables.PROVIDER, new QRecord().withValue("name", "relay").withValue("label", "Relay").withValue("kind", "SMTP") + runtime.asSystem(() -> AlertsDb.insert("email_provider", new QRecord().withValue("name", "relay").withValue("label", "Relay").withValue("kind", "SMTP") .withValue("host", "127.0.0.1").withValue("port", 2525).withValue("security", "NONE").withValue("fromAddress", "sender@example.com") .withValue("enabled", true).withValue("isDefault", true))); } @@ -124,12 +126,18 @@ class EmailQBitSendingTest extends BaseAlertsTest /******************************************************************************* - ** The queued messages, oldest first. + ** Find messages through the API, oldest first. Read the fixture rows with + ** core QQQ for content assertions: the API deliberately omits bodies, + ** recipients and sender details from its metadata projection. *******************************************************************************/ private List messages() throws QException { - return (runtime.asSystem(() -> AlertsDb.query(EmailTables.MESSAGE, new com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter().withOrderBy( - new com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterOrderBy("id"))))); + List messages = new ArrayList<>(); + for(EmailApiMessage message : EmailApi.findMessagesByIdempotencyKeyPrefix("alerts:", 100)) + { + messages.add(runtime.asSystem(() -> AlertsDb.byId("email_message", message.id()))); + } + return (messages); } @@ -139,7 +147,7 @@ class EmailQBitSendingTest extends BaseAlertsTest *******************************************************************************/ private String templateName(QRecord message) throws QException { - return (runtime.asSystem(() -> AlertsDb.byId(EmailTables.TEMPLATE, message.getValue("templateId"))).getValueString("name")); + return (EmailApi.getMessage(message.getValueInteger("id")).orElseThrow().templateName()); } @@ -184,7 +192,7 @@ class EmailQBitSendingTest extends BaseAlertsTest { assertThat(AlertsEmail.isActive()).isTrue(); assertThat(AlertsEmail.isActive(null)).isFalse(); - assertThat(EmailRuntime.config(qInstance)).isNotNull(); + assertThat(EmailApi.isInstalled(qInstance)).isTrue(); assertThat(new EmailChannel(new AlertsQBitConfig(), ChannelType.EMAIL).validate(null, null)).isEmpty(); assertThat(new NotificationEmailer(runtime).isConfigured()).isTrue(); } @@ -201,7 +209,8 @@ class EmailQBitSendingTest extends BaseAlertsTest { addProvider(); EmailChannel channel = new EmailChannel(config, ChannelType.EMAIL); - assertThat(channel.deliver(alertMessage(), null, "ops@example.com", null).outcome()).isEqualTo(DeliveryResult.Outcome.SENT); + DeliveryResult delivered = channel.deliver(alertMessage(), null, "ops@example.com", null); + assertThat(delivered.outcome()).as("Delivery outcome: %s", delivered.error()).isEqualTo(DeliveryResult.Outcome.SENT); List messages = messages(); assertThat(messages).hasSize(1); @@ -267,7 +276,7 @@ class EmailQBitSendingTest extends BaseAlertsTest void testSuppressedAddress() throws Exception { addProvider(); - runtime.asSystem(() -> AlertsDb.insert(EmailTables.SUPPRESSION, new QRecord().withValue("address", "ops@example.com").withValue("scope", "ALL").withValue("reason", "HARD_BOUNCE"))); + runtime.asSystem(() -> AlertsDb.insert("email_suppression", new QRecord().withValue("address", "ops@example.com").withValue("scope", "ALL").withValue("reason", "HARD_BOUNCE"))); DeliveryResult result = new EmailChannel(config, ChannelType.EMAIL).deliver(alertMessage(), null, "ops@example.com", null); assertThat(result.outcome()).isEqualTo(DeliveryResult.Outcome.FAILED); assertThat(result.error()).contains("suppressed"); @@ -344,7 +353,7 @@ class EmailQBitSendingTest extends BaseAlertsTest loginAs(ADMIN); QAlerts.notify(new NotificationSignal("Export failed").withSeverity(NotificationSeverity.WARNING).withCategory(NotificationCategory.JOB) .withAudience(NotificationAudience.users("viewer-1"))); - runtime.asSystem(() -> AlertsDb.insert(EmailTables.SUPPRESSION, new QRecord().withValue("address", "viewer-1@example.com").withValue("scope", "ALL").withValue("reason", "HARD_BOUNCE"))); + runtime.asSystem(() -> AlertsDb.insert("email_suppression", new QRecord().withValue("address", "viewer-1@example.com").withValue("scope", "ALL").withValue("reason", "HARD_BOUNCE"))); NotificationEmailer.Result suppressed = new NotificationEmailer(runtime).run(); assertThat(suppressed.skipped()).isEqualTo(1); assertThat(suppressed.sent()).isEqualTo(0); diff --git a/src/test/resources/email-h2.sql b/src/test/resources/email-h2.sql index ae1ea2e..264fdd4 100644 --- a/src/test/resources/email-h2.sql +++ b/src/test/resources/email-h2.sql @@ -75,6 +75,7 @@ CREATE TABLE email_message ( phi_approved BOOLEAN, unsubscribe_url TEXT, idempotency_key VARCHAR(500), + rfc_message_id VARCHAR(500), provider_id INTEGER, provider_message_id VARCHAR(500), attempt_count INTEGER, @@ -93,7 +94,8 @@ CREATE TABLE email_message ( sender_process VARCHAR(500), suppressed_addresses TEXT, send_after TIMESTAMP, - UNIQUE (idempotency_key) + UNIQUE (idempotency_key), + UNIQUE (rfc_message_id) ); CREATE TABLE email_message_attempt ( -- 2.54.0