From 44e405d22b7b3d4fa613568a5de52e9e502c8d29 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 5 Jul 2018 22:00:38 +0000 Subject: [PATCH 01/26] We now store activities in a separate table --- boot.php | 2 +- database.sql | 16 ++- src/Database/DBStructure.php | 15 +++ src/Model/Item.php | 241 +++++++++++++++++++++++++++++------ 4 files changed, 233 insertions(+), 41 deletions(-) diff --git a/boot.php b/boot.php index a4588b3f73..249fd9a891 100644 --- a/boot.php +++ b/boot.php @@ -41,7 +41,7 @@ define('FRIENDICA_PLATFORM', 'Friendica'); define('FRIENDICA_CODENAME', 'The Tazmans Flax-lily'); define('FRIENDICA_VERSION', '2018.08-dev'); define('DFRN_PROTOCOL_VERSION', '2.23'); -define('DB_UPDATE_VERSION', 1274); +define('DB_UPDATE_VERSION', 1275); define('NEW_UPDATE_ROUTINE_VERSION', 1170); /** diff --git a/database.sql b/database.sql index b3d8f5dd05..1f6545225e 100644 --- a/database.sql +++ b/database.sql @@ -1,6 +1,6 @@ -- ------------------------------------------ -- Friendica 2018.08-dev (The Tazmans Flax-lily) --- DB_UPDATE_VERSION 1274 +-- DB_UPDATE_VERSION 1275 -- ------------------------------------------ @@ -478,6 +478,7 @@ CREATE TABLE IF NOT EXISTS `item` ( `author-link` varchar(255) NOT NULL DEFAULT '' COMMENT 'Link to the profile page of the author of this item', `author-avatar` varchar(255) NOT NULL DEFAULT '' COMMENT 'Link to the avatar picture of the author of this item', `icid` int unsigned COMMENT 'Id of the item-content table entry that contains the whole item content', + `iaid` int unsigned COMMENT 'Id of the item-activity table entry that contains the activity data', `title` varchar(255) NOT NULL DEFAULT '' COMMENT 'item title', `content-warning` varchar(255) NOT NULL DEFAULT '' COMMENT '', `body` mediumtext COMMENT 'item body content', @@ -542,6 +543,19 @@ CREATE TABLE IF NOT EXISTS `item` ( INDEX `icid` (`icid`) ) DEFAULT COLLATE utf8mb4_general_ci COMMENT='Structure for all posts'; +-- +-- TABLE item-activity +-- +CREATE TABLE IF NOT EXISTS `item-activity` ( + `id` int unsigned NOT NULL auto_increment, + `uri` varchar(255) NOT NULL DEFAULT '' COMMENT '', + `uri-hash` char(80) NOT NULL DEFAULT '' COMMENT 'SHA-1 and RIPEMD-160 hash from uri', + `activity` smallint unsigned NOT NULL DEFAULT 0 COMMENT '', + PRIMARY KEY(`id`), + UNIQUE INDEX `uri-hash` (`uri-hash`), + INDEX `uri` (`uri`(191)) +) DEFAULT COLLATE utf8mb4_general_ci COMMENT='Activities for items'; + -- -- TABLE item-content -- diff --git a/src/Database/DBStructure.php b/src/Database/DBStructure.php index db6997ec62..e615b1bc6e 100644 --- a/src/Database/DBStructure.php +++ b/src/Database/DBStructure.php @@ -1182,6 +1182,7 @@ class DBStructure "author-link" => ["type" => "varchar(255)", "not null" => "1", "default" => "", "comment" => "Link to the profile page of the author of this item"], "author-avatar" => ["type" => "varchar(255)", "not null" => "1", "default" => "", "comment" => "Link to the avatar picture of the author of this item"], "icid" => ["type" => "int unsigned", "relation" => ["item-content" => "id"], "comment" => "Id of the item-content table entry that contains the whole item content"], + "iaid" => ["type" => "int unsigned", "relation" => ["item-activity" => "id"], "comment" => "Id of the item-activity table entry that contains the activity data"], "title" => ["type" => "varchar(255)", "not null" => "1", "default" => "", "comment" => "item title"], "content-warning" => ["type" => "varchar(255)", "not null" => "1", "default" => "", "comment" => ""], "body" => ["type" => "mediumtext", "comment" => "item body content"], @@ -1248,6 +1249,20 @@ class DBStructure "icid" => ["icid"], ] ]; + $database["item-activity"] = [ + "comment" => "Activities for items", + "fields" => [ + "id" => ["type" => "int unsigned", "not null" => "1", "extra" => "auto_increment", "primary" => "1", "relation" => ["thread" => "iid"]], + "uri" => ["type" => "varchar(255)", "not null" => "1", "default" => "", "comment" => ""], + "uri-hash" => ["type" => "char(80)", "not null" => "1", "default" => "", "comment" => "SHA-1 and RIPEMD-160 hash from uri"], + "activity" => ["type" => "smallint unsigned", "not null" => "1", "default" => "0", "comment" => ""], + ], + "indexes" => [ + "PRIMARY" => ["id"], + "uri-hash" => ["UNIQUE", "uri-hash"], + "uri" => ["uri(191)"], + ] + ]; $database["item-content"] = [ "comment" => "Content for all posts", "fields" => [ diff --git a/src/Model/Item.php b/src/Model/Item.php index ac53bb0220..b6af5e7d9d 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -67,7 +67,7 @@ class Item extends BaseObject // All fields in the item table const ITEM_FIELDLIST = ['id', 'uid', 'parent', 'uri', 'parent-uri', 'thr-parent', 'guid', - 'contact-id', 'type', 'wall', 'gravity', 'extid', 'icid', + 'contact-id', 'type', 'wall', 'gravity', 'extid', 'icid', 'iaid', 'created', 'edited', 'commented', 'received', 'changed', 'verb', 'postopts', 'plink', 'resource-id', 'event-id', 'tag', 'attach', 'inform', 'file', 'allow_cid', 'allow_gid', 'deny_cid', 'deny_gid', @@ -78,6 +78,40 @@ class Item extends BaseObject 'author-id', 'author-link', 'author-name', 'author-avatar', 'owner-id', 'owner-link', 'owner-name', 'owner-avatar']; + const ACTIVITIES = [ACTIVITY_LIKE, ACTIVITY_DISLIKE, ACTIVITY_ATTEND, ACTIVITY_ATTENDNO, ACTIVITY_ATTENDMAYBE]; + + /** + * @brief 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; + } + + /** + * @brief returns an activity string from an activity index + * + * @param integer $index activity index + * @return string Activity string + */ + public static function indexToActivity($index) + { + if (!isset(self::ACTIVITIES[$index])) { + return ''; + } + + return self::ACTIVITIES[$index]; + } + /** * @brief Fetch a single item row * @@ -88,14 +122,12 @@ class Item extends BaseObject { $row = dba::fetch($stmt); - // Fetch data from the item-content table whenever there is content there - foreach (self::MIXED_CONTENT_FIELDLIST as $field) { - if (empty($row[$field]) && !empty($row['item-' . $field])) { - $row[$field] = $row['item-' . $field]; - } - unset($row['item-' . $field]); + if (is_bool($row)) { + return $row; } + // ---------------------- Transform item structure data ---------------------- + // We prefer the data from the user's contact over the public one if (!empty($row['author-link']) && !empty($row['contact-link']) && ($row['author-link'] == $row['contact-link'])) { @@ -117,22 +149,64 @@ class Item extends BaseObject } } - // Build the tag string out of the term entries - if (isset($row['id']) && array_key_exists('tag', $row)) { - $row['tag'] = Term::tagTextFromItemId($row['id']); - } - - // Build the file string out of the term entries - if (isset($row['id']) && array_key_exists('file', $row)) { - $row['file'] = Term::fileTextFromItemId($row['id']); - } - // We can always comment on posts from these networks - if (isset($row['writable']) && !empty($row['network']) && - in_array($row['network'], [NETWORK_DFRN, NETWORK_DIASPORA, NETWORK_OSTATUS])) { + if (array_key_exists('writable', $row) && + in_array($row['internal-network'], [NETWORK_DFRN, NETWORK_DIASPORA, NETWORK_OSTATUS])) { $row['writable'] = true; } + // ---------------------- Transform item content data ---------------------- + + // Fetch data from the item-content table whenever there is content there + foreach (self::MIXED_CONTENT_FIELDLIST as $field) { + if (empty($row[$field]) && !empty($row['internal-item-' . $field])) { + $row[$field] = $row['internal-item-' . $field]; + } + unset($row['internal-item-' . $field]); + } + + if (!empty($row['internal-iaid']) && array_key_exists('verb', $row)) { + $row['verb'] = self::indexToActivity($row['internal-activity']); + if (array_key_exists('title', $row)) { + $row['title'] = ''; + } + if (array_key_exists('body', $row)) { + $row['body'] = $row['verb']; + } + if (array_key_exists('object', $row)) { + $row['object'] = ''; + } + if (array_key_exists('object-type', $row)) { + $row['object-type'] = ACTIVITY_OBJ_NOTE; + } + } elseif (in_array($row['verb'], ['', ACTIVITY_POST, ACTIVITY_SHARE])) { + // Posts don't have an object or target - but having tags or files. + // We safe some performance by building tag and file strings only here. + // We remove object and target since they aren't used for this type. + if (array_key_exists('object', $row)) { + $row['object'] = ''; + } + if (array_key_exists('target', $row)) { + $row['target'] = ''; + } + // Build the tag string out of the term entries + if (array_key_exists('tag', $row)) { + $row['tag'] = Term::tagTextFromItemId($row['internal-iid']); + } + + // Build the file string out of the term entries + if (array_key_exists('file', $row)) { + $row['file'] = Term::fileTextFromItemId($row['internal-iid']); + } + } + + // Remove internal fields + unset($row['internal-activity']); + unset($row['internal-network']); + unset($row['internal-iid']); + unset($row['internal-iaid']); + unset($row['internal-icid']); + return $row; } @@ -417,7 +491,11 @@ class Item extends BaseObject 'file', 'allow_cid', 'allow_gid', 'deny_cid', 'deny_gid', 'private', 'pubmail', 'moderated', 'visible', 'starred', 'bookmark', 'unseen', 'deleted', 'origin', 'forum_mode', 'mention', 'global', - 'id' => 'item_id', 'network', 'icid']; + 'id' => 'item_id', 'network', 'icid', 'iaid', 'id' => 'internal-iid', + 'network' => 'internal-network', 'icid' => 'internal-icid', + 'iaid' => 'internal-iaid']; + + $fields['item-activity'] = ['activity' => 'internal-activity']; $fields['item-content'] = array_merge(self::CONTENT_FIELDLIST, self::MIXED_CONTENT_FIELDLIST); @@ -522,6 +600,10 @@ class Item extends BaseObject $joins .= " LEFT JOIN `sign` ON `sign`.`iid` = `item`.`id`"; } + if (strpos($sql_commands, "`item-activity`.") !== false) { + $joins .= " LEFT JOIN `item-activity` ON `item-activity`.`id` = `item`.`iaid`"; + } + if (strpos($sql_commands, "`item-content`.") !== false) { $joins .= " LEFT JOIN `item-content` ON `item-content`.`id` = `item`.`icid`"; } @@ -547,14 +629,15 @@ class Item extends BaseObject */ private static function constructSelectFields($fields, $selected) { - // To be able to fetch the tags we need the item id - if (in_array('tag', $selected) && !in_array('id', $selected)) { - $selected[] = 'id'; + if (!empty($selected)) { + $selected[] = 'internal-iid'; + $selected[] = 'internal-iaid'; + $selected[] = 'internal-icid'; + $selected[] = 'internal-network'; } - // To be able to fetch the files we need the item id - if (in_array('file', $selected) && !in_array('id', $selected)) { - $selected[] = 'id'; + if (in_array('verb', $selected)) { + $selected[] = 'internal-activity'; } $selection = []; @@ -562,7 +645,7 @@ class Item extends BaseObject foreach ($table_fields as $field => $select) { if (empty($selected) || in_array($select, $selected)) { if (in_array($select, self::MIXED_CONTENT_FIELDLIST)) { - $selection[] = "`item`.`".$select."` AS `item-" . $select . "`"; + $selection[] = "`item`.`".$select."` AS `internal-item-" . $select . "`"; } if (is_int($field)) { $selection[] = "`" . $table . "`.`" . $select . "`"; @@ -626,7 +709,7 @@ class Item extends BaseObject // 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', 'plink', 'icid'], $condition); + $items = dba::select('item', ['id', 'origin', 'uri', 'plink', 'iaid', 'icid'], $condition); $content_fields = []; foreach (array_merge(self::CONTENT_FIELDLIST, self::MIXED_CONTENT_FIELDLIST) as $field) { @@ -667,19 +750,21 @@ class Item extends BaseObject if (!empty($item['plink'])) { $content_fields['plink'] = $item['plink']; } - self::updateContent($content_fields, ['uri' => $item['uri']]); + if (!self::updateActivity($content_fields, ['uri' => $item['uri']])) { + self::updateContent($content_fields, ['uri' => $item['uri']]); - if (empty($item['icid'])) { - $item_content = dba::selectFirst('item-content', [], ['uri' => $item['uri']]); - if (DBM::is_result($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 (in_array($field, self::MIXED_CONTENT_FIELDLIST) && !empty($item_content[$field])) { - $item_fields[$field] = ''; + if (empty($item['icid'])) { + $item_content = dba::selectFirst('item-content', [], ['uri' => $item['uri']]); + if (DBM::is_result($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 (in_array($field, self::MIXED_CONTENT_FIELDLIST) && !empty($item_content[$field])) { + $item_fields[$field] = ''; + } } + dba::update('item', $item_fields, ['id' => $item['id']]); } - dba::update('item', $item_fields, ['id' => $item['id']]); } } @@ -1422,7 +1507,9 @@ class Item extends BaseObject } // We are doing this outside of the transaction to avoid timing problems - self::insertContent($item); + if (!self::insertActivity($item)) { + self::insertContent($item); + } dba::transaction(); $ret = dba::insert('item', $item); @@ -1577,6 +1664,55 @@ class Item extends BaseObject return $current_post; } + /** + * @brief Insert a new item content entry + * + * @param array $item The item fields that are to be inserted + */ + private static function insertActivity(&$item) + { + $activity_index = self::activityToIndex($item['verb']); + + if ($activity_index < 0) { + return false; + } + + $fields = ['uri' => $item['uri'], 'activity' => $activity_index, + 'uri-hash' => hash('sha1', $item['uri']) . hash('ripemd160', $item['uri'])]; + + $saved_item = $item; + + // We just remove everything that is content + foreach (array_merge(self::CONTENT_FIELDLIST, self::MIXED_CONTENT_FIELDLIST) as $field) { + unset($item[$field]); + } + + // To avoid timing problems, we are using locks. + $locked = Lock::set('item_insert_activity'); + if (!$locked) { + logger("Couldn't acquire lock for URI " . $item['uri'] . " - proceeding anyway."); + } + + // Do we already have this content? + $item_activity = dba::selectFirst('item-activity', ['id'], ['uri' => $item['uri']]); + if (DBM::is_result($item_activity)) { + $item['iaid'] = $item_activity['id']; + logger('Fetched activity for URI ' . $item['uri'] . ' (' . $item['iaid'] . ')'); + } elseif (dba::insert('item-activity', $fields)) { + $item['iaid'] = dba::lastInsertId(); + logger('Inserted activity for URI ' . $item['uri'] . ' (' . $item['iaid'] . ')'); + } else { + // This shouldn't happen. But if it does, we simply store it in the item-content table + logger('Could not insert activity for URI ' . $item['uri'] . ' - should not happen'); + $item = $saved_item; + return false; + } + if ($locked) { + Lock::remove('item_insert_activity'); + } + return true; + } + /** * @brief Insert a new item content entry * @@ -1635,6 +1771,33 @@ class Item extends BaseObject } } + /** + * @brief 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 + */ + private static function updateActivity($item, $condition) + { + if (empty($item['verb'])) { + return false; + } + + $activity_index = self::activityToIndex($item['verb']); + + if (!$activity_index) { + return false; + } + + $fields = ['activity' => $activity_index]; + + logger('Update activity for URI ' . $condition['uri']); + + dba::update('item-activity', $fields, $condition, true); + + return true; + } + /** * @brief Update existing item content entries * From 014eea8e1222c38677fdbd1fdcc8634e35aa25b0 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 5 Jul 2018 22:07:50 +0000 Subject: [PATCH 02/26] Improve expire for item-content and item-activity --- src/Model/Item.php | 4 ++++ src/Worker/Expire.php | 7 +++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index b6af5e7d9d..1382e5217e 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -766,6 +766,10 @@ class Item extends BaseObject dba::update('item', $item_fields, ['id' => $item['id']]); } } + } else { + if (empty($item['iaid'])) { + // To-Do + } } if (!empty($tags)) { diff --git a/src/Worker/Expire.php b/src/Worker/Expire.php index 713bfa25e0..342408894e 100644 --- a/src/Worker/Expire.php +++ b/src/Worker/Expire.php @@ -26,10 +26,13 @@ class Expire { if ($param == 'delete') { logger('Delete expired items', LOGGER_DEBUG); // physically remove anything that has been deleted for more than two months - $r = dba::p("SELECT `id`, `icid` FROM `item` WHERE `deleted` AND `changed` < UTC_TIMESTAMP() - INTERVAL 60 DAY"); + $r = dba::p("SELECT `id`, `iaid`, `icid` FROM `item` WHERE `deleted` AND `changed` < UTC_TIMESTAMP() - INTERVAL 60 DAY"); while ($row = dba::fetch($r)) { dba::delete('item', ['id' => $row['id']]); - if (!dba::exists('item', ['icid' => $row['icid']])) { + if (!empty($row['iaid']) && !dba::exists('item', ['iaid' => $row['iaid']])) { + dba::delete('item-content', ['id' => $row['icid']]); + } + if (!empty($row['icid']) && !dba::exists('item', ['icid' => $row['icid']])) { dba::delete('item-content', ['id' => $row['icid']]); } } From 5203f41d42084ba49075168f775b8fb85aa239c3 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 5 Jul 2018 22:50:33 +0000 Subject: [PATCH 03/26] Set iaid after update --- src/Model/Item.php | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 1382e5217e..bfaebf4b29 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -748,9 +748,22 @@ class Item extends BaseObject while ($item = dba::fetch($items)) { if (!empty($item['plink'])) { - $content_fields['plink'] = $item['plink']; + $content_fields['plink'] = $item['plink']; } - if (!self::updateActivity($content_fields, ['uri' => $item['uri']])) { + if (self::updateActivity($content_fields, ['uri' => $item['uri']])) { + if (empty($item['iaid'])) { + $item_activity = dba::selectFirst('item-activity', ['id'], ['uri' => $item['uri']]); + if (DBM::is_result($item_activity)) { + $item_fields = ['iaid' => $item_activity['id'], 'icid' => 0, + 'verb' => '', 'object' => '', 'body' => 'activity']; + 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']]); + } + } + } + } else { self::updateContent($content_fields, ['uri' => $item['uri']]); if (empty($item['icid'])) { @@ -766,10 +779,6 @@ class Item extends BaseObject dba::update('item', $item_fields, ['id' => $item['id']]); } } - } else { - if (empty($item['iaid'])) { - // To-Do - } } if (!empty($tags)) { From f33bd5fc8e5d3cf9462869815e6c68df6fbfa039 Mon Sep 17 00:00:00 2001 From: Michael Date: Fri, 6 Jul 2018 05:16:40 +0000 Subject: [PATCH 04/26] Update does work now --- src/Model/Item.php | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index bfaebf4b29..1f0d496ab8 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -86,7 +86,7 @@ class Item extends BaseObject * @param string $activity activity string * @return integer Activity index */ - public static function activityToIndex($activity) + private static function activityToIndex($activity) { $index = array_search($activity, self::ACTIVITIES); @@ -103,7 +103,7 @@ class Item extends BaseObject * @param integer $index activity index * @return string Activity string */ - public static function indexToActivity($index) + private static function indexToActivity($index) { if (!isset(self::ACTIVITIES[$index])) { return ''; @@ -754,8 +754,10 @@ class Item extends BaseObject if (empty($item['iaid'])) { $item_activity = dba::selectFirst('item-activity', ['id'], ['uri' => $item['uri']]); if (DBM::is_result($item_activity)) { - $item_fields = ['iaid' => $item_activity['id'], 'icid' => 0, - 'verb' => '', 'object' => '', 'body' => 'activity']; + $item_fields = ['iaid' => $item_activity['id'], 'icid' => null]; + foreach (self::MIXED_CONTENT_FIELDLIST as $field) { + $item_fields[$field] = ''; + } dba::update('item', $item_fields, ['id' => $item['id']]); if (!empty($item['icid']) && !dba::exists('item', ['icid' => $item['icid']])) { @@ -1795,14 +1797,14 @@ class Item extends BaseObject if (empty($item['verb'])) { return false; } - $activity_index = self::activityToIndex($item['verb']); - if (!$activity_index) { + if ($activity_index < 0) { return false; } - $fields = ['activity' => $activity_index]; + $fields = ['activity' => $activity_index, + 'uri-hash' => hash('sha1', $condition['uri']) . hash('ripemd160', $condition['uri'])]; logger('Update activity for URI ' . $condition['uri']); From da954b92c7be15078fc5eb206fe618e727ca9337 Mon Sep 17 00:00:00 2001 From: Michael Date: Fri, 6 Jul 2018 05:17:44 +0000 Subject: [PATCH 05/26] New post update to fill the item-activity table --- src/Database/PostUpdate.php | 42 +++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/Database/PostUpdate.php b/src/Database/PostUpdate.php index e50250ff5f..88bec31b74 100644 --- a/src/Database/PostUpdate.php +++ b/src/Database/PostUpdate.php @@ -34,6 +34,9 @@ class PostUpdate if (!self::update1274()) { return; } + if (!self::update1275()) { + return; + } } /** @@ -272,6 +275,45 @@ class PostUpdate } dba::close($items); + logger("Processed rows: " . $rows, LOGGER_DEBUG); + return true; + } + /** + * @brief update the "item-activity" table + * + * @return bool "true" when the job is done + */ + private static function update1275() + { + // Was the script completed? + if (Config::get("system", "post_update_version") >= 1275) { + return true; + } + + logger("Start", LOGGER_DEBUG); + + $fields = ['id', 'verb']; + + $condition = ["`iaid` IS NULL AND NOT `icid` IS NULL AND `verb` IN (?, ?, ?, ?, ?)", + ACTIVITY_LIKE, ACTIVITY_DISLIKE, ACTIVITY_ATTEND, ACTIVITY_ATTENDNO, ACTIVITY_ATTENDMAYBE]; + + $params = ['limit' => 10000]; + $items = Item::select($fields, $condition, $params); + + if (!DBM::is_result($items)) { + Config::set("system", "post_update_version", 1275); + logger("Done", LOGGER_DEBUG); + return true; + } + + $rows = 0; + + while ($item = Item::fetch($items)) { + Item::update($item, ['id' => $item['id']]); + ++$rows; + } + dba::close($items); + logger("Processed rows: " . $rows, LOGGER_DEBUG); return true; } From 32d398cc93e38b89957c9589a281714d3348a297 Mon Sep 17 00:00:00 2001 From: Michael Date: Fri, 6 Jul 2018 05:39:25 +0000 Subject: [PATCH 06/26] Unified content that is stored for a like --- src/Protocol/Diaspora.php | 47 +-------------------------------------- src/Protocol/OStatus.php | 7 +++--- 2 files changed, 5 insertions(+), 49 deletions(-) diff --git a/src/Protocol/Diaspora.php b/src/Protocol/Diaspora.php index 47101d5ad7..7b55fc6820 100644 --- a/src/Protocol/Diaspora.php +++ b/src/Protocol/Diaspora.php @@ -1924,50 +1924,6 @@ class Diaspora return true; } - /** - * @brief Creates the body for a "like" message - * - * @param array $contact The contact that send us the "like" - * @param array $parent_item The item array of the parent item - * @param string $guid message guid - * - * @return string the body - */ - private static function constructLikeBody($contact, $parent_item, $guid) - { - $bodyverb = L10n::t('%1$s likes %2$s\'s %3$s'); - - $ulink = "[url=".$contact["url"]."]".$contact["name"]."[/url]"; - $alink = "[url=".$parent_item["author-link"]."]".$parent_item["author-name"]."[/url]"; - $plink = "[url=".System::baseUrl()."/display/".urlencode($guid)."]".L10n::t("status")."[/url]"; - - return sprintf($bodyverb, $ulink, $alink, $plink); - } - - /** - * @brief Creates a XML object for a "like" - * - * @param array $importer Array of the importer user - * @param array $parent_item The item array of the parent item - * - * @return string The XML - */ - private static function constructLikeObject($importer, $parent_item) - { - $objtype = ACTIVITY_OBJ_NOTE; - $link = ''; - $parent_body = $parent_item["body"]; - - $xmldata = ["object" => ["type" => $objtype, - "local" => "1", - "id" => $parent_item["uri"], - "link" => $link, - "title" => "", - "content" => $parent_body]]; - - return XML::fromArray($xmldata, $xml, true); - } - /** * @brief Processes "like" messages * @@ -2046,9 +2002,8 @@ class Diaspora $datarray["parent-uri"] = $parent_item["uri"]; $datarray["object-type"] = ACTIVITY_OBJ_NOTE; - $datarray["object"] = self::constructLikeObject($importer, $parent_item); - $datarray["body"] = self::constructLikeBody($contact, $parent_item, $guid); + $datarray["body"] = $verb; // like on comments have the comment as parent. So we need to fetch the toplevel parent if ($parent_item["id"] != $parent_item["parent"]) { diff --git a/src/Protocol/OStatus.php b/src/Protocol/OStatus.php index 5a30bfd2c3..cf35581ab8 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -338,7 +338,7 @@ class OStatus $header = []; $header["uid"] = $importer["uid"]; $header["network"] = NETWORK_OSTATUS; - $header["type"] = "remote"; + $header["type"] = "remote-comment"; $header["wall"] = 0; $header["origin"] = 0; $header["gravity"] = GRAVITY_COMMENT; @@ -449,9 +449,11 @@ class OStatus $orig_uri = $xpath->query("activity:object/atom:id", $entry)->item(0)->nodeValue; logger("Favorite ".$orig_uri." ".print_r($item, true)); + $item["type"] = "activity"; $item["verb"] = ACTIVITY_LIKE; $item["parent-uri"] = $orig_uri; $item["gravity"] = GRAVITY_ACTIVITY; + $item["object-type"] = ACTIVITY_OBJ_NOTE; } // http://activitystrea.ms/schema/1.0/rsvp-yes @@ -681,11 +683,10 @@ class OStatus } else { logger('Reply with URI '.$item["uri"].' already existed for user '.$importer["uid"].'.', LOGGER_DEBUG); } - - $item["type"] = 'remote-comment'; } else { $item["parent-uri"] = $item["uri"]; $item["gravity"] = GRAVITY_PARENT; + $item["type"] = "remote"; } if (($item['author-link'] != '') && !empty($item['protocol'])) { From 5a070425601987a5705689e701b131a5b8634489 Mon Sep 17 00:00:00 2001 From: Michael Date: Fri, 6 Jul 2018 06:37:33 +0000 Subject: [PATCH 07/26] Added warning --- src/Model/Item.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Model/Item.php b/src/Model/Item.php index 1f0d496ab8..7c4a3cbffe 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -78,6 +78,8 @@ class Item extends BaseObject 'author-id', 'author-link', 'author-name', 'author-avatar', 'owner-id', 'owner-link', 'owner-name', 'owner-avatar']; + // 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]; /** From 7f78540454b97dc2848f06be335bdcd1481be5e4 Mon Sep 17 00:00:00 2001 From: Michael Date: Fri, 6 Jul 2018 06:45:30 +0000 Subject: [PATCH 08/26] Expire does work now for activities as well --- src/Worker/Expire.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Worker/Expire.php b/src/Worker/Expire.php index 342408894e..5d56308faf 100644 --- a/src/Worker/Expire.php +++ b/src/Worker/Expire.php @@ -26,17 +26,18 @@ class Expire { if ($param == 'delete') { logger('Delete expired items', LOGGER_DEBUG); // physically remove anything that has been deleted for more than two months - $r = dba::p("SELECT `id`, `iaid`, `icid` FROM `item` WHERE `deleted` AND `changed` < UTC_TIMESTAMP() - INTERVAL 60 DAY"); - while ($row = dba::fetch($r)) { + $condition = ["`deleted` AND `changed` < UTC_TIMESTAMP() - INTERVAL 60 DAY"]; + $rows = dba::select('item', ['id', 'iaid', 'icid'], $condition); + while ($row = dba::fetch($rows)) { dba::delete('item', ['id' => $row['id']]); if (!empty($row['iaid']) && !dba::exists('item', ['iaid' => $row['iaid']])) { - dba::delete('item-content', ['id' => $row['icid']]); + dba::delete('item-activity', ['id' => $row['iaid']]); } if (!empty($row['icid']) && !dba::exists('item', ['icid' => $row['icid']])) { dba::delete('item-content', ['id' => $row['icid']]); } } - dba::close($r); + dba::close($rows); logger('Delete expired items - done', LOGGER_DEBUG); From 0c51159111b7837ac960dc532d2add29a1594a47 Mon Sep 17 00:00:00 2001 From: Michael Date: Fri, 6 Jul 2018 06:46:44 +0000 Subject: [PATCH 09/26] New index that is needed for the expiry --- database.sql | 3 ++- src/Database/DBStructure.php | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/database.sql b/database.sql index 1f6545225e..0b0607e62e 100644 --- a/database.sql +++ b/database.sql @@ -540,7 +540,8 @@ CREATE TABLE IF NOT EXISTS `item` ( INDEX `deleted_changed` (`deleted`,`changed`), INDEX `uid_wall_changed` (`uid`,`wall`,`changed`), INDEX `uid_eventid` (`uid`,`event-id`), - INDEX `icid` (`icid`) + INDEX `icid` (`icid`), + INDEX `iaid` (`iaid`) ) DEFAULT COLLATE utf8mb4_general_ci COMMENT='Structure for all posts'; -- diff --git a/src/Database/DBStructure.php b/src/Database/DBStructure.php index e615b1bc6e..d8eb742dde 100644 --- a/src/Database/DBStructure.php +++ b/src/Database/DBStructure.php @@ -1247,6 +1247,7 @@ class DBStructure "uid_wall_changed" => ["uid","wall","changed"], "uid_eventid" => ["uid","event-id"], "icid" => ["icid"], + "iaid" => ["iaid"], ] ]; $database["item-activity"] = [ From 9e262e6157b068175349428325fe24a8fa9c2187 Mon Sep 17 00:00:00 2001 From: Michael Date: Fri, 6 Jul 2018 22:08:41 +0000 Subject: [PATCH 10/26] Better handling of empty content --- src/Model/Item.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 7c4a3cbffe..be317b2882 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1832,7 +1832,9 @@ class Item extends BaseObject } if (empty($fields)) { - return; + // when there are no fields at all, just use the condition + // This is to ensure that we always store content. + $fields = $condition; } if (!empty($item['plink'])) { From b421e7708ff31d2bb4e367974798aa11a56bfd79 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 7 Jul 2018 07:43:13 +0000 Subject: [PATCH 11/26] Removing of orphaned activity and content data --- src/Worker/Expire.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/Worker/Expire.php b/src/Worker/Expire.php index 5d56308faf..b09db5c677 100644 --- a/src/Worker/Expire.php +++ b/src/Worker/Expire.php @@ -39,12 +39,24 @@ class Expire { } dba::close($rows); - logger('Delete expired items - done', LOGGER_DEBUG); + // Normally we shouldn't have orphaned data at all. + // If we do have some, then we have to check why. + logger('Deleting orphaned item activities - start', LOGGER_DEBUG); + $condition = ["NOT EXISTS (SELECT `iaid` FROM `item` WHERE `item`.`uri` = `item-activity`.`uri`)"]; + dba::delete('item-activity', $condition); + logger('Orphaned item activities deleted: ' . dba::affected_rows(), LOGGER_DEBUG); + + logger('Deleting orphaned item content - start', LOGGER_DEBUG); + $condition = ["NOT EXISTS (SELECT `icid` FROM `item` WHERE `item`.`uri` = `item-content`.`uri`)"]; + dba::delete('item-content', $condition); + logger('Orphaned item content deleted: ' . dba::affected_rows(), LOGGER_DEBUG); // make this optional as it could have a performance impact on large sites if (intval(Config::get('system', 'optimize_items'))) { dba::e("OPTIMIZE TABLE `item`"); } + + logger('Delete expired items - done', LOGGER_DEBUG); return; } elseif (intval($param) > 0) { $user = dba::selectFirst('user', ['uid', 'username', 'expire'], ['uid' => $param]); From 3389e9b21391126945510d10c2ff0acc91a68b73 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 7 Jul 2018 10:43:43 +0000 Subject: [PATCH 12/26] Removing of likes should work now on comments as well --- src/Model/Item.php | 68 ++++++++++------------------------------------ 1 file changed, 15 insertions(+), 53 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index be317b2882..0757a09b1a 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -497,7 +497,7 @@ class Item extends BaseObject 'network' => 'internal-network', 'icid' => 'internal-icid', 'iaid' => 'internal-iaid']; - $fields['item-activity'] = ['activity' => 'internal-activity']; + $fields['item-activity'] = ['activity', 'activity' => 'internal-activity']; $fields['item-content'] = array_merge(self::CONTENT_FIELDLIST, self::MIXED_CONTENT_FIELDLIST); @@ -2789,27 +2789,22 @@ class Item extends BaseObject switch ($verb) { case 'like': case 'unlike': - $bodyverb = L10n::t('%1$s likes %2$s\'s %3$s'); $activity = ACTIVITY_LIKE; break; case 'dislike': case 'undislike': - $bodyverb = L10n::t('%1$s doesn\'t like %2$s\'s %3$s'); $activity = ACTIVITY_DISLIKE; break; case 'attendyes': case 'unattendyes': - $bodyverb = L10n::t('%1$s is attending %2$s\'s %3$s'); $activity = ACTIVITY_ATTEND; break; case 'attendno': case 'unattendno': - $bodyverb = L10n::t('%1$s is not attending %2$s\'s %3$s'); $activity = ACTIVITY_ATTENDNO; break; case 'attendmaybe': case 'unattendmaybe': - $bodyverb = L10n::t('%1$s may attend %2$s\'s %3$s'); $activity = ACTIVITY_ATTENDMAYBE; break; default: @@ -2828,6 +2823,8 @@ class Item extends BaseObject return false; } + $item_uri = $item['uri']; + $uid = $item['uid']; if (($uid == 0) && local_user()) { $uid = local_user(); @@ -2848,7 +2845,7 @@ class Item extends BaseObject // Retrieve the current logged in user's public contact $author_id = public_contact(); - $author_contact = dba::selectFirst('contact', [], ['id' => $author_id]); + $author_contact = dba::selectFirst('contact', ['url'], ['id' => $author_id]); if (!DBM::is_result($author_contact)) { logger('like: unknown author ' . $author_id); return false; @@ -2872,26 +2869,21 @@ class Item extends BaseObject // we need to eradicate your first choice. if ($event_verb_flag) { $verbs = [ACTIVITY_ATTEND, ACTIVITY_ATTENDNO, ACTIVITY_ATTENDMAYBE]; + + // Translate to the index based activity index + $activities = []; + foreach ($verbs as $verb) { + $activities[] = self::activityToIndex($verb); + } } else { - $verbs = $activity; + $activities = self::activityToIndex($activity); } - $base_condition = ['verb' => $verbs, 'deleted' => false, 'gravity' => GRAVITY_ACTIVITY, - 'author-id' => $author_contact['id'], 'uid' => $item['uid']]; + $condition = ['activity' => $activities, 'deleted' => false, 'gravity' => GRAVITY_ACTIVITY, + 'author-id' => $author_id, 'uid' => $item['uid'], 'thr-parent' => $item_uri]; - $condition = array_merge($base_condition, ['parent' => $item_id]); $like_item = self::selectFirst(['id', 'guid', 'verb'], $condition); - if (!DBM::is_result($like_item)) { - $condition = array_merge($base_condition, ['parent-uri' => $item_id]); - $like_item = self::selectFirst(['id', 'guid', 'verb'], $condition); - } - - if (!DBM::is_result($like_item)) { - $condition = array_merge($base_condition, ['thr-parent' => $item_id]); - $like_item = self::selectFirst(['id', 'guid', 'verb'], $condition); - } - // If it exists, mark it as deleted if (DBM::is_result($like_item)) { // Already voted, undo it @@ -2917,30 +2909,7 @@ class Item extends BaseObject return true; } - // Else or if event verb different from existing row, create a new item row - $post_type = (($item['resource-id']) ? L10n::t('photo') : L10n::t('status')); - if ($item['object-type'] === ACTIVITY_OBJ_EVENT) { - $post_type = L10n::t('event'); - } $objtype = $item['resource-id'] ? ACTIVITY_OBJ_IMAGE : ACTIVITY_OBJ_NOTE ; - $link = xmlify('' . "\n") ; - $body = $item['body']; - - $obj = <<< EOT - - - $objtype - 1 - {$item['uri']} - $link - - $body - -EOT; - - $ulink = '[url=' . $author_contact['url'] . ']' . $author_contact['name'] . '[/url]'; - $alink = '[url=' . $item['author-link'] . ']' . $item['author-name'] . '[/url]'; - $plink = '[url=' . System::baseUrl() . '/display/' . $owner_self_contact['nick'] . '/' . $item['id'] . ']' . $post_type . '[/url]'; $new_item = [ 'guid' => get_guid(32), @@ -2955,17 +2924,10 @@ EOT; 'parent-uri' => $item['uri'], 'thr-parent' => $item['uri'], 'owner-id' => $item['owner-id'], - 'owner-name' => $item['owner-name'], - 'owner-link' => $item['owner-link'], - 'owner-avatar' => $item['owner-avatar'], - 'author-id' => $author_contact['id'], - 'author-name' => $author_contact['name'], - 'author-link' => $author_contact['url'], - 'author-avatar' => $author_contact['thumb'], - 'body' => sprintf($bodyverb, $ulink, $alink, $plink), + 'author-id' => $author_id, + 'body' => $activity, 'verb' => $activity, 'object-type' => $objtype, - 'object' => $obj, 'allow_cid' => $item['allow_cid'], 'allow_gid' => $item['allow_gid'], 'deny_cid' => $item['deny_cid'], From 575662c2d7eecdacd6b757ef0853c90c6365d0da Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 7 Jul 2018 11:39:37 +0000 Subject: [PATCH 13/26] Just some code cleanup --- src/Model/Item.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 0757a09b1a..a95bc184f1 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -2892,12 +2892,9 @@ class Item extends BaseObject dba::update('item', $fields, ['id' => $like_item['id']]); // Clean up the Diaspora signatures for this like - // Go ahead and do it even if Diaspora support is disabled. We still want to clean up - // if it had been enabled in the past dba::delete('sign', ['iid' => $like_item['id']]); - $like_item_id = $like_item['id']; - Worker::add(PRIORITY_HIGH, "Notifier", "like", $like_item_id); + Worker::add(PRIORITY_HIGH, "Notifier", "like", $like_item['id']); if (!$event_verb_flag || $like_item['verb'] == $activity) { return true; From d6af9515ba13bf6350fd3a2714e44e0d2765a121 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 7 Jul 2018 16:38:01 +0000 Subject: [PATCH 14/26] Avoid storing an icid value when iaid is stored/Fix item retraction --- src/Model/Item.php | 4 +++- src/Protocol/Diaspora.php | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index a95bc184f1..87ea80005e 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -752,7 +752,9 @@ class Item extends BaseObject if (!empty($item['plink'])) { $content_fields['plink'] = $item['plink']; } - if (self::updateActivity($content_fields, ['uri' => $item['uri']])) { + if ((self::activityToIndex($item['verb']) >= 0) || !empty($item['iaid'])) { + self::updateActivity($content_fields, ['uri' => $item['uri']]); + if (empty($item['iaid'])) { $item_activity = dba::selectFirst('item-activity', ['id'], ['uri' => $item['uri']]); if (DBM::is_result($item_activity)) { diff --git a/src/Protocol/Diaspora.php b/src/Protocol/Diaspora.php index 7b55fc6820..87f798427c 100644 --- a/src/Protocol/Diaspora.php +++ b/src/Protocol/Diaspora.php @@ -2709,10 +2709,11 @@ class Diaspora // When we receive a public retraction, we delete every item that we find. if ($importer['uid'] == 0) { - $condition = ["`guid` = ? AND NOT `file` LIKE '%%[%%' AND NOT `deleted`", $target_guid]; + $condition = ['guid' => $target_guid, 'deleted' => false]; } else { - $condition = ["`guid` = ? AND `uid` = ? AND NOT `file` LIKE '%%[%%' AND NOT `deleted`", $target_guid, $importer['uid']]; + $condition = ['guid' => $target_guid, 'deleted' => false, 'uid' => $importer['uid']]; } + $r = Item::select($fields, $condition); if (!DBM::is_result($r)) { logger("Target guid ".$target_guid." was not found on this system for user ".$importer['uid']."."); From 4d35e228c41a1fc6a6de9f50b8ec5c5bb45a9c2d Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 7 Jul 2018 18:14:16 +0000 Subject: [PATCH 15/26] More item abstraction / making remote deletion work again --- include/api.php | 12 ++++++------ mod/acl.php | 2 +- mod/display.php | 2 +- mod/item.php | 4 ++-- src/Object/Post.php | 3 ++- src/Protocol/DFRN.php | 12 ++++++++---- src/Protocol/Diaspora.php | 7 ++++++- src/Protocol/Feed.php | 2 +- src/Worker/Delivery.php | 6 +++--- src/Worker/Notifier.php | 2 +- 10 files changed, 31 insertions(+), 21 deletions(-) diff --git a/include/api.php b/include/api.php index 32fe6c651d..19d4a944d5 100644 --- a/include/api.php +++ b/include/api.php @@ -1272,7 +1272,7 @@ function api_status_show($type) // get last public wall message $condition = ['owner-id' => $user_info['pid'], 'uid' => api_user(), 'gravity' => [GRAVITY_PARENT, GRAVITY_COMMENT]]; - $lastwall = dba::selectFirst('item', [], $condition, ['order' => ['id' => true]]); + $lastwall = Item::selectFirst(Item::ITEM_FIELDLIST, $condition, ['order' => ['id' => true]]); if (DBM::is_result($lastwall)) { $in_reply_to = api_in_reply_to($lastwall); @@ -1357,7 +1357,7 @@ function api_users_show($type) $condition = ['owner-id' => $user_info['pid'], 'uid' => api_user(), 'gravity' => [GRAVITY_PARENT, GRAVITY_COMMENT], 'private' => false]; - $lastwall = dba::selectFirst('item', [], $condition, ['order' => ['id' => true]]); + $lastwall = Item::selectFirst(Item::ITEM_FIELDLIST, $condition, ['order' => ['id' => true]]); if (DBM::is_result($lastwall)) { $in_reply_to = api_in_reply_to($lastwall); @@ -1817,12 +1817,12 @@ function api_statuses_show($type) $conversation = (x($_REQUEST, 'conversation') ? 1 : 0); // try to fetch the item for the local user - or the public item, if there is no local one - $uri_item = dba::selectFirst('item', ['uri'], ['id' => $id]); + $uri_item = Item::selectFirst(['uri'], ['id' => $id]); if (!DBM::is_result($uri_item)) { throw new BadRequestException("There is no status with this id."); } - $item = dba::selectFirst('item', ['id'], ['uri' => $uri_item['uri'], 'uid' => [0, api_user()]], ['order' => ['uid' => true]]); + $item = Item::selectFirst(['id'], ['uri' => $uri_item['uri'], 'uid' => [0, api_user()]], ['order' => ['uid' => true]]); if (!DBM::is_result($item)) { throw new BadRequestException("There is no status with this id."); } @@ -1897,12 +1897,12 @@ function api_conversation_show($type) logger('API: api_conversation_show: '.$id); // try to fetch the item for the local user - or the public item, if there is no local one - $item = dba::selectFirst('item', ['parent-uri'], ['id' => $id]); + $item = Item::selectFirst(['parent-uri'], ['id' => $id]); if (!DBM::is_result($item)) { throw new BadRequestException("There is no status with this id."); } - $parent = dba::selectFirst('item', ['id'], ['uri' => $item['parent-uri'], 'uid' => [0, api_user()]], ['order' => ['uid' => true]]); + $parent = Item::selectFirst(['id'], ['uri' => $item['parent-uri'], 'uid' => [0, api_user()]], ['order' => ['uid' => true]]); if (!DBM::is_result($parent)) { throw new BadRequestException("There is no status with this id."); } diff --git a/mod/acl.php b/mod/acl.php index c16b3bab6e..dd5dd90281 100644 --- a/mod/acl.php +++ b/mod/acl.php @@ -241,7 +241,7 @@ function acl_content(App $a) if ($conv_id) { // In multi threaded posts the conv_id is not the parent of the whole thread - $parent_item = dba::selectFirst('item', ['parent'], ['id' => $conv_id]); + $parent_item = Item::selectFirst(['parent'], ['id' => $conv_id]); if (DBM::is_result($parent_item)) { $conv_id = $parent_item['parent']; } diff --git a/mod/display.php b/mod/display.php index 919b12fbc3..920cb454f9 100644 --- a/mod/display.php +++ b/mod/display.php @@ -202,7 +202,7 @@ function display_content(App $a, $update = false, $update_uid = 0) if ($update) { $item_id = $_REQUEST['item_id']; - $item = dba::selectFirst('item', ['uid', 'parent', 'parent-uri'], ['id' => $item_id]); + $item = Item::selectFirst(['uid', 'parent', 'parent-uri'], ['id' => $item_id]); if ($item['uid'] != 0) { $a->profile = ['uid' => intval($item['uid']), 'profile_uid' => intval($item['uid'])]; } else { diff --git a/mod/item.php b/mod/item.php index be9fa09c47..61de1e096c 100644 --- a/mod/item.php +++ b/mod/item.php @@ -106,7 +106,7 @@ function item_post(App $a) { $thr_parent_contact = Contact::getDetailsByURL($parent_item["author-link"]); if ($parent_item['id'] != $parent_item['parent']) { - $parent_item = dba::selectFirst('item', [], ['id' => $parent_item['parent']]); + $parent_item = Item::selectFirst(Item::ITEM_FIELDLIST, ['id' => $parent_item['parent']]); } } @@ -170,7 +170,7 @@ function item_post(App $a) { $orig_post = null; if ($post_id) { - $orig_post = dba::selectFirst('item', [], ['id' => $post_id]); + $orig_post = Item::selectFirst(Item::ITEM_FIELDLIST, ['id' => $post_id]); } $user = dba::selectFirst('user', [], ['uid' => $profile_uid]); diff --git a/src/Object/Post.php b/src/Object/Post.php index b131246fd4..216008974a 100644 --- a/src/Object/Post.php +++ b/src/Object/Post.php @@ -13,6 +13,7 @@ use Friendica\Core\L10n; use Friendica\Core\PConfig; use Friendica\Database\DBM; use Friendica\Model\Contact; +use Friendica\Model\Item; use Friendica\Util\DateTimeFormat; use Friendica\Util\Temporal; use dba; @@ -178,7 +179,7 @@ class Post extends BaseObject if (!$origin) { /// @todo This shouldn't be done as query here, but better during the data creation. // it is now done here, since during the RC phase we shouldn't make to intense changes. - $parent = dba::selectFirst('item', ['origin'], ['id' => $item['parent']]); + $parent = Item::selectFirst(['origin'], ['id' => $item['parent']]); if (DBM::is_result($parent)) { $origin = $parent['origin']; } diff --git a/src/Protocol/DFRN.php b/src/Protocol/DFRN.php index f979d6b003..72ab89d925 100644 --- a/src/Protocol/DFRN.php +++ b/src/Protocol/DFRN.php @@ -2414,8 +2414,7 @@ class DFRN $item["edited"] = $xpath->query("atom:updated/text()", $entry)->item(0)->nodeValue; - $current = dba::selectFirst('item', - ['id', 'uid', 'edited', 'body'], + $current = Item::selectFirst(['id', 'uid', 'edited', 'body'], ['uri' => $item["uri"], 'uid' => $importer["importer_uid"]] ); // Is there an existing item? @@ -2747,13 +2746,18 @@ class DFRN return false; } - $condition = ["`uri` = ? AND `uid` = ? AND NOT `file` LIKE '%[%'", $uri, $importer["importer_uid"]]; - $item = dba::selectFirst('item', ['id', 'parent', 'contact-id'], $condition); + $condition = ['uri' => $uri, 'uid' => $importer["importer_uid"]]; + $item = Item::selectFirst(['id', 'parent', 'contact-id', 'file'], $condition); if (!DBM::is_result($item)) { logger("Item with uri " . $uri . " for user " . $importer["importer_uid"] . " wasn't found.", LOGGER_DEBUG); return; } + if (strstr($item['file'], '[')) { + logger("Item with uri " . $uri . " for user " . $importer["importer_uid"] . " is filed. So it won't be deleted.", LOGGER_DEBUG); + return; + } + // When it is a starting post it has to belong to the person that wants to delete it if (($item['id'] == $item['parent']) && ($item['contact-id'] != $importer["id"])) { logger("Item with uri " . $uri . " don't belong to contact " . $importer["id"] . " - ignoring deletion.", LOGGER_DEBUG); diff --git a/src/Protocol/Diaspora.php b/src/Protocol/Diaspora.php index 87f798427c..d567be144d 100644 --- a/src/Protocol/Diaspora.php +++ b/src/Protocol/Diaspora.php @@ -2705,7 +2705,7 @@ class Diaspora } // Fetch items that are about to be deleted - $fields = ['uid', 'id', 'parent', 'parent-uri', 'author-link']; + $fields = ['uid', 'id', 'parent', 'parent-uri', 'author-link', 'file']; // When we receive a public retraction, we delete every item that we find. if ($importer['uid'] == 0) { @@ -2721,6 +2721,11 @@ class Diaspora } while ($item = Item::fetch($r)) { + if (strstr($item['file'], '[')) { + logger("Target guid " . $target_guid . " for user " . $item['uid'] . " is filed. So it won't be deleted.", LOGGER_DEBUG); + continue; + } + // Fetch the parent item $parent = Item::selectFirst(['author-link'], ['id' => $item["parent"]]); diff --git a/src/Protocol/Feed.php b/src/Protocol/Feed.php index c04e40b5e5..29ab21d1f1 100644 --- a/src/Protocol/Feed.php +++ b/src/Protocol/Feed.php @@ -250,7 +250,7 @@ class Feed { if (!$simulate) { $condition = ["`uid` = ? AND `uri` = ? AND `network` IN (?, ?)", $importer["uid"], $item["uri"], NETWORK_FEED, NETWORK_DFRN]; - $previous = dba::selectFirst('item', ['id'], $condition); + $previous = Item::selectFirst(['id'], $condition); if (DBM::is_result($previous)) { logger("Item with uri ".$item["uri"]." for user ".$importer["uid"]." already existed under id ".$previous["id"], LOGGER_DEBUG); continue; diff --git a/src/Worker/Delivery.php b/src/Worker/Delivery.php index e505f4bd70..0c8ff27faf 100644 --- a/src/Worker/Delivery.php +++ b/src/Worker/Delivery.php @@ -53,7 +53,7 @@ class Delivery extends BaseObject } elseif ($cmd == self::RELOCATION) { $uid = $item_id; } else { - $item = dba::selectFirst('item', ['parent'], ['id' => $item_id]); + $item = Item::selectFirst(['parent'], ['id' => $item_id]); if (!DBM::is_result($item) || empty($item['parent'])) { return; } @@ -436,12 +436,12 @@ class Delivery extends BaseObject if (empty($target_item['title'])) { $condition = ['uri' => $target_item['parent-uri'], 'uid' => $owner['uid']]; - $title = dba::selectFirst('item', ['title'], $condition); + $title = Item::selectFirst(['title'], $condition); if (DBM::is_result($title) && ($title['title'] != '')) { $subject = $title['title']; } else { $condition = ['parent-uri' => $target_item['parent-uri'], 'uid' => $owner['uid']]; - $title = dba::selectFirst('item', ['title'], $condition); + $title = Item::selectFirst(['title'], $condition); if (DBM::is_result($title) && ($title['title'] != '')) { $subject = $title['title']; } diff --git a/src/Worker/Notifier.php b/src/Worker/Notifier.php index fcf36bd55a..96549233e5 100644 --- a/src/Worker/Notifier.php +++ b/src/Worker/Notifier.php @@ -167,7 +167,7 @@ class Notifier { $fields = ['network', 'author-id', 'owner-id']; $condition = ['uri' => $target_item["thr-parent"], 'uid' => $target_item["uid"]]; - $thr_parent = dba::selectFirst('item', $fields, $condition); + $thr_parent = Item::selectFirst($fields, $condition); logger('GUID: '.$target_item["guid"].': Parent is '.$parent['network'].'. Thread parent is '.$thr_parent['network'], LOGGER_DEBUG); From afa194200beb2b814612119acb5ab5cf54ae4d36 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 7 Jul 2018 19:39:00 +0000 Subject: [PATCH 16/26] Fix: Ensure that the value is set. --- src/Model/Item.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 87ea80005e..4a7d8639d5 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -752,7 +752,7 @@ class Item extends BaseObject if (!empty($item['plink'])) { $content_fields['plink'] = $item['plink']; } - if ((self::activityToIndex($item['verb']) >= 0) || !empty($item['iaid'])) { + if (!empty($item['iaid']) || (!empty($content_fields['verb']) && (self::activityToIndex($content_fields['verb']) >= 0))) { self::updateActivity($content_fields, ['uri' => $item['uri']]); if (empty($item['iaid'])) { @@ -768,6 +768,12 @@ class Item extends BaseObject 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' => $item['uri']]); From 4467dd4972f2b726b0fc443c68313ac6fdaeb3ca Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 7 Jul 2018 23:03:28 +0000 Subject: [PATCH 17/26] Clear legacy item fields --- src/Model/Item.php | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 4a7d8639d5..3738d396f0 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -192,12 +192,12 @@ class Item extends BaseObject $row['target'] = ''; } // Build the tag string out of the term entries - if (array_key_exists('tag', $row)) { + if (array_key_exists('tag', $row) && empty($row['tag'])) { $row['tag'] = Term::tagTextFromItemId($row['internal-iid']); } // Build the file string out of the term entries - if (array_key_exists('file', $row)) { + if (array_key_exists('file', $row) && empty($row['file'])) { $row['file'] = Term::fileTextFromItemId($row['internal-iid']); } } @@ -711,13 +711,24 @@ class Item extends BaseObject // 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', 'plink', 'iaid', 'icid'], $condition); + $items = dba::select('item', ['id', 'origin', 'uri', 'plink', 'iaid', 'icid', 'tag', 'file'], $condition); $content_fields = []; foreach (array_merge(self::CONTENT_FIELDLIST, self::MIXED_CONTENT_FIELDLIST) as $field) { if (isset($fields[$field])) { $content_fields[$field] = $fields[$field]; - unset($fields[$field]); + if (in_array($field, self::CONTENT_FIELDLIST)) { + unset($fields[$field]); + } else { + $fields[$field] = null; + } + } + } + + $author_owner_fields = ['author-name', 'author-avatar', 'author-link', 'owner-name', 'owner-avatar', 'owner-link']; + foreach ($author_owner_fields as $field) { + if (isset($fields[$field])) { + $fields[$field] = null; } } @@ -795,10 +806,16 @@ class Item extends BaseObject if (!empty($tags)) { Term::insertFromTagFieldByItemId($item['id'], $tags); + if (!empty($item['tag'])) { + dba::update('item', ['tag' => ''], ['id' => $item['id']]); + } } if (!empty($files)) { Term::insertFromFileFieldByItemId($item['id'], $files); + if (!empty($item['file'])) { + dba::update('item', ['file' => ''], ['id' => $item['id']]); + } } self::updateThread($item['id']); From 4673560c55469ed167e0ed1ceb6875ecc4d89b20 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 7 Jul 2018 23:19:28 +0000 Subject: [PATCH 18/26] Don't use isset --- src/Model/Item.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 3738d396f0..fb9fe984d3 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -107,7 +107,7 @@ class Item extends BaseObject */ private static function indexToActivity($index) { - if (!isset(self::ACTIVITIES[$index])) { + if (!is_int($index) || !array_key_exists($index, self::ACTIVITIES)) { return ''; } From 75aa831b32ec55b635fdbb3631b10f3cd0248aca Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 7 Jul 2018 23:31:30 +0000 Subject: [PATCH 19/26] Better not check too strict --- src/Model/Item.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index fb9fe984d3..3086a869be 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -107,7 +107,7 @@ class Item extends BaseObject */ private static function indexToActivity($index) { - if (!is_int($index) || !array_key_exists($index, self::ACTIVITIES)) { + if (is_null($index) || !array_key_exists($index, self::ACTIVITIES)) { return ''; } From 376045628455d2da99cb50ea3aa700d69f85beba Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 7 Jul 2018 23:52:02 +0000 Subject: [PATCH 20/26] We already used DB version 1275 (Why hadn't been there a merge conflict?) --- boot.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boot.php b/boot.php index 249fd9a891..faefa6c15b 100644 --- a/boot.php +++ b/boot.php @@ -41,7 +41,7 @@ define('FRIENDICA_PLATFORM', 'Friendica'); define('FRIENDICA_CODENAME', 'The Tazmans Flax-lily'); define('FRIENDICA_VERSION', '2018.08-dev'); define('DFRN_PROTOCOL_VERSION', '2.23'); -define('DB_UPDATE_VERSION', 1275); +define('DB_UPDATE_VERSION', 1276); define('NEW_UPDATE_ROUTINE_VERSION', 1170); /** From 43a7a1647d176b4fa180d02c1f58c5c213bb4106 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 7 Jul 2018 23:53:30 +0000 Subject: [PATCH 21/26] Update version number --- database.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database.sql b/database.sql index 0b0607e62e..4d807a6fdc 100644 --- a/database.sql +++ b/database.sql @@ -1,6 +1,6 @@ -- ------------------------------------------ -- Friendica 2018.08-dev (The Tazmans Flax-lily) --- DB_UPDATE_VERSION 1275 +-- DB_UPDATE_VERSION 1276 -- ------------------------------------------ From 5ee2db8a164ae27e2a5821ff4036f3d686b0c40e Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 8 Jul 2018 04:55:45 +0000 Subject: [PATCH 22/26] Avoid undefined index --- src/Model/Item.php | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 3086a869be..6fff08c715 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -181,7 +181,7 @@ class Item extends BaseObject if (array_key_exists('object-type', $row)) { $row['object-type'] = ACTIVITY_OBJ_NOTE; } - } elseif (in_array($row['verb'], ['', ACTIVITY_POST, ACTIVITY_SHARE])) { + } elseif (array_key_exists('verb', $row) && in_array($row['verb'], ['', ACTIVITY_POST, ACTIVITY_SHARE])) { // Posts don't have an object or target - but having tags or files. // We safe some performance by building tag and file strings only here. // We remove object and target since they aren't used for this type. @@ -191,15 +191,16 @@ class Item extends BaseObject if (array_key_exists('target', $row)) { $row['target'] = ''; } - // Build the tag string out of the term entries - if (array_key_exists('tag', $row) && empty($row['tag'])) { - $row['tag'] = Term::tagTextFromItemId($row['internal-iid']); - } + } - // Build the file string out of the term entries - if (array_key_exists('file', $row) && empty($row['file'])) { - $row['file'] = Term::fileTextFromItemId($row['internal-iid']); - } + // Build the tag string out of the term entries + if (array_key_exists('tag', $row) && empty($row['tag'])) { + $row['tag'] = Term::tagTextFromItemId($row['internal-iid']); + } + + // Build the file string out of the term entries + if (array_key_exists('file', $row) && empty($row['file'])) { + $row['file'] = Term::fileTextFromItemId($row['internal-iid']); } // Remove internal fields From 271b6cb8f93e6827e950de2ea99bc2403ab099f9 Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 8 Jul 2018 05:10:45 +0000 Subject: [PATCH 23/26] Hopefully this fixes the tests --- src/Object/Image.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Object/Image.php b/src/Object/Image.php index 9692b84715..9621badaba 100644 --- a/src/Object/Image.php +++ b/src/Object/Image.php @@ -781,9 +781,12 @@ class Image $img_str = Network::fetchUrl($url, true, $redirects, 4); $filesize = strlen($img_str); - if (function_exists("getimagesizefromstring")) { - $data = getimagesizefromstring($img_str); - } else { + // The tests are failing with an read error. This can be caused by memory shortage + // See https://stackoverflow.com/questions/10175758/getimagesize-read-error + // So we use the alternate method instead + //if (function_exists("getimagesizefromstring")) { + // $data = getimagesizefromstring($img_str); + //} else { $tempfile = tempnam(get_temppath(), "cache"); $a = get_app(); @@ -793,7 +796,7 @@ class Image $data = getimagesize($tempfile); unlink($tempfile); - } + //} if ($data) { $data["size"] = $filesize; From 69db696ab1b5d505394fafae1221d0da844a317f Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 8 Jul 2018 05:29:06 +0000 Subject: [PATCH 24/26] Improve test behaviour --- include/dba.php | 2 ++ src/App.php | 12 +++++++++--- src/Object/Image.php | 29 +++++++++++++++-------------- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/include/dba.php b/include/dba.php index 061f5399c7..b95589970e 100644 --- a/include/dba.php +++ b/include/dba.php @@ -955,6 +955,8 @@ class dba { * @return boolean Was the command executed successfully? */ public static function rollback() { + $ret = false; + switch (self::$driver) { case 'pdo': if (!self::$db->inTransaction()) { diff --git a/src/App.php b/src/App.php index 92ff057138..a2bdd03df6 100644 --- a/src/App.php +++ b/src/App.php @@ -412,11 +412,17 @@ class App public function set_baseurl($url) { $parsed = @parse_url($url); + $hostname = ''; if (x($parsed)) { - $this->scheme = $parsed['scheme']; + if (!empty($parsed['scheme'])) { + $this->scheme = $parsed['scheme']; + } + + if (!empty($parsed['host'])) { + $hostname = $parsed['host']; + } - $hostname = $parsed['host']; if (x($parsed, 'port')) { $hostname .= ':' . $parsed['port']; } @@ -432,7 +438,7 @@ class App $this->hostname = Config::get('config', 'hostname'); } - if (!isset($this->hostname) || ( $this->hostname == '')) { + if (!isset($this->hostname) || ($this->hostname == '')) { $this->hostname = $hostname; } } diff --git a/src/Object/Image.php b/src/Object/Image.php index 9621badaba..7e6b758f1c 100644 --- a/src/Object/Image.php +++ b/src/Object/Image.php @@ -781,22 +781,23 @@ class Image $img_str = Network::fetchUrl($url, true, $redirects, 4); $filesize = strlen($img_str); - // The tests are failing with an read error. This can be caused by memory shortage - // See https://stackoverflow.com/questions/10175758/getimagesize-read-error - // So we use the alternate method instead - //if (function_exists("getimagesizefromstring")) { - // $data = getimagesizefromstring($img_str); - //} else { - $tempfile = tempnam(get_temppath(), "cache"); + try { + if (function_exists("getimagesizefromstring")) { + $data = getimagesizefromstring($img_str); + } else { + $tempfile = tempnam(get_temppath(), "cache"); - $a = get_app(); - $stamp1 = microtime(true); - file_put_contents($tempfile, $img_str); - $a->save_timestamp($stamp1, "file"); + $a = get_app(); + $stamp1 = microtime(true); + file_put_contents($tempfile, $img_str); + $a->save_timestamp($stamp1, "file"); - $data = getimagesize($tempfile); - unlink($tempfile); - //} + $data = getimagesize($tempfile); + unlink($tempfile); + } + } catch (Exception $e) { + return false; + } if ($data) { $data["size"] = $filesize; From c2d4b557aeb4b6eb9eee2a6b65b18ab7bf6efe39 Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 8 Jul 2018 05:44:35 +0000 Subject: [PATCH 25/26] Only build tag and file when really needed --- src/Model/Item.php | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index b5903b9999..8a96e54405 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -189,14 +189,16 @@ class Item extends BaseObject } } - // Build the tag string out of the term entries - if (array_key_exists('tag', $row) && empty($row['tag'])) { - $row['tag'] = Term::tagTextFromItemId($row['internal-iid']); - } + if (!array_key_exists('verb', $row) || in_array($row['verb'], ['', ACTIVITY_POST, ACTIVITY_SHARE])) { + // Build the tag string out of the term entries + if (array_key_exists('tag', $row) && empty($row['tag'])) { + $row['tag'] = Term::tagTextFromItemId($row['internal-iid']); + } - // Build the file string out of the term entries - if (array_key_exists('file', $row) && empty($row['file'])) { - $row['file'] = Term::fileTextFromItemId($row['internal-iid']); + // Build the file string out of the term entries + if (array_key_exists('file', $row) && empty($row['file'])) { + $row['file'] = Term::fileTextFromItemId($row['internal-iid']); + } } // Remove internal fields From c25c3b5981787df9a2de9ef85d76f9934440f2d7 Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 8 Jul 2018 08:32:50 +0000 Subject: [PATCH 26/26] urgent bugfix: Wrong function name for locks --- src/Model/Item.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 8a96e54405..162387185e 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1731,7 +1731,7 @@ class Item extends BaseObject } // To avoid timing problems, we are using locks. - $locked = Lock::set('item_insert_activity'); + $locked = Lock::acquire('item_insert_activity'); if (!$locked) { logger("Couldn't acquire lock for URI " . $item['uri'] . " - proceeding anyway."); } @@ -1751,7 +1751,7 @@ class Item extends BaseObject return false; } if ($locked) { - Lock::remove('item_insert_activity'); + Lock::release('item_insert_activity'); } return true; }