diff --git a/actions/emailsettings.php b/actions/emailsettings.php index 1a81304f56..d83fa7b32b 100644 --- a/actions/emailsettings.php +++ b/actions/emailsettings.php @@ -92,7 +92,7 @@ class EmailsettingsAction extends SettingsAction $this->element('legend', null, _('Email address')); $this->hidden('token', common_session_token()); - if ($user->email) { + if (!$user->isNull('email')) { $this->element('p', array('id' => 'form_confirmed'), $user->email); // TRANS: Form note in e-mail settings form. $this->element('p', array('class' => 'form_note'), _('Current confirmed email address.')); @@ -163,7 +163,7 @@ class EmailsettingsAction extends SettingsAction $this->elementStart('div', array('id' => 'emailincoming')); - if ($user->incomingemail) { + if (!$user->isNull('incomingemail')) { $this->elementStart('p'); $this->element('span', 'address', $user->incomingemail); // @todo XXX: Looks a little awkward in the UI. @@ -180,7 +180,7 @@ class EmailsettingsAction extends SettingsAction } $this->elementStart('p'); - if ($user->incomingemail) { + if (!$user->isNull('incomingemail')) { // TRANS: Instructions for incoming e-mail address input form, when an address has already been assigned. $msg = _('Make a new email address for posting to; '. 'cancels the old one.'); @@ -435,7 +435,7 @@ class EmailsettingsAction extends SettingsAction } $original = clone($user); - $user->email = DB_DataObject_Cast::sql('NULL'); + $user->email = $user->sqlValue('NULL'); // Throws exception on failure. Also performs it within a transaction. $user->updateWithKeys($original); @@ -458,7 +458,7 @@ class EmailsettingsAction extends SettingsAction } $orig = clone($user); - $user->incomingemail = DB_DataObject_Cast::sql('NULL'); + $user->incomingemail = $user->sqlValue('NULL'); $user->emailpost = false; // Throws exception on failure. Also performs it within a transaction. $user->updateWithKeys($orig); diff --git a/actions/smssettings.php b/actions/smssettings.php index 1b1909f74b..ee081ea036 100644 --- a/actions/smssettings.php +++ b/actions/smssettings.php @@ -98,7 +98,7 @@ class SmssettingsAction extends SettingsAction $this->element('legend', null, _('SMS address')); $this->hidden('token', common_session_token()); - if ($user->sms) { + if (!$user->isNull('sms')) { $carrier = $user->getCarrier(); $this->element( 'p', @@ -170,13 +170,13 @@ class SmssettingsAction extends SettingsAction } $this->elementEnd('fieldset'); - if ($user->sms) { + if (!$user->isNull('sms')) { $this->elementStart('fieldset', ['id' => 'settings_sms_incoming_email']); // XXX: Confused! This is about SMS. Should this message be updated? // TRANS: Form legend for incoming SMS settings form. $this->element('legend', null, _('Incoming email')); - if ($user->incomingemail) { + if (!$user->isNull('incomingemail')) { $this->element('p', 'form_unconfirmed', $user->incomingemail); $this->element( 'p', @@ -417,9 +417,9 @@ class SmssettingsAction extends SettingsAction $original = clone($user); - $user->sms = DB_DataObject_Cast::sql('NULL'); - $user->carrier = DB_DataObject_Cast::sql('NULL'); - $user->smsemail = DB_DataObject_Cast::sql('NULL'); + $user->sms = $user->sqlValue('NULL'); + $user->carrier = $user->sqlValue('NULL'); + $user->smsemail = $user->sqlValue('NULL'); // Throws exception on failure. Also performs it within a transaction. $user->updateWithKeys($original); @@ -531,7 +531,7 @@ class SmssettingsAction extends SettingsAction $orig = clone($user); - $user->incomingemail = DB_DataObject_Cast::sql('NULL'); + $user->incomingemail = $user->sqlValue('NULL'); // Throws exception on failure. Also performs it within a transaction. $user->updateWithKeys($orig); diff --git a/classes/Conversation.php b/classes/Conversation.php index 4251ba2efa..eeecd34579 100644 --- a/classes/Conversation.php +++ b/classes/Conversation.php @@ -106,7 +106,7 @@ class Conversation extends Managed_DataObject common_random_hexstr(8) ); // locally generated Conversation objects don't get static URLs stored - $conv->url = DB_DataObject_Cast::sql('NULL'); + $conv->url = $conv->sqlValue('NULL'); } // This insert throws exceptions on failure $conv->insert(); diff --git a/classes/Managed_DataObject.php b/classes/Managed_DataObject.php index 5684d8080a..271205eebd 100644 --- a/classes/Managed_DataObject.php +++ b/classes/Managed_DataObject.php @@ -283,6 +283,8 @@ abstract class Managed_DataObject extends Memcached_DataObject * Memcached_DataObject doesn't have enough info to handle properly. * * @return array of strings + * @throws MethodNotImplementedException + * @throws ServerException */ public function _allCacheKeys() { @@ -440,6 +442,32 @@ abstract class Managed_DataObject extends Memcached_DataObject return intval($this->id); } + /** + * Check whether the column is NULL in SQL + * + * @param string $key column property name + * + * @return bool + */ + public function isNull(string $key): bool + { + if (array_key_exists($key, get_object_vars($this)) + && is_null($this->$key)) { + // If there was no fetch, this is a false positive. + return true; + } elseif (is_object($this->$key) + && $this->$key instanceof DB_DataObject_Cast + && $this->$key->type === 'sql') { + // This is cast to raw SQL, let's see if it's NULL. + return (strcasecmp($this->$key->value, 'NULL') == 0); + } elseif (DB_DataObject::_is_null($this, $key)) { + // DataObject's NULL magic should be disabled, + // this is just for completeness. + return true; + } + return false; + } + /** * WARNING: Only use this on Profile and Notice. We should probably do * this with traits/"implements" or whatever, but that's over the top @@ -516,10 +544,20 @@ abstract class Managed_DataObject extends Memcached_DataObject // do it in a transaction $this->query('BEGIN'); - $parts = array(); + $parts = []; foreach ($this->keys() as $k) { - if (strcmp($this->$k, $orig->$k) != 0) { - $parts[] = $k . ' = ' . $this->_quote($this->$k); + $v = $this->table()[$k]; + if ($this->$k !== $orig->$k) { + if (is_object($this->$k) && $this->$k instanceof DB_DataObject_Cast) { + $value = $this->$k->toString($v, $this->getDatabaseConnection()); + } elseif (DB_DataObject::_is_null($this, $k)) { + $value = 'NULL'; + } elseif ($v & DB_DATAOBJECT_STR) { // if a string + $value = $this->_quote((string) $this->$k); + } else { + $value = (int) $this->$k; + } + $parts[] = "{$k} = {$value}"; } } if (count($parts) == 0) { diff --git a/classes/Memcached_DataObject.php b/classes/Memcached_DataObject.php index 8833134120..2784701130 100644 --- a/classes/Memcached_DataObject.php +++ b/classes/Memcached_DataObject.php @@ -972,9 +972,15 @@ class Memcached_DataObject extends Safe_DataObject case 'date': $vstr = "{$v->year} - {$v->month} - {$v->day}"; break; + case 'sql': + if (strcasecmp($v->value, 'NULL') == 0) { + // Very selectively handling NULLs. + $vstr = ''; + break; + } + // fallthrough case 'blob': case 'string': - case 'sql': case 'datetime': case 'time': // Low level exception. No need for i18n as discussed with Brion. diff --git a/extlib/DB/DataObject.php b/extlib/DB/DataObject.php index 590b8615c4..fc63d25056 100644 --- a/extlib/DB/DataObject.php +++ b/extlib/DB/DataObject.php @@ -4168,8 +4168,7 @@ class DB_DataObject $method = $value; $value = func_get_arg(1); } - //require_once 'DB/DataObject/Cast.php'; - require_once 'Cast.php'; + require_once 'DB/DataObject/Cast.php'; return call_user_func(array('DB_DataObject_Cast', $method), $value); } diff --git a/lib/util/default.php b/lib/util/default.php index cfe27010fb..7df0f6172a 100644 --- a/lib/util/default.php +++ b/lib/util/default.php @@ -74,6 +74,7 @@ $default = 'mirror' => null, 'utf8' => true, 'db_driver' => 'DB', # XXX: JanRain libs only work with DB + 'disable_null_strings' => true, // 'NULL' can be harmful 'quote_identifiers' => true, 'type' => 'mysql', 'schemacheck' => 'runtime', // 'runtime' or 'script' diff --git a/modules/Favorite/FavoriteModule.php b/modules/Favorite/FavoriteModule.php index 073df30975..c72214f401 100644 --- a/modules/Favorite/FavoriteModule.php +++ b/modules/Favorite/FavoriteModule.php @@ -75,7 +75,7 @@ class FavoriteModule extends ActivityVerbHandlerModule while ($user->fetch()) { $user->setPref('email', 'notify_fave', $user->emailnotifyfav); $orig = clone($user); - $user->emailnotifyfav = 'null'; // flag this preference as migrated + $user->emailnotifyfav = $user->sqlValue('NULL'); // flag this preference as migrated $user->update($orig); } printfnq("DONE.\n"); diff --git a/plugins/OStatus/classes/FeedSub.php b/plugins/OStatus/classes/FeedSub.php index c2a88eabf4..c9c1d925f0 100644 --- a/plugins/OStatus/classes/FeedSub.php +++ b/plugins/OStatus/classes/FeedSub.php @@ -480,7 +480,7 @@ class FeedSub extends Managed_DataObject $this->sub_end = common_sql_date(time() + $lease_seconds); } else { // Backwards compatibility to StatusNet (PuSH <0.4 supported permanent subs) - $this->sub_end = DB_DataObject_Cast::sql('NULL'); + $this->sub_end = $this->sqlValue('NULL'); } $this->modified = common_sql_now(); @@ -496,10 +496,10 @@ class FeedSub extends Managed_DataObject { $original = clone($this); - $this->secret = DB_DataObject_Cast::sql('NULL'); + $this->secret = $this->sqlValue('NULL'); $this->sub_state = 'inactive'; - $this->sub_start = DB_DataObject_Cast::sql('NULL'); - $this->sub_end = DB_DataObject_Cast::sql('NULL'); + $this->sub_start = $this->sqlValue('NULL'); + $this->sub_end = $this->sqlValue('NULL'); $this->modified = common_sql_now(); return $this->update($original); diff --git a/scripts/upgrade.php b/scripts/upgrade.php index 1178d59769..734057accc 100755 --- a/scripts/upgrade.php +++ b/scripts/upgrade.php @@ -62,6 +62,7 @@ function main() fixupNoticeConversation(); initConversation(); + fixupUserBadNulls(); fixupGroupURI(); if ($iterate_files) { printfnq("Running file iterations:\n"); @@ -121,6 +122,26 @@ function updateSchemaPlugins() printfnq("DONE.\n"); } +function fixupUserBadNulls(): void +{ + printfnq("Ensuring all users have no empty strings for NULLs..."); + + foreach (['email', 'incomingemail', 'sms', 'smsemail'] as $col) { + $user = new User(); + $user->whereAdd("{$col} = ''"); + + if ($user->find()) { + while ($user->fetch()) { + $sql = "UPDATE {$user->escapedTableName()} SET {$col} = NULL " + . "WHERE id = {$user->id}"; + $user->query($sql); + } + } + } + + printfnq("DONE.\n"); +} + function fixupNoticeConversation() { printfnq("Ensuring all notices have a conversation ID...");