Page MenuHomePhabricator

Factor code for user email out of the User class
Open, Needs TriagePublic

Description

I suggest that the code dealing with a user's email be moved to a new UserEmailManager service

The following would be affected:

public $mEmail
public $mEmailAuthenticated
protected $mEmailToken
protected $mEmailTokenExpires
public static function newFromConfirmationCode()
public function getEmail()
public function getEmailAuthenticationTimestamp()
public function setEmail()
public function setEmailWithConfirmation()
public function sendConfirmationMail()
public function sendMail()
protected function confirmationToken()
protected function confirmationTokenUrl()
public function confirmEmail()
public function invalidateEmail()
public function setEmailAuthenticationTimestamp()
public function canSendEmail()
public function canReceiveEmail()
public function isEmailConfirmed()
public function isEmailConfirmationPending()

These methods use multiple globals and services that could be injected.

  • an ILoadBalancer (currently databases are retrieved via wfGetDB)
  • (following migration of Hook calls) a HookContainer
  • $wgEnableEmail, $wgEmailAuthentication
  • $wgLang
  • (if a message localizer can be injected) wfMessage can be replaced
  • $wgPasswordSender
  • $wgUserEmailConfirmationTokenExpiry
  • $wgEnableUserEmail
  • PermissionManager (permissions currently checked via User::isAllowed)
  • UserOptionsLookup (options currently retrieved via User::getOption)

The new UserEmailManager would provide lower-level interaction with the database

  • Read user email, email authentication, and token information
  • Save email, email authentication, and token information
  • Determine, using database and a user's options, whether they can recieve email
  • Determine, using database and a user's permissions, whether they can send email

Thoughts?

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

I am wondering is that this can/should be done well until we introduce a UserRecord, or something alike.

The components we've been pulling out of the User class so far were backed by tables other then user with one exception of UserEditTracker (might have been a mistake to use UserIdentity there). However, if we are going to pull apart more and more of the features backed by the user table, we will end up with really complex initialization/purging logic to maintain correctness. Also, reconstructing the user row for writing into the database would e hard with all the caches in all such services.

Thus, I believe it would be better to postpone this until we have a proper data transfer object for User.