Rework contact termination to separate protocol-level and contact-level

- Mail and Feed contacts are now removed automatically on relationship termination
- Added logging and notice messages for different results

doc/Addons.md

@@ -494,7 +494,8 @@ Called when unfollowing a remote contact on a non-native network (like Twitter)
 
 Hook data:
 - **contact** (input): the remote contact (uid = local unfollowing user id) array.
-- **dissolve** (input): whether to stop sharing with the remote contact as well.
+- **two_way** (input): wether to stop sharing with the remote contact as well.
+- **result** (output): wether the unfollowing is successful or not.
 
 ## Complete list of hook callbacks
 

include/api.php

@@ -3778,11 +3778,11 @@ api_register_func('api/direct_messages/destroy', 'api_direct_messages_destroy',
  *
  * @param string $type Known types are 'atom', 'rss', 'xml' and 'json'
  * @return string|array
- * @throws BadRequestException
- * @throws ForbiddenException
- * @throws ImagickException
- * @throws InternalServerErrorException
- * @throws NotFoundException
+ * @throws HTTPException\BadRequestException
+ * @throws HTTPException\ExpectationFailedException
+ * @throws HTTPException\ForbiddenException
+ * @throws HTTPException\InternalServerErrorException
+ * @throws HTTPException\NotFoundException
  * @see   https://developer.twitter.com/en/docs/accounts-and-users/follow-search-get-users/api-reference/post-friendships-destroy.html
  */
 function api_friendships_destroy($type)
@@ -3790,25 +3790,31 @@ function api_friendships_destroy($type)
 	$uid = api_user();
 
 	if ($uid === false) {
-		throw new ForbiddenException();
+		throw new HTTPException\ForbiddenException();
+	}
+
+	$owner = User::getOwnerDataById($uid);
+	if (!$owner) {
+		Logger::notice(API_LOG_PREFIX . 'No owner {uid} found', ['module' => 'api', 'action' => 'friendships_destroy', 'uid' => $uid]);
+		throw new HTTPException\NotFoundException('Error Processing Request');
 	}
 
 	$contact_id = $_REQUEST['user_id'] ?? 0;
 
 	if (empty($contact_id)) {
 		Logger::notice(API_LOG_PREFIX . 'No user_id specified', ['module' => 'api', 'action' => 'friendships_destroy']);
-		throw new BadRequestException("no user_id specified");
+		throw new HTTPException\BadRequestException('no user_id specified');
 	}
 
 	// Get Contact by given id
 	$contact = DBA::selectFirst('contact', ['url'], ['id' => $contact_id, 'uid' => 0, 'self' => false]);
 
 	if(!DBA::isResult($contact)) {
-		Logger::notice(API_LOG_PREFIX . 'No contact found for ID {contact}', ['module' => 'api', 'action' => 'friendships_destroy', 'contact' => $contact_id]);
-		throw new NotFoundException("no contact found to given ID");
+		Logger::notice(API_LOG_PREFIX . 'No public contact found for ID {contact}', ['module' => 'api', 'action' => 'friendships_destroy', 'contact' => $contact_id]);
+		throw new HTTPException\NotFoundException('no contact found to given ID');
 	}
 
-	$url = $contact["url"];
+	$url = $contact['url'];
 
 	$condition = ["`uid` = ? AND (`rel` = ? OR `rel` = ?) AND (`nurl` = ? OR `alias` = ? OR `alias` = ?)",
 			$uid, Contact::SHARING, Contact::FRIEND, Strings::normaliseLink($url),
@@ -3817,40 +3823,35 @@ function api_friendships_destroy($type)
 
 	if (!DBA::isResult($contact)) {
 		Logger::notice(API_LOG_PREFIX . 'Not following contact', ['module' => 'api', 'action' => 'friendships_destroy']);
-		throw new NotFoundException("Not following Contact");
-	}
-
-	if (!in_array($contact['network'], Protocol::NATIVE_SUPPORT)) {
-		Logger::notice(API_LOG_PREFIX . 'Not supported for {network}', ['module' => 'api', 'action' => 'friendships_destroy', 'network' => $contact['network']]);
-		throw new ExpectationFailedException("Not supported");
+		throw new HTTPException\NotFoundException('Not following Contact');
 	}
 
 	$dissolve = ($contact['rel'] == Contact::SHARING);
 
-	$owner = User::getOwnerDataById($uid);
-	if ($owner) {
-		Contact::terminateFriendship($owner, $contact, $dissolve);
-	}
-	else {
-		Logger::notice(API_LOG_PREFIX . 'No owner {uid} found', ['module' => 'api', 'action' => 'friendships_destroy', 'uid' => $uid]);
-		throw new NotFoundException("Error Processing Request");
+	try {
+		$result = Contact::terminateFriendship($owner, $contact, $dissolve);
+
+		if ($result === null) {
+			Logger::notice(API_LOG_PREFIX . 'Not supported for {network}', ['module' => 'api', 'action' => 'friendships_destroy', 'network' => $contact['network']]);
+			throw new HTTPException\ExpectationFailedException('Unfollowing is currently not supported by this contact\'s network.');
 		}
 
-	// Sharing-only contacts get deleted as there no relationship any more
-	if ($dissolve) {
-		Contact::remove($contact['id']);
-	} else {
-		Contact::update(['rel' => Contact::FOLLOWER], ['id' => $contact['id']]);
+		if ($result === false) {
+			throw new HTTPException\ServiceUnavailableException('Unable to unfollow this contact, please retry in a few minutes or contact your administrator.');
+		}
+	} catch (Exception $e) {
+		Logger::error(API_LOG_PREFIX . $e->getMessage(), ['owner' => $owner, 'contact' => $contact, 'dissolve' => $dissolve]);
+		throw new HTTPException\InternalServerErrorException('Unable to unfollow this contact, please contact your administrator');
 	}
 
 	// "uid" and "self" are only needed for some internal stuff, so remove it from here
-	unset($contact["uid"]);
-	unset($contact["self"]);
+	unset($contact['uid']);
+	unset($contact['self']);
 
 	// Set screen_name since Twidere requests it
-	$contact["screen_name"] = $contact["nick"];
+	$contact['screen_name'] = $contact['nick'];
 
-	return api_format_data("friendships-destroy", $type, ['user' => $contact]);
+	return api_format_data('friendships-destroy', $type, ['user' => $contact]);
 }
 api_register_func('api/friendships/destroy', 'api_friendships_destroy', true, API_METHOD_POST);
 

mod/unfollow.php

@@ -120,6 +120,12 @@ function unfollow_process(string $url)
 
 	$uid = local_user();
 
+	$owner = User::getOwnerDataById($uid);
+	if (!$owner) {
+		\Friendica\Module\Security\Logout::init();
+		// NOTREACHED
+	}
+
 	$condition = ["`uid` = ? AND (`rel` = ? OR `rel` = ?) AND (`nurl` = ? OR `alias` = ? OR `alias` = ?)",
 		$uid, Contact::SHARING, Contact::FRIEND, Strings::normaliseLink($url),
 		Strings::normaliseLink($url), $url];
@@ -131,27 +137,30 @@ function unfollow_process(string $url)
 		// NOTREACHED
 	}
 
-	if (!in_array($contact['network'], Protocol::NATIVE_SUPPORT)) {
-		notice(DI::l10n()->t('Unfollowing is currently not supported by your network.'));
-		DI::baseUrl()->redirect($base_return_path . '/' . $contact['id']);
-		// NOTREACHED
-	}
-
 	$dissolve = ($contact['rel'] == Contact::SHARING);
 
-	$owner = User::getOwnerDataById($uid);
-	if ($owner) {
-		Contact::terminateFriendship($owner, $contact, $dissolve);
-	}
-
-	// Sharing-only contacts get deleted as there no relationship anymore
-	if ($dissolve) {
-		Contact::remove($contact['id']);
-		$return_path = $base_return_path;
-	} else {
-		Contact::update(['rel' => Contact::FOLLOWER], ['id' => $contact['id']]);
+	$notice_message = '';
 	$return_path = $base_return_path . '/' . $contact['id'];
+
+	try {
+		$result = Contact::terminateFriendship($owner, $contact, $dissolve);
+
+		if ($result === null) {
+			$notice_message = DI::l10n()->t('Unfollowing is currently not supported by this contact\'s network.');
 		}
 
+		if ($result === false) {
+			$notice_message = DI::l10n()->t('Unable to unfollow this contact, please retry in a few minutes or contact your administrator.');
+		}
+
+		if ($result === true) {
+			$notice_message = DI::l10n()->t('Contact was successfully unfollowed');
+		}
+	} catch (Exception $e) {
+		DI::logger()->error($e->getMessage(), ['owner' => $owner, 'contact' => $contact, 'dissolve' => $dissolve]);
+		$notice_message = DI::l10n()->t('Unable to unfollow this contact, please contact your administrator');
+	}
+
+	notice($notice_message);
 	DI::baseUrl()->redirect($return_path);
 }

src/Console/Contact.php

@@ -23,6 +23,7 @@ namespace Friendica\Console;
 
 use Console_Table;
 use Friendica\App;
+use Friendica\DI;
 use Friendica\Model\Contact as ContactModel;
 use Friendica\Model\User as UserModel;
 use Friendica\Network\Probe;
@@ -177,11 +178,12 @@ HELP;
 	}
 
 	/**
-	 * Sends an unfriend message.  Does not remove the contact
+	 * Sends an unfriend message.
 	 *
 	 * @return bool True, if the command was successful
+	 * @throws \Exception
 	 */
-	private function terminateContact()
+	private function terminateContact(): bool
 	{
 		$cid = $this->getArgument(1);
 		if (empty($cid)) {
@@ -199,7 +201,23 @@ HELP;
 
 		$user = UserModel::getById($contact['uid']);
 
+		try {
 			$result = ContactModel::terminateFriendship($user, $contact);
+			if ($result === null) {
+				throw new RuntimeException('Unfollowing is currently not supported by this contact\'s network.');
+			}
+
+			if ($result === false) {
+				throw new RuntimeException('Unable to unfollow this contact, please retry in a few minutes or check the logs.');
+			}
+
+			$this->out('Contact was successfully unfollowed');
+
+			return true;
+		} catch (\Exception $e) {
+			DI::logger()->error($e->getMessage(), ['owner' => $user, 'contact' => $contact]);
+			throw new RuntimeException('Unable to unfollow this contact, please check the log');
+		}
 	}
 
 	/**

src/Core/Protocol.php

@@ -22,6 +22,12 @@
 namespace Friendica\Core;
 
 use Friendica\DI;
+use Friendica\Network\HTTPException;
+use Friendica\Protocol\Activity;
+use Friendica\Protocol\ActivityPub;
+use Friendica\Protocol\Diaspora;
+use Friendica\Protocol\OStatus;
+use Friendica\Protocol\Salmon;
 
 /**
  * Manage compatibility with federated networks
@@ -157,4 +163,63 @@ class Protocol
 	{
 		return $display_name . ' (' . self::getAddrFromProfileUrl($profile_url) . ')';
 	}
+
+	/**
+	 * Sends an unfriend message. Does not remove the contact
+	 *
+	 * @param array   $user    User unfriending
+	 * @param array   $contact Contact unfriended
+	 * @param boolean $two_way Revoke eventual inbound follow as well
+	 * @return bool|null true if successful, false if not, null if no action was performed
+	 * @throws HTTPException\InternalServerErrorException
+	 * @throws \ImagickException
+	 */
+	public static function terminateFriendship(array $user, array $contact, bool $two_way = false): bool
+	{
+		if (empty($contact['network'])) {
+			throw new \InvalidArgumentException('Missing network key in contact array');
+		}
+
+		$protocol = $contact['network'];
+		if (($protocol == Protocol::DFRN) && !empty($contact['protocol'])) {
+			$protocol = $contact['protocol'];
+		}
+
+		if (in_array($protocol, [Protocol::OSTATUS, Protocol::DFRN])) {
+			// create an unfollow slap
+			$item = [];
+			$item['verb'] = Activity::O_UNFOLLOW;
+			$item['gravity'] = GRAVITY_ACTIVITY;
+			$item['follow'] = $contact['url'];
+			$item['body'] = '';
+			$item['title'] = '';
+			$item['guid'] = '';
+			$item['uri-id'] = 0;
+			$slap = OStatus::salmon($item, $user);
+
+			if (empty($contact['notify'])) {
+				throw new \InvalidArgumentException('Missing expected "notify" key in OStatus/DFRN contact');
+			}
+
+			return Salmon::slapper($user, $contact['notify'], $slap) === 0;
+		} elseif ($protocol == Protocol::DIASPORA) {
+			return Diaspora::sendUnshare($user, $contact) > 0;
+		} elseif ($protocol == Protocol::ACTIVITYPUB) {
+			if ($two_way) {
+				ActivityPub\Transmitter::sendContactReject($contact['url'], $contact['hub-verify'], $user['uid']);
+			}
+
+			return ActivityPub\Transmitter::sendContactUndo($contact['url'], $contact['id'], $user['uid']);
+		}
+
+		// Catch-all addon hook
+		$hook_data = [
+			'contact' => $contact,
+			'two_way' => $two_way,
+			'result' => null
+		];
+		Hook::callAll('unfollow', $hook_data);
+
+		return $hook_data['result'];
+	}
 }

src/Model/Contact.php

@@ -809,7 +809,6 @@ class Contact
 	 * Marks a contact for removal
 	 *
 	 * @param int $id contact id
-	 * @return null
 	 * @throws HTTPException\InternalServerErrorException
 	 */
 	public static function remove($id)
@@ -828,56 +827,26 @@ class Contact
 	}
 
 	/**
-	 * Sends an unfriend message. Does not remove the contact
+	 * Sends an unfriend message. Removes the contact for two-way unfriending or sharing only protocols (feed an mail)
 	 *
 	 * @param array   $user    User unfriending
 	 * @param array   $contact Contact unfriended
-	 * @param boolean $dissolve Remove the contact on the remote side
-	 * @return void
+	 * @param boolean $two_way Revoke eventual inbound follow as well
+	 * @return bool|null true if successful, false if not, null if no action was performed
 	 * @throws HTTPException\InternalServerErrorException
 	 * @throws \ImagickException
 	 */
-	public static function terminateFriendship(array $user, array $contact, $dissolve = false)
+	public static function terminateFriendship(array $user, array $contact, bool $two_way = false): bool
 	{
-		if (empty($contact['network'])) {
-			return;
-		}
+		$result = Protocol::terminateFriendship($user, $contact, $two_way);
 
-		$protocol = $contact['network'];
-		if (($protocol == Protocol::DFRN) && !empty($contact['protocol'])) {
-			$protocol = $contact['protocol'];
-		}
-
-		if (in_array($protocol, [Protocol::OSTATUS, Protocol::DFRN])) {
-			// create an unfollow slap
-			$item = [];
-			$item['verb'] = Activity::O_UNFOLLOW;
-			$item['gravity'] = GRAVITY_ACTIVITY;
-			$item['follow'] = $contact["url"];
-			$item['body'] = '';
-			$item['title'] = '';
-			$item['guid'] = '';
-			$item['uri-id'] = 0;
-			$slap = OStatus::salmon($item, $user);
-
-			if (!empty($contact['notify'])) {
-				Salmon::slapper($user, $contact['notify'], $slap);
-			}
-		} elseif ($protocol == Protocol::DIASPORA) {
-			Diaspora::sendUnshare($user, $contact);
-		} elseif ($protocol == Protocol::ACTIVITYPUB) {
-			ActivityPub\Transmitter::sendContactUndo($contact['url'], $contact['id'], $user['uid']);
-
-			if ($dissolve) {
-				ActivityPub\Transmitter::sendContactReject($contact['url'], $contact['hub-verify'], $user['uid']);
-			}
+		if ($two_way || in_array($contact['network'], [Protocol::FEED, Protocol::MAIL])) {
+			self::remove($contact['id']);
 		} else {
-			$hook_data = [
-				'contact' => $contact,
-				'dissolve' => $dissolve,
-			];
-			Hook::callAll('unfollow', $hook_data);
+			self::update(['rel' => Contact::FOLLOWER], ['id' => $contact['id']]);
 		}
+
+		return $result;
 	}
 
 	/**

src/Worker/Notifier.php

@@ -703,7 +703,7 @@ class Notifier
 		}
 
 		while($contact = DBA::fetch($contacts_stmt)) {
-			Contact::terminateFriendship($owner, $contact, true);
+			Protocol::terminateFriendship($owner, $contact, true);
 		}
 		DBA::close($contacts_stmt);