Merge pull request #2836 from Hypolite/Issue-#2816-2

Fix Issue #2816 - Unable to save config items present in .htconfig.php but not in DB

boot.php

@@ -38,7 +38,7 @@ define ( 'FRIENDICA_PLATFORM',     'Friendica');
 define ( 'FRIENDICA_CODENAME',     'Asparagus');
 define ( 'FRIENDICA_VERSION',      '3.5.1-dev' );
 define ( 'DFRN_PROTOCOL_VERSION',  '2.23'    );
-define ( 'DB_UPDATE_VERSION',      1204      );
+define ( 'DB_UPDATE_VERSION',      1205      );
 
 /**
  * @brief Constant with a HTML line break.
@@ -53,7 +53,7 @@ define ( 'ATOM_TIME',              'Y-m-d\TH:i:s\Z' );
 
 /**
  * @brief Image storage quality.
- * 
+ *
  * Lower numbers save space at cost of image detail.
  * For ease of upgrade, please do not change here. Change jpeg quality with
  * $a->config['system']['jpeg_quality'] = n;
@@ -95,7 +95,7 @@ define ( 'DEFAULT_DB_ENGINE',  'MyISAM'  );
 
 /**
  * @name SSL Policy
- * 
+ *
  * SSL redirection policies
  * @{
  */
@@ -106,7 +106,7 @@ define ( 'SSL_POLICY_SELFSIGN',     2 );
 
 /**
  * @name Logger
- * 
+ *
  * log levels
  * @{
  */
@@ -119,7 +119,7 @@ define ( 'LOGGER_ALL',             4 );
 
 /**
  * @name Cache
- * 
+ *
  * Cache levels
  * @{
  */
@@ -131,7 +131,7 @@ define ( 'CACHE_HOUR',             3 );
 
 /**
  * @name Register
- * 
+ *
  * Registration policies
  * @{
  */
@@ -142,7 +142,7 @@ define ( 'REGISTER_OPEN',          2 );
 
 /**
  * @name Contact_is
- * 
+ *
  * Relationship types
  * @{
  */
@@ -153,7 +153,7 @@ define ( 'CONTACT_IS_FRIEND',   3);
 
 /**
  * @name Update
- * 
+ *
  * DB update return values
  * @{
  */
@@ -205,7 +205,7 @@ define ( 'ACCOUNT_TYPE_COMMUNITY',   3 );
 
 /**
  * @name CP
- * 
+ *
  * Type of the community page
  * @{
  */
@@ -216,7 +216,7 @@ define ( 'CP_GLOBAL_COMMUNITY',    1 );
 
 /**
  * @name Network
- * 
+ *
  * Network and protocol family types
  * @{
  */
@@ -288,7 +288,7 @@ define ( 'ZCURL_TIMEOUT' , (-1));
 
 /**
  * @name Notify
- * 
+ *
  * Email notification options
  * @{
  */
@@ -310,7 +310,7 @@ define ( 'NOTIFY_SYSTEM',   0x8000 );
 
 /**
  * @name Term
- * 
+ *
  * Tag/term types
  * @{
  */
@@ -330,7 +330,7 @@ define ( 'TERM_OBJ_PHOTO', 2 );
 
 /**
  * @name Namespaces
- * 
+ *
  * Various namespaces we may need to parse
  * @{
  */
@@ -353,7 +353,7 @@ define ( 'NAMESPACE_ATOM1',           'http://www.w3.org/2005/Atom' );
 
 /**
  * @name Activity
- * 
+ *
  * Activity stream defines
  * @{
  */
@@ -399,7 +399,7 @@ define ( 'ACTIVITY_OBJ_QUESTION', 'http://activityschema.org/object/question' );
 
 /**
  * @name Gravity
- * 
+ *
  * Item weight for query ordering
  * @{
  */
@@ -466,9 +466,9 @@ function startup() {
 /**
  *
  * class: App
- * 
+ *
  * @brief Our main application structure for the life of this page.
- * 
+ *
  * Primarily deals with the URL that got us here
  * and tries to make some sense of it, and
  * stores our page contents and config storage
@@ -1015,9 +1015,9 @@ class App {
 
 	/**
 	 * @brief Register template engine class
-	 * 
+	 *
 	 * If $name is "", is used class static property $class::$name
-	 * 
+	 *
 	 * @param string $class
 	 * @param string $name
 	 */
@@ -1035,7 +1035,7 @@ class App {
 
 	/**
 	 * @brief Return template engine instance.
-	 * 
+	 *
 	 * If $name is not defined, return engine defined by theme,
 	 * or default
 	 *
@@ -1358,7 +1358,7 @@ class App {
 
 /**
  * @brief Retrieve the App structure
- * 
+ *
  * Useful in functions which require it but don't get it passed to them
  */
 function get_app() {
@@ -1612,7 +1612,7 @@ function run_update_function($x) {
  * and mark it uninstalled in the database (for now we'll remove it).
  * Then go through the config list and if we have a plugin that isn't installed,
  * call the install procedure and add it to the database.
- * 
+ *
  * @param App $a
  *
 	 */
@@ -1678,17 +1678,17 @@ function get_guid($size=16, $prefix = "") {
 	}
 }
 
-/** 
+/**
  * @brief Wrapper for adding a login box.
- * 
+ *
  * @param bool $register
  *	If $register == true provide a registration link.
  *	This will most always depend on the value of $a->config['register_policy'].
  * @param bool $hiddens
- * 
+ *
  * @return string
  *	Returns the complete html for inserting into the page
- * 
+ *
  * @hooks 'login_hook'
  *	string $o
  */
@@ -1778,7 +1778,7 @@ function goaway($s) {
 
 /**
  * @brief Returns the user id of locally logged in user or false.
- * 
+ *
  * @return int|bool user id or false
  */
 function local_user() {
@@ -1789,7 +1789,7 @@ function local_user() {
 
 /**
  * @brief Returns contact id of authenticated site visitor or false
- * 
+ *
  * @return int|bool visitor_id or false
  */
 function remote_user() {
@@ -1846,13 +1846,13 @@ function get_max_import_size() {
  *	so plugins can take part in process :)
  *
  * @param (string|integer) $cmd program to run or priority
- * 
+ *
  * next args are passed as $cmd command line
  * e.g.: proc_run("ls","-la","/tmp");
  * or: proc_run(PRIORITY_HIGH, "include/notifier.php", "drop", $drop_id);
  *
  * @note $cmd and string args are surrounded with ""
- * 
+ *
  * @hooks 'proc_run'
  *	array $arr
  */
@@ -2011,9 +2011,9 @@ function current_theme(){
 
 /**
  * @brief Return full URL to theme which is currently in effect.
- * 
+ *
  * Provide a sane default if nothing is chosen or the specified theme does not exist.
- * 
+ *
  * @return string
  */
 function current_theme_url() {
@@ -2360,7 +2360,7 @@ function current_load() {
 
 /**
  * @brief get c-style args
- * 
+ *
  * @return int
  */
 function argc() {
@@ -2369,7 +2369,7 @@ function argc() {
 
 /**
  * @brief Returns the value of a argv key
- * 
+ *
  * @param int $x argv key
  * @return string Value of the argv key
  */
@@ -2382,12 +2382,12 @@ function argv($x) {
 
 /**
  * @brief Get the data which is needed for infinite scroll
- * 
+ *
  * For invinite scroll we need the page number of the actual page
  * and the the URI where the content of the next page comes from.
  * This data is needed for the js part in main.js.
  * Note: infinite scroll does only work for the network page (module)
- * 
+ *
  * @param string $module The name of the module (e.g. "network")
  * @return array Of infinite scroll data
  *	'pageno' => $pageno The number of the actual page

database.sql

@@ -1,6 +1,6 @@
 -- ------------------------------------------
 -- Friendica 3.5.1-dev (Asparagus)
--- DB_UPDATE_VERSION 1204
+-- DB_UPDATE_VERSION 1205
 -- ------------------------------------------
 
 
@@ -97,7 +97,7 @@ CREATE TABLE IF NOT EXISTS `config` (
 	`k` varchar(255) NOT NULL DEFAULT '',
 	`v` text,
 	 PRIMARY KEY(`id`),
-	 INDEX `cat_k` (`cat`(30),`k`(30))
+	 UNIQUE INDEX `cat_k` (`cat`(30),`k`(30))
 ) DEFAULT CHARSET=utf8mb4;
 
 --
@@ -707,7 +707,7 @@ CREATE TABLE IF NOT EXISTS `pconfig` (
 	`k` varchar(255) NOT NULL DEFAULT '',
 	`v` mediumtext,
 	 PRIMARY KEY(`id`),
-	 INDEX `uid_cat_k` (`uid`,`cat`(30),`k`(30))
+	 UNIQUE INDEX `uid_cat_k` (`uid`,`cat`(30),`k`(30))
 ) DEFAULT CHARSET=utf8mb4;
 
 --

doc/upgrade.md

@@ -0,0 +1,34 @@
+# Considerations before upgrading Friendica
+
+* [Home](help)
+
+## MySQL >= 5.7.4
+
+Starting from MySQL version 5.7.4, the IGNORE keyword in ALTER TABLE statements is ignored.
+This prevents automatic table deduplication if a UNIQUE index is added to a Friendica table's structure.
+If a DB update fails for you while creating a UNIQUE index, make sure to manually deduplicate the table before trying the update again.
+
+### Manual deduplication
+
+There are two main ways of doing it, either by manually removing the duplicates or by recreating the table.
+Manually removing the duplicates is usually faster if they're not too numerous.
+To manually remove the duplicates, you need to know the UNIQUE index columns available in `database.sql`.
+
+```SQL
+SELECT GROUP_CONCAT(id), <index columns>, count(*) as count FROM users
+GROUP BY <index columns> HAVING count >= 2;
+
+/* delete or merge duplicate from above query */;
+```
+
+If there are too many rows to handle manually, you can create a new table with the same structure as the table with duplicates and insert the existing content with INSERT IGNORE.
+To recreate the table you need to know the table structure available in `database.sql`.
+
+```SQL
+CREATE TABLE <table_name>_new <rest of the CREATE TABLE>;
+INSERT IGNORE INTO <table_name>_new SELECT * FROM <table_name>;
+DROP TABLE <table_name>;
+RENAME TABLE <table_name>_new TO <table_name>;
+```
+
+This method is slower overall, but it is better suited for large numbers of duplicates.

include/Core/Config.php

@@ -126,37 +126,19 @@ class Config {
 	public static function set($family,$key,$value) {
 		global $a;
 
-		// If $a->config[$family] has been previously set to '!<unset>!', then
+		$a->config[$family][$key] = $value;
-		// $a->config[$family][$key] will evaluate to $a->config[$family][0], and
-		// $a->config[$family][$key] = $value will be equivalent to
-		// $a->config[$family][0] = $value[0] (this causes infuriating bugs),
-		// so unset the family before assigning a value to a family's key
-		if($a->config[$family] === '!<unset>!')
-			unset($a->config[$family]);
 
 		// manage array value
 		$dbvalue = (is_array($value)?serialize($value):$value);
 		$dbvalue = (is_bool($dbvalue) ? intval($dbvalue) : $dbvalue);
-		if(is_null(self::get($family,$key,null,true))) {
-			$a->config[$family][$key] = $value;
-			$ret = q("INSERT INTO `config` ( `cat`, `k`, `v` ) VALUES ( '%s', '%s', '%s' ) ",
-				dbesc($family),
-				dbesc($key),
-				dbesc($dbvalue)
-			);
-			if($ret)
-				return $value;
-			return $ret;
-		}
 
-		$ret = q("UPDATE `config` SET `v` = '%s' WHERE `cat` = '%s' AND `k` = '%s'",
+		$ret = q("INSERT INTO `config` ( `cat`, `k`, `v` ) VALUES ( '%s', '%s', '%s' )
-			dbesc($dbvalue),
+ON DUPLICATE KEY UPDATE `v` = '%s'",
 			dbesc($family),
-			dbesc($key)
+			dbesc($key),
+			dbesc($dbvalue),
+			dbesc($dbvalue)
 		);
-
-		$a->config[$family][$key] = $value;
-
 		if($ret)
 			return $value;
 		return $ret;

include/Core/PConfig.php

@@ -126,27 +126,16 @@ class PConfig {
 		// manage array value
 		$dbvalue = (is_array($value)?serialize($value):$value);
 
-		if(is_null(self::get($uid,$family,$key,null, true))) {
-			$a->config[$uid][$family][$key] = $value;
-			$ret = q("INSERT INTO `pconfig` ( `uid`, `cat`, `k`, `v` ) VALUES ( %d, '%s', '%s', '%s' ) ",
-				intval($uid),
-				dbesc($family),
-				dbesc($key),
-				dbesc($dbvalue)
-			);
-			if($ret)
-				return $value;
-			return $ret;
-		}
-		$ret = q("UPDATE `pconfig` SET `v` = '%s' WHERE `uid` = %d AND `cat` = '%s' AND `k` = '%s'",
-			dbesc($dbvalue),
-			intval($uid),
-			dbesc($family),
-			dbesc($key)
-		);
-
 		$a->config[$uid][$family][$key] = $value;
 
+		$ret = q("INSERT INTO `pconfig` ( `uid`, `cat`, `k`, `v` ) VALUES ( %d, '%s', '%s', '%s' )
+ON DUPLICATE KEY UPDATE `v` = '%s'",
+			intval($uid),
+			dbesc($family),
+			dbesc($key),
+			dbesc($dbvalue),
+			dbesc($dbvalue)
+		);
 		if($ret)
 			return $value;
 		return $ret;

include/dba.php

@@ -91,6 +91,23 @@ class dba {
 		return $this->db;
 	}
 
+	/**
+	 * @brief Returns the MySQL server version string
+	 * 
+	 * This function discriminate between the deprecated mysql API and the current
+	 * object-oriented mysqli API. Example of returned string: 5.5.46-0+deb8u1
+	 *
+	 * @return string
+	 */
+	public function server_info() {
+		if ($this->mysqli) {
+			$return = $this->db->server_info;
+		} else {
+			$return = mysql_get_server_info($this->db);
+		}
+		return $return;
+	}
+
 	public function q($sql, $onlyquery = false) {
 		global $a;
 

include/dbstructure.php

@@ -78,6 +78,10 @@ function table_structure($table) {
 			if ($index["Index_type"] == "FULLTEXT")
 				continue;
 
+			if ($index['Key_name'] != 'PRIMARY' && $index['Non_unique'] == '0' && !isset($indexdata[$index["Key_name"]])) {
+				$indexdata[$index["Key_name"]] = array('UNIQUE');
+			}
+
 			$column = $index["Column_name"];
 			// On utf8mb4 a varchar index can only have a length of 191
 			// To avoid the need to add this to every index definition we just ignore it here.
@@ -154,6 +158,19 @@ function update_structure($verbose, $action, $tables=null, $definition=null) {
 	if (is_null($definition))
 		$definition = db_definition($charset);
 
+	// Ensure index conversion to unique removes duplicates
+	$sql_config = "SET session old_alter_table=1;";
+	if ($verbose)
+		echo $sql_config."\n";
+	if ($action)
+		@$db->q($sql_config);
+
+	// MySQL >= 5.7.4 doesn't support the IGNORE keyword in ALTER TABLE statements
+	if (version_compare($db->server_info(), '5.7.4') >= 0) {
+		$ignore = '';
+	}else {
+		$ignore = ' IGNORE';
+	}
 
 	// Compare it
 	foreach ($definition AS $name => $structure) {
@@ -223,7 +240,7 @@ function update_structure($verbose, $action, $tables=null, $definition=null) {
 					$sql2=db_create_index($indexname, $fieldnames);
 					if ($sql2 != "") {
 						if ($sql3 == "")
-							$sql3 = "ALTER TABLE `".$name."` ".$sql2;
+							$sql3 = "ALTER" . $ignore . " TABLE `".$name."` ".$sql2;
 						else
 							$sql3 .= ", ".$sql2;
 					}
@@ -330,6 +347,11 @@ function db_create_index($indexname, $fieldnames, $method="ADD") {
 		killme();
 	}
 
+	if ($fieldnames[0] == "UNIQUE") {
+		array_shift($fieldnames);
+		$method .= ' UNIQUE';
+	}
+
 	$names = "";
 	foreach ($fieldnames AS $fieldname) {
 		if ($names != "")
@@ -457,7 +479,7 @@ function db_definition($charset) {
 					),
 			"indexes" => array(
 					"PRIMARY" => array("id"),
-					"cat_k" => array("cat(30)","k(30)"),
+					"cat_k" => array("UNIQUE", "cat(30)","k(30)"),
 					)
 			);
 	$database["contact"] = array(
@@ -1067,7 +1089,7 @@ function db_definition($charset) {
 					),
 			"indexes" => array(
 					"PRIMARY" => array("id"),
-					"uid_cat_k" => array("uid","cat(30)","k(30)"),
+					"uid_cat_k" => array("UNIQUE", "uid","cat(30)","k(30)"),
 					)
 			);
 	$database["photo"] = array(
@@ -1491,6 +1513,9 @@ function dbstructure_run(&$argv, &$argc) {
 
 	if ($argc==2) {
 		switch ($argv[1]) {
+			case "dryrun":
+				update_structure(true, false);
+				return;
 			case "update":
 				update_structure(true, true);
 
@@ -1523,7 +1548,8 @@ function dbstructure_run(&$argv, &$argc) {
 	// print help
 	echo $argv[0]." <command>\n";
 	echo "\n";
-	echo "commands:\n";
+	echo "Commands:\n";
+	echo "dryrun		show database update schema queries without running them\n";
 	echo "update		update database schema\n";
 	echo "dumpsql		dump database schema\n";
 	return;

update.php

@@ -1,6 +1,6 @@
 <?php
 
-define('UPDATE_VERSION' , 1204);
+define('UPDATE_VERSION' , 1205);
 
 /**
  *
@@ -1677,7 +1677,7 @@ function update_1190() {
 			$idx = array_search($plugin, $plugins_arr);
 			if ($idx !== false){
 				unset($plugins_arr[$idx]);
-				//delete forumlist manually from addon and hook table 
+				//delete forumlist manually from addon and hook table
 				// since uninstall_plugin() don't work here
 				q("DELETE FROM `addon` WHERE `name` = 'forumlist' ");
 				q("DELETE FROM `hook` WHERE `file` = 'addon/forumlist/forumlist.php' ");
@@ -1727,4 +1727,4 @@ function update_1190() {
 function update_1202() {
 	$r = q("UPDATE `user` SET `account-type` = %d WHERE `page-flags` IN (%d, %d)",
 		dbesc(ACCOUNT_TYPE_COMMUNITY), dbesc(PAGE_COMMUNITY), dbesc(PAGE_PRVGROUP));
-}
+}