Merge pull request #8690 from annando/no-activity

The "item-activity" is removed

include/api.php

@@ -43,6 +43,7 @@ use Friendica\Model\Notify;
 use Friendica\Model\Photo;
 use Friendica\Model\User;
 use Friendica\Model\UserItem;
+use Friendica\Model\Verb;
 use Friendica\Network\FKOAuth1;
 use Friendica\Network\HTTPException;
 use Friendica\Network\HTTPException\BadRequestException;
@@ -5102,8 +5103,7 @@ function api_get_announce($item)
 	}
 
 	$fields = ['author-id', 'author-name', 'author-link', 'author-avatar'];
-	$activity = Item::activityToIndex(Activity::ANNOUNCE);
+	$condition = ['parent-uri' => $item['uri'], 'gravity' => GRAVITY_ACTIVITY, 'uid' => [0, $item['uid']], 'vid' => Verb::getID(Activity::ANNOUNCE)];
-	$condition = ['parent-uri' => $item['uri'], 'gravity' => GRAVITY_ACTIVITY, 'uid' => [0, $item['uid']], 'activity' => $activity];
 	$announce = Item::selectFirstForUser($item['uid'], $fields, $condition, ['order' => ['received' => true]]);
 	if (!DBA::isResult($announce)) {
 		return [];

include/conversation.php

@@ -34,6 +34,7 @@ use Friendica\Model\Contact;
 use Friendica\Model\Item;
 use Friendica\Model\Profile;
 use Friendica\Model\Tag;
+use Friendica\Model\Verb;
 use Friendica\Object\Post;
 use Friendica\Object\Thread;
 use Friendica\Protocol\Activity;
@@ -769,11 +770,9 @@ function conversation_add_children(array $parents, $block_authors, $order, $uid)
 
 	$items = [];
 
-	$follow = Item::activityToIndex(Activity::FOLLOW);
-
 	foreach ($parents AS $parent) {
-		$condition = ["`item`.`parent-uri` = ? AND `item`.`uid` IN (0, ?) AND (`activity` != ? OR `activity` IS NULL)",
+		$condition = ["`item`.`parent-uri` = ? AND `item`.`uid` IN (0, ?) AND `vid` != ?",
-			$parent['uri'], $uid, $follow];
+			$parent['uri'], $uid, Verb::getID(Activity::FOLLOW)];
 		$items = conversation_fetch_items($parent, $items, $condition, $block_authors, $params);
 	}
 

mod/ping.php

@@ -30,6 +30,7 @@ use Friendica\Model\Contact;
 use Friendica\Model\Group;
 use Friendica\Model\Item;
 use Friendica\Model\Notify\Type;
+use Friendica\Model\Verb;
 use Friendica\Protocol\Activity;
 use Friendica\Util\DateTimeFormat;
 use Friendica\Util\Temporal;
@@ -135,8 +136,8 @@ function ping_init(App $a)
 
 		$notifs = ping_get_notifications(local_user());
 
-		$condition = ["`unseen` AND `uid` = ? AND `contact-id` != ? AND (`activity` != ? OR `activity` IS NULL)",
+		$condition = ["`unseen` AND `uid` = ? AND `contact-id` != ? AND `vid` != ?",
-			local_user(), local_user(), Item::activityToIndex(Activity::FOLLOW)];
+			local_user(), local_user(), Verb::getID(Activity::FOLLOW)];
 		$fields = ['id', 'parent', 'verb', 'author-name', 'unseen', 'author-link', 'author-avatar', 'contact-avatar',
 			'network', 'created', 'object', 'parent-author-name', 'parent-author-link', 'parent-guid', 'wall', 'activity'];
 		$params = ['order' => ['received' => true]];

src/Model/APContact.php

@@ -295,6 +295,8 @@ class APContact
 			$apcontact['gsid'] = GServer::getID($apcontact['baseurl']);
 		} elseif (!empty($fetched_contact['gsid'])) {
 			$apcontact['gsid'] = $fetched_contact['gsid'];
+		} else {
+			$apcontact['gsid'] = null;
 		}
 
 		if ($apcontact['url'] == $apcontact['alias']) {

src/Model/Item.php

@@ -95,7 +95,7 @@ class Item
 	// All fields in the item table
 	const ITEM_FIELDLIST = ['id', 'uid', 'parent', 'uri', 'parent-uri', 'thr-parent',
 			'guid', 'uri-id', 'parent-uri-id', 'thr-parent-id', 'vid',
-			'contact-id', 'type', 'wall', 'gravity', 'extid', 'icid', 'iaid', 'psid',
+			'contact-id', 'type', 'wall', 'gravity', 'extid', 'icid', 'psid',
 			'created', 'edited', 'commented', 'received', 'changed', 'verb',
 			'postopts', 'plink', 'resource-id', 'event-id', 'attach', 'inform',
 			'file', 'allow_cid', 'allow_gid', 'deny_cid', 'deny_gid', 'post-type',
@@ -106,8 +106,8 @@ class Item
 			'author-id', 'author-link', 'author-name', 'author-avatar', 'author-network',
 			'owner-id', 'owner-link', 'owner-name', 'owner-avatar'];
 
+	// List of all verbs that don't need additional content data.
 	// Never reorder or remove entries from this list. Just add new ones at the end, if needed.
-	// The item-activity table only stores the index and needs this array to know the matching activity.
 	const ACTIVITIES = [
 		Activity::LIKE, Activity::DISLIKE,
 		Activity::ATTEND, Activity::ATTENDNO, Activity::ATTENDMAYBE,
@@ -203,38 +203,6 @@ class Item
 		return self::selectThreadForUser($uid, $selected, $condition, $params);
 	}
 
-	/**
-	 * returns an activity index from an activity string
-	 *
-	 * @param string $activity activity string
-	 * @return integer Activity index
-	 */
-	public static function activityToIndex($activity)
-	{
-		$index = array_search($activity, self::ACTIVITIES);
-
-		if (is_bool($index)) {
-			$index = -1;
-		}
-
-		return $index;
-	}
-
-	/**
-	 * returns an activity string from an activity index
-	 *
-	 * @param integer $index activity index
-	 * @return string Activity string
-	 */
-	private static function indexToActivity($index)
-	{
-		if (is_null($index) || !array_key_exists($index, self::ACTIVITIES)) {
-			return '';
-		}
-
-		return self::ACTIVITIES[$index];
-	}
-
 	/**
 	 * Fetch a single item row
 	 *
@@ -294,8 +262,6 @@ class Item
 		if (array_key_exists('verb', $row)) {
 			if (!is_null($row['internal-verb'])) {
 				$row['verb'] = $row['internal-verb'];
-			} elseif (!is_null($row['internal-activity'])) {
-				$row['verb'] = self::indexToActivity($row['internal-activity']);
 			}
 
 			if (in_array($row['verb'], self::ACTIVITIES)) {
@@ -346,7 +312,6 @@ class Item
 		}
 
 		// Remove internal fields
-		unset($row['internal-activity']);
 		unset($row['internal-network']);
 		unset($row['internal-uri-id']);
 		unset($row['internal-uid']);
@@ -675,7 +640,7 @@ class Item
 			'resource-id', 'event-id', 'attach', 'post-type', 'file',
 			'private', 'pubmail', 'moderated', 'visible', 'starred', 'bookmark',
 			'unseen', 'deleted', 'origin', 'forum_mode', 'mention', 'global',
-			'id' => 'item_id', 'network', 'icid', 'iaid',
+			'id' => 'item_id', 'network', 'icid',
 			'uri-id' => 'internal-uri-id', 'uid' => 'internal-uid',
 			'network' => 'internal-network', 'psid' => 'internal-psid'];
 
@@ -683,8 +648,6 @@ class Item
 			$fields['user-item'] = ['pinned', 'notification-type', 'ignored' => 'internal-user-ignored'];
 		}
 
-		$fields['item-activity'] = ['activity', 'activity' => 'internal-activity'];
-
 		$fields['item-content'] = array_merge(self::CONTENT_FIELDLIST, self::MIXED_CONTENT_FIELDLIST);
 
 		$fields['post-delivery-data'] = array_merge(Post\DeliveryData::LEGACY_FIELD_LIST, Post\DeliveryData::FIELD_LIST);
@@ -803,10 +766,6 @@ class Item
 			$joins .= " LEFT JOIN `diaspora-interaction` ON `diaspora-interaction`.`uri-id` = `item`.`uri-id`";
 		}
 
-		if (strpos($sql_commands, "`item-activity`.") !== false) {
-			$joins .= " LEFT JOIN `item-activity` ON `item-activity`.`uri-id` = `item`.`uri-id`";
-		}
-
 		if (strpos($sql_commands, "`item-content`.") !== false) {
 			$joins .= " LEFT JOIN `item-content` ON `item-content`.`uri-id` = `item`.`uri-id`";
 		}
@@ -849,7 +808,7 @@ class Item
 		}
 
 		if (in_array('verb', $selected)) {
-			$selected = array_merge($selected, ['internal-activity', 'internal-verb']);
+			$selected = array_merge($selected, ['internal-verb']);
 		}
 
 		if (in_array('ignored', $selected)) {
@@ -928,7 +887,7 @@ class Item
 		// We cannot simply expand the condition to check for origin entries
 		// The condition needn't to be a simple array but could be a complex condition.
 		// And we have to execute this query before the update to ensure to fetch the same data.
-		$items = DBA::select('item', ['id', 'origin', 'uri', 'uri-id', 'iaid', 'icid', 'uid', 'file'], $condition);
+		$items = DBA::select('item', ['id', 'origin', 'uri', 'uri-id', 'icid', 'uid', 'file'], $condition);
 
 		$content_fields = [];
 		foreach (array_merge(self::CONTENT_FIELDLIST, self::MIXED_CONTENT_FIELDLIST) as $field) {
@@ -958,6 +917,10 @@ class Item
 			$files = null;
 		}
 
+		if (!empty($content_fields['verb'])) {
+			$fields['vid'] = Verb::getID($content_fields['verb']);
+		}
+
 		if (!empty($fields)) {
 			$success = DBA::update('item', $fields, $condition);
 
@@ -974,34 +937,7 @@ class Item
 		$notify_items = [];
 
 		while ($item = DBA::fetch($items)) {
-			if (!empty($item['iaid']) || (!empty($content_fields['verb']) && (self::activityToIndex($content_fields['verb']) >= 0))) {
+			if (empty($content_fields['verb']) || !in_array($content_fields['verb'], self::ACTIVITIES)) {
-				self::updateActivity($content_fields, ['uri-id' => $item['uri-id']]);
-
-				if (empty($item['iaid'])) {
-					$item_activity = DBA::selectFirst('item-activity', ['id'], ['uri-id' => $item['uri-id']]);
-					if (DBA::isResult($item_activity)) {
-						$item_fields = ['iaid' => $item_activity['id'], 'icid' => null];
-						foreach (self::MIXED_CONTENT_FIELDLIST as $field) {
-							if (self::isLegacyMode()) {
-								$item_fields[$field] = null;
-							} else {
-								unset($item_fields[$field]);
-							}
-						}
-						DBA::update('item', $item_fields, ['id' => $item['id']]);
-
-						if (!empty($item['icid']) && !DBA::exists('item', ['icid' => $item['icid']])) {
-							DBA::delete('item-content', ['id' => $item['icid']]);
-						}
-					}
-				} elseif (!empty($item['icid'])) {
-					DBA::update('item', ['icid' => null], ['id' => $item['id']]);
-
-					if (!DBA::exists('item', ['icid' => $item['icid']])) {
-						DBA::delete('item-content', ['id' => $item['icid']]);
-					}
-				}
-			} else {
 				self::updateContent($content_fields, ['uri-id' => $item['uri-id']]);
 
 				if (empty($item['icid'])) {
@@ -1009,12 +945,10 @@ class Item
 					if (DBA::isResult($item_content)) {
 						$item_fields = ['icid' => $item_content['id']];
 						// Clear all fields in the item table that have a content in the item-content table
-						foreach ($item_content as $field => $content) {
+						if (self::isLegacyMode()) {
-							if (in_array($field, self::MIXED_CONTENT_FIELDLIST) && !empty($item_content[$field])) {
+							foreach ($item_content as $field => $content) {
-								if (self::isLegacyMode()) {
+								if (in_array($field, self::MIXED_CONTENT_FIELDLIST) && !empty($content)) {
 									$item_fields[$field] = null;
-								} else {
-									unset($item_fields[$field]);
 								}
 							}
 						}
@@ -1113,7 +1047,7 @@ class Item
 		$fields = ['id', 'uri', 'uri-id', 'uid', 'parent', 'parent-uri', 'origin',
 			'deleted', 'file', 'resource-id', 'event-id', 'attach',
 			'verb', 'object-type', 'object', 'target', 'contact-id',
-			'icid', 'iaid', 'psid'];
+			'icid', 'psid'];
 		$item = self::selectFirst($fields, ['id' => $item_id]);
 		if (!DBA::isResult($item)) {
 			Logger::info('Item not found.', ['id' => $item_id]);
@@ -1192,8 +1126,6 @@ class Item
 
 		Post\DeliveryData::delete($item['uri-id']);
 
-		// We don't delete the item-activity here, since we need some of the data for ActivityPub
-
 		if (!empty($item['icid']) && !self::exists(['icid' => $item['icid'], 'deleted' => false])) {
 			DBA::delete('item-content', ['id' => $item['icid']], ['cascade' => false]);
 		}
@@ -1855,10 +1787,11 @@ class Item
 			$notify_type = Delivery::POST;
 		}
 
-		// We are doing this outside of the transaction to avoid timing problems
+		$like_no_comment = DI::config()->get('system', 'like_no_comment');
-		if (in_array($item['verb'], self::ACTIVITIES)) {
+
-			$item['iaid'] = self::insertActivity($item);
+		DBA::transaction();
-		} else {
+
+		if (!in_array($item['verb'], self::ACTIVITIES)) {
 			$item['icid'] = self::insertContent($item);
 		}
 
@@ -1869,10 +1802,6 @@ class Item
 			unset($item[$field]);
 		}
 
-		$like_no_comment = DI::config()->get('system', 'like_no_comment');
-
-		DBA::transaction();
-
 		// Filling item related side tables
 
 		// Diaspora signature
@@ -2023,43 +1952,6 @@ class Item
 		return $current_post;
 	}
 
-	/**
-	 * Insert a new item content entry
-	 *
-	 * @param array $item The item fields that are to be inserted
-	 * @return bool
-	 * @throws \Exception
-	 */
-	private static function insertActivity(array $item)
-	{
-		$fields = ['activity' => self::activityToIndex($item['verb']),
-			'uri-hash' => (string)$item['uri-id'], 'uri-id' => $item['uri-id']];
-
-		// To avoid timing problems, we are using locks.
-		$locked = DI::lock()->acquire('item_insert_activity');
-		if (!$locked) {
-			Logger::log("Couldn't acquire lock for URI " . $item['uri'] . " - proceeding anyway.");
-		}
-
-		// Do we already have this content?
-		$item_activity = DBA::selectFirst('item-activity', ['id'], ['uri-id' => $item['uri-id']]);
-		if (DBA::isResult($item_activity)) {
-			$iaid = $item_activity['id'];
-			Logger::log('Fetched activity for URI ' . $item['uri'] . ' (' . $iaid . ')');
-		} elseif (DBA::insert('item-activity', $fields)) {
-			$iaid = DBA::lastInsertId();
-			Logger::log('Inserted activity for URI ' . $item['uri'] . ' (' . $iaid . ')');
-		} else {
-			// This shouldn't happen.
-			$iaid = null;
-			Logger::log('Could not insert activity for URI ' . $item['uri'] . ' - should not happen');
-		}
-		if ($locked) {
-			DI::lock()->release('item_insert_activity');
-		}
-		return $iaid;
-	}
-
 	/**
 	 * Insert a new item content entry
 	 *
@@ -2076,57 +1968,33 @@ class Item
 			}
 		}
 
-		// To avoid timing problems, we are using locks.
-		$locked = DI::lock()->acquire('item_insert_content');
-		if (!$locked) {
-			Logger::log("Couldn't acquire lock for URI " . $item['uri'] . " - proceeding anyway.");
-		}
-
-		// Do we already have this content?
 		$item_content = DBA::selectFirst('item-content', ['id'], ['uri-id' => $item['uri-id']]);
 		if (DBA::isResult($item_content)) {
 			$icid = $item_content['id'];
-			Logger::log('Fetched content for URI ' . $item['uri'] . ' (' . $icid . ')');
+			Logger::info('Content found', ['icid' => $icid, 'uri' => $item['uri']]);
-		} elseif (DBA::insert('item-content', $fields)) {
+			return $icid;
-			$icid = DBA::lastInsertId();
-			Logger::log('Inserted content for URI ' . $item['uri'] . ' (' . $icid . ')');
-		} else {
-			// This shouldn't happen.
-			$icid = null;
-			Logger::log('Could not insert content for URI ' . $item['uri'] . ' - should not happen');
-		}
-		if ($locked) {
-			DI::lock()->release('item_insert_content');
-		}
-		return $icid;
-	}
-
-	/**
-	 * Update existing item content entries
-	 *
-	 * @param array $item      The item fields that are to be changed
-	 * @param array $condition The condition for finding the item content entries
-	 * @return bool
-	 * @throws \Exception
-	 */
-	private static function updateActivity($item, $condition)
-	{
-		if (empty($item['verb'])) {
-			return false;
-		}
-		$activity_index = self::activityToIndex($item['verb']);
-
-		if ($activity_index < 0) {
-			return false;
 		}
 
-		$fields = ['activity' => $activity_index];
+		DBA::insert('item-content', $fields, true);
+		$icid = DBA::lastInsertId();
+		if ($icid != 0) {
+			Logger::info('Content inserted', ['icid' => $icid, 'uri' => $item['uri']]);
+			return $icid;
+		}
 
-		Logger::log('Update activity for ' . json_encode($condition));
+		// Possibly there can be timing issues. Then the same content could be inserted multiple times.
+		// Due to the indexes this doesn't happen, but "lastInsertId" will be empty in these situations.
+		// So we have to fetch the id manually. This is no bug and there is no data loss.
+		$item_content = DBA::selectFirst('item-content', ['id'], ['uri-id' => $item['uri-id']]);
+		if (DBA::isResult($item_content)) {
+			$icid = $item_content['id'];
+			Logger::notice('Content inserted with empty lastInsertId', ['icid' => $icid, 'uri' => $item['uri']]);
+			return $icid;
+		}
 
-		DBA::update('item-activity', $fields, $condition, true);
+		// This shouldn't happen.
-
+		Logger::error("Content wasn't inserted", $item);
-		return true;
+		return null;
 	}
 
 	/**
@@ -2152,9 +2020,8 @@ class Item
 			$fields = $condition;
 		}
 
-		Logger::log('Update content for ' . json_encode($condition));
-
 		DBA::update('item-content', $fields, $condition, true);
+		Logger::info('Updated content', ['condition' => $condition]);
 	}
 
 	/**
@@ -3167,15 +3034,15 @@ class Item
 			$verbs = [Activity::ATTEND, Activity::ATTENDNO, Activity::ATTENDMAYBE];
 
 			// Translate to the index based activity index
-			$activities = [];
+			$vids = [];
 			foreach ($verbs as $verb) {
-				$activities[] = self::activityToIndex($verb);
+				$vids[] = Verb::getID($verb);
 			}
 		} else {
-			$activities = self::activityToIndex($activity);
+			$vids = Verb::getID($activity);
 		}
 
-		$condition = ['activity' => $activities, 'deleted' => false, 'gravity' => GRAVITY_ACTIVITY,
+		$condition = ['vid' => $vids, 'deleted' => false, 'gravity' => GRAVITY_ACTIVITY,
 			'author-id' => $author_id, 'uid' => $item['uid'], 'thr-parent' => $item_uri];
 
 		$like_item = self::selectFirst(['id', 'guid', 'verb'], $condition);

src/Model/Profile.php

@@ -601,7 +601,7 @@ class Profile
 
 			while ($rr = DBA::fetch($s)) {
 				$condition = ['parent-uri' => $rr['uri'], 'uid' => $rr['uid'], 'author-id' => public_contact(),
-					'activity' => [Item::activityToIndex( Activity::ATTEND), Item::activityToIndex(Activity::ATTENDMAYBE)],
+					'vid' => [Verb::getID(Activity::ATTEND), Verb::getID(Activity::ATTENDMAYBE)],
 					'visible' => true, 'deleted' => false];
 				if (!Item::exists($condition)) {
 					continue;

src/Worker/Expire.php

@@ -52,11 +52,6 @@ class Expire
 
 			// Normally we shouldn't have orphaned data at all.
 			// If we do have some, then we have to check why.
-			Logger::log('Deleting orphaned item activities - start', Logger::DEBUG);
-			$condition = ["NOT EXISTS (SELECT `iaid` FROM `item` WHERE `item`.`iaid` = `item-activity`.`id`)"];
-			DBA::delete('item-activity', $condition);
-			Logger::log('Orphaned item activities deleted: ' . DBA::affectedRows(), Logger::DEBUG);
-
 			Logger::log('Deleting orphaned item content - start', Logger::DEBUG);
 			$condition = ["NOT EXISTS (SELECT `icid` FROM `item` WHERE `item`.`icid` = `item-content`.`id`)"];
 			DBA::delete('item-content', $condition);