Addshore has uploaded a new change for review.
https://gerrit.wikimedia.org/r/89631
Change subject: Use FieldValueEscaping DNM
......................................................................
Use FieldValueEscaping DNM
Change-Id: I225a78ed3789b69b3b50582c3c04691c909de26a
---
M src/MediaWiki/MWTableBuilderBuilder.php
M src/MySQL/MySQLFieldSqlBuilder.php
M src/MySQL/MySQLIndexSqlBuilder.php
M src/MySQL/MySQLSchemaSqlBuilder.php
M src/MySQL/MySQLTableSqlBuilder.php
M tests/phpunit/MySQL/MySQLFieldSqlBuilderTest.php
M tests/phpunit/MySQL/MySQLIndexSqlBuilderTest.php
M tests/phpunit/MySQL/MySQLSchemaSqlBuilderTest.php
M tests/phpunit/MySQL/MySQLTableSqlBuilderTest.php
9 files changed, 82 insertions(+), 31 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseDatabase
refs/changes/31/89631/1
diff --git a/src/MediaWiki/MWTableBuilderBuilder.php
b/src/MediaWiki/MWTableBuilderBuilder.php
index b416980..042b6bc 100644
--- a/src/MediaWiki/MWTableBuilderBuilder.php
+++ b/src/MediaWiki/MWTableBuilderBuilder.php
@@ -65,7 +65,8 @@
return new MySQLTableSqlBuilder(
$this->connectionProvider->getConnection()->getDBname(),
$this->newEscaper(),
- $this->newTableNameFormatter()
+ $this->newTableNameFormatter(),
+ new MySQLFieldSqlBuilder( $this->newEscaper() )
);
}
diff --git a/src/MySQL/MySQLFieldSqlBuilder.php
b/src/MySQL/MySQLFieldSqlBuilder.php
index 4034902..3ec54c1 100644
--- a/src/MySQL/MySQLFieldSqlBuilder.php
+++ b/src/MySQL/MySQLFieldSqlBuilder.php
@@ -24,8 +24,7 @@
}
public function getFieldSQL( FieldDefinition $field ){
- //todo escape name once identifier escaping is implemented
- $sql = $field->getName() . ' ';
+ $sql = $this->escaper->getEscapedIdentifier( $field->getName()
) . ' ';
$sql .= $this->getFieldType( $field->getType() );
diff --git a/src/MySQL/MySQLIndexSqlBuilder.php
b/src/MySQL/MySQLIndexSqlBuilder.php
index 5140173..147199a 100644
--- a/src/MySQL/MySQLIndexSqlBuilder.php
+++ b/src/MySQL/MySQLIndexSqlBuilder.php
@@ -3,6 +3,7 @@
namespace Wikibase\Database\MySQL;
use RuntimeException;
+use Wikibase\Database\Escaper;
use Wikibase\Database\Schema\Definitions\IndexDefinition;
use Wikibase\Database\Schema\Definitions\TableDefinition;
use Wikibase\Database\Schema\IndexSqlBuilder;
@@ -15,10 +16,15 @@
*/
class MySQLIndexSqlBuilder extends IndexSqlBuilder {
+ protected $escaper;
+ protected $tableNameFormatter;
+
/**
+ * @param Escaper $escaper
* @param TableNameFormatter $tableNameFormatter
*/
- public function __construct( TableNameFormatter $tableNameFormatter ) {
+ public function __construct( Escaper $escaper, TableNameFormatter
$tableNameFormatter ) {
+ $this->escaper = $escaper;
$this->tableNameFormatter = $tableNameFormatter;
}
@@ -27,7 +33,7 @@
$sql .= $this->getIndexType( $index->getType() );
if( $index->getType() !== IndexDefinition::TYPE_PRIMARY ){
- $sql .= ' `'.$index->getName().'`';//todo escape index
name?
+ $sql .= ' ' . $this->escaper->getEscapedIdentifier(
$index->getName() );
}
$sql .= ' ON ' . $this->tableNameFormatter->formatTableName(
$tableName );
diff --git a/src/MySQL/MySQLSchemaSqlBuilder.php
b/src/MySQL/MySQLSchemaSqlBuilder.php
index 75baf07..5b737ba 100644
--- a/src/MySQL/MySQLSchemaSqlBuilder.php
+++ b/src/MySQL/MySQLSchemaSqlBuilder.php
@@ -18,11 +18,13 @@
*/
class MySQLSchemaSqlBuilder implements SchemaModificationSqlBuilder {
+ protected $escaper;
protected $fieldSqlBuilder;
protected $tableNameFormatter;
- public function __construct( Escaper $fieldValueEscaper,
TableNameFormatter $tableNameFormatter ) {
- $this->fieldSqlBuilder = new MySQLFieldSqlBuilder(
$fieldValueEscaper );
+ public function __construct( Escaper $escaper, TableNameFormatter
$tableNameFormatter ) {
+ $this->escaper = $escaper;
+ $this->fieldSqlBuilder = new MySQLFieldSqlBuilder( $escaper );
$this->tableNameFormatter = $tableNameFormatter;
}
@@ -34,7 +36,8 @@
*/
public function getRemoveFieldSql( $tableName, $fieldName ) {
$tableName = $this->tableNameFormatter->formatTableName(
$tableName );
- return "ALTER TABLE {$tableName} DROP {$fieldName}"; //todo
escape $fieldName?
+ $fieldName = $this->escaper->getEscapedIdentifier( $fieldName );
+ return "ALTER TABLE {$tableName} DROP {$fieldName}";
}
/**
@@ -56,6 +59,7 @@
*/
public function getRemoveIndexSql( $tableName, $indexName ){
$tableName = $this->tableNameFormatter->formatTableName(
$tableName );
+ $indexName = $this->escaper->getEscapedIdentifier( $indexName );
return "DROP INDEX {$indexName} ON {$tableName}";
}
@@ -66,7 +70,7 @@
* @return string
*/
public function getAddIndexSql( $tableName, IndexDefinition $index ){
- $indexSqlBuilder = new MySQLIndexSqlBuilder(
$this->tableNameFormatter );
+ $indexSqlBuilder = new MySQLIndexSqlBuilder( $this->escaper,
$this->tableNameFormatter );
return $indexSqlBuilder->getIndexSQL( $index, $tableName );
}
diff --git a/src/MySQL/MySQLTableSqlBuilder.php
b/src/MySQL/MySQLTableSqlBuilder.php
index 41d3818..2d77597 100644
--- a/src/MySQL/MySQLTableSqlBuilder.php
+++ b/src/MySQL/MySQLTableSqlBuilder.php
@@ -27,12 +27,12 @@
/**
* @param string $dbName
- * @param Escaper $fieldValueEscaper
+ * @param Escaper $escaper
* @param TableNameFormatter $tableNameFormatter
*/
- public function __construct( $dbName, Escaper $fieldValueEscaper,
TableNameFormatter $tableNameFormatter ) {
+ public function __construct( $dbName, Escaper $escaper,
TableNameFormatter $tableNameFormatter, FieldSqlBuilder $fieldSqlBuilder ) {
$this->dbName = $dbName;
- $this->escaper = $fieldValueEscaper;
+ $this->escaper = $escaper;
$this->tableNameFormatter = $tableNameFormatter;
//TODO inject sqlbuilder
$this->fieldSqlBuilder = new MySQLFieldSqlBuilder(
$this->escaper );
@@ -80,8 +80,7 @@
$sql = $this->getIndexType( $index->getType() );
if( $index->getType() !== IndexDefinition::TYPE_PRIMARY ){
- //todo escape name once identifier escaping is
implemented
- $sql .= ' `'.$index->getName().'`';
+ $sql .= ' ' . $this->escaper->getEscapedIdentifier(
$index->getName() );
}
$columnNames = array();
diff --git a/tests/phpunit/MySQL/MySQLFieldSqlBuilderTest.php
b/tests/phpunit/MySQL/MySQLFieldSqlBuilderTest.php
index 86e2a69..4f2df32 100644
--- a/tests/phpunit/MySQL/MySQLFieldSqlBuilderTest.php
+++ b/tests/phpunit/MySQL/MySQLFieldSqlBuilderTest.php
@@ -28,6 +28,11 @@
->will( $this->returnCallback( function( $value ) {
return '|' . $value . '|';
} ) );
+ $mockEscaper->expects( $this->any() )
+ ->method( 'getEscapedIdentifier' )
+ ->will( $this->returnCallback( function( $input ) {
+ return "|$input|";
+ } ) );
$sqlBuilder = new MySQLFieldSqlBuilder( $mockEscaper );
@@ -44,7 +49,7 @@
'fieldName',
FieldDefinition::TYPE_INTEGER
),
- 'fieldName INT NULL'
+ '|fieldName| INT NULL'
);
$argLists[] = array(
@@ -56,7 +61,7 @@
FieldDefinition::NO_ATTRIB,
FieldDefinition::AUTOINCREMENT
),
- 'fieldName INT NULL AUTO_INCREMENT'
+ '|fieldName| INT NULL AUTO_INCREMENT'
);
$argLists[] = array(
@@ -65,7 +70,7 @@
FieldDefinition::TYPE_FLOAT,
FieldDefinition::NOT_NULL
),
- 'fieldName FLOAT NOT NULL'
+ '|fieldName| FLOAT NOT NULL'
);
$argLists[] = array(
@@ -75,7 +80,7 @@
FieldDefinition::NOT_NULL,
'1'
),
- 'fieldName TINYINT DEFAULT |1| NOT NULL'
+ '|fieldName| TINYINT DEFAULT |1| NOT NULL'
);
$argLists[] = array(
@@ -85,7 +90,7 @@
FieldDefinition::NOT_NULL,
'foo'
),
- 'fieldName BLOB DEFAULT |foo| NOT NULL'
+ '|fieldName| BLOB DEFAULT |foo| NOT NULL'
);
return $argLists;
diff --git a/tests/phpunit/MySQL/MySQLIndexSqlBuilderTest.php
b/tests/phpunit/MySQL/MySQLIndexSqlBuilderTest.php
index c476522..b6ff350 100644
--- a/tests/phpunit/MySQL/MySQLIndexSqlBuilderTest.php
+++ b/tests/phpunit/MySQL/MySQLIndexSqlBuilderTest.php
@@ -26,7 +26,14 @@
->method( 'formatTableName' )
->will( $this->returnArgument(0) );
- $sqlBuilder = new MySQLIndexSqlBuilder( $mockTableNameFormatter
);
+ $mockEscaper = $this->getMock( 'Wikibase\Database\Escaper' );
+ $mockEscaper->expects( $this->any() )
+ ->method( 'getEscapedIdentifier' )
+ ->will( $this->returnCallback( function( $input ) {
+ return "|$input|";
+ } ) );
+
+ $sqlBuilder = new MySQLIndexSqlBuilder( $mockEscaper,
$mockTableNameFormatter );
$sql = $sqlBuilder->getIndexSQL( $index, 'tableName' );
$this->assertEquals( $expectedSQL, $sql );
}
@@ -43,7 +50,7 @@
array( 'intField' => 0, 'textField' => 0 ),
IndexDefinition::TYPE_INDEX
),
- 'CREATE INDEX `indexName` ON tableName
(`intField`,`textField`)'
+ 'CREATE INDEX |indexName| ON tableName
(`intField`,`textField`)'
);
$argLists[] = array(
@@ -52,7 +59,7 @@
array( 'intField' => 0, 'textField' => 0 ),
IndexDefinition::TYPE_UNIQUE
),
- 'CREATE UNIQUE INDEX `indexName` ON tableName
(`intField`,`textField`)'
+ 'CREATE UNIQUE INDEX |indexName| ON tableName
(`intField`,`textField`)'
);
$argLists[] = array(
@@ -81,7 +88,19 @@
->method( 'getType' )
->will( $this->returnValue( 'foobar' ) );
- $sqlBuilder = new MySQLIndexSqlBuilder( $tableNameFormatter );
+ $mockEscaper = $this->getMock( 'Wikibase\Database\Escaper' );
+ $mockEscaper->expects( $this->any() )
+ ->method( 'getEscapedValue' )
+ ->will( $this->returnCallback( function( $input ) {
+ return "|$input|";
+ } ) );
+ $mockEscaper->expects( $this->any() )
+ ->method( 'getEscapedIdentifier' )
+ ->will( $this->returnCallback( function( $input ) {
+ return "|$input|";
+ } ) );
+
+ $sqlBuilder = new MySQLIndexSqlBuilder( $mockEscaper,
$tableNameFormatter );
$sqlBuilder->getIndexSQL( $indexDefinition, 'tableName' );
}
diff --git a/tests/phpunit/MySQL/MySQLSchemaSqlBuilderTest.php
b/tests/phpunit/MySQL/MySQLSchemaSqlBuilderTest.php
index a77b37b..73aa4e7 100644
--- a/tests/phpunit/MySQL/MySQLSchemaSqlBuilderTest.php
+++ b/tests/phpunit/MySQL/MySQLSchemaSqlBuilderTest.php
@@ -25,6 +25,11 @@
->will( $this->returnCallback( function( $input ) {
return "|$input|";
} ) );
+ $mockEscaper->expects( $this->any() )
+ ->method( 'getEscapedIdentifier' )
+ ->will( $this->returnCallback( function( $input ) {
+ return "|$input|";
+ } ) );
$mockTableNameFormatter = $this->getMock(
'Wikibase\Database\TableNameFormatter' );
$mockTableNameFormatter->expects( $this->atLeastOnce() )
@@ -39,26 +44,26 @@
public function testGetRemoveFieldSql(){
$instance = $this->newInstance();
$sql = $instance->getRemoveFieldSql( 'tableName', 'fieldName' );
- $this->assertEquals( "ALTER TABLE ||tableName|| DROP
fieldName", $sql );
+ $this->assertEquals( "ALTER TABLE ||tableName|| DROP
|fieldName|", $sql );
}
public function testGetAddFieldSql(){
$instance = $this->newInstance();
$field = new FieldDefinition( 'intField',
FieldDefinition::TYPE_INTEGER, FieldDefinition::NOT_NULL, 42 );
$sql = $instance->getAddFieldSql( 'tableName', $field );
- $this->assertEquals( 'ALTER TABLE ||tableName|| ADD intField
INT DEFAULT |42| NOT NULL', $sql );
+ $this->assertEquals( 'ALTER TABLE ||tableName|| ADD |intField|
INT DEFAULT |42| NOT NULL', $sql );
}
public function testGetRemoveIndexSql(){
$instance = $this->newInstance();
$sql = $instance->getRemoveIndexSql( 'tableName', 'indexName' );
- $this->assertEquals( "DROP INDEX indexName ON ||tableName||",
$sql );
+ $this->assertEquals( "DROP INDEX |indexName| ON ||tableName||",
$sql );
}
public function testGetAddIndexSql(){
$instance = $this->newInstance();
- $sql = $instance->getAddIndexSql( 'tableName', new
IndexDefinition( 'name', array( 'a' => 0, 'b' => 0 ),
IndexDefinition::TYPE_INDEX ) );
- $this->assertEquals( "CREATE INDEX `name` ON ||tableName||
(`a`,`b`)", $sql );
+ $sql = $instance->getAddIndexSql( 'tableName', new
IndexDefinition( 'indexName', array( 'a' => 0, 'b' => 0 ),
IndexDefinition::TYPE_INDEX ) );
+ $this->assertEquals( "CREATE INDEX |indexName| ON ||tableName||
(`a`,`b`)", $sql );
}
}
\ No newline at end of file
diff --git a/tests/phpunit/MySQL/MySQLTableSqlBuilderTest.php
b/tests/phpunit/MySQL/MySQLTableSqlBuilderTest.php
index 67032ab..b673b98 100644
--- a/tests/phpunit/MySQL/MySQLTableSqlBuilderTest.php
+++ b/tests/phpunit/MySQL/MySQLTableSqlBuilderTest.php
@@ -39,10 +39,23 @@
->method( 'formatTableName' )
->will( $this->returnArgument(0) );
+ $mockEscaper = $this->getMock( 'Wikibase\Database\Escaper' );
+ $mockEscaper->expects( $this->any() )
+ ->method( 'getEscapedValue' )
+ ->will( $this->returnCallback( function( $input ) {
+ return "|$input|";
+ } ) );
+ $mockEscaper->expects( $this->any() )
+ ->method( 'getEscapedIdentifier' )
+ ->will( $this->returnCallback( function( $input ) {
+ return "|$input|";
+ } ) );
+
return new MySQLTableSqlBuilder(
self::DB_NAME,
$mockEscaper,
- $mockTableNameFormatter
+ $mockTableNameFormatter,
+ $mockFieldSqlBuilder
);
}
@@ -106,7 +119,7 @@
),
)
),
- 'CREATE TABLE `dbName`.tableName (textField BLOB NULL,
intField INT DEFAULT 42 NOT NULL, INDEX `indexName` (`textField`,`intField`))
ENGINE=InnoDB, DEFAULT CHARSET=binary;'
+ 'CREATE TABLE `dbName`.tableName (<FIELDSQL>,
<FIELDSQL>, INDEX |indexName| (`textField`,`intField`)) ENGINE=InnoDB, DEFAULT
CHARSET=binary;'
);
$argLists[] = array(
@@ -132,7 +145,7 @@
),
)
),
- 'CREATE TABLE `dbName`.tableName (textField BLOB NULL,
intField INT DEFAULT 42 NOT NULL, textField2 BLOB NULL, INDEX `indexName`
(`intField`), UNIQUE INDEX `uniqueIndexName` (`textField2`)) ENGINE=InnoDB,
DEFAULT CHARSET=binary;'
+ 'CREATE TABLE `dbName`.tableName (<FIELDSQL>,
<FIELDSQL>, <FIELDSQL>, INDEX |indexName| (`intField`), UNIQUE INDEX
|uniqueIndexName| (`textField2`)) ENGINE=InnoDB, DEFAULT CHARSET=binary;'
);
return $argLists;
--
To view, visit https://gerrit.wikimedia.org/r/89631
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I225a78ed3789b69b3b50582c3c04691c909de26a
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseDatabase
Gerrit-Branch: master
Gerrit-Owner: Addshore <addshorew...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
