message: add checkValidity method & deprecate valid

eleith · Aug 29, 2021 · 6f435a4 · 6f435a4
1 parent 0516e38
commit 6f435a4
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 83 deletions.
diff --git a/smtp/client.ts b/smtp/client.ts
@@ -55,18 +55,18 @@ export class SMTPClient {
 			return;
 		}
 
-		message.valid((valid, why) => {
-			if (valid) {
-				const stack = this.createMessageStack(message, callback);
-				if (stack.to.length === 0) {
-					return callback(new Error('No recipients found in message'), msg);
-				}
-				this.queue.push(stack);
-				this._poll();
-			} else {
-				callback(new Error(why), msg);
+		const { isValid, validationError } = message.checkValidity();
+
+		if (isValid) {
+			const stack = this.createMessageStack(message, callback);
+			if (stack.to.length === 0) {
+				return callback(new Error('No recipients found in message'), msg);
 			}
-		});
+			this.queue.push(stack);
+			this._poll();
+		} else {
+			callback(new Error(validationError), msg);
+		}
 	}
 
 	/**

diff --git a/smtp/message.ts b/smtp/message.ts
@@ -193,30 +193,35 @@ export class Message {
 
 	/**
 	 * @public
-	 * @param {function(isValid: boolean, invalidReason: string): void} callback .
-	 * @returns {void}
+	 * @returns {{ isValid: boolean, validationError: (string | undefined) }} an object specifying whether this message is validly formatted, and the first validation error if it is not.
 	 */
-	public valid(callback: (isValid: boolean, invalidReason?: string) => void) {
+	public checkValidity() {
 		if (
 			typeof this.header.from !== 'string' &&
 			Array.isArray(this.header.from) === false
 		) {
-			callback(false, 'Message must have a `from` header');
-		} else if (
+			return {
+				isValid: false,
+				validationError: 'Message must have a `from` header',
+			};
+		}
+
+		if (
 			typeof this.header.to !== 'string' &&
 			Array.isArray(this.header.to) === false &&
 			typeof this.header.cc !== 'string' &&
 			Array.isArray(this.header.cc) === false &&
 			typeof this.header.bcc !== 'string' &&
 			Array.isArray(this.header.bcc) === false
 		) {
-			callback(
-				false,
-				'Message must have at least one `to`, `cc`, or `bcc` header'
-			);
-		} else if (this.attachments.length === 0) {
-			callback(true, undefined);
-		} else {
+			return {
+				isValid: false,
+				validationError:
+					'Message must have at least one `to`, `cc`, or `bcc` header',
+			};
+		}
+
+		if (this.attachments.length > 0) {
 			const failed: string[] = [];
 
 			this.attachments.forEach((attachment) => {
@@ -232,9 +237,24 @@ export class Message {
 					failed.push('attachment has no data associated with it');
 				}
 			});
-
-			callback(failed.length === 0, failed.join(', '));
+			return {
+				isValid: failed.length === 0,
+				validationError: failed.join(', '),
+			};
 		}
+
+		return { isValid: true, validationError: undefined };
+	}
+
+	/**
+	 * @public
+	 * @deprecated does not conform to the `errback` style followed by the rest of the library, and will be removed in the next major version. use `checkValidity` instead.
+	 * @param {function(isValid: boolean, invalidReason: (string | undefined)): void} callback .
+	 * @returns {void}
+	 */
+	public valid(callback: (isValid: boolean, invalidReason?: string) => void) {
+		const { isValid, validationError } = this.checkValidity();
+		callback(isValid, validationError);
 	}
 
 	/**

diff --git a/test/client.ts b/test/client.ts
@@ -140,7 +140,7 @@ test('client accepts array recipients', async (t) => {
 	msg.header.cc = [msg.header.cc as string];
 	msg.header.bcc = [msg.header.bcc as string];
 
-	const isValid = await new Promise((r) => msg.valid(r));
+	const { isValid } = msg.checkValidity();
 	const stack = client.createMessageStack(msg);
 
 	t.true(isValid);
@@ -158,7 +158,7 @@ test('client accepts array sender', async (t) => {
 	});
 	msg.header.from = [msg.header.from as string];
 
-	const isValid = await new Promise((r) => msg.valid(r));
+	const { isValid } = msg.checkValidity();
 	t.true(isValid);
 });
 

diff --git a/test/message.ts b/test/message.ts
@@ -63,31 +63,6 @@ function send(headers: Partial<MessageHeaders>) {
 	});
 }
 
-function validate(headers: Partial<MessageHeaders>) {
-	const { to, cc, bcc } = headers;
-	const msg = new Message(headers);
-
-	if (Array.isArray(to)) {
-		msg.header.to = to;
-	}
-	if (Array.isArray(cc)) {
-		msg.header.to = to;
-	}
-	if (Array.isArray(bcc)) {
-		msg.header.to = to;
-	}
-
-	return new Promise((resolve, reject) => {
-		msg.valid((isValid, reason) => {
-			if (isValid) {
-				resolve(isValid);
-			} else {
-				reject(new Error(reason));
-			}
-		});
-	});
-}
-
 test.before(async (t) => {
 	server.listen(port, t.pass);
 });
@@ -401,69 +376,80 @@ test('streams message', async (t) => {
 });
 
 test('message validation fails without `from` header', async (t) => {
-	const { message: error } = await t.throwsAsync(validate({}));
-	t.is(error, 'Message must have a `from` header');
+	const msg = new Message({});
+	const { isValid, validationError } = msg.checkValidity();
+	t.false(isValid);
+	t.is(validationError, 'Message must have a `from` header');
 });
 
 test('message validation fails without `to`, `cc`, or `bcc` header', async (t) => {
-	const { message: error } = await t.throwsAsync(
-		validate({
-			from: 'piglet@gmail.com',
-		})
+	const { isValid, validationError } = new Message({
+		from: 'piglet@gmail.com',
+	}).checkValidity();
+
+	t.false(isValid);
+	t.is(
+		validationError,
+		'Message must have at least one `to`, `cc`, or `bcc` header'
 	);
-	t.is(error, 'Message must have at least one `to`, `cc`, or `bcc` header');
 });
 
 test('message validation succeeds with only `to` recipient header (string)', async (t) => {
-	const isValid = validate({
+	const { isValid, validationError } = new Message({
 		from: 'piglet@gmail.com',
 		to: 'pooh@gmail.com',
-	});
-	await t.notThrowsAsync(isValid);
-	t.true(await isValid);
+	}).checkValidity();
+
+	t.true(isValid);
+	t.is(validationError, undefined);
 });
 
 test('message validation succeeds with only `to` recipient header (array)', async (t) => {
-	const isValid = validate({
+	const { isValid, validationError } = new Message({
 		from: 'piglet@gmail.com',
 		to: ['pooh@gmail.com'],
-	});
-	await t.notThrowsAsync(isValid);
-	t.true(await isValid);
+	}).checkValidity();
+
+	t.true(isValid);
+	t.is(validationError, undefined);
 });
 
 test('message validation succeeds with only `cc` recipient header (string)', async (t) => {
-	const isValid = validate({
+	const { isValid, validationError } = new Message({
 		from: 'piglet@gmail.com',
 		cc: 'pooh@gmail.com',
-	});
-	await t.notThrowsAsync(isValid);
-	t.true(await isValid);
+	}).checkValidity();
+
+	t.true(isValid);
+	t.is(validationError, undefined);
 });
 
 test('message validation succeeds with only `cc` recipient header (array)', async (t) => {
-	const isValid = validate({
+	const { isValid, validationError } = new Message({
 		from: 'piglet@gmail.com',
 		cc: ['pooh@gmail.com'],
-	});
-	await t.notThrowsAsync(isValid);
-	t.true(await isValid);
+	}).checkValidity();
+
+	t.true(isValid);
+	t.is(validationError, undefined);
 });
 
 test('message validation succeeds with only `bcc` recipient header (string)', async (t) => {
-	const isValid = validate({
+	const { isValid, validationError } = new Message({
 		from: 'piglet@gmail.com',
 		bcc: 'pooh@gmail.com',
-	});
-	await t.notThrowsAsync(isValid);
-	t.true(await isValid);
+	}).checkValidity();
+
+	t.true(isValid);
+	t.is(validationError, undefined);
 });
 
 test('message validation succeeds with only `bcc` recipient header (array)', async (t) => {
-	const isValid = validate({
+	const { isValid, validationError } = new Message({
 		from: 'piglet@gmail.com',
 		bcc: ['pooh@gmail.com'],
-	});
-	await t.notThrowsAsync(isValid);
-	t.true(await isValid);
+	}).checkValidity();
+
+	t.true(isValid);
+	t.is(validationError, undefined);
 });