Merge pull request #9959 from annando/more-database-stuff

Fixes several database errors, removes "relation" handling

src/Core/Hook.php

@@ -218,7 +218,7 @@ class Hook
 		} else {
 			// remove orphan hooks
 			$condition = ['hook' => $name, 'file' => $hook[0], 'function' => $hook[1]];
-			self::delete($condition, ['cascade' => false]);
+			self::delete($condition);
 		}
 	}
 
@@ -250,13 +250,12 @@ class Hook
 	 * We have to clear the cached routerDispatchData because addons can provide routes
 	 *
 	 * @param array $condition
-	 * @param array $options
 	 * @return bool
 	 * @throws \Exception
 	 */
-	public static function delete(array $condition, array $options = [])
+	public static function delete(array $condition)
 	{
-		$result = DBA::delete('hook', $condition, $options);
+		$result = DBA::delete('hook', $condition);
 
 		if ($result) {
 			DI::cache()->delete('routerDispatchData');

src/Database/DBA.php

@@ -388,9 +388,6 @@ class DBA
 	 *
 	 * @param string|array $table      Table name
 	 * @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

src/Database/Database.php

@@ -1217,175 +1217,29 @@ class Database
 		return $ret;
 	}
 
-	/**
-	 * Build the array with the table relations
-	 *
-	 * The array is build from the database definitions in DBStructure.php
-	 *
-	 * This process must only be started once, since the value is cached.
-	 */
-	private function buildRelationData()
-	{
-		$definition = DBStructure::definition($this->configCache->get('system', 'basepath'));
-
-		foreach ($definition AS $table => $structure) {
-			foreach ($structure['fields'] AS $field => $field_struct) {
-				if (isset($field_struct['relation'])) {
-					foreach ($field_struct['relation'] AS $rel_table => $rel_field) {
-						$this->relation[$rel_table][$rel_field][$table][] = $field;
-					}
-				}
-			}
-		}
-	}
-
 	/**
 	 * Delete a row from a table
 	 *
-	 * Note: this methods does NOT accept schema => table arrays because of the complex relation stuff.
-	 *
 	 * @param string $table      Table name
 	 * @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)
-	 * @param array  $callstack  Internal use: prevent endless loops
 	 *
 	 * @return boolean was the delete successful?
 	 * @throws \Exception
 	 */
-	public function delete($table, array $conditions, array $options = [], array &$callstack = [])
+	public function delete($table, array $conditions)
 	{
 		if (empty($table) || empty($conditions)) {
 			$this->logger->info('Table and conditions have to be set');
 			return false;
 		}
 
-		$commands = [];
+		$table_string = DBA::buildTableString($table);
 
-		// Create a key for the loop prevention
+		$condition_string = DBA::buildCondition($conditions);
-		$key = $table . ':' . json_encode($conditions);
 
-		// We quit when this key already exists in the callstack.
+		$sql = "DELETE FROM " . $table_string . " " . $condition_string;
-		if (isset($callstack[$key])) {
+		$this->logger->debug($this->replaceParameters($sql, $conditions), ['callstack' => System::callstack(6)]);
-			return true;
+		return $this->e($sql, $conditions);
-		}
-
-		$callstack[$key] = true;
-
-		$commands[$key] = ['table' => $table, 'conditions' => $conditions];
-
-		// Don't use "defaults" here, since it would set "false" to "true"
-		if (isset($options['cascade'])) {
-			$cascade = $options['cascade'];
-		} else {
-			$cascade = true;
-		}
-
-		// To speed up the whole process we cache the table relations
-		if ($cascade && count($this->relation) == 0) {
-			$this->buildRelationData();
-		}
-
-		// Is there a relation entry for the table?
-		if ($cascade && isset($this->relation[$table])) {
-			// We only allow a simple "one field" relation.
-			$field   = array_keys($this->relation[$table])[0];
-			$rel_def = array_values($this->relation[$table])[0];
-
-			// Create a key for preventing double queries
-			$qkey = $field . '-' . $table . ':' . json_encode($conditions);
-
-			// When the search field is the relation field, we don't need to fetch the rows
-			// This is useful when the leading record is already deleted in the frontend but the rest is done in the backend
-			if ((count($conditions) == 1) && ($field == array_keys($conditions)[0])) {
-				foreach ($rel_def AS $rel_table => $rel_fields) {
-					foreach ($rel_fields AS $rel_field) {
-						$this->delete($rel_table, [$rel_field => array_values($conditions)[0]], $options, $callstack);
-					}
-				}
-				// We quit when this key already exists in the callstack.
-			} elseif (!isset($callstack[$qkey])) {
-				$callstack[$qkey] = true;
-
-				// Fetch all rows that are to be deleted
-				$data = $this->select($table, [$field], $conditions);
-
-				while ($row = $this->fetch($data)) {
-					$this->delete($table, [$field => $row[$field]], $options, $callstack);
-				}
-
-				$this->close($data);
-
-				// Since we had split the delete command we don't need the original command anymore
-				unset($commands[$key]);
-			}
-		}
-
-		// Now we finalize the process
-		$do_transaction = !$this->in_transaction;
-
-		if ($do_transaction) {
-			$this->transaction();
-		}
-
-		$compacted = [];
-		$counter   = [];
-
-		foreach ($commands AS $command) {
-			$conditions = $command['conditions'];
-			reset($conditions);
-			$first_key = key($conditions);
-
-			$condition_string = DBA::buildCondition($conditions);
-
-			if ((count($command['conditions']) > 1) || is_int($first_key)) {
-				$sql = "DELETE FROM " . DBA::quoteIdentifier($command['table']) . " " . $condition_string;
-				$this->logger->info($this->replaceParameters($sql, $conditions), ['callstack' => System::callstack(6), 'internal_callstack' => $callstack]);
-
-				if (!$this->e($sql, $conditions)) {
-					if ($do_transaction) {
-						$this->rollback();
-					}
-					return false;
-				}
-			} else {
-				$key_table     = $command['table'];
-				$key_condition = array_keys($command['conditions'])[0];
-				$value         = array_values($command['conditions'])[0];
-
-				// Split the SQL queries in chunks of 100 values
-				// We do the $i stuff here to make the code better readable
-				$i = isset($counter[$key_table][$key_condition]) ? $counter[$key_table][$key_condition] : 0;
-				if (isset($compacted[$key_table][$key_condition][$i]) && count($compacted[$key_table][$key_condition][$i]) > 100) {
-					++$i;
-				}
-
-				$compacted[$key_table][$key_condition][$i][$value] = $value;
-				$counter[$key_table][$key_condition]               = $i;
-			}
-		}
-		foreach ($compacted AS $table => $values) {
-			foreach ($values AS $field => $field_value_list) {
-				foreach ($field_value_list AS $field_values) {
-					$sql = "DELETE FROM " . DBA::quoteIdentifier($table) . " WHERE " . DBA::quoteIdentifier($field) . " IN (" .
-					       substr(str_repeat("?, ", count($field_values)), 0, -2) . ");";
-
-					$this->logger->info($this->replaceParameters($sql, $field_values), ['callstack' => System::callstack(6), 'internal_callstack' => $callstack]);
-
-					if (!$this->e($sql, $field_values)) {
-						if ($do_transaction) {
-							$this->rollback();
-						}
-						return false;
-					}
-				}
-			}
-		}
-		if ($do_transaction) {
-			$this->commit();
-		}
-		return true;
 	}
 
 	/**

src/Model/Event.php

@@ -242,7 +242,7 @@ class Event
 			return;
 		}
 
-		DBA::delete('event', ['id' => $event_id], ['cascade' => false]);
+		DBA::delete('event', ['id' => $event_id]);
 		Logger::log("Deleted event ".$event_id, Logger::DEBUG);
 	}
 

src/Model/Item.php

@@ -318,14 +318,6 @@ class Item
 
 		Post\DeliveryData::delete($item['uri-id']);
 
-		// When the permission set will be used in photo and events as well,
-		// this query here needs to be extended.
-		// @todo Currently deactivated. We need the permission set in the deletion process.
-		// This is a reminder to add the removal somewhere else.
-		//if (!empty($item['psid']) && !self::exists(['psid' => $item['psid'], 'deleted' => false])) {
-		//	DBA::delete('permissionset', ['id' => $item['psid']], ['cascade' => false]);
-		//}
-
 		// If it's the parent of a comment thread, kill all the kids
 		if ($item['gravity'] == GRAVITY_PARENT) {
 			self::markForDeletion(['parent' => $item['parent'], 'deleted' => false], $priority);

src/Module/Conversation/Community.php

@@ -332,7 +332,7 @@ class Community extends BaseModule
 		$params = ['order' => ['commented' => true], 'limit' => $itemspage];
 
 		if (!empty($item_id)) {
-			$condition[0] .= " AND `iid` = ?";
+			$condition[0] .= " AND `id` = ?";
 			$condition[] = $item_id;
 		} else {
 			if (local_user() && !empty($_REQUEST['no_sharer'])) {

src/Worker/Delivery.php

@@ -183,7 +183,7 @@ class Delivery
 		}
 
 		if (empty($items)) {
-			Logger::log('No delivery data for  ' . $cmd . ' - Item ID: ' .$target_id . ' - Contact ID: ' . $contact_id);
+			Logger::notice('No delivery data', ['command' => $cmd, 'uri-id' => $post_uriid, 'cid' => $contact_id]);
 		}
 
 		$owner = Model\User::getOwnerDataById($uid);
@@ -221,7 +221,7 @@ class Delivery
 			$contact['network'] = Protocol::DFRN;
 		}
 
-		Logger::notice('Delivering', ['cmd' => $cmd, 'target' => $target_id, 'followup' => $followup, 'network' => $contact['network']]);
+		Logger::notice('Delivering', ['cmd' => $cmd, 'uri-id' => $post_uriid, 'followup' => $followup, 'network' => $contact['network']]);
 
 		switch ($contact['network']) {
 			case Protocol::DFRN:

src/Worker/MergeContact.php

@@ -23,6 +23,7 @@ namespace Friendica\Worker;
 
 use Friendica\Core\Logger;
 use Friendica\Database\DBA;
+use Friendica\Database\DBStructure;
 use Friendica\Model\Post;
 
 class MergeContact
@@ -48,6 +49,12 @@ class MergeContact
 		DBA::update('mail', ['contact-id' => $new_cid], ['contact-id' => $old_cid]);
 		DBA::update('photo', ['contact-id' => $new_cid], ['contact-id' => $old_cid]);
 		DBA::update('event', ['cid' => $new_cid], ['cid' => $old_cid]);
+		if (DBStructure::existsTable('item')) {
+			DBA::update('item', ['contact-id' => $new_cid], ['contact-id' => $old_cid]);
+		}
+		if (DBStructure::existsTable('thread')) {
+			DBA::update('thread', ['contact-id' => $new_cid], ['contact-id' => $old_cid]);
+		}
 
 		// These fields only contain public contact entries (uid = 0)
 		if ($uid == 0) {
@@ -56,6 +63,16 @@ class MergeContact
 			Post::update(['author-id' => $new_cid], ['author-id' => $old_cid]);
 			Post::update(['owner-id' => $new_cid], ['owner-id' => $old_cid]);
 			Post::update(['causer-id' => $new_cid], ['causer-id' => $old_cid]);
+			if (DBStructure::existsTable('item')) {
+				DBA::update('item', ['author-id' => $new_cid], ['author-id' => $old_cid]);
+				DBA::update('item', ['owner-id' => $new_cid], ['owner-id' => $old_cid]);
+				DBA::update('item', ['causer-id' => $new_cid], ['causer-id' => $old_cid]);
+			}
+			if (DBStructure::existsTable('thread')) {
+				DBA::update('thread', ['author-id' => $new_cid], ['author-id' => $old_cid]);
+				DBA::update('thread', ['owner-id' => $new_cid], ['owner-id' => $old_cid]);
+				DBA::update('thread', ['causer-id' => $new_cid], ['causer-id' => $old_cid]);
+			}
 		} else {
 			/// @todo Check if some other data needs to be adjusted as well, possibly the "rel" status?
 		}

src/Worker/RemoveContact.php

@@ -54,6 +54,8 @@ class RemoveContact {
 			DBA::delete('item', ['contact-id' => $id]);
 		}
 
+		DBA::delete('mail', ['contact-id' => $id]);
+
 		Post\ThreadUser::delete(['author-id' => $id]);
 		Post\ThreadUser::delete(['owner-id' => $id]);
 		Post\ThreadUser::delete(['causer-id' => $id]);

static/dbstructure.config.php

@@ -46,7 +46,7 @@
  * ],
  *
  * Whenever possible prefer "foreign" before "relation" with the foreign keys.
- * "foreign" adds true foreign keys on the database level, while "relation" simulates this behaviour.
+ * "foreign" adds true foreign keys on the database level, while "relation" is just an indicator of a table relation without any consequences
  *
  * If you need to make any change, make sure to increment the DB_UPDATE_VERSION constant value below.
  *