Managed_DataObject->updateWithKeys throws its own exception

diff --git a/actions/confirmaddress.php b/actions/confirmaddress.php
index 1e2d3a92d6d3104a8bc6650d9fe45e9726308c6e..806851001e3cf45e4a0f13717b9589b8cd89060d 100644 (file)
--- 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 483131c779f9d415f55311b8d0723ccfbfab2f82..0c2033d821d0587a0bcbf5aeb974d17ad3024ebd 100644 (file)
--- 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 46df772b73214047dcd05351e5cda52ef216e6ae..4839a036c0e4be2dcb0f632f3d3fd32c71204336 100644 (file)
--- 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 ec2fcfd89e94cd6694eb88280f45121c6659f934..ec8841281d2bf07c544910080002aacd7181e556 100644 (file)
--- 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 cc088249db1d2986b49237ce8670b4d72f87be0f..a221f96633a0781e1ff5051299155040a8cc6eff 100644 (file)
--- 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 ea99244e3189db8a06cd3c0ae0f45db6a1e4c70c..12672d21577a7a877c4f0085499e1dfbfaee3f5c 100644 (file)
--- 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 c5343c7b4f7f7a5a85ef3cc8ea4ce34bd7984167..29c6f08dd28d6338d5a6ea9746142816b3d6f133 100644 (file)
--- 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 465814342d1bd44eca0e2acebc3089ab6a6c6086..cc4b646f6b024c37e84fb01d370af8f8687b7498 100644 (file)
--- 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 4a7de8a3d91fb3d29c5cc27d593f3fa0d0f3685c..4ba24c275a37031ac0efe583cdde5affb0d376b0 100755 (executable)
--- 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 cc0bd5598de088e7d72808470e7734c93cdff914..97caf645cd6eb812904d9c68da110e958e30ca49 100644 (file)
--- 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) {