diff --git a/actions/apifriendshipscreate.php b/actions/apifriendshipscreate.php index e7caf96869..3997a8b51c 100644 --- a/actions/apifriendshipscreate.php +++ b/actions/apifriendshipscreate.php @@ -100,8 +100,8 @@ class ApiFriendshipsCreateAction extends ApiAuthAction try { Subscription::start($this->scoped, $this->other); - } catch (Exception $e) { - $this->clientError($e->getMessage(), 403); + } catch (AlreadyFulfilledException $e) { + $this->clientError($e->getMessage(), 409); } $this->initDocument($this->format); diff --git a/actions/atompubsubscriptionfeed.php b/actions/atompubsubscriptionfeed.php index 413277a5d3..6a483b95c0 100644 --- a/actions/atompubsubscriptionfeed.php +++ b/actions/atompubsubscriptionfeed.php @@ -230,18 +230,11 @@ class AtompubsubscriptionfeedAction extends AtompubAction $this->clientError(sprintf(_('Unknown profile %s.'), $person->id)); } - if (Subscription::exists($this->_profile, $profile)) { + try { + $sub = Subscription::start($this->_profile, $profile); + } catch (AlreadyFulfilledException $e) { // 409 Conflict - // TRANS: Client error displayed trying to subscribe to an already subscribed profile. - // TRANS: %s is the profile the user already has a subscription on. - $this->clientError(sprintf(_('Already subscribed to %s.'), - $person->id), - 409); - } - - if (Subscription::start($this->_profile, $profile)) { - $sub = Subscription::pkeyGet(array('subscriber' => $this->_profile->id, - 'subscribed' => $profile->id)); + $this->clientError($e->getMessage(), 409); } Event::handle('EndAtomPubNewActivity', array($activity, $sub)); diff --git a/actions/invite.php b/actions/invite.php index f99dd4d783..89b7e83bf6 100644 --- a/actions/invite.php +++ b/actions/invite.php @@ -118,7 +118,7 @@ class InviteAction extends Action $this->already[] = $other; } else { try { - Subscription::start($profile, $other); + Subscription::ensureStart($profile, $other); $this->subbed[] = $other; } catch (Exception $e) { // subscription failed, but keep working diff --git a/actions/subscribe.php b/actions/subscribe.php index 4002c9fbb0..320409afa0 100644 --- a/actions/subscribe.php +++ b/actions/subscribe.php @@ -122,7 +122,7 @@ class SubscribeAction extends Action { // Throws exception on error - $sub = Subscription::start($this->user->getProfile(), + $sub = Subscription::ensureStart($this->user->getProfile(), $this->other); if ($this->boolean('ajax')) { diff --git a/classes/Subscription.php b/classes/Subscription.php index 5c5101ad1d..ec9ae51841 100644 --- a/classes/Subscription.php +++ b/classes/Subscription.php @@ -94,6 +94,7 @@ class Subscription extends Managed_DataObject if (Event::handle('StartSubscribe', array($subscriber, $other))) { $otherUser = User::getKV('id', $other->id); if ($otherUser instanceof User && $otherUser->subscribe_policy == User::SUBSCRIBE_POLICY_MODERATE && !$force) { + // Will throw an AlreadyFulfilledException if this queue item already exists. $sub = Subscription_queue::saveNew($subscriber, $other); $sub->notify(); } else { @@ -132,6 +133,17 @@ class Subscription extends Managed_DataObject return $sub; } + static function ensureStart(Profile $subscriber, Profile $other, $force=false) + { + try { + $sub = self::start($subscriber, $other, $force); + } catch (AlreadyFulfilledException $e) { + // Nothing to see here, move along... + return self::getSubscription($subscriber, $other); + } + return $sub; + } + /** * Low-level subscription save. * Outside callers should use Subscription::start() diff --git a/classes/Subscription_queue.php b/classes/Subscription_queue.php index 405eca93fd..02cc72f1f2 100644 --- a/classes/Subscription_queue.php +++ b/classes/Subscription_queue.php @@ -36,6 +36,9 @@ class Subscription_queue extends Managed_DataObject public static function saveNew(Profile $subscriber, Profile $subscribed) { + if (self::exists($subscriber, $subscribed)) { + throw new AlreadyFulfilledException(_('This subscription request is already in progress.')); + } $rq = new Subscription_queue(); $rq->subscriber = $subscriber->id; $rq->subscribed = $subscribed->id; diff --git a/classes/User.php b/classes/User.php index 2f4670a2df..f543a75528 100644 --- a/classes/User.php +++ b/classes/User.php @@ -354,7 +354,7 @@ class User extends Managed_DataObject common_log(LOG_WARNING, sprintf("Default user %s does not exist.", $defnick), __FILE__); } else { - Subscription::start($profile, $defuser->getProfile()); + Subscription::ensureStart($profile, $defuser->getProfile()); } } diff --git a/lib/activityimporter.php b/lib/activityimporter.php index 5bef4cfb07..c4dd797e6d 100644 --- a/lib/activityimporter.php +++ b/lib/activityimporter.php @@ -109,7 +109,7 @@ class ActivityImporter extends QueueHandler // XXX: don't do this for untrusted input! - Subscription::start($otherProfile, $profile); + Subscription::ensureStart($otherProfile, $profile); } else if (empty($activity->actor) || $activity->actor->id == $author->id) { @@ -123,7 +123,7 @@ class ActivityImporter extends QueueHandler throw new ClientException(_('Unknown profile.')); } - Subscription::start($profile, $otherProfile); + Subscription::ensureStart($profile, $otherProfile); } else { // TRANS: Client exception thrown when trying to import an event not related to the importing user. throw new Exception(_('This activity seems unrelated to our user.')); diff --git a/lib/activitymover.php b/lib/activitymover.php index fe33e9081e..ac828d9491 100644 --- a/lib/activitymover.php +++ b/lib/activitymover.php @@ -146,7 +146,7 @@ class ActivityMover extends QueueHandler "Changing sub to {$act->objects[0]->id}". "by {$act->actor->id} to {$remote->nickname}."); $otherProfile = $otherUser->getProfile(); - Subscription::start($otherProfile, $remote); + Subscription::ensureStart($otherProfile, $remote); Subscription::cancel($otherProfile, $user->getProfile()); } else { $this->log(LOG_NOTICE,