Merge pull request #11849 from annando/contact-id

Improved contact-id detection

src/Model/Contact.php

@@ -329,12 +329,13 @@ class Contact
 	 *
 	 * @param int  $cid    Either public contact id or user's contact id
 	 * @param int  $uid    User ID
+	 * @param bool $strict If "true" then contact mustn't be set to pending or readonly
 	 *
 	 * @return boolean is the contact id a follower?
 	 * @throws HTTPException\InternalServerErrorException
 	 * @throws \ImagickException
 	 */
-	public static function isFollower(int $cid, int $uid): bool
+	public static function isFollower(int $cid, int $uid, bool $strict = false): bool
 	{
 		if (Contact\User::isBlocked($cid, $uid)) {
 			return false;
@@ -346,6 +347,9 @@ class Contact
 		}
 
 		$condition = ['id' => $cdata['user'], 'rel' => [self::FOLLOWER, self::FRIEND]];
+		if ($strict) {
+			$condition = array_merge($condition, ['pending' => false, 'readonly' => false, 'blocked' => false]);
+		}
 		return DBA::exists('contact', $condition);
 	}
 
@@ -354,12 +358,13 @@ class Contact
 	 *
 	 * @param string $url    Contact URL
 	 * @param int    $uid    User ID
+	 * @param bool   $strict If "true" then contact mustn't be set to pending or readonly
 	 *
 	 * @return boolean is the contact id a follower?
 	 * @throws HTTPException\InternalServerErrorException
 	 * @throws \ImagickException
 	 */
-	public static function isFollowerByURL(string $url, int $uid): bool
+	public static function isFollowerByURL(string $url, int $uid, bool $strict = false): bool
 	{
 		$cid = self::getIdForURL($url, $uid);
 
@@ -367,7 +372,7 @@ class Contact
 			return false;
 		}
 
-		return self::isFollower($cid, $uid);
+		return self::isFollower($cid, $uid, $strict);
 	}
 
 	/**
@@ -375,12 +380,13 @@ class Contact
 	 *
 	 * @param int  $cid    Either public contact id or user's contact id
 	 * @param int  $uid    User ID
+	 * @param bool $strict If "true" then contact mustn't be set to pending or readonly
 	 *
 	 * @return boolean is the contact sharing with given user?
 	 * @throws HTTPException\InternalServerErrorException
 	 * @throws \ImagickException
 	 */
-	public static function isSharing(int $cid, int $uid): bool
+	public static function isSharing(int $cid, int $uid, bool $strict = false): bool
 	{
 		if (Contact\User::isBlocked($cid, $uid)) {
 			return false;
@@ -392,6 +398,9 @@ class Contact
 		}
 
 		$condition = ['id' => $cdata['user'], 'rel' => [self::SHARING, self::FRIEND]];
+		if ($strict) {
+			$condition = array_merge($condition, ['pending' => false, 'readonly' => false, 'blocked' => false]);
+		}
 		return DBA::exists('contact', $condition);
 	}
 
@@ -400,12 +409,13 @@ class Contact
 	 *
 	 * @param string $url    Contact URL
 	 * @param int    $uid    User ID
+	 * @param bool   $strict If "true" then contact mustn't be set to pending or readonly
 	 *
 	 * @return boolean is the contact url being followed?
 	 * @throws HTTPException\InternalServerErrorException
 	 * @throws \ImagickException
 	 */
-	public static function isSharingByURL(string $url, int $uid): bool
+	public static function isSharingByURL(string $url, int $uid, bool $strict = false): bool
 	{
 		$cid = self::getIdForURL($url, $uid);
 
@@ -413,7 +423,7 @@ class Contact
 			return false;
 		}
 
-		return self::isSharing($cid, $uid);
+		return self::isSharing($cid, $uid, $strict);
 	}
 
 	/**

src/Model/Item.php

@@ -448,21 +448,40 @@ class Item
 	 */
 	private static function contactId(array $item): int
 	{
-		if (!empty($item['contact-id']) && DBA::exists('contact', ['self' => true, 'id' => $item['contact-id']])) {
+		if ($item['uid'] == 0) {
-			return $item['contact-id'];
-		} elseif (($item['gravity'] == GRAVITY_PARENT) && !empty($item['uid']) && !empty($item['contact-id']) && Contact::isSharing($item['contact-id'], $item['uid'])) {
-			return $item['contact-id'];
-		} elseif (!empty($item['uid']) && !Contact::isSharing($item['author-id'], $item['uid'])) {
 			return $item['author-id'];
-		} elseif (!empty($item['contact-id'])) {
+		}
-			return $item['contact-id'];
+		if (!empty($item['causer-id']) && Contact::isSharing($item['causer-id'], $item['uid'], true)) {
+			$cdata = Contact::getPublicAndUserContactID($item['causer-id'], $item['uid']);
+			if (!empty($cdata['user'])) {
+				return $cdata['user'];
+			}
+		}
+
+		if ($item['gravity'] == GRAVITY_PARENT) {
+			if (Contact::isSharingByURL($item['owner-link'], $item['uid'], true)) {
+				$contact_id = Contact::getIdForURL($item['owner-link'], $item['uid']);
 			} else {
-			$contact_id = Contact::getIdForURL($item['author-link'], $item['uid']);
+				$contact_id = Contact::getIdForURL($item['owner-link']);
+			}
 			if (!empty($contact_id)) {
 				return $contact_id;
 			}
 		}
-		return $item['author-id'];
+
+		if (Contact::isSharingByURL($item['author-link'], $item['uid'], true)) {
+			$contact_id = Contact::getIdForURL($item['author-link'], $item['uid']);
+		} else {
+			$contact_id = Contact::getIdForURL($item['author-link']);
+		}
+
+		if (!empty($contact_id)) {
+			return $contact_id;
+		}
+
+		Logger::warning('contact-id could not be fetched, using self contact instead.', ['uid' => $item['uid'], 'item' => $item]);
+		$self = Contact::selectFirst(['id'], ['self' => true, 'uid' => $item['uid']]);
+		return $self['id'];
 	}
 
 	/**
@@ -911,8 +930,9 @@ class Item
 		Contact::checkAvatarCache($item['author-id']);
 		Contact::checkAvatarCache($item['owner-id']);
 
-		// The contact-id should be set before "self::insert" was called - but there seems to be issues sometimes
+		if (!Contact::isSharing($item['contact-id'], $item['uid'])) {
 			$item['contact-id'] = self::contactId($item);
+		}
 
 		if (!empty($item['direction']) && in_array($item['direction'], [Conversation::PUSH, Conversation::RELAY]) &&
 			empty($item['origin']) && self::isTooOld($item)) {
@@ -1660,25 +1680,7 @@ class Item
 		$item['origin'] = 0;
 		$item['wall'] = 0;
 
-		if ($item['gravity'] == GRAVITY_PARENT) {
+		$item['contact-id'] = self::contactId($item);
-			$contact = Contact::getByURLForUser($item['owner-link'], $uid, false, ['id']);
-		} else {
-			$contact = Contact::getByURLForUser($item['author-link'], $uid, false, ['id']);
-		}
-
-		if (!empty($contact['id'])) {
-			$item['contact-id'] = $contact['id'];
-		} else {
-			// Shouldn't happen at all
-			Logger::warning('contact-id could not be fetched', ['uid' => $uid, 'item' => $item]);
-			$self = DBA::selectFirst('contact', ['id'], ['self' => true, 'uid' => $uid]);
-			if (!DBA::isResult($self)) {
-				// Shouldn't happen even less
-				Logger::warning('self contact could not be fetched', ['uid' => $uid, 'item' => $item]);
-				return 0;
-			}
-			$item['contact-id'] = $self['id'];
-		}
 
 		$notify = false;
 		if ($item['gravity'] == GRAVITY_PARENT) {