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

Reply via email to