We now store the item delivery data in a separate table (#5413)

* We now store the item delivery data in a separate table

* Reorganized table structure

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',      1278);
+define('DB_UPDATE_VERSION',      1279);
 define('NEW_UPDATE_ROUTINE_VERSION', 1170);
 
 /**

database.sql

@@ -1,6 +1,6 @@
 -- ------------------------------------------
 -- Friendica 2018.08-dev (The Tazmans Flax-lily)
--- DB_UPDATE_VERSION 1278
+-- DB_UPDATE_VERSION 1279
 -- ------------------------------------------
 
 
@@ -491,11 +491,11 @@ CREATE TABLE IF NOT EXISTS `item` (
 	`allow_gid` mediumtext COMMENT 'Access Control - list of allowed groups',
 	`deny_cid` mediumtext COMMENT 'Access Control - list of denied contact.id',
 	`deny_gid` mediumtext COMMENT 'Access Control - list of denied groups',
-	`postopts` text COMMENT 'External post connectors add their network name to this comma-separated string to identify that they should be delivered to these networks during delivery',
-	`inform` mediumtext COMMENT 'Additional receivers of this post',
 	`resource-id` varchar(32) NOT NULL DEFAULT '' COMMENT 'Used to link other tables to items, it identifies the linked resource (e.g. photo) and if set must also set resource_type',
 	`event-id` int unsigned NOT NULL DEFAULT 0 COMMENT 'Used to link to the event.id',
 	`attach` mediumtext COMMENT 'JSON structure representing attachments to this item',
+	`postopts` text COMMENT 'Deprecated',
+	`inform` mediumtext COMMENT 'Deprecated',
 	`type` varchar(20) COMMENT 'Deprecated',
 	`bookmark` boolean COMMENT 'Deprecated',
 	`file` mediumtext COMMENT 'Deprecated',
@@ -588,6 +588,16 @@ CREATE TABLE IF NOT EXISTS `item-content` (
 	 INDEX `uri` (`uri`(191))
 ) DEFAULT COLLATE utf8mb4_general_ci COMMENT='Content for all posts';
 
+--
+-- TABLE item-delivery-data
+--
+CREATE TABLE IF NOT EXISTS `item-delivery-data` (
+	`iid` int unsigned NOT NULL COMMENT 'Item id',
+	`postopts` text COMMENT 'External post connectors add their network name to this comma-separated string to identify that they should be delivered to these networks during delivery',
+	`inform` mediumtext COMMENT 'Additional receivers of the linked item',
+	 PRIMARY KEY(`iid`)
+) DEFAULT COLLATE utf8mb4_general_ci COMMENT='Delivery data for items';
+
 --
 -- TABLE locks
 --

src/Database/DBStructure.php

@@ -1196,15 +1196,14 @@ class DBStructure
 						"allow_gid" => ["type" => "mediumtext", "comment" => "Access Control - list of allowed groups"],
 						"deny_cid" => ["type" => "mediumtext", "comment" => "Access Control - list of denied contact.id"],
 						"deny_gid" => ["type" => "mediumtext", "comment" => "Access Control - list of denied groups"],
-						// These fields will be moved into some item-delivery-information table
-						"postopts" => ["type" => "text", "comment" => "External post connectors add their network name to this comma-separated string to identify that they should be delivered to these networks during delivery"],
-						"inform" => ["type" => "mediumtext", "comment" => "Additional receivers of this post"],
 						// It is to be decided whether these fields belong to the user or the structure
 						"resource-id" => ["type" => "varchar(32)", "not null" => "1", "default" => "", "comment" => "Used to link other tables to items, it identifies the linked resource (e.g. photo) and if set must also set resource_type"],
 						"event-id" => ["type" => "int unsigned", "not null" => "1", "default" => "0", "relation" => ["event" => "id"], "comment" => "Used to link to the event.id"],
 						// Could possibly be replaced by the "attach" table?
 						"attach" => ["type" => "mediumtext", "comment" => "JSON structure representing attachments to this item"],
 						// Deprecated fields. Will be removed in upcoming versions
+						"postopts" => ["type" => "text", "comment" => "Deprecated"],
+						"inform" => ["type" => "mediumtext", "comment" => "Deprecated"],
 						"type" => ["type" => "varchar(20)", "comment" => "Deprecated"],
 						"bookmark" => ["type" => "boolean", "comment" => "Deprecated"],
 						"file" => ["type" => "mediumtext", "comment" => "Deprecated"],
@@ -1301,6 +1300,17 @@ class DBStructure
 						"uri" => ["uri(191)"],
 						]
 				];
+		$database["item-delivery-data"] = [
+				"comment" => "Delivery data for items",
+				"fields" => [
+						"iid" => ["type" => "int unsigned", "not null" => "1", "primary" => "1", "relation" => ["item" => "id"], "comment" => "Item id"],
+						"postopts" => ["type" => "text", "comment" => "External post connectors add their network name to this comma-separated string to identify that they should be delivered to these networks during delivery"],
+						"inform" => ["type" => "mediumtext", "comment" => "Additional receivers of the linked item"],
+						],
+				"indexes" => [
+						"PRIMARY" => ["iid"],
+						]
+				];
 		$database["locks"] = [
 				"comment" => "",
 				"fields" => [

src/Database/PostUpdate.php

@@ -32,7 +32,7 @@ class PostUpdate
 		if (!self::update1206()) {
 			return;
 		}
-		if (!self::update1278()) {
+		if (!self::update1279()) {
 			return;
 		}
 	}
@@ -228,20 +228,21 @@ class PostUpdate
 	 *
 	 * @return bool "true" when the job is done
 	 */
-	private static function update1278()
+	private static function update1279()
 	{
 		// Was the script completed?
-		if (Config::get("system", "post_update_version") >= 1278) {
+		if (Config::get("system", "post_update_version") >= 1279) {
 			return true;
 		}
 
-		$id = Config::get("system", "post_update_version_1278_id", 0);
+		$id = Config::get("system", "post_update_version_1279_id", 0);
 
 		logger("Start from item " . $id, LOGGER_DEBUG);
 
 		$fields = array_merge(Item::MIXED_CONTENT_FIELDLIST, ['network', 'author-id', 'owner-id', 'tag', 'file',
 			'author-name', 'author-avatar', 'author-link', 'owner-name', 'owner-avatar', 'owner-link', 'id',
-			'uid', 'allow_cid', 'allow_gid', 'deny_cid', 'deny_gid', 'psid', 'post-type', 'bookmark', 'type']);
+			'uid', 'allow_cid', 'allow_gid', 'deny_cid', 'deny_gid', 'psid', 'post-type', 'bookmark', 'type',
+			'inform']);
 
 		$start_id = $id;
 		$rows = 0;
@@ -285,12 +286,12 @@ class PostUpdate
 		}
 		dba::close($items);
 
-		Config::set("system", "post_update_version_1278_id", $id);
+		Config::set("system", "post_update_version_1279_id", $id);
 
 		logger("Processed rows: " . $rows . " - last processed item:  " . $id, LOGGER_DEBUG);
 
 		if ($start_id == $id) {
-			Config::set("system", "post_update_version", 1278);
+			Config::set("system", "post_update_version", 1279);
 			logger("Done", LOGGER_DEBUG);
 			return true;
 		}

src/Model/Item.php

@@ -73,6 +73,9 @@ class Item extends BaseObject
 	// Field list for "item-content" table that is not present in the "item" table
 	const CONTENT_FIELDLIST = ['language'];
 
+	// Field list for additional delivery data
+	const DELIVERY_DATA_FIELDLIST = ['postopts', 'inform'];
+
 	// 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', 'iaid', 'psid',
@@ -95,7 +98,7 @@ class Item extends BaseObject
 	public static function isLegacyMode()
 	{
 		if (is_null(self::$legacy_mode)) {
-			self::$legacy_mode = (Config::get("system", "post_update_version") < 1276);
+			self::$legacy_mode = (Config::get("system", "post_update_version") < 1279);
 		}
 
 		return self::$legacy_mode;
@@ -180,7 +183,8 @@ class Item extends BaseObject
 
 		// Fetch data from the item-content table whenever there is content there
 		if (self::isLegacyMode()) {
-			foreach (self::MIXED_CONTENT_FIELDLIST as $field) {
+			$legacy_fields = array_merge(self::DELIVERY_DATA_FIELDLIST, self::MIXED_CONTENT_FIELDLIST);
+			foreach ($legacy_fields as $field) {
 				if (empty($row[$field]) && !empty($row['internal-item-' . $field])) {
 					$row[$field] = $row['internal-item-' . $field];
 				}
@@ -512,8 +516,8 @@ class Item extends BaseObject
 
 		$fields['item'] = ['id', 'uid', 'parent', 'uri', 'parent-uri', 'thr-parent', 'guid',
 			'contact-id', 'owner-id', 'author-id', 'type', 'wall', 'gravity', 'extid',
-			'created', 'edited', 'commented', 'received', 'changed', 'postopts',
+			'created', 'edited', 'commented', 'received', 'changed',
-			'resource-id', 'event-id', 'tag', 'attach', 'inform', 'post-type',
+			'resource-id', 'event-id', 'tag', 'attach', 'post-type',
 			'file', 'allow_cid', 'allow_gid', 'deny_cid', 'deny_gid', 'psid',
 			'private', 'pubmail', 'moderated', 'visible', 'starred', 'bookmark',
 			'unseen', 'deleted', 'origin', 'forum_mode', 'mention', 'global',
@@ -525,6 +529,8 @@ class Item extends BaseObject
 
 		$fields['item-content'] = array_merge(self::CONTENT_FIELDLIST, self::MIXED_CONTENT_FIELDLIST);
 
+		$fields['item-delivery-data'] = self::DELIVERY_DATA_FIELDLIST;
+
 		$fields['author'] = ['url' => 'author-link', 'name' => 'author-name',
 			'thumb' => 'author-avatar', 'nick' => 'author-nick', 'network' => 'author-network'];
 
@@ -634,6 +640,10 @@ class Item extends BaseObject
 			$joins .= " LEFT JOIN `item-content` ON `item-content`.`id` = `item`.`icid`";
 		}
 
+		if (strpos($sql_commands, "`item-delivery-data`.") !== false) {
+			$joins .= " LEFT JOIN `item-delivery-data` ON `item-delivery-data`.`iid` = `item`.`id`";
+		}
+
 		if ((strpos($sql_commands, "`parent-item`.") !== false) || (strpos($sql_commands, "`parent-author`.") !== false)) {
 			$joins .= " STRAIGHT_JOIN `item` AS `parent-item` ON `parent-item`.`id` = `item`.`parent`";
 		}
@@ -670,7 +680,8 @@ class Item extends BaseObject
 		foreach ($fields as $table => $table_fields) {
 			foreach ($table_fields as $field => $select) {
 				if (empty($selected) || in_array($select, $selected)) {
-					if (self::isLegacyMode() && in_array($select, self::MIXED_CONTENT_FIELDLIST)) {
+					$legacy_fields = array_merge(self::DELIVERY_DATA_FIELDLIST, self::MIXED_CONTENT_FIELDLIST);
+					if (self::isLegacyMode() && in_array($select, $legacy_fields)) {
 						$selection[] = "`item`.`".$select."` AS `internal-item-" . $select . "`";
 					}
 					if (is_int($field)) {
@@ -783,6 +794,12 @@ class Item extends BaseObject
 			$files = '';
 		}
 
+		$delivery_data = ['postopts' => defaults($fields, 'postopts', ''),
+			'inform' => defaults($fields, 'inform', '')];
+
+		$fields['postopts'] = null;
+		$fields['inform'] = null;
+
 		if (!empty($fields)) {
 			$success = dba::update('item', $fields, $condition);
 
@@ -797,6 +814,7 @@ class Item extends BaseObject
 		$rows = dba::affected_rows();
 
 		while ($item = dba::fetch($items)) {
+
 			// This part here can safely be removed when the legacy fields in the item had been removed
 			if (empty($item['uri-hash']) && !empty($item['uri']) && !empty($item['created'])) {
 
@@ -887,6 +905,8 @@ class Item extends BaseObject
 				}
 			}
 
+			self::updateDeliveryData($item['id'], $delivery_data);
+
 			self::updateThread($item['id']);
 
 			// We only need to notfiy others when it is an original entry from us.
@@ -1033,6 +1053,8 @@ class Item extends BaseObject
 			self::delete(['uri' => $item['uri'], 'uid' => 0, 'deleted' => false], $priority);
 		}
 
+		dba::delete('item-delivery-data', ['iid' => $item['id']]);
+
 		// If it's the parent of a comment thread, kill all the kids
 		if ($item['id'] == $item['parent']) {
 			self::delete(['parent' => $item['parent'], 'deleted' => false], $priority);
@@ -1626,6 +1648,12 @@ class Item extends BaseObject
 			self::insertContent($item);
 		}
 
+		$delivery_data = ['postopts' => defaults($item, 'postopts', ''),
+			'inform' => defaults($item, 'inform', '')];
+
+		unset($item['postopts']);
+		unset($item['inform']);
+
 		dba::transaction();
 		$ret = dba::insert('item', $item);
 
@@ -1650,6 +1678,10 @@ class Item extends BaseObject
 			$spoolpath = get_spoolpath();
 			if ($spoolpath != "") {
 				$spool = $spoolpath.'/'.$file;
+
+				// Ensure to have the removed data from above again in the item array
+				$item = array_merge($item, $delivery_data);
+
 				file_put_contents($spool, json_encode($item));
 				logger("Item wasn't stored - Item was spooled into file ".$file, LOGGER_DEBUG);
 			}
@@ -1748,6 +1780,10 @@ class Item extends BaseObject
 			self::updateThread($parent_id);
 		}
 
+		$delivery_data['iid'] = $current_post;
+
+		self::insertDeliveryData($delivery_data);
+
 		dba::commit();
 
 		/*
@@ -1779,6 +1815,35 @@ class Item extends BaseObject
 		return $current_post;
 	}
 
+	/**
+	 * @brief Insert a new item delivery data entry
+	 *
+	 * @param array $item The item fields that are to be inserted
+	 */
+	private static function insertDeliveryData($delivery_data)
+	{
+		if (empty($delivery_data['iid']) || (empty($delivery_data['postopts']) && empty($delivery_data['inform']))) {
+			return;
+		}
+
+		dba::insert('item-delivery-data', $delivery_data);
+	}
+
+	/**
+	 * @brief Update an existing item delivery data entry
+	 *
+	 * @param integer $id The item id that is to be updated
+	 * @param array $item The item fields that are to be inserted
+	 */
+	private static function updateDeliveryData($id, $delivery_data)
+	{
+		if (empty($id) || (empty($delivery_data['postopts']) && empty($delivery_data['inform']))) {
+			return;
+		}
+
+		dba::update('item-delivery-data', $delivery_data, ['iid' => $id], true);
+	}
+
 	/**
 	 * @brief Insert a new item content entry
 	 *
@@ -2072,6 +2137,8 @@ class Item extends BaseObject
 			unset($item['mention']);
 			unset($item['origin']);
 			unset($item['starred']);
+			unset($item['postopts']);
+			unset($item['inform']);
 			if ($item['uri'] == $item['parent-uri']) {
 				$item['contact-id'] = $item['owner-id'];
 			} else {
@@ -2131,6 +2198,8 @@ class Item extends BaseObject
 		unset($item['mention']);
 		unset($item['origin']);
 		unset($item['starred']);
+		unset($item['postopts']);
+		unset($item['inform']);
 		$item['contact-id'] = Contact::getIdForURL($item['author-link']);
 
 		$public_shadow = self::insert($item, false, false, true);