Merge pull request #9672 from owncloud/mysql-affectedrows

Make MySQL affected row count consistent with other DBMSes

* owncloud/mysql-affectedrows:
  Added test of OCS privatedata to trigger key duplication
  Document why we have to check with defined() first.
  Make MySQL return "number of found rows" instead of number of "affected rows".

3 files changed, 75 insertions, 1 deletions

diff --git a/lib/private/db/connectionfactory.php b/lib/private/db/connectionfactory.php
index 8f852cf7127..033065bcb77 100644
--- a/lib/private/db/connectionfactory.php
+++ b/lib/private/db/connectionfactory.php
@@ -60,7 +60,15 @@ class ConnectionFactory {
 		if (!isset($this->defaultConnectionParams[$normalizedType])) {
 			throw new \InvalidArgumentException("Unsupported type: $type");
 		}
-		return $this->defaultConnectionParams[$normalizedType];
+		$result = $this->defaultConnectionParams[$normalizedType];
+		// \PDO::MYSQL_ATTR_FOUND_ROWS may not be defined, e.g. when the MySQL
+		// driver is missing. In this case, we won't be able to connect anyway.
+		if ($normalizedType === 'mysql' && defined('\PDO::MYSQL_ATTR_FOUND_ROWS')) {
+			$result['driverOptions'] = array(
+				\PDO::MYSQL_ATTR_FOUND_ROWS => true,
+			);
+		}
+		return $result;
 	}
 
 	/**
diff --git a/tests/lib/db.php b/tests/lib/db.php
index 2fca67b5638..4b1a474c4ef 100644
--- a/tests/lib/db.php
+++ b/tests/lib/db.php
@@ -200,4 +200,45 @@ class Test_DB extends PHPUnit_Framework_TestCase {
 		}
 	}
 
+	public function testUpdateAffectedRowsNoMatch() {
+		$this->insertCardData('fullname1', 'uri1');
+		// The WHERE clause does not match any rows
+		$this->assertSame(0, $this->updateCardData('fullname3', 'uri2'));
+	}
+
+	public function testUpdateAffectedRowsDifferent() {
+		$this->insertCardData('fullname1', 'uri1');
+		// The WHERE clause matches a single row and the value we are updating
+		// is different from the one already present.
+		$this->assertSame(1, $this->updateCardData('fullname1', 'uri2'));
+	}
+
+	public function testUpdateAffectedRowsSame() {
+		$this->insertCardData('fullname1', 'uri1');
+		// The WHERE clause matches a single row and the value we are updating
+		// to is the same as the one already present. MySQL reports 0 here when
+		// the PDO::MYSQL_ATTR_FOUND_ROWS flag is not specified.
+		$this->assertSame(1, $this->updateCardData('fullname1', 'uri1'));
+	}
+
+	public function testUpdateAffectedRowsMultiple() {
+		$this->insertCardData('fullname1', 'uri1');
+		$this->insertCardData('fullname2', 'uri2');
+		// The WHERE clause matches two rows. One row contains a value that
+		// needs to be updated, the other one already contains the value we are
+		// updating to. MySQL reports 1 here when the PDO::MYSQL_ATTR_FOUND_ROWS
+		// flag is not specified.
+		$query = OC_DB::prepare("UPDATE `*PREFIX*{$this->table2}` SET `uri` = ?");
+		$this->assertSame(2, $query->execute(array('uri1')));
+	}
+
+	protected function insertCardData($fullname, $uri) {
+		$query = OC_DB::prepare("INSERT INTO `*PREFIX*{$this->table2}` (`fullname`, `uri`, `carddata`) VALUES (?, ?, ?)");
+		$this->assertSame(1, $query->execute(array($fullname, $uri, uniqid())));
+	}
+
+	protected function updateCardData($fullname, $uri) {
+		$query = OC_DB::prepare("UPDATE `*PREFIX*{$this->table2}` SET `uri` = ? WHERE `fullname` = ?");
+		return $query->execute(array($uri, $fullname));
+	}
 }
diff --git a/tests/lib/ocs/privatedata.php b/tests/lib/ocs/privatedata.php
index 498ab718621..530750fabea 100644
--- a/tests/lib/ocs/privatedata.php
+++ b/tests/lib/ocs/privatedata.php
@@ -79,6 +79,31 @@ class Test_OC_OCS_Privatedata extends PHPUnit_Framework_TestCase
 		$this->assertEquals('updated', $data['value']);
 	}
 
+	public function testSetSameValue() {
+		$_POST = array('value' => 123456789);
+		$params = array('app' => $this->appKey, 'key' => 'k-10');
+		$result = OC_OCS_Privatedata::set($params);
+		$this->assertEquals(100, $result->getStatusCode());
+
+		$result = OC_OCS_Privatedata::get($params);
+		$this->assertOcsResult(1, $result);
+		$data = $result->getData();
+		$data = $data[0];
+		$this->assertEquals('123456789', $data['value']);
+
+		// set the same value again
+		$_POST = array('value' => 123456789);
+		$params = array('app' => $this->appKey, 'key' => 'k-10');
+		$result = OC_OCS_Privatedata::set($params);
+		$this->assertEquals(100, $result->getStatusCode());
+
+		$result = OC_OCS_Privatedata::get($params);
+		$this->assertOcsResult(1, $result);
+		$data = $result->getData();
+		$data = $data[0];
+		$this->assertEquals('123456789', $data['value']);
+	}
+
 	public function testSetMany() {
 		$_POST = array('value' => 123456789);