From f3df984c794a0ffce6d11780328d6dde0cd37e0d Mon Sep 17 00:00:00 2001 From: addshore Date: Sun, 4 Mar 2018 13:23:39 +0000 Subject: [PATCH] Introduce IDatabase::buildSubstring Change-Id: I96f3e0c4920d52f63175cb6767c149f20a8a8cde --- includes/db/DatabaseOracle.php | 9 +++ includes/libs/rdbms/database/DBConnRef.php | 4 ++ includes/libs/rdbms/database/Database.php | 34 +++++++++++ .../libs/rdbms/database/DatabaseMssql.php | 13 ++++ .../libs/rdbms/database/DatabaseSqlite.php | 9 +++ includes/libs/rdbms/database/IDatabase.php | 15 +++++ .../includes/db/DatabaseOracleTest.php | 51 ++++++++++++++++ .../includes/db/DatabaseTestHelper.php | 1 + .../libs/rdbms/database/DatabaseMssqlTest.php | 54 +++++++++++++++++ .../libs/rdbms/database/DatabaseSQLTest.php | 35 +++++++++++ .../database/DatabaseSqliteRdbmsTest.php | 59 +++++++++++++++++++ 11 files changed, 284 insertions(+) create mode 100644 tests/phpunit/includes/db/DatabaseOracleTest.php create mode 100644 tests/phpunit/includes/libs/rdbms/database/DatabaseMssqlTest.php create mode 100644 tests/phpunit/includes/libs/rdbms/database/DatabaseSqliteRdbmsTest.php diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php index 225a36c421..3362f0f907 100644 --- a/includes/db/DatabaseOracle.php +++ b/includes/db/DatabaseOracle.php @@ -1358,6 +1358,15 @@ class DatabaseOracle extends Database { return '(' . $this->selectSQLText( $table, $fld, $conds, null, [], $join_conds ) . ')'; } + public function buildSubstring( $input, $startPosition, $length = null ) { + $this->assertBuildSubstringParams( $startPosition, $length ); + $params = [ $input, $startPosition ]; + if ( $length !== null ) { + $params[] = $length; + } + return 'SUBSTR(' . implode( ',', $params ) . ')'; + } + /** * @param string $field Field or column to cast * @return string diff --git a/includes/libs/rdbms/database/DBConnRef.php b/includes/libs/rdbms/database/DBConnRef.php index ef2953ec9c..910d42f11d 100644 --- a/includes/libs/rdbms/database/DBConnRef.php +++ b/includes/libs/rdbms/database/DBConnRef.php @@ -350,6 +350,10 @@ class DBConnRef implements IDatabase { return $this->__call( __FUNCTION__, func_get_args() ); } + public function buildSubstring( $input, $startPosition, $length = null ) { + return $this->__call( __FUNCTION__, func_get_args() ); + } + public function buildStringCast( $field ) { return $this->__call( __FUNCTION__, func_get_args() ); } diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index 8ccccc34d1..fc0db57bef 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/includes/libs/rdbms/database/Database.php @@ -1854,6 +1854,40 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware return '(' . $this->selectSQLText( $table, $fld, $conds, null, [], $join_conds ) . ')'; } + public function buildSubstring( $input, $startPosition, $length = null ) { + $this->assertBuildSubstringParams( $startPosition, $length ); + $functionBody = "$input FROM $startPosition"; + if ( $length !== null ) { + $functionBody .= " FOR $length"; + } + return 'SUBSTRING(' . $functionBody . ')'; + } + + /** + * Check type and bounds for parameters to self::buildSubstring() + * + * All supported databases have substring functions that behave the same for + * positive $startPosition and non-negative $length, but behaviors differ when + * given 0 or negative $startPosition or negative $length. The simplest + * solution to that is to just forbid those values. + * + * @param int $startPosition + * @param int|null $length + * @since 1.31 + */ + protected function assertBuildSubstringParams( $startPosition, $length ) { + if ( !is_int( $startPosition ) || $startPosition <= 0 ) { + throw new InvalidArgumentException( + '$startPosition must be a positive integer' + ); + } + if ( !( is_int( $length ) && $length >= 0 || $length === null ) ) { + throw new InvalidArgumentException( + '$length must be null or an integer greater than or equal to 0' + ); + } + } + public function buildStringCast( $field ) { return $field; } diff --git a/includes/libs/rdbms/database/DatabaseMssql.php b/includes/libs/rdbms/database/DatabaseMssql.php index 771e2e5a21..b6428c7b21 100644 --- a/includes/libs/rdbms/database/DatabaseMssql.php +++ b/includes/libs/rdbms/database/DatabaseMssql.php @@ -1225,6 +1225,19 @@ class DatabaseMssql extends Database { return $sql; } + public function buildSubstring( $input, $startPosition, $length = null ) { + $this->assertBuildSubstringParams( $startPosition, $length ); + if ( $length === null ) { + /** + * MSSQL doesn't allow an empty length parameter, so when we don't want to limit the + * length returned use the default maximum size of text. + * @see https://docs.microsoft.com/en-us/sql/t-sql/statements/set-textsize-transact-sql + */ + $length = 2147483647; + } + return 'SUBSTRING(' . implode( ',', [ $input, $startPosition, $length ] ) . ')'; + } + /** * Returns an associative array for fields that are of type varbinary, binary, or image * $table can be either a raw table name or passed through tableName() first diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php index 3d6cee350f..83c88149b0 100644 --- a/includes/libs/rdbms/database/DatabaseSqlite.php +++ b/includes/libs/rdbms/database/DatabaseSqlite.php @@ -818,6 +818,15 @@ class DatabaseSqlite extends Database { } } + public function buildSubstring( $input, $startPosition, $length = null ) { + $this->assertBuildSubstringParams( $startPosition, $length ); + $params = [ $input, $startPosition ]; + if ( $length !== null ) { + $params[] = $length; + } + return 'SUBSTR(' . implode( ',', $params ) . ')'; + } + /** * @param string $field Field or column to cast * @return string diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php index 9ad78a7868..9311c074d7 100644 --- a/includes/libs/rdbms/database/IDatabase.php +++ b/includes/libs/rdbms/database/IDatabase.php @@ -19,6 +19,7 @@ */ namespace Wikimedia\Rdbms; +use InvalidArgumentException; use Wikimedia\ScopedCallback; use RuntimeException; use UnexpectedValueException; @@ -1050,6 +1051,20 @@ interface IDatabase { $delim, $table, $field, $conds = '', $join_conds = [] ); + /** + * Build a SUBSTRING function. + * + * Behavior for non-ASCII values is undefined. + * + * @param string $input Field name + * @param int $startPosition Positive integer + * @param int|null $length Non-negative integer length or null for no limit + * @throws InvalidArgumentException + * @return string SQL text + * @since 1.31 + */ + public function buildSubString( $input, $startPosition, $length = null ); + /** * @param string $field Field or column to cast * @return string diff --git a/tests/phpunit/includes/db/DatabaseOracleTest.php b/tests/phpunit/includes/db/DatabaseOracleTest.php new file mode 100644 index 0000000000..b03734e8bd --- /dev/null +++ b/tests/phpunit/includes/db/DatabaseOracleTest.php @@ -0,0 +1,51 @@ +getMockBuilder( DatabaseOracle::class ) + ->disableOriginalConstructor() + ->setMethods( null ) + ->getMock(); + } + + public function provideBuildSubstring() { + yield [ 'someField', 1, 2, 'SUBSTR(someField,1,2)' ]; + yield [ 'someField', 1, null, 'SUBSTR(someField,1)' ]; + } + + /** + * @covers DatabaseOracle::buildSubstring + * @dataProvider provideBuildSubstring + */ + public function testBuildSubstring( $input, $start, $length, $expected ) { + $mockDb = $this->getMockDb(); + $output = $mockDb->buildSubstring( $input, $start, $length ); + $this->assertSame( $expected, $output ); + } + + public function provideBuildSubstring_invalidParams() { + yield [ -1, 1 ]; + yield [ 1, -1 ]; + yield [ 1, 'foo' ]; + yield [ 'foo', 1 ]; + yield [ null, 1 ]; + yield [ 0, 1 ]; + } + + /** + * @covers DatabaseOracle::buildSubstring + * @dataProvider provideBuildSubstring_invalidParams + */ + public function testBuildSubstring_invalidParams( $start, $length ) { + $mockDb = $this->getMockDb(); + $this->setExpectedException( InvalidArgumentException::class ); + $mockDb->buildSubstring( 'foo', $start, $length ); + } + +} diff --git a/tests/phpunit/includes/db/DatabaseTestHelper.php b/tests/phpunit/includes/db/DatabaseTestHelper.php index 606c12d58e..89501523ae 100644 --- a/tests/phpunit/includes/db/DatabaseTestHelper.php +++ b/tests/phpunit/includes/db/DatabaseTestHelper.php @@ -2,6 +2,7 @@ use Wikimedia\Rdbms\TransactionProfiler; use Wikimedia\Rdbms\DatabaseDomain; +use Wikimedia\Rdbms\Database; /** * Helper for testing the methods from the Database class diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseMssqlTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseMssqlTest.php new file mode 100644 index 0000000000..bb85f5a365 --- /dev/null +++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseMssqlTest.php @@ -0,0 +1,54 @@ +getMockBuilder( DatabaseMssql::class ) + ->disableOriginalConstructor() + ->setMethods( null ) + ->getMock(); + } + + public function provideBuildSubstring() { + yield [ 'someField', 1, 2, 'SUBSTRING(someField,1,2)' ]; + yield [ 'someField', 1, null, 'SUBSTRING(someField,1,2147483647)' ]; + yield [ 'someField', 1, 3333333333, 'SUBSTRING(someField,1,3333333333)' ]; + } + + /** + * @covers Wikimedia\Rdbms\DatabaseMssql::buildSubstring + * @dataProvider provideBuildSubstring + */ + public function testBuildSubstring( $input, $start, $length, $expected ) { + $mockDb = $this->getMockDb(); + $output = $mockDb->buildSubstring( $input, $start, $length ); + $this->assertSame( $expected, $output ); + } + + public function provideBuildSubstring_invalidParams() { + yield [ -1, 1 ]; + yield [ 1, -1 ]; + yield [ 1, 'foo' ]; + yield [ 'foo', 1 ]; + yield [ null, 1 ]; + yield [ 0, 1 ]; + } + + /** + * @covers Wikimedia\Rdbms\DatabaseMssql::buildSubstring + * @dataProvider provideBuildSubstring_invalidParams + */ + public function testBuildSubstring_invalidParams( $start, $length ) { + $mockDb = $this->getMockDb(); + $this->setExpectedException( InvalidArgumentException::class ); + $mockDb->buildSubstring( 'foo', $start, $length ); + } + +} diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php index 5c1943b128..b3c8cce3b7 100644 --- a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php +++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php @@ -1,5 +1,6 @@ assertFalse( $this->database->tableExists( "tmp_table_2", __METHOD__ ) ); $this->assertFalse( $this->database->tableExists( "tmp_table_3", __METHOD__ ) ); } + + public function provideBuildSubstring() { + yield [ 'someField', 1, 2, 'SUBSTRING(someField FROM 1 FOR 2)' ]; + yield [ 'someField', 1, null, 'SUBSTRING(someField FROM 1)' ]; + } + + /** + * @covers Wikimedia\Rdbms\Database::buildSubstring + * @dataProvider provideBuildSubstring + */ + public function testBuildSubstring( $input, $start, $length, $expected ) { + $output = $this->database->buildSubstring( $input, $start, $length ); + $this->assertSame( $expected, $output ); + } + + public function provideBuildSubstring_invalidParams() { + yield [ -1, 1 ]; + yield [ 1, -1 ]; + yield [ 1, 'foo' ]; + yield [ 'foo', 1 ]; + yield [ null, 1 ]; + yield [ 0, 1 ]; + } + + /** + * @covers Wikimedia\Rdbms\Database::buildSubstring + * @covers Wikimedia\Rdbms\Database::assertBuildSubstringParams + * @dataProvider provideBuildSubstring_invalidParams + */ + public function testBuildSubstring_invalidParams( $start, $length ) { + $this->setExpectedException( InvalidArgumentException::class ); + $this->database->buildSubstring( 'foo', $start, $length ); + } + } diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseSqliteRdbmsTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseSqliteRdbmsTest.php new file mode 100644 index 0000000000..d963a5dcd2 --- /dev/null +++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseSqliteRdbmsTest.php @@ -0,0 +1,59 @@ +getMockBuilder( DatabaseSqlite::class ) + ->disableOriginalConstructor() + ->setMethods( null ) + ->getMock(); + } + + public function provideBuildSubstring() { + yield [ 'someField', 1, 2, 'SUBSTR(someField,1,2)' ]; + yield [ 'someField', 1, null, 'SUBSTR(someField,1)' ]; + } + + /** + * @covers Wikimedia\Rdbms\DatabaseSqlite::buildSubstring + * @dataProvider provideBuildSubstring + */ + public function testBuildSubstring( $input, $start, $length, $expected ) { + $dbMock = $this->getMockDb(); + $output = $dbMock->buildSubstring( $input, $start, $length ); + $this->assertSame( $expected, $output ); + } + + public function provideBuildSubstring_invalidParams() { + yield [ -1, 1 ]; + yield [ 1, -1 ]; + yield [ 1, 'foo' ]; + yield [ 'foo', 1 ]; + yield [ null, 1 ]; + yield [ 0, 1 ]; + } + + /** + * @covers Wikimedia\Rdbms\DatabaseSqlite::buildSubstring + * @dataProvider provideBuildSubstring_invalidParams + */ + public function testBuildSubstring_invalidParams( $start, $length ) { + $dbMock = $this->getMockDb(); + $this->setExpectedException( InvalidArgumentException::class ); + $dbMock->buildSubstring( 'foo', $start, $length ); + } + +} -- 2.20.1