[buddypress-trac] [BuddyPress Trac] #8003: Setting Reply-To and From causes excessive database overhead in cases where addresses don't match a user

[buddypress-trac]

#8003: Setting Reply-To and From causes excessive database overhead in cases where
addresses don't match a user
--------------------------+---------------------------------
 Reporter:  boonebgorges  |      Owner:  DJPaul
     Type:  enhancement   |     Status:  new
 Priority:  normal        |  Milestone:  Under Consideration
Component:  Emails        |    Version:
 Severity:  normal        |   Keywords:
--------------------------+---------------------------------
 When setting the `Reply-To` and `From` headers of an outgoing email,
 `BP_Email` uses the `BP_Email_Recipient` class, which does a WP user
 lookup:
 https://buddypress.trac.wordpress.org/browser/tags/3.2.0/src/bp-
 core/classes/class-bp-email.php?marks=698#L688
 https://buddypress.trac.wordpress.org/browser/tags/3.2.0/src/bp-
 core/classes/class-bp-email.php?marks=770#L759

 If the installation uses a From or Reply-To address that does not match a
 WP user account, the WP user lookup will fail. And because of WP's
 `get_user_by()` internals, failed lookups are not cached.
 https://core.trac.wordpress.org/browser/tags/4.9.8/src/wp-includes/class-
 wp-user.php#L214

 One or two extra database queries is not a serious problem, but in cases
 where many emails are sent in a single request - as in BuddyPress Group
 Email Subscription, or as when the group admin checks "notify members of
 this change" - the result can be dozens or hundreds of additional database
 hits. See https://redmine.gc.cuny.edu/issues/10720 for a real-world case.

 I imagine it's very common, especially for larger installations, to use a
 From and Reply-To address on outgoing emails that does not correspond to a
 WP user. There are many use cases: custom Reply-To for reply-by-email or
 for bounce handling; an inbox that is shared by members of a support team;
 a no-reply inbox.

 A few suggested approaches:

 1. Don't use `BP_Email_Recipient` in `set_replyto()` and `set_from()`.
 Aside from general DRY principles, it looks like the primary purpose for
 this class in the case of *recipients* is to validate email addresses and
 to get canonical display names for the "To" field. These motivations don't
 apply, or apply differently, in the case of email *senders*. Specifically,
 the `$from_address` and `$from_name` are trusted values that go through WP
 filters `wp_mail_from` and `wp_mail_from_name`, so it's not clear that the
 extra lookup is necessary. Skipping the use of `BP_Email_Recipient` in
 this case would mean a non-trivial patch, since `BP_Email` expects the
 `BP_Email_Recipient` interface. We could, for example, introduce a
 `BP_Email_User` interface that is implemented by both `BP_Email_Recipient`
 and `BP_Email_Sender`.

 2. Provide a short-circuit filter in `BP_Email_Recipient` that allows
 plugins or sites to skip the database lookup. For example, something near
 the beginning of
 https://buddypress.trac.wordpress.org/browser/tags/3.2.0/src/bp-
 core/classes/class-bp-email-recipient.php#L46; a plugin would then be able
 to filter, and if a certain email address is passed, then it could send
 back a set of dummy values, or something like that.

 I'm happy to work up a patch, but was hoping first to get some
 architectural feedback from @djpaul.

-- 
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/8003>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac