diff --git a/actions/confirmaddress.php b/actions/confirmaddress.php index 1e2d3a92d6..806851001e 100644 --- a/actions/confirmaddress.php +++ b/actions/confirmaddress.php @@ -108,13 +108,8 @@ class ConfirmaddressAction extends Action $cur->smsemail = $carrier->toEmailAddress($cur->sms); } - $result = $cur->updateWithKeys($orig_user); - - if (!$result) { - common_log_db_error($cur, 'UPDATE', __FILE__); - // TRANS: Server error displayed when confirming an e-mail address or IM address fails. - $this->serverError(_('Could not update user.')); - } + // Throws exception on failure. + $cur->updateWithKeys($orig_user); if ($type == 'email') { $cur->emailChanged(); diff --git a/actions/emailsettings.php b/actions/emailsettings.php index 483131c779..0c2033d821 100644 --- a/actions/emailsettings.php +++ b/actions/emailsettings.php @@ -478,20 +478,12 @@ class EmailsettingsAction extends SettingsAction return; } - $user->query('BEGIN'); - $original = clone($user); $user->email = null; - $result = $user->updateWithKeys($original); - - if (!$result) { - common_log_db_error($user, 'UPDATE', __FILE__); - // TRANS: Server error thrown on database error removing a registered e-mail address. - $this->serverError(_('Could not update user.')); - } - $user->query('COMMIT'); + // Throws exception on failure. Also performs it within a transaction. + $user->updateWithKeys($original); // TRANS: Message given after successfully removing a registered e-mail address. $this->showForm(_('The email address was removed.'), true); @@ -517,11 +509,8 @@ class EmailsettingsAction extends SettingsAction $user->incomingemail = null; $user->emailpost = 0; - if (!$user->updateWithKeys($orig)) { - common_log_db_error($user, 'UPDATE', __FILE__); - // TRANS: Server error thrown on database error removing incoming e-mail address. - $this->serverError(_('Could not update user record.')); - } + // Throws exception on failure. Also performs it within a transaction. + $user->updateWithKeys($orig); // TRANS: Message given after successfully removing an incoming e-mail address. $this->showForm(_('Incoming email address removed.'), true); @@ -541,11 +530,8 @@ class EmailsettingsAction extends SettingsAction $user->incomingemail = mail_new_incoming_address(); $user->emailpost = 1; - if (!$user->updateWithKeys($orig)) { - common_log_db_error($user, 'UPDATE', __FILE__); - // TRANS: Server error thrown on database error adding incoming e-mail address. - $this->serverError(_('Could not update user record.')); - } + // Throws exception on failure. Also performs it within a transaction. + $user->updateWithKeys($orig); // TRANS: Message given after successfully adding an incoming e-mail address. $this->showForm(_('New incoming email address added.'), true); diff --git a/actions/recoverpassword.php b/actions/recoverpassword.php index 46df772b73..4839a036c0 100644 --- a/actions/recoverpassword.php +++ b/actions/recoverpassword.php @@ -105,12 +105,8 @@ class RecoverpasswordAction extends Action if (!$user->email) { $orig = clone($user); $user->email = $email; - $result = $user->updateWithKeys($orig); - if (!$result) { - common_log_db_error($user, 'UPDATE', __FILE__); - // TRANS: Server error displayed when updating a user's e-mail address in the database fails while recovering a password. - $this->serverError(_('Could not update user with confirmed email address.')); - } + // Throws exception on failure. + $user->updateWithKeys($orig); } // Success! diff --git a/actions/smssettings.php b/actions/smssettings.php index ec2fcfd89e..ec8841281d 100644 --- a/actions/smssettings.php +++ b/actions/smssettings.php @@ -436,21 +436,14 @@ class SmssettingsAction extends SettingsAction return; } - $user->query('BEGIN'); - $original = clone($user); $user->sms = null; $user->carrier = null; $user->smsemail = null; - $result = $user->updateWithKeys($original); - if (!$result) { - common_log_db_error($user, 'UPDATE', __FILE__); - // TRANS: Server error thrown on database error removing a registered SMS phone number. - $this->serverError(_('Could not update user.')); - } - $user->query('COMMIT'); + // Throws exception on failure. Also performs it within a transaction. + $user->updateWithKeys($original); // TRANS: Message given after successfully removing a registered SMS phone number. $this->showForm(_('The SMS phone number was removed.'), true); @@ -556,11 +549,8 @@ class SmssettingsAction extends SettingsAction $user->incomingemail = null; - if (!$user->updateWithKeys($orig)) { - common_log_db_error($user, 'UPDATE', __FILE__); - // TRANS: Server error displayed when the user could not be updated in SMS settings. - $this->serverError(_('Could not update user record.')); - } + // Throws exception on failure. Also performs it within a transaction. + $user->updateWithKeys($orig); // TRANS: Confirmation text after updating SMS settings. $this->showForm(_('Incoming email address removed.'), true); @@ -581,11 +571,8 @@ class SmssettingsAction extends SettingsAction $user->incomingemail = mail_new_incoming_address(); - if (!$user->updateWithKeys($orig)) { - common_log_db_error($user, 'UPDATE', __FILE__); - // TRANS: Server error displayed when the user could not be updated in SMS settings. - $this->serverError(_('Could not update user record.')); - } + // Throws exception on failure. Also performs it within a transaction. + $user->updateWithKeys($orig); // TRANS: Confirmation text after updating SMS settings. $this->showForm(_('New incoming email address added.'), true); diff --git a/classes/Managed_DataObject.php b/classes/Managed_DataObject.php index cc088249db..a221f96633 100644 --- a/classes/Managed_DataObject.php +++ b/classes/Managed_DataObject.php @@ -339,7 +339,13 @@ abstract class Managed_DataObject extends Memcached_DataObject } } if (count($parts) == 0) { - // No changes, unless made in the ->update call + // No changes to keys, it's safe to run ->update(...) + if ($this->update($orig) === false) { + common_log_db_error($this, 'UPDATE', __FILE__); + // rollback as something bad occurred + $this->query('ROLLBACK'); + throw new ServerException("Could not UPDATE non-keys for {$this->__table}"); + } return true; } $toupdate = implode(', ', $parts); @@ -357,6 +363,8 @@ abstract class Managed_DataObject extends Memcached_DataObject } // Update non-keys too, if the previous endeavour worked. + // The ->update call uses "$this" values for keys, that's why we can't do this until + // the keys are updated (because they might differ from $orig and update the wrong entries). if ($this->update($orig) === false) { common_log_db_error($this, 'UPDATE', __FILE__); // rollback as something bad occurred diff --git a/classes/Profile.php b/classes/Profile.php index ea99244e31..12672d2157 100644 --- a/classes/Profile.php +++ b/classes/Profile.php @@ -838,12 +838,8 @@ class Profile extends Managed_DataObject common_debug("Updating User ({$this->id}) nickname from {$dataObject->nickname} to {$this->nickname}"); $origuser = clone($local); $local->nickname = $this->nickname; - $result = $local->updateWithKeys($origuser); - if ($result === false) { - common_log_db_error($local, 'UPDATE', __FILE__); - // TRANS: Server error thrown when user profile settings could not be updated. - throw new ServerException(_('Could not update user nickname.')); - } + // updateWithKeys throws exception on failure. + $local->updateWithKeys($origuser); // Clear the site owner, in case nickname changed if ($local->hasRole(Profile_role::OWNER)) { diff --git a/plugins/Oembed/OembedPlugin.php b/plugins/Oembed/OembedPlugin.php index c5343c7b4f..29c6f08dd2 100644 --- a/plugins/Oembed/OembedPlugin.php +++ b/plugins/Oembed/OembedPlugin.php @@ -288,10 +288,8 @@ class OembedPlugin extends Plugin $thumbnail->filename = $filename; $thumbnail->width = $info[0]; // array indexes documented on php.net: $thumbnail->height = $info[1]; // https://php.net/manual/en/function.getimagesize.php - if (!$thumbnail->update($orig)) { - unlink($fullpath); // delete the file if database failed to write - throw new ServerException(_('Failed to update remotely downloaded file info in database.')); - } + // Throws exception on failure. + $thumbnail->updateWithKeys($orig); } public function onPluginVersion(array &$versions) diff --git a/plugins/RequireValidatedEmail/actions/confirmfirstemail.php b/plugins/RequireValidatedEmail/actions/confirmfirstemail.php index 465814342d..cc4b646f6b 100644 --- a/plugins/RequireValidatedEmail/actions/confirmfirstemail.php +++ b/plugins/RequireValidatedEmail/actions/confirmfirstemail.php @@ -150,6 +150,7 @@ class ConfirmfirstemailAction extends Action $this->user->email = $this->confirm->address; + // Throws exception on failure. $this->user->updateWithKeys($orig); $this->user->emailChanged(); diff --git a/scripts/clear_jabber.php b/scripts/clear_jabber.php index 4a7de8a3d9..4ba24c275a 100755 --- a/scripts/clear_jabber.php +++ b/scripts/clear_jabber.php @@ -75,7 +75,11 @@ function clear_jabber($id) } else { $original = clone($user); $user->jabber = null; - $result = $user->updateWithKeys($original); + try { + $user->updateWithKeys($original); + } catch (Exception $e) { + echo "WARNING: user update failed (setting jabber to null): ".$e->getMessage()."\n"; + } } echo "\n"; } else if (!$user) { diff --git a/scripts/registeruser.php b/scripts/registeruser.php index cc0bd5598d..97caf645cd 100644 --- a/scripts/registeruser.php +++ b/scripts/registeruser.php @@ -69,10 +69,8 @@ try { $user->email = $email; - if (!$user->updateWithKeys($orig)) { - print "Failed!\n"; - throw new Exception("Can't update email address."); - } + // Throws exception on failure. + $user->updateWithKeys($orig); } } catch (Exception $e) {