From e76009507d8543b87b189e69840d7fd97be14de9 Mon Sep 17 00:00:00 2001 From: Edimar Cardoso Date: Thu, 28 Jul 2022 18:50:53 -0300 Subject: [PATCH] Refactor email send methods and organize tests - Make Email.send using Email.sendAsync with Promisse.await() (Fibers) - Allow send method to receive a stream object and use in the devSendMode method for tests. - Remove usage of Fibers Future - Code formatting --- packages/email/email.js | 131 +++++++-------------------- packages/email/email_test_helpers.js | 12 +-- packages/email/email_tests.js | 27 +++--- 3 files changed, 54 insertions(+), 116 deletions(-) diff --git a/packages/email/email.js b/packages/email/email.js index 68056e1000..cbe588c2ae 100644 --- a/packages/email/email.js +++ b/packages/email/email.js @@ -2,7 +2,6 @@ import { Meteor } from 'meteor/meteor'; import { Log } from 'meteor/logging'; import { Hook } from 'meteor/callback-hook'; -import Future from 'fibers/future'; import url from 'url'; import nodemailer from 'nodemailer'; import wellKnow from 'nodemailer/lib/well-known'; @@ -25,7 +24,7 @@ export const EmailInternals = { const MailComposer = EmailInternals.NpmModules.mailcomposer.module; -const makeTransport = function(mailUrlString) { +const makeTransport = function (mailUrlString) { const mailUrl = new URL(mailUrlString); if (mailUrl.protocol !== 'smtp:' && mailUrl.protocol !== 'smtps:') { @@ -60,7 +59,7 @@ const makeTransport = function(mailUrlString) { }; // More info: https://nodemailer.com/smtp/well-known/ -const knownHostsTransport = function(settings = undefined, url = undefined) { +const knownHostsTransport = function (settings = undefined, url = undefined) { let service, user, password; const hasSettings = settings && Object.keys(settings).length; @@ -110,7 +109,7 @@ const knownHostsTransport = function(settings = undefined, url = undefined) { }; EmailTest.knowHostsTransport = knownHostsTransport; -const getTransport = function() { +const getTransport = function () { const packageSettings = Meteor.settings.packages?.email || {}; // We delay this check until the first call to Email.send, in case someone // set process.env.MAIL_URL in startup code. Then we store in a cache until @@ -138,44 +137,37 @@ const getTransport = function() { }; let nextDevModeMailId = 0; -let output_stream = process.stdout; EmailTest._getAndIncNextDevModeMailId = function () { return nextDevModeMailId++; }; // Testing hooks -EmailTest.overrideOutputStream = function(stream) { +EmailTest.resetNextDevModeMailId = function () { nextDevModeMailId = 0; - output_stream = stream; }; -EmailTest.restoreOutputStream = function() { - output_stream = process.stdout; -}; +const devModeSendAsync = async function (mail, stream) { + return new Promise((resolve, reject) => { + let devModeMailId = EmailTest._getAndIncNextDevModeMailId(); -const devModeSend = function(mail) { - let devModeMailId = EmailTest._getAndIncNextDevModeMailId(); - - const stream = output_stream; - - // This approach does not prevent other writers to stdout from interleaving. - stream.write('====== BEGIN MAIL #' + devModeMailId + ' ======\n'); - stream.write( - '(Mail not sent; to enable sending, set the MAIL_URL ' + - 'environment variable.)\n' - ); - const readStream = new MailComposer(mail).compile().createReadStream(); - readStream.pipe(stream, { end: false }); - const future = new Future(); - readStream.on('end', function() { - stream.write('====== END MAIL #' + devModeMailId + ' ======\n'); - future.return(); + // This approach does not prevent other writers to stdout from interleaving. + stream.write('====== BEGIN MAIL #' + devModeMailId + ' ======\n'); + stream.write( + '(Mail not sent; to enable sending, set the MAIL_URL ' + + 'environment variable.)\n' + ); + const readStream = new MailComposer(mail).compile().createReadStream(); + readStream.pipe(stream, { end: false }); + readStream.on('end', function () { + stream.write('====== END MAIL #' + devModeMailId + ' ======\n'); + resolve(); + }); + readStream.on('error', (err) => reject(err)); }); - future.wait(); }; -const smtpSend = function(transport, mail) { +const smtpSend = function (transport, mail) { transport._syncSendMail(mail); }; @@ -190,7 +182,7 @@ const sendHooks = new Hook(); * false to skip sending. * @returns {{ stop: function, callback: function }} */ -Email.hookSend = function(f) { +Email.hookSend = function (f) { return sendHooks.register(f); }; @@ -231,67 +223,16 @@ Email.customTransport = undefined; * @param {Object[]} [options.attachments] Array of attachment objects, as * described in the [nodemailer documentation](https://nodemailer.com/message/attachments/). * @param {MailComposer} [options.mailComposer] A [MailComposer](https://nodemailer.com/extras/mailcomposer/#e-mail-message-fields) + * @param {Object} [options.stream] Output stream to write email on development environment * object representing the message to be sent. Overrides all other options. * You can create a `MailComposer` object via * `new EmailInternals.NpmModules.mailcomposer.module`. */ -Email.send = function(options) { - if (options.mailComposer) { - options = options.mailComposer.mail; - } - - let send = true; - sendHooks.forEach(hook => { - send = hook(options); - return send; - }); - if (!send) return; - - const customTransport = Email.customTransport; - if (customTransport) { - const packageSettings = Meteor.settings.packages?.email || {}; - customTransport({ packageSettings, ...options }); - return; - } - - const mailUrlEnv = process.env.MAIL_URL; - const mailUrlSettings = Meteor.settings.packages?.email; - - if (Meteor.isProduction && !mailUrlEnv && !mailUrlSettings) { - // This check is mostly necessary when using the flag --production when running locally. - // And it works as a reminder to properly set the mail URL when running locally. - throw new Error( - 'You have not provided a mail URL. You can provide it by using the environment variable MAIL_URL or your settings. You can read more about it here: https://docs.meteor.com/api/email.html.' - ); - } - - if (mailUrlEnv || mailUrlSettings) { - const transport = getTransport(); - smtpSend(transport, options); - return; - } - devModeSend(options); +Email.send = function (options) { + // Using Fibers Promise.await + Promise.await(Email.sendAsync(options)); }; -const devModeSendAsync = async function (mail, stream) { - return new Promise((resolve, reject) => { - let devModeMailId = EmailTest._getAndIncNextDevModeMailId(); - - // This approach does not prevent other writers to stdout from interleaving. - stream.write('====== BEGIN MAIL #' + devModeMailId + ' ======\n'); - stream.write( - '(Mail not sent; to enable sending, set the MAIL_URL ' + - 'environment variable.)\n' - ); - const readStream = new MailComposer(mail).compile().createReadStream(); - readStream.pipe(stream, { end: false }); - readStream.on('end', function () { - stream.write('====== END MAIL #' + devModeMailId + ' ======\n'); - resolve(stream); - }); - readStream.on('error', (err) => reject(err)); - }); -}; // TODO Rewrite summary. /** * @summary Send an email with asyncronous method. Capture Throws an `Error` on failure to contact mail server @@ -322,30 +263,28 @@ const devModeSendAsync = async function (mail, stream) { * @param {Object[]} [options.attachments] Array of attachment objects, as * described in the [nodemailer documentation](https://nodemailer.com/message/attachments/). * @param {MailComposer} [options.mailComposer] A [MailComposer](https://nodemailer.com/extras/mailcomposer/#e-mail-message-fields) + * @param {Object} [options.stream] Output stream to write email on development environment * object representing the message to be sent. Overrides all other options. * You can create a `MailComposer` object via * `new EmailInternals.NpmModules.mailcomposer.module`. */ Email.sendAsync = async function (options) { - const stream = output_stream; - const { isTestMode } = Email; - if (options.mailComposer) { - options = options.mailComposer.mail; - } + const { stream = process.stdout, ...rest } = options; + const email = rest.mailComposer ? rest.mailComposer.mail : rest; let send = true; sendHooks.forEach((hook) => { - send = hook(options); + send = hook(email); return send; }); if (!send) { - return isTestMode && stream; + return; } if (Email.customTransport) { const packageSettings = Meteor.settings.packages?.email || {}; - Email.customTransport({ packageSettings, ...options }); - return isTestMode && stream; + Email.customTransport({ packageSettings, ...email }); + return; } const mailUrlEnv = process.env.MAIL_URL; @@ -361,10 +300,10 @@ Email.sendAsync = async function (options) { if (mailUrlEnv || mailUrlSettings) { const transport = getTransport(); - smtpSend(transport, options); + smtpSend(transport, email); return; } - return devModeSendAsync(options, stream).catch((err) => { + return devModeSendAsync(email, stream).catch((err) => { throw err; }); }; diff --git a/packages/email/email_test_helpers.js b/packages/email/email_test_helpers.js index c0c7d0c847..a8706ab1c9 100644 --- a/packages/email/email_test_helpers.js +++ b/packages/email/email_test_helpers.js @@ -7,15 +7,9 @@ export const devWarningBanner = export const smokeEmailTest = (testFunction) => { // This only tests dev mode, so don't run the test if this is deployed. if (process.env.MAIL_URL) return; - - try { - const stream = new streamBuffers.WritableStreamBuffer(); - EmailTest.overrideOutputStream(stream); - - testFunction(stream); - } finally { - EmailTest.restoreOutputStream(); - } + const stream = new streamBuffers.WritableStreamBuffer(); + EmailTest.resetNextDevModeMailId(); + testFunction(stream); }; export const canonicalize = (string) => { diff --git a/packages/email/email_tests.js b/packages/email/email_tests.js index 472d33131b..c01bd8f377 100644 --- a/packages/email/email_tests.js +++ b/packages/email/email_tests.js @@ -1,15 +1,14 @@ +import { Email } from 'meteor/email'; import { smokeEmailTest } from './email_test_helpers'; import { TEST_CASES } from './email_tests_data'; -Email.isTestMode = true; - // Create dynamic sync tests TEST_CASES.forEach(({ title, options, testCalls }) => { Tinytest.add(`[Sync] ${title}`, function (test) { smokeEmailTest((stream) => { Object.entries(options).forEach(([key, option]) => { const testCall = testCalls[key]; - Email.send(option); + Email.send({ ...option, stream }); testCall(test, stream); }); }); @@ -19,10 +18,10 @@ TEST_CASES.forEach(({ title, options, testCalls }) => { // Create dynamic async tests TEST_CASES.forEach(({ title, options, testCalls }) => { Tinytest.addAsync(`[Async] ${title}`, function (test, onComplete) { - smokeEmailTest(() => { + smokeEmailTest((stream) => { const allPromises = Object.entries(options).map(([key, option]) => { const testCall = testCalls[key]; - return Email.sendAsync(option).then((stream) => { + return Email.sendAsync({ ...option, stream }).then(() => { testCall(test, stream); }); }); @@ -45,6 +44,7 @@ Tinytest.add( to: 'bar@example.com', text: '*Cool*, man', html: 'Cool, man', + stream, }); test.equal(stream.getContentsAsString('utf8'), false); }); @@ -63,6 +63,7 @@ Tinytest.add( to: 'bar@example.com', text: '*Cool*, man', html: 'Cool, man', + stream, }); test.equal(stream.getContentsAsString('utf8'), false); @@ -93,6 +94,7 @@ Tinytest.add('[Sync] email - hooks stop the sending', function (test) { to: 'bar@example.com', text: '*Cool*, man', html: 'Cool, man', + stream, }); test.equal(stream.getContentsAsString('utf8'), false); @@ -108,7 +110,7 @@ Tinytest.addAsync( '[Async] email - alternate API is used for sending gets data', function (test, onComplete) { const allPromises = []; - smokeEmailTest(() => { + smokeEmailTest((stream) => { Email.customTransport = (options) => { test.equal(options.from, 'foo@example.com'); }; @@ -118,13 +120,14 @@ Tinytest.addAsync( to: 'bar@example.com', text: '*Cool*, man', html: 'Cool, man', - }).then((stream) => { + stream, + }).then(() => { test.equal(stream.getContentsAsString('utf8'), false); }) ); }); - smokeEmailTest(function () { + smokeEmailTest(function (stream) { Meteor.settings.packages = { email: { service: '1on1', user: 'test', password: 'pwd' }, }; @@ -139,7 +142,8 @@ Tinytest.addAsync( to: 'bar@example.com', text: '*Cool*, man', html: 'Cool, man', - }).then((stream) => { + stream, + }).then(() => { test.equal(stream.getContentsAsString('utf8'), false); }) ); @@ -169,13 +173,14 @@ Tinytest.addAsync( const hook3 = Email.hookSend(() => { console.log('FAIL'); }); - smokeEmailTest(() => { + smokeEmailTest((stream) => { Email.sendAsync({ from: 'foo@example.com', to: 'bar@example.com', text: '*Cool*, man', html: 'Cool, man', - }).then((stream) => { + stream, + }).then(() => { test.equal(stream.getContentsAsString('utf8'), false); hook1.stop(); hook2.stop();