Jeroen De Dauw has submitted this change and it was merged. Change subject: Expand TableCreateModifyDeleteTest integrationTest ......................................................................
Expand TableCreateModifyDeleteTest integrationTest Change-Id: Iba9a796ef0777427b4494108be02b3f4771a0a52 --- M src/MediaWiki/MediaWikiSchemaModifier.php M src/MediaWiki/MediaWikiTableBuilder.php M src/SQLite/SQLiteSchemaSqlBuilder.php M tests/integration/MediaWiki/Schema/TableCreateModifyDeleteTest.php M tests/phpunit/SQLite/SQLiteSchemaSqlBuilderTest.php 5 files changed, 123 insertions(+), 40 deletions(-) Approvals: Jeroen De Dauw: Looks good to me, approved diff --git a/src/MediaWiki/MediaWikiSchemaModifier.php b/src/MediaWiki/MediaWikiSchemaModifier.php index 58e804a..8ac27c4 100644 --- a/src/MediaWiki/MediaWikiSchemaModifier.php +++ b/src/MediaWiki/MediaWikiSchemaModifier.php @@ -50,18 +50,20 @@ * @throws FieldRemovalFailedException */ public function removeField( $tableName, $fieldName ) { - $success = $this->getDB()->query( - $this->sqlBuilder->getRemoveFieldSql( $tableName, $fieldName ), - __METHOD__ - ); + $sql = $this->sqlBuilder->getRemoveFieldSql( $tableName, $fieldName ); - if ( $success === false ) { - throw new FieldRemovalFailedException( - $tableName, - $fieldName, - $this->getDB()->lastQuery() - ); + foreach( explode( PHP_EOL, $sql ) as $query ) { + $success = $this->getDB()->query( $query ); + + if ( $success === false ) { + throw new FieldRemovalFailedException( + $tableName, + $fieldName, + $this->getDB()->lastQuery() + ); + } } + } /** diff --git a/src/MediaWiki/MediaWikiTableBuilder.php b/src/MediaWiki/MediaWikiTableBuilder.php index c5fcf6c..4718e51 100644 --- a/src/MediaWiki/MediaWikiTableBuilder.php +++ b/src/MediaWiki/MediaWikiTableBuilder.php @@ -71,7 +71,7 @@ public function createTable( TableDefinition $table ) { $sql = $this->tableSqlBuilder->getCreateTableSql( $table ); - foreach( explode( PHP_EOL, $sql ) as $query ){ + foreach( explode( PHP_EOL, $sql ) as $query ) { $success = $this->getDB()->query( $query ); if ( $success === false ) { diff --git a/src/SQLite/SQLiteSchemaSqlBuilder.php b/src/SQLite/SQLiteSchemaSqlBuilder.php index e12966e..0e37fa3 100644 --- a/src/SQLite/SQLiteSchemaSqlBuilder.php +++ b/src/SQLite/SQLiteSchemaSqlBuilder.php @@ -86,14 +86,15 @@ } /** - * @param string $tableName + * @param string $tableName Ignored by this method * @param string $indexName * + * @see http://www.sqlite.org/lang_dropindex.html * @return string */ public function getRemoveIndexSql( $tableName, $indexName ){ - $tableName = $this->tableNameFormatter->formatTableName( $tableName ); - return "DROP INDEX IF EXISTS {$tableName}.{$indexName}"; + $indexName = $this->escaper->getEscapedIdentifier( $indexName ); + return "DROP INDEX IF EXISTS {$indexName}"; } /** diff --git a/tests/integration/MediaWiki/Schema/TableCreateModifyDeleteTest.php b/tests/integration/MediaWiki/Schema/TableCreateModifyDeleteTest.php index 930b1f5..2d6037f 100644 --- a/tests/integration/MediaWiki/Schema/TableCreateModifyDeleteTest.php +++ b/tests/integration/MediaWiki/Schema/TableCreateModifyDeleteTest.php @@ -4,7 +4,6 @@ use Wikibase\Database\LazyDBConnectionProvider; use Wikibase\Database\MediaWiki\MediaWikiQueryInterface; -use Wikibase\Database\MediaWiki\MediaWikiSchemaModifier; use Wikibase\Database\MediaWiki\MediaWikiSchemaModifierBuilder; use Wikibase\Database\MediaWiki\MWTableBuilderBuilder; use Wikibase\Database\MediaWiki\MWTableDefinitionReaderBuilder; @@ -34,7 +33,7 @@ protected function dropTablesIfStillThere( $tablesToDrop ) { $tableBuilder = $this->newTableBuilder(); - foreach( $tablesToDrop as $tableName ){ + foreach( $tablesToDrop as $tableName ) { if ( $tableBuilder->tableExists( $tableName ) ) { $tableBuilder->dropTable( $tableName ); } @@ -61,7 +60,7 @@ return $trBuilder->setConnection( $connectionProvider )->getTableDefinitionReader( $this->newQueryInterface() ); } - protected function newSchemaModifier(){ + protected function newSchemaModifier() { $connectionProvider = new LazyDBConnectionProvider( DB_MASTER ); $schemaModifierBuilder = new MediaWikiSchemaModifierBuilder(); return $schemaModifierBuilder @@ -70,15 +69,113 @@ ->getSchemaModifier(); } - public function testModifyTable(){ - $tableBuilder = $this->newTableBuilder(); + public function getType(){ + $connectionProvider = new LazyDBConnectionProvider( DB_MASTER ); + return $connectionProvider->getConnection()->getType(); + } + + public function testAddField() { $table = new TableDefinition( 'modify_table_test', array( new FieldDefinition( 'startField', FieldDefinition::TYPE_TEXT ) ) ); + $this->setupTestTable( $table ); + $newField = new FieldDefinition( 'secondField', FieldDefinition::TYPE_INTEGER ); + $this->newSchemaModifier()->addField( $table->getName(), $newField ); + $table = $table->mutateFields( array_merge( $table->getFields(), array( $newField ) ) ); + $this->assertTable( $this->newTableBuilder(), $table, 'assert field added' ); + } + + public function testAddIndex() { + $table = new TableDefinition( + 'modify_table_test', + array( + new FieldDefinition( 'startField', FieldDefinition::TYPE_INTEGER ) + ) + ); + $this->setupTestTable( $table ); + + $newIndex = new IndexDefinition( 'indexName', array( 'startField' => 0 ) ); + $this->newSchemaModifier()->addIndex( $table->getName(), $newIndex ); + $table = $table->mutateIndexes( array_merge( $table->getIndexes(), array( $newIndex ) ) ); + $this->assertTable( $this->newTableBuilder(), $table, 'assert index added' ); + } + + public function testRemoveField() { + $table = new TableDefinition( + 'modify_table_test', + array( + new FieldDefinition( 'startField1', FieldDefinition::TYPE_TEXT ), + new FieldDefinition( 'startField2', FieldDefinition::TYPE_TEXT ), + ) + ); + $this->setupTestTable( $table ); + + $removeField = new FieldDefinition( 'startField2', FieldDefinition::TYPE_INTEGER ); + $this->newSchemaModifier()->removeField( $table->getName(), $removeField->getName() ); + $table = $table->mutateFieldAway( $removeField->getName() ); + $this->assertTable( $this->newTableBuilder(), $table, 'assert field removed' ); + } + + public function testRemoveIndex() { + $table = new TableDefinition( + 'modify_table_test', + array( + new FieldDefinition( 'startField', FieldDefinition::TYPE_INTEGER ) + ), + array( + new IndexDefinition( 'indexName', array( 'startField' => 0 ) ) + ) + ); + $this->setupTestTable( $table ); + + $removeIndex = new IndexDefinition( 'indexName', array( 'startField' => 0 ) ); + $this->newSchemaModifier()->removeIndex( $table->getName(), $removeIndex->getName() ); + $table = $table->mutateIndexAway( $removeIndex->getName() ); + $this->assertTable( $this->newTableBuilder(), $table, 'assert index removed' ); + } + + public function testFieldAddRemoveRoundtrip() { + $startTable = new TableDefinition( + 'modify_table_test', + array( + new FieldDefinition( 'startField', FieldDefinition::TYPE_TEXT ) + ) + ); + $this->setupTestTable( $startTable ); + $field = new FieldDefinition( 'secondField', FieldDefinition::TYPE_INTEGER ); + + $this->newSchemaModifier()->addField( $startTable->getName(), $field ); + $newTable = $startTable->mutateFields( array_merge( $startTable->getFields(), array( $field ) ) ); + $this->assertTable( $this->newTableBuilder(), $newTable, 'assert field added' ); + + $this->newSchemaModifier()->removeField( $startTable->getName(), $field->getName() ); + $this->assertTable( $this->newTableBuilder(), $startTable, 'assert field remove' ); + } + + public function testIndexAddRemoveRoundtrip() { + $startTable = new TableDefinition( + 'modify_table_test', + array( + new FieldDefinition( 'startField', FieldDefinition::TYPE_INTEGER ) + ) + ); + $this->setupTestTable( $startTable ); + $index = new IndexDefinition( 'indexName', array( 'startField' => 0 ) ); + + $this->newSchemaModifier()->addIndex( $startTable->getName(), $index ); + $newTable = $startTable->mutateIndexes( array_merge( $startTable->getIndexes(), array( $index ) ) ); + $this->assertTable( $this->newTableBuilder(), $newTable, 'assert index added' ); + + $this->newSchemaModifier()->removeIndex( $startTable->getName(), $index->getName() ); + $this->assertTable( $this->newTableBuilder(), $startTable, 'assert index remove' ); + } + + public function setupTestTable( TableDefinition $table ) { + $tableBuilder = $this->newTableBuilder(); $this->assertFalse( $tableBuilder->tableExists( $table->getName() ), 'Table should not exist before creation' @@ -86,26 +183,9 @@ $tableBuilder->createTable( $table ); $this->assertTable( $tableBuilder, $table, 'assert table after creation' ); - - $schemaModifer = $this->newSchemaModifier(); - - //add a new field - $newField = new FieldDefinition( 'secondField', FieldDefinition::TYPE_INTEGER ); - $schemaModifer->addField( $table->getName(), $newField ); - $table = $table->mutateFields( array_merge( $table->getFields(), array( $newField ) ) ); - $this->assertTable( $tableBuilder, $table, 'assert field added' ); - - //remove a new index - $newIndex = new IndexDefinition( 'indexName', array( 'secondField' => 0 ) ); - $schemaModifer->addIndex( $table->getName(), $newIndex ); - $table = $table->mutateIndexes( array_merge( $table->getIndexes(), array( $newIndex ) ) ); - $this->assertTable( $tableBuilder, $table, 'assert index added' ); - - //TODO remove and field - //TODO remove an index } - protected function assertTable( TableBuilder $tableBuilder, TableDefinition $table, $message = '' ){ + protected function assertTable( TableBuilder $tableBuilder, TableDefinition $table, $message = '' ) { $this->assertTrue( $tableBuilder->tableExists( $table->getName() ), $message . ' (tableExists)' @@ -116,7 +196,7 @@ $this->assertEquals( $table, $tableReader->readDefinition( $table->getName() ), - $message . '(definitionEquals)' + $message . ' (definitionEquals)' ); } diff --git a/tests/phpunit/SQLite/SQLiteSchemaSqlBuilderTest.php b/tests/phpunit/SQLite/SQLiteSchemaSqlBuilderTest.php index e49badc..57e563a 100644 --- a/tests/phpunit/SQLite/SQLiteSchemaSqlBuilderTest.php +++ b/tests/phpunit/SQLite/SQLiteSchemaSqlBuilderTest.php @@ -34,7 +34,7 @@ } ) ); $mockTableNameFormatter = $this->getMock( 'Wikibase\Database\TableNameFormatter' ); - $mockTableNameFormatter->expects( $this->atLeastOnce() ) + $mockTableNameFormatter->expects( $this->any() ) ->method( 'formatTableName' ) ->will( $this->returnArgument(0) ); @@ -93,7 +93,7 @@ public function testGetRemoveIndexSql(){ $instance = $this->newInstance( ); $sql = $instance->getRemoveIndexSql( 'tableName', 'textField' ); - $this->assertEquals( "DROP INDEX IF EXISTS tableName.textField", $sql ); + $this->assertEquals( "DROP INDEX IF EXISTS -textField-", $sql ); } public function testGetAddIndexSql(){ -- To view, visit https://gerrit.wikimedia.org/r/89712 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iba9a796ef0777427b4494108be02b3f4771a0a52 Gerrit-PatchSet: 5 Gerrit-Project: mediawiki/extensions/WikibaseDatabase Gerrit-Branch: master Gerrit-Owner: Addshore <addshorew...@gmail.com> Gerrit-Reviewer: Jeroen De Dauw <jeroended...@gmail.com> Gerrit-Reviewer: jenkins-bot _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits