From cac9d23498808a60dfb890aef437606e98106a6d Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 18 Mar 2010 14:26:32 -0700 Subject: [PATCH] Fix for xmpp/sms notification options appearing to be disabled on new subscriptions. Base problem is that our caching-on-insert interferes with relying on column default values; the cached object is missing those fields, so they appear to be empty (null) when the object is retrieved from cache. Now explicitly setting them when inserting subscriptions, and cleaned up some code that had alternate code paths. May also have made auto-subscription work for remote OStatus subscribers, but can't test until magic sigs are working again. --- classes/Subscription.php | 56 +++++++++++---------- classes/User.php | 27 ++-------- plugins/OStatus/actions/ostatussub.php | 2 +- plugins/OStatus/classes/Ostatus_profile.php | 46 ----------------- 4 files changed, 35 insertions(+), 96 deletions(-) diff --git a/classes/Subscription.php b/classes/Subscription.php index 5ac95f9222..97c44a2e46 100644 --- a/classes/Subscription.php +++ b/classes/Subscription.php @@ -75,20 +75,7 @@ class Subscription extends Memcached_DataObject } if (Event::handle('StartSubscribe', array($subscriber, $other))) { - - $sub = new Subscription(); - - $sub->subscriber = $subscriber->id; - $sub->subscribed = $other->id; - $sub->created = common_sql_now(); - - $result = $sub->insert(); - - if (!$result) { - common_log_db_error($sub, 'INSERT', __FILE__); - throw new Exception(_('Could not save subscription.')); - } - + $sub = self::saveNew($subscriber->id, $other->id); $sub->notify(); self::blow('user:notices_with_friends:%d', $subscriber->id); @@ -103,20 +90,11 @@ class Subscription extends Memcached_DataObject !self::exists($other, $subscriber) && !$subscriber->hasBlocked($other)) { - $auto = new Subscription(); - - $auto->subscriber = $other->id; - $auto->subscribed = $subscriber->id; - $auto->created = common_sql_now(); - - $result = $auto->insert(); - - if (!$result) { - common_log_db_error($auto, 'INSERT', __FILE__); - throw new Exception(_('Could not save subscription.')); + try { + self::start($other, $subscriber); + } catch (Exception $e) { + common_log(LOG_ERR, "Exception during autosubscribe of {$other->nickname} to profile {$subscriber->id}: {$e->getMessage()}"); } - - $auto->notify(); } Event::handle('EndSubscribe', array($subscriber, $other)); @@ -125,6 +103,30 @@ class Subscription extends Memcached_DataObject return true; } + /** + * Low-level subscription save. + * Outside callers should use Subscription::start() + */ + protected function saveNew($subscriber_id, $other_id) + { + $sub = new Subscription(); + + $sub->subscriber = $subscriber_id; + $sub->subscribed = $other_id; + $sub->jabber = 1; + $sub->sms = 1; + $sub->created = common_sql_now(); + + $result = $sub->insert(); + + if (!$result) { + common_log_db_error($sub, 'INSERT', __FILE__); + throw new Exception(_('Could not save subscription.')); + } + + return $sub; + } + function notify() { # XXX: add other notifications (Jabber, SMS) here diff --git a/classes/User.php b/classes/User.php index 16c256535b..8ad2ec63d5 100644 --- a/classes/User.php +++ b/classes/User.php @@ -153,19 +153,12 @@ class User extends Memcached_DataObject return Sms_carrier::staticGet('id', $this->carrier); } + /** + * @deprecated use Subscription::start($sub, $other); + */ function subscribeTo($other) { - $sub = new Subscription(); - $sub->subscriber = $this->id; - $sub->subscribed = $other->id; - - $sub->created = common_sql_now(); // current time - - if (!$sub->insert()) { - return false; - } - - return true; + return Subscription::start($this->getProfile(), $other); } function hasBlocked($other) @@ -346,17 +339,7 @@ class User extends Memcached_DataObject common_log(LOG_WARNING, sprintf("Default user %s does not exist.", $defnick), __FILE__); } else { - $defsub = new Subscription(); - $defsub->subscriber = $user->id; - $defsub->subscribed = $defuser->id; - $defsub->created = $user->created; - - $result = $defsub->insert(); - - if (!$result) { - common_log_db_error($defsub, 'INSERT', __FILE__); - return false; - } + Subscription::start($user, $defuser); } } diff --git a/plugins/OStatus/actions/ostatussub.php b/plugins/OStatus/actions/ostatussub.php index 07081c2c6f..994af6e95c 100644 --- a/plugins/OStatus/actions/ostatussub.php +++ b/plugins/OStatus/actions/ostatussub.php @@ -299,7 +299,7 @@ class OStatusSubAction extends Action if ($user->isSubscribed($local)) { // TRANS: OStatus remote subscription dialog error. $this->showForm(_m('Already subscribed!')); - } elseif ($this->oprofile->subscribeLocalToRemote($user)) { + } elseif (Subscription::start($user, $local)) { $this->success(); } else { // TRANS: OStatus remote subscription dialog error. diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php index f8fda4162a..90a8d0ef47 100644 --- a/plugins/OStatus/classes/Ostatus_profile.php +++ b/plugins/OStatus/classes/Ostatus_profile.php @@ -194,52 +194,6 @@ class Ostatus_profile extends Memcached_DataObject } } - /** - * Subscribe a local user to this remote user. - * PuSH subscription will be started if necessary, and we'll - * send a Salmon notification to the remote server if available - * notifying them of the sub. - * - * @param User $user - * @return boolean success - * @throws FeedException - */ - public function subscribeLocalToRemote(User $user) - { - if ($this->isGroup()) { - throw new ServerException("Can't subscribe to a remote group"); - } - - if ($this->subscribe()) { - if ($user->subscribeTo($this->localProfile())) { - $this->notify($user->getProfile(), ActivityVerb::FOLLOW, $this); - return true; - } - } - return false; - } - - /** - * Mark this remote profile as subscribing to the given local user, - * and send appropriate notifications to the user. - * - * This will generally be in response to a subscription notification - * from a foreign site to our local Salmon response channel. - * - * @param User $user - * @return boolean success - */ - public function subscribeRemoteToLocal(User $user) - { - if ($this->isGroup()) { - throw new ServerException("Remote groups can't subscribe to local users"); - } - - Subscription::start($this->localProfile(), $user->getProfile()); - - return true; - } - /** * Send a subscription request to the hub for this feed. * The hub will later send us a confirmation POST to /main/push/callback.