From d1269f815e976d1306471311db373a2a0c806b7f Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 6 Apr 2022 20:59:14 +0800 Subject: [PATCH 1/2] identify a newsletter by checking both list-url and unsubscribe-url in headers and email sender address --- packages/api/src/services/newsletters.ts | 2 -- .../inbound-email-handler/src/bloomberg-handler.ts | 2 +- packages/inbound-email-handler/src/golang-handler.ts | 2 +- packages/inbound-email-handler/src/index.ts | 10 +++++++--- packages/inbound-email-handler/src/newsletter.ts | 12 ++++++++---- .../inbound-email-handler/src/substack-handler.ts | 6 +++--- 6 files changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/api/src/services/newsletters.ts b/packages/api/src/services/newsletters.ts index d091a18c3..ac6391146 100644 --- a/packages/api/src/services/newsletters.ts +++ b/packages/api/src/services/newsletters.ts @@ -3,9 +3,7 @@ import { nanoid } from 'nanoid' import { User } from '../entity/user' import { CreateNewsletterEmailErrorCode } from '../generated/graphql' import { env } from '../env' -import { AppDataSource } from '../server' import { getRepository } from '../entity/utils' - import addressparser = require('nodemailer/lib/addressparser') const parsedAddress = (emailAddress: string): string | undefined => { diff --git a/packages/inbound-email-handler/src/bloomberg-handler.ts b/packages/inbound-email-handler/src/bloomberg-handler.ts index a60736461..3239ab176 100644 --- a/packages/inbound-email-handler/src/bloomberg-handler.ts +++ b/packages/inbound-email-handler/src/bloomberg-handler.ts @@ -5,6 +5,6 @@ export class BloombergHandler extends NewsletterHandler { super() this.senderRegex = /<.+@mail.bloomberg.*.com>/ this.urlRegex = // this.urlRegex = /Read on the Web<\/a>/ - this.defaultUrl = 'https://golangweekly.com/' + this.defaultUrl = 'https://golangweekly.com' } } diff --git a/packages/inbound-email-handler/src/index.ts b/packages/inbound-email-handler/src/index.ts index 6f4caa502..724e2c064 100644 --- a/packages/inbound-email-handler/src/index.ts +++ b/packages/inbound-email-handler/src/index.ts @@ -30,10 +30,11 @@ const NEWSLETTER_HANDLERS = [ export const getNewsletterHandler = ( rawUrl: string, - from: string + from: string, + rawUnSubUrl: string ): NewsletterHandler | undefined => { return NEWSLETTER_HANDLERS.find((h) => { - return h.isNewsletter(rawUrl, from) + return h.isNewsletter(rawUrl, from, rawUnSubUrl) }) } @@ -72,9 +73,12 @@ export const inboundEmailHandler = Sentry.GCPFunction.wrapHttpFunction( ? forwardedAddress.toString() : parsed.to const rawUrl = headers['list-post'] ? headers['list-post'].toString() : '' + const rawUnSubUrl = headers['list-unsubscribe'] + ? headers['list-unsubscribe'].toString() + : '' // check if it is a forwarding confirmation email or newsletter - const newsletterHandler = getNewsletterHandler(rawUrl, from) + const newsletterHandler = getNewsletterHandler(rawUrl, from, rawUnSubUrl) try { if (newsletterHandler) { console.log('handleNewsletter', from, recipientAddress) diff --git a/packages/inbound-email-handler/src/newsletter.ts b/packages/inbound-email-handler/src/newsletter.ts index 858bdcf97..8c59f4fc4 100644 --- a/packages/inbound-email-handler/src/newsletter.ts +++ b/packages/inbound-email-handler/src/newsletter.ts @@ -1,4 +1,5 @@ import { PubSub } from '@google-cloud/pubsub' +import { v4 as uuidv4 } from 'uuid' import addressparser from 'addressparser' const pubsub = new PubSub() @@ -14,10 +15,10 @@ export class NewsletterHandler { protected urlRegex = /NEWSLETTER_URL_REGEX/ protected defaultUrl = 'NEWSLETTER_DEFAULT_URL' - isNewsletter(_rawUrl: string, from: string): boolean { + isNewsletter(rawUrl: string, from: string, rawUnSubUrl: string): boolean { // Axios newsletter is from const re = new RegExp(this.senderRegex) - return re.test(from) + return re.test(from) && (!!rawUrl || !!rawUnSubUrl) } getNewsletterUrl(_rawUrl: string, html: string): string | undefined { @@ -55,8 +56,11 @@ export class NewsletterHandler { } // fallback to default url if newsletter url does not exist - const url = this.getNewsletterUrl(rawUrl, html) || this.defaultUrl - const author = this.getAuthor(from) + // assign a random uuid to the default url to avoid duplicate url + const url = + this.getNewsletterUrl(rawUrl, html) || + `${this.defaultUrl}?source=newsletters&id=${uuidv4()}` + const author = this.getAuthor(from) || 'Unknown' const message = { email: email, diff --git a/packages/inbound-email-handler/src/substack-handler.ts b/packages/inbound-email-handler/src/substack-handler.ts index 02530c30b..1870a17e4 100644 --- a/packages/inbound-email-handler/src/substack-handler.ts +++ b/packages/inbound-email-handler/src/substack-handler.ts @@ -4,7 +4,7 @@ import addressparser from 'addressparser' export class SubstackHandler extends NewsletterHandler { constructor() { super() - this.defaultUrl = 'https://www.substack.com/' + this.defaultUrl = 'https://www.substack.com' } getNewsletterUrl(rawUrl: string, _html: string): string | undefined { @@ -15,7 +15,7 @@ export class SubstackHandler extends NewsletterHandler { : undefined } - isNewsletter(rawUrl: string, _from: string): boolean { - return !!rawUrl + isNewsletter(rawUrl: string, _from: string, rawUnSubUrl: string): boolean { + return !!rawUrl || !!rawUnSubUrl } } From 0a67032fd370285709438682c9c708094f951358 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 6 Apr 2022 21:09:22 +0800 Subject: [PATCH 2/2] fix tests --- packages/inbound-email-handler/src/index.ts | 8 +++---- .../inbound-email-handler/src/newsletter.ts | 4 ++-- .../src/substack-handler.ts | 4 ++-- .../test/newsletter.test.ts | 24 +++++++++++++++---- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/packages/inbound-email-handler/src/index.ts b/packages/inbound-email-handler/src/index.ts index 724e2c064..71c1d018b 100644 --- a/packages/inbound-email-handler/src/index.ts +++ b/packages/inbound-email-handler/src/index.ts @@ -31,10 +31,10 @@ const NEWSLETTER_HANDLERS = [ export const getNewsletterHandler = ( rawUrl: string, from: string, - rawUnSubUrl: string + unSubRawUrl: string ): NewsletterHandler | undefined => { return NEWSLETTER_HANDLERS.find((h) => { - return h.isNewsletter(rawUrl, from, rawUnSubUrl) + return h.isNewsletter(rawUrl, from, unSubRawUrl) }) } @@ -73,12 +73,12 @@ export const inboundEmailHandler = Sentry.GCPFunction.wrapHttpFunction( ? forwardedAddress.toString() : parsed.to const rawUrl = headers['list-post'] ? headers['list-post'].toString() : '' - const rawUnSubUrl = headers['list-unsubscribe'] + const unSubRawUrl = headers['list-unsubscribe'] ? headers['list-unsubscribe'].toString() : '' // check if it is a forwarding confirmation email or newsletter - const newsletterHandler = getNewsletterHandler(rawUrl, from, rawUnSubUrl) + const newsletterHandler = getNewsletterHandler(rawUrl, from, unSubRawUrl) try { if (newsletterHandler) { console.log('handleNewsletter', from, recipientAddress) diff --git a/packages/inbound-email-handler/src/newsletter.ts b/packages/inbound-email-handler/src/newsletter.ts index 8c59f4fc4..1e96a70bd 100644 --- a/packages/inbound-email-handler/src/newsletter.ts +++ b/packages/inbound-email-handler/src/newsletter.ts @@ -15,10 +15,10 @@ export class NewsletterHandler { protected urlRegex = /NEWSLETTER_URL_REGEX/ protected defaultUrl = 'NEWSLETTER_DEFAULT_URL' - isNewsletter(rawUrl: string, from: string, rawUnSubUrl: string): boolean { + isNewsletter(rawUrl: string, from: string, unSubRawUrl: string): boolean { // Axios newsletter is from const re = new RegExp(this.senderRegex) - return re.test(from) && (!!rawUrl || !!rawUnSubUrl) + return re.test(from) && (!!rawUrl || !!unSubRawUrl) } getNewsletterUrl(_rawUrl: string, html: string): string | undefined { diff --git a/packages/inbound-email-handler/src/substack-handler.ts b/packages/inbound-email-handler/src/substack-handler.ts index 1870a17e4..4aa888383 100644 --- a/packages/inbound-email-handler/src/substack-handler.ts +++ b/packages/inbound-email-handler/src/substack-handler.ts @@ -15,7 +15,7 @@ export class SubstackHandler extends NewsletterHandler { : undefined } - isNewsletter(rawUrl: string, _from: string, rawUnSubUrl: string): boolean { - return !!rawUrl || !!rawUnSubUrl + isNewsletter(rawUrl: string, _from: string, _unSubRawUrl: string): boolean { + return !!rawUrl } } diff --git a/packages/inbound-email-handler/test/newsletter.test.ts b/packages/inbound-email-handler/test/newsletter.test.ts index 46f427421..9710e7407 100644 --- a/packages/inbound-email-handler/test/newsletter.test.ts +++ b/packages/inbound-email-handler/test/newsletter.test.ts @@ -34,26 +34,36 @@ describe('Newsletter email test', () => { it('returns SubstackHandler when email is from SubStack', () => { const rawUrl = '' - expect(getNewsletterHandler(rawUrl, '')).to.be.instanceof(SubstackHandler) + expect(getNewsletterHandler(rawUrl, '', '')).to.be.instanceof( + SubstackHandler + ) }) it('returns AxiosHandler when email is from Axios', () => { const from = 'Mike Allen ' + const unSubRawUrl = + '' - expect(getNewsletterHandler('', from)).to.be.instanceof(AxiosHandler) + expect(getNewsletterHandler('', from, unSubRawUrl)).to.be.instanceof( + AxiosHandler + ) }) context('when email is from Bloomberg', () => { it('should return BloombergHandler when email is from Bloomberg Business', () => { const from = 'From: Bloomberg ' - expect(getNewsletterHandler('', from)).to.be.instanceof( + const unSubRawUrl = '' + + expect(getNewsletterHandler('', from, unSubRawUrl)).to.be.instanceof( BloombergHandler ) }) it('should return BloombergHandler when email is from Bloomberg View', () => { const from = 'From: Bloomberg ' - expect(getNewsletterHandler('', from)).to.be.instanceof( + const unSubRawUrl = '' + + expect(getNewsletterHandler('', from, unSubRawUrl)).to.be.instanceof( BloombergHandler ) }) @@ -61,7 +71,11 @@ describe('Newsletter email test', () => { it('should return GolangHandler when email is from Golang Weekly', () => { const from = 'Golang Weekly ' - expect(getNewsletterHandler('', from)).to.be.instanceof(GolangHandler) + const unSubRawUrl = '' + + expect(getNewsletterHandler('', from, unSubRawUrl)).to.be.instanceof( + GolangHandler + ) }) })