From 9a9674d5fb6d599169ed1fa9a4ad6050a7b37196 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 28 Jan 2021 22:45:54 +0000 Subject: [PATCH 1/3] New "post-user" structure, new update functionality --- src/Model/Item.php | 88 ++++++++------------------ src/Model/Post.php | 113 ++++++++++++++++++++++++++++++++++ src/Model/Post/User.php | 24 +++++++- src/Worker/Expire.php | 4 +- src/Worker/RemoveContact.php | 3 +- static/dbstructure.config.php | 10 +-- static/dbview.config.php | 2 + update.php | 25 ++++++++ 8 files changed, 200 insertions(+), 69 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 7e5ae10e8..39fa8963a 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -196,84 +196,45 @@ class Item return false; } - $data_fields = $fields; - - // To ensure the data integrity we do it in an transaction - DBA::transaction(); - - // 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', 'uid'], $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 (!empty($fields['verb'])) { + $fields['vid'] = Verb::getID($fields['verb']); } - $delivery_data = Post\DeliveryData::extractFields($fields); - - $clear_fields = ['bookmark', 'type', 'author-name', 'author-avatar', 'author-link', 'owner-name', 'owner-avatar', 'owner-link', 'postopts', 'inform']; - foreach ($clear_fields as $field) { - unset($fields[$field]); + $rows = Post::update($fields, $condition); + if (is_bool($rows)) { + return $rows; } - if (array_key_exists('file', $fields)) { - $files = $fields['file']; - unset($fields['file']); - } else { - $files = null; + // We only need to call the line by line update for specific fields + if (empty($fields['body']) && empty($fields['file']) && + empty($fields['attach']) && empty($fields['edited'])) { + return $rows; } - if (!empty($content_fields['verb'])) { - $fields['vid'] = Verb::getID($content_fields['verb']); - } + Logger::info('Updating per single row method', ['fields' => $fields, 'condition' => $condition]); - if (!empty($fields)) { - $success = DBA::update('item', $fields, $condition); - - if (!$success) { - DBA::close($items); - DBA::rollback(); - return false; - } - } - - // When there is no content for the "old" item table, this will count the fetched items - $rows = DBA::affectedRows(); + $items = Post::select(['id', 'origin', 'uri-id', 'uid'], $condition); $notify_items = []; while ($item = DBA::fetch($items)) { - Post\User::update($item['uri-id'], $item['uid'], $data_fields); - - if (empty($content_fields['verb']) || !in_array($content_fields['verb'], self::ACTIVITIES)) { - if (!empty($content_fields['body'])) { - $content_fields['raw-body'] = trim($content_fields['raw-body'] ?? $content_fields['body']); - - // Remove all media attachments from the body and store them in the post-media table - $content_fields['raw-body'] = Post\Media::insertFromBody($item['uri-id'], $content_fields['raw-body']); - $content_fields['raw-body'] = self::setHashtags($content_fields['raw-body']); - } - + if (!empty($fields['body'])) { + $content_fields = ['raw-body' => trim($fields['raw-body'] ?? $fields['body'])]; + + // Remove all media attachments from the body and store them in the post-media table + $content_fields['raw-body'] = Post\Media::insertFromBody($item['uri-id'], $content_fields['raw-body']); + $content_fields['raw-body'] = self::setHashtags($content_fields['raw-body']); self::updateContent($content_fields, ['uri-id' => $item['uri-id']]); } - if (!is_null($files)) { - Post\Category::storeTextByURIId($item['uri-id'], $item['uid'], $files); + if (!empty($fields['file'])) { + Post\Category::storeTextByURIId($item['uri-id'], $item['uid'], $fields['file']); } if (!empty($fields['attach'])) { Post\Media::insertFromAttachment($item['uri-id'], $fields['attach']); } - Post\DeliveryData::update($item['uri-id'], $delivery_data); - - self::updateThread($item['id']); - // We only need to notfiy others when it is an original entry from us. // Only call the notifier when the item has some content relevant change. if ($item['origin'] && in_array('edited', array_keys($fields))) { @@ -282,7 +243,6 @@ class Item } DBA::close($items); - DBA::commit(); foreach ($notify_items as $notify_item) { Worker::add(PRIORITY_HIGH, "Notifier", Delivery::POST, $notify_item); @@ -1140,7 +1100,8 @@ class Item Tag::storeFromBody($item['uri-id'], $body); } - if (Post\User::insert($item['uri-id'], $item['uid'], $item)) { + $id = Post\User::insert($item['uri-id'], $item['uid'], $item); + if ($id) { // Remove all fields that aren't part of the item table foreach ($item as $field => $value) { if (!in_array($field, $structure['item'])) { @@ -1148,6 +1109,9 @@ class Item } } + // We syncronize the id value of the of the post-user table with the item table + $item['id'] = $id; + $condition = ['uri-id' => $item['uri-id'], 'uid' => $item['uid'], 'network' => $item['network']]; if (Post::exists($condition)) { Logger::notice('Item is already inserted - aborting', $condition); @@ -1991,7 +1955,8 @@ class Item if (($community_page || $prvgroup) && !$item['wall'] && !$item['origin'] && ($item['gravity'] == GRAVITY_PARENT)) { Logger::info('Delete private group/communiy top-level item without mention', ['id' => $item_id, 'guid'=> $item['guid']]); - DBA::delete('item', ['id' => $item_id]); + DBA::delete('item', ['uri-id' => $item['uri-id'], 'uid' => $item['uid']]); + Post\User::delete(['uri-id' => $item['uri-id'], 'uid' => $item['uid']]); return true; } return false; @@ -2670,6 +2635,7 @@ class Item $condition = ["`uri-id` = ? AND NOT `deleted` AND NOT (`uid` IN (?, 0))", $uri_id, $item["uid"]]; if (!Post::exists($condition)) { DBA::delete('item', ['uri-id' => $uri_id, 'uid' => 0]); + Post\User::delete(['uri-id' => $uri_id, 'uid' => 0]); Logger::debug('Deleted shadow item', ['id' => $itemid, 'uri-id' => $uri_id]); } } diff --git a/src/Model/Post.php b/src/Model/Post.php index ec348c60a..b418c30e4 100644 --- a/src/Model/Post.php +++ b/src/Model/Post.php @@ -21,7 +21,9 @@ namespace Friendica\Model; +use Friendica\Core\Logger; use Friendica\Database\DBA; +use Friendica\Database\DBStructure; use Friendica\Protocol\Activity; class Post @@ -400,4 +402,115 @@ class Post return self::selectThreadForUser($uid, $selected, $condition, $params); } + + /** + * Update existing post entries + * + * @param array $fields The fields that are to be changed + * @param array $condition The condition for finding the item entries + * + * A return value of "0" doesn't mean an error - but that 0 rows had been changed. + * + * @return integer|boolean number of affected rows - or "false" if there was an error + * @throws \Friendica\Network\HTTPException\InternalServerErrorException + */ + public static function update(array $fields, array $condition) + { + $affected = 0; + + Logger::info('Start Update', ['fields' => $fields, 'condition' => $condition]); + + // Don't allow changes to fields that are responsible for the relation between the records + unset($fields['id']); + unset($fields['parent']); + unset($fields['uid']); + unset($fields['uri']); + unset($fields['uri-id']); + unset($fields['thr-parent']); + unset($fields['thr-parent-id']); + unset($fields['parent-uri']); + unset($fields['parent-uri-id']); + + // To ensure the data integrity we do it in an transaction + DBA::transaction(); + + $update_fields = DBStructure::getFieldsForTable('post-user', $fields); + if (!empty($update_fields)) { + $rows = DBA::selectToArray('post-view', ['post-user-id'], $condition); + $puids = array_column($rows, 'post-user-id'); + if (!DBA::update('post-user', $update_fields, ['id' => $puids])) { + DBA::rollback(); + Logger::notice('Updating post-user failed', ['fields' => $update_fields, 'condition' => $condition]); + return false; + } + $affected = DBA::affectedRows(); + } + + $update_fields = DBStructure::getFieldsForTable('item-content', $fields); + if (!empty($update_fields)) { + $rows = DBA::selectToArray('post-view', ['uri-id'], $condition, ['group_by' => ['uri-id']]); + $uriids = array_column($rows, 'uri-id'); + if (!DBA::update('item-content', $update_fields, ['uri-id' => $uriids])) { + DBA::rollback(); + Logger::notice('Updating item-content failed', ['fields' => $update_fields, 'condition' => $condition]); + return false; + } + $affected = max($affected, DBA::affectedRows()); + } + + $update_fields = Post\DeliveryData::extractFields($fields); + if (!empty($update_fields)) { + if (empty($uriids)) { + $rows = DBA::selectToArray('post-view', ['uri-id'], $condition, ['group_by' => ['uri-id']]); + $uriids = array_column($rows, 'uri-id'); + } + if (!DBA::update('post-delivery-data', $update_fields, ['uri-id' => $uriids])) { + DBA::rollback(); + Logger::notice('Updating post-delivery-data failed', ['fields' => $update_fields, 'condition' => $condition]); + return false; + } + $affected = max($affected, DBA::affectedRows()); + } + + $update_fields = DBStructure::getFieldsForTable('thread', $fields); + if (!empty($update_fields)) { + $rows = DBA::selectToArray('post-view', ['id'], $condition); + $ids = array_column($rows, 'id'); + if (!DBA::update('thread', $update_fields, ['iid' => $ids])) { + DBA::rollback(); + Logger::notice('Updating thread failed', ['fields' => $update_fields, 'condition' => $condition]); + return false; + } + $affected = max($affected, DBA::affectedRows()); + } + + $item_fields = ['guid', 'type', 'wall', 'gravity', 'extid', 'created', 'edited', 'commented', 'received', 'changed', + 'resource-id', 'post-type', 'private', 'pubmail', 'moderated', 'visible', 'starred', 'bookmark', + 'unseen', 'deleted', 'origin', 'forum_mode', 'mention', 'global', 'network', 'vid', 'psid', + 'contact-id', 'author-id', 'owner-id', 'causer-id', 'event-id']; + + $update_fields = []; + foreach ($item_fields as $field) { + if (array_key_exists($field, $fields)) { + $update_fields[$field] = $fields[$field]; + } + } + if (!empty($update_fields)) { + if (empty($ids)) { + $rows = DBA::selectToArray('post-view', ['id'], $condition, []); + $ids = array_column($rows, 'id'); + } + if (!DBA::update('item', $update_fields, ['id' => $ids])) { + DBA::rollback(); + Logger::notice('Updating item failed', ['fields' => $update_fields, 'condition' => $condition]); + return false; + } + $affected = max($affected, DBA::affectedRows()); + } + + DBA::commit(); + + Logger::info('Updated posts', ['rows' => $affected]); + return $affected; + } } diff --git a/src/Model/Post/User.php b/src/Model/Post/User.php index e5fe96cfb..6c2176656 100644 --- a/src/Model/Post/User.php +++ b/src/Model/Post/User.php @@ -34,7 +34,7 @@ class User * @param integer $uri_id * @param integer $uid * @param array $fields - * @return bool + * @return int ID of inserted post-user * @throws \Exception */ public static function insert(int $uri_id, int $uid, array $data = []) @@ -58,7 +58,11 @@ class User $fields['unseen'] = false; } - return DBA::insert('post-user', $fields, Database::INSERT_IGNORE); + if (!DBA::insert('post-user', $fields, Database::INSERT_IGNORE)) { + return 0; + } + + return DBA::lastInsertId(); } /** @@ -89,4 +93,20 @@ class User return DBA::update('post-user', $fields, ['uri-id' => $uri_id, 'uid' => $uid], $insert_if_missing ? true : []); } + + /** + * Delete a row from the post-user table + * + * @param array $conditions Field condition(s) + * @param array $options + * - cascade: If true we delete records in other tables that depend on the one we're deleting through + * relations (default: true) + * + * @return boolean was the delete successful? + * @throws \Exception + */ + public static function delete(array $conditions, array $options = []) + { + return DBA::delete('post-user', $conditions, $options); + } } diff --git a/src/Worker/Expire.php b/src/Worker/Expire.php index 7ad427d6b..52e0912f5 100644 --- a/src/Worker/Expire.php +++ b/src/Worker/Expire.php @@ -27,6 +27,7 @@ use Friendica\Core\Worker; use Friendica\Database\DBA; use Friendica\DI; use Friendica\Model\Item; +use Friendica\Model\Post; /** * Expires old item entries @@ -43,10 +44,11 @@ class Expire Logger::log('Delete expired items', Logger::DEBUG); // physically remove anything that has been deleted for more than two months $condition = ["`deleted` AND `changed` < UTC_TIMESTAMP() - INTERVAL 60 DAY"]; - $rows = DBA::select('item', ['id', 'guid'], $condition); + $rows = DBA::select('item', ['id', 'guid', 'uri-id', 'uid'], $condition); while ($row = DBA::fetch($rows)) { Logger::info('Delete expired item', ['id' => $row['id'], 'guid' => $row['guid']]); DBA::delete('item', ['id' => $row['id']]); + Post\User::delete(['uri-id' => $row['uri-id'], 'uid' => $row['uid']]); } DBA::close($rows); diff --git a/src/Worker/RemoveContact.php b/src/Worker/RemoveContact.php index 6662e58a5..25d61f555 100644 --- a/src/Worker/RemoveContact.php +++ b/src/Worker/RemoveContact.php @@ -47,10 +47,11 @@ class RemoveContact { $condition = ['uid' => $contact['uid'], 'contact-id' => $id]; } do { - $items = Post::select(['id', 'guid'], $condition, ['limit' => 100]); + $items = Post::select(['id', 'guid', 'uri-id', 'uid'], $condition, ['limit' => 100]); while ($item = Post::fetch($items)) { Logger::info('Delete removed contact item', ['id' => $item['id'], 'guid' => $item['guid']]); DBA::delete('item', ['id' => $item['id']]); + Post\User::delete(['uri-id' => $item['uri-id'], 'uid' => $item['uid']]); } DBA::close($items); } while (Post::exists($condition)); diff --git a/static/dbstructure.config.php b/static/dbstructure.config.php index feba3c920..bda704b09 100644 --- a/static/dbstructure.config.php +++ b/static/dbstructure.config.php @@ -55,7 +55,7 @@ use Friendica\Database\DBA; if (!defined('DB_UPDATE_VERSION')) { - define('DB_UPDATE_VERSION', 1394); + define('DB_UPDATE_VERSION', 1395); } return [ @@ -1204,8 +1204,9 @@ return [ "post-user" => [ "comment" => "User specific post data", "fields" => [ - "uri-id" => ["type" => "int unsigned", "not null" => "1", "primary" => "1", "foreign" => ["item-uri" => "id"], "comment" => "Id of the item-uri table entry that contains the item uri"], - "uid" => ["type" => "mediumint unsigned", "not null" => "1", "primary" => "1", "foreign" => ["user" => "uid"], "comment" => "Owner id which owns this copy of the item"], + "id" => ["type" => "int unsigned", "not null" => "1", "extra" => "auto_increment", "primary" => "1"], + "uri-id" => ["type" => "int unsigned", "not null" => "1", "foreign" => ["item-uri" => "id"], "comment" => "Id of the item-uri table entry that contains the item uri"], + "uid" => ["type" => "mediumint unsigned", "not null" => "1", "foreign" => ["user" => "uid"], "comment" => "Owner id which owns this copy of the item"], "protocol" => ["type" => "tinyint unsigned", "comment" => "Protocol used to deliver the item for this user"], "contact-id" => ["type" => "int unsigned", "not null" => "1", "default" => "0", "foreign" => ["contact" => "id"], "comment" => "contact.id"], "unseen" => ["type" => "boolean", "not null" => "1", "default" => "1", "comment" => "post has not been seen"], @@ -1215,7 +1216,8 @@ return [ "psid" => ["type" => "int unsigned", "foreign" => ["permissionset" => "id", "on delete" => "restrict"], "comment" => "ID of the permission set of this post"], ], "indexes" => [ - "PRIMARY" => ["uid", "uri-id"], + "PRIMARY" => ["id"], + "uid_uri-id" => ["UNIQUE", "uid", "uri-id"], "uri-id" => ["uri-id"], "contact-id" => ["contact-id"], "psid" => ["psid"], diff --git a/static/dbview.config.php b/static/dbview.config.php index b369ee9fa..5d1231d5a 100644 --- a/static/dbview.config.php +++ b/static/dbview.config.php @@ -41,6 +41,7 @@ "fields" => [ "id" => ["item", "id"], "item_id" => ["item", "id"], + "post-user-id" => ["post-user", "id"], "uid" => ["item", "uid"], "parent" => ["item", "parent"], "uri" => ["item", "uri"], @@ -175,6 +176,7 @@ "parent-author-network" => ["parent-item-author", "network"], ], "query" => "FROM `item` + LEFT JOIN `post-user` ON `post-user`.`uri-id` = `item`.`uri-id` AND `post-user`.`uid` = `item`.`uid` STRAIGHT_JOIN `contact` ON `contact`.`id` = `item`.`contact-id` STRAIGHT_JOIN `contact` AS `author` ON `author`.`id` = `item`.`author-id` STRAIGHT_JOIN `contact` AS `owner` ON `owner`.`id` = `item`.`owner-id` diff --git a/update.php b/update.php index 1d4fd3843..e8f8da9fb 100644 --- a/update.php +++ b/update.php @@ -665,3 +665,28 @@ function update_1380() return Update::SUCCESS; } + +function pre_update_1395() +{ + if (DBStructure::existsTable('post-user') && !DBA::e("DROP TABLE `post-user`")) { + return Update::FAILED; + } + return Update::SUCCESS; +} + +function update_1395() +{ + if (!DBA::e("INSERT INTO `post-user`(`id`, `uri-id`, `uid`, `contact-id`, `unseen`, `origin`, `psid`) + SELECT `id`, `uri-id`, `uid`, `contact-id`, `unseen`, `origin`, `psid` FROM `item` + ON DUPLICATE KEY UPDATE `contact-id` = `item`.`contact-id`, `unseen` = `item`.`unseen`, `origin` = `item`.`origin`, `psid` = `item`.`psid`")) { + return Update::FAILED; + } + + if (!DBA::e("INSERT INTO `post-user`(`uri-id`, `uid`, `hidden`, `notification-type`) + SELECT `uri-id`, `user-item`.`uid`, `hidden`,`notification-type` FROM `user-item` + INNER JOIN `item` ON `item`.`id` = `user-item`.`iid` + ON DUPLICATE KEY UPDATE `hidden` = `user-item`.`hidden`, `notification-type` = `user-item`.`notification-type`")) { + return Update::FAILED; + } + return Update::SUCCESS; +} From 57ed388f9ac309a58dfa54b3ff9aa02286d24229 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 28 Jan 2021 22:52:05 +0000 Subject: [PATCH 2/3] Updated database.sql --- database.sql | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/database.sql b/database.sql index c9017fd2e..28a011a21 100644 --- a/database.sql +++ b/database.sql @@ -1,6 +1,6 @@ -- ------------------------------------------ -- Friendica 2021.03-dev (Red Hot Poker) --- DB_UPDATE_VERSION 1394 +-- DB_UPDATE_VERSION 1395 -- ------------------------------------------ @@ -1153,6 +1153,7 @@ CREATE TABLE IF NOT EXISTS `post-tag` ( -- TABLE post-user -- CREATE TABLE IF NOT EXISTS `post-user` ( + `id` int unsigned NOT NULL auto_increment, `uri-id` int unsigned NOT NULL COMMENT 'Id of the item-uri table entry that contains the item uri', `uid` mediumint unsigned NOT NULL COMMENT 'Owner id which owns this copy of the item', `protocol` tinyint unsigned COMMENT 'Protocol used to deliver the item for this user', @@ -1162,7 +1163,8 @@ CREATE TABLE IF NOT EXISTS `post-user` ( `notification-type` tinyint unsigned NOT NULL DEFAULT 0 COMMENT '', `origin` boolean NOT NULL DEFAULT '0' COMMENT 'item originated at this site', `psid` int unsigned COMMENT 'ID of the permission set of this post', - PRIMARY KEY(`uid`,`uri-id`), + PRIMARY KEY(`id`), + UNIQUE INDEX `uid_uri-id` (`uid`,`uri-id`), INDEX `uri-id` (`uri-id`), INDEX `contact-id` (`contact-id`), INDEX `psid` (`psid`), @@ -1493,6 +1495,7 @@ DROP VIEW IF EXISTS `post-view`; CREATE VIEW `post-view` AS SELECT `item`.`id` AS `id`, `item`.`id` AS `item_id`, + `post-user`.`id` AS `post-user-id`, `item`.`uid` AS `uid`, `item`.`parent` AS `parent`, `item`.`uri` AS `uri`, @@ -1626,6 +1629,7 @@ CREATE VIEW `post-view` AS SELECT `parent-item-author`.`name` AS `parent-author-name`, `parent-item-author`.`network` AS `parent-author-network` FROM `item` + LEFT JOIN `post-user` ON `post-user`.`uri-id` = `item`.`uri-id` AND `post-user`.`uid` = `item`.`uid` STRAIGHT_JOIN `contact` ON `contact`.`id` = `item`.`contact-id` STRAIGHT_JOIN `contact` AS `author` ON `author`.`id` = `item`.`author-id` STRAIGHT_JOIN `contact` AS `owner` ON `owner`.`id` = `item`.`owner-id` From 8deabf5c2e2d64f3f6b9a252efbdefb534e205c5 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 28 Jan 2021 23:17:58 +0000 Subject: [PATCH 3/3] Providing more test data --- tests/datasets/api.fixture.php | 53 +++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/tests/datasets/api.fixture.php b/tests/datasets/api.fixture.php index 2e4689dd8..89a3f8dba 100644 --- a/tests/datasets/api.fixture.php +++ b/tests/datasets/api.fixture.php @@ -222,7 +222,58 @@ return [ 'body' => 'Friend user status', 'plink' => 'http://localhost/display/6', ], - ], + ], + 'post-user' => [ + [ + 'id' => 1, + 'uri-id' => 1, + 'uid' => 42, + 'contact-id' => 42, + 'unseen' => 1, + 'origin' => 1, + ], + [ + 'id' => 2, + 'uri-id' => 2, + 'uid' => 42, + 'contact-id' => 42, + 'unseen' => 0, + 'origin' => 1, + ], + [ + 'id' => 3, + 'uri-id' => 3, + 'uid' => 42, + 'contact-id' => 43, + 'unseen' => 0, + 'origin' => 1, + ], + [ + 'id' => 4, + 'uri-id' => 4, + 'uid' => 42, + 'contact-id' => 44, + 'unseen' => 0, + 'origin' => 1, + ], + [ + 'id' => 5, + 'uri-id' => 5, + 'uid' => 42, + 'contact-id' => 42, + 'unseen' => 0, + 'origin' => 1, + ], + [ + 'id' => 6, + 'uri-id' => 6, + 'uid' => 42, + 'contact-id' => 44, + 'unseen' => 0, + 'origin' => 1, + ], + + ], 'item' => [ [ 'id' => 1,