From 38c21de0ee17bb0eacd6cc8f0e1ed71cbee0b0c9 Mon Sep 17 00:00:00 2001 From: marc tobias Date: Mon, 13 Apr 2020 17:48:20 +0200 Subject: [PATCH 1/2] Nominatim::DB tests against separate postgresql database --- .travis.yml | 2 +- lib/DB.php | 70 +++++++++++++++++- lib/setup/SetupClass.php | 6 +- test/README.md | 4 +- test/php/Nominatim/DBTest.php | 126 +++++++++++++++++++++++++++++++- utils/check_import_finished.php | 2 +- 6 files changed, 200 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index f5344742..dd973aec 100644 --- a/.travis.yml +++ b/.travis.yml @@ -25,7 +25,7 @@ script: - cd $TRAVIS_BUILD_DIR/ - if [[ $TEST_SUITE == "tests" ]]; then phpcs --report-width=120 . ; fi - cd $TRAVIS_BUILD_DIR/test/php - - if [[ $TEST_SUITE == "tests" ]]; then /usr/bin/phpunit ./ ; fi + - if [[ $TEST_SUITE == "tests" ]]; then UNIT_TEST_DSN='pgsql:dbname=nominatim_unit_tests' /usr/bin/phpunit ./ ; fi - cd $TRAVIS_BUILD_DIR/test/bdd - # behave --format=progress3 api - if [[ $TEST_SUITE == "tests" ]]; then behave -DREMOVE_TEMPLATE=1 --format=progress3 db ; fi diff --git a/lib/DB.php b/lib/DB.php index 6307d6ca..5915b2e8 100644 --- a/lib/DB.php +++ b/lib/DB.php @@ -240,6 +240,28 @@ class DB return ($this->getOne($sSQL, array(':tablename' => $sTableName)) == 1); } + /** + * Returns a list of table names in the database + * + * @return array[] + */ + public function getListOfTables() + { + return $this->getCol("SELECT tablename FROM pg_tables WHERE schemaname='public'"); + } + + /** + * Deletes a table. Returns true on success. Returns true if the table didn't exist. + * + * @param string $sTableName + * + * @return boolean + */ + public function deleteTable($sTableName) + { + return $this->exec('DROP TABLE IF EXISTS '.$sTableName.' CASCADE') == 0; + } + /** * Check if an index exists in the database. Optional filtered by tablename * @@ -311,11 +333,11 @@ END; } /** - * Since the DSN includes the database name, checks if the connection works. + * Tries to connect to the database but on failure doesn't throw an exception. * * @return boolean */ - public function databaseExists() + public function checkConnection() { $bExists = true; try { @@ -350,6 +372,13 @@ END; return (float) ($aMatches[1].'.'.$aMatches[2]); } + /** + * Returns an associate array of postgresql database connection settings. Keys can + * be 'database', 'hostspec', 'port', 'username', 'password'. + * Returns empty array on failure, thus check if at least 'database' is set. + * + * @return array[] + */ public static function parseDSN($sDSN) { // https://secure.php.net/manual/en/ref.pdo-pgsql.connection.php @@ -365,4 +394,41 @@ END; } return $aInfo; } + + /** + * Takes an array of settings and return the DNS string. Key names can be + * 'database', 'hostspec', 'port', 'username', 'password' but aliases + * 'dbname', 'host' and 'user' are also supported. + * + * @return string + * + */ + public static function generateDSN($aInfo) + { + $sDSN = 'pgsql:'; + if (isset($aInfo['host'])) { + $sDSN .= 'host=' . $aInfo['host'] . ';'; + } elseif (isset($aInfo['hostspec'])) { + $sDSN .= 'host=' . $aInfo['hostspec'] . ';'; + } + if (isset($aInfo['port'])) { + $sDSN .= 'port=' . $aInfo['port'] . ';'; + } + if (isset($aInfo['dbname'])) { + $sDSN .= 'dbname=' . $aInfo['dbname'] . ';'; + } elseif (isset($aInfo['database'])) { + $sDSN .= 'dbname=' . $aInfo['database'] . ';'; + } + if (isset($aInfo['user'])) { + $sDSN .= 'user=' . $aInfo['user'] . ';'; + } elseif (isset($aInfo['username'])) { + $sDSN .= 'user=' . $aInfo['username'] . ';'; + } + if (isset($aInfo['password'])) { + $sDSN .= 'password=' . $aInfo['password'] . ';'; + } + $sDSN = preg_replace('/;$/', '', $sDSN); + + return $sDSN; + } } diff --git a/lib/setup/SetupClass.php b/lib/setup/SetupClass.php index ac0f8f02..7c1c628e 100755 --- a/lib/setup/SetupClass.php +++ b/lib/setup/SetupClass.php @@ -84,7 +84,7 @@ class SetupFunctions info('Create DB'); $oDB = new \Nominatim\DB; - if ($oDB->databaseExists()) { + if ($oDB->checkConnection()) { fail('database already exists ('.CONST_Database_DSN.')'); } @@ -651,7 +651,7 @@ class SetupFunctions ); $aDropTables = array(); - $aHaveTables = $this->oDB->getCol("SELECT tablename FROM pg_tables WHERE schemaname='public'"); + $aHaveTables = $this->oDB->getListOfTables(); foreach ($aHaveTables as $sTable) { $bFound = false; @@ -858,7 +858,7 @@ class SetupFunctions private function dropTable($sName) { if ($this->bVerbose) echo "Dropping table $sName\n"; - $this->oDB->exec('DROP TABLE IF EXISTS '.$sName.' CASCADE'); + $this->oDB->deleteTable($sName); } /** diff --git a/test/README.md b/test/README.md index ab5f7f4c..5de08759 100644 --- a/test/README.md +++ b/test/README.md @@ -46,11 +46,13 @@ Very low coverage. To execute the test suite run cd test/php - phpunit ../ + UNIT_TEST_DSN='pgsql:dbname=nominatim_unit_tests' phpunit ../ It will read phpunit.xml which points to the library, test path, bootstrap strip and set other parameters. +The database set by `UNIT_TEST_DSN` will be deleted and recreated. Not setting +it will skip some tests as pending, but not fail the tests. BDD Functional Tests ==================== diff --git a/test/php/Nominatim/DBTest.php b/test/php/Nominatim/DBTest.php index 38874c88..e4791975 100644 --- a/test/php/Nominatim/DBTest.php +++ b/test/php/Nominatim/DBTest.php @@ -24,10 +24,10 @@ class DBTest extends \PHPUnit\Framework\TestCase $this->assertTrue($oDB->connect()); } - public function testDatabaseExists() + public function testCheckConnection() { $oDB = new \Nominatim\DB(''); - $this->assertFalse($oDB->databaseExists()); + $this->assertFalse($oDB->checkConnection()); } public function testErrorHandling() @@ -113,4 +113,126 @@ class DBTest extends \PHPUnit\Framework\TestCase \Nominatim\DB::parseDSN('pgsql:dbname=db1;host=machine1;port=1234;user=john;password=secret') ); } + + public function testGenerateDSN() + { + $this->assertEquals( + 'pgsql:', + \Nominatim\DB::generateDSN(array()) + ); + $this->assertEquals( + 'pgsql:host=machine1;dbname=db1', + \Nominatim\DB::generateDSN(\Nominatim\DB::parseDSN('pgsql:host=machine1;dbname=db1')) + ); + } + + public function testAgainstDatabase() + { + if (getenv('UNIT_TEST_DSN') == false) $this->markTestSkipped('UNIT_TEST_DSN not set'); + + ## Create the database. + { + $aDSNParsed = \Nominatim\DB::parseDSN(getenv('UNIT_TEST_DSN')); + $sDbname = $aDSNParsed['database']; + $aDSNParsed['database'] = 'postgres'; + + $oDB = new \Nominatim\DB(\Nominatim\DB::generateDSN($aDSNParsed)); + $oDB->connect(); + $oDB->exec('DROP DATABASE IF EXISTS ' . $sDbname); + $oDB->exec('CREATE DATABASE ' . $sDbname); + } + + $oDB = new \Nominatim\DB(getenv('UNIT_TEST_DSN')); + $oDB->connect(); + + $this->assertTrue( + $oDB->checkConnection($sDbname) + ); + + # Tables, Indices + { + $this->assertEmpty($oDB->getListOfTables()); + $oDB->exec('CREATE TABLE table1 (id integer, city varchar, country varchar)'); + $oDB->exec('CREATE TABLE table2 (id integer, city varchar, country varchar)'); + $this->assertEquals( + array('table1', 'table2'), + $oDB->getListOfTables() + ); + $this->assertTrue($oDB->deleteTable('table2')); + $this->assertTrue($oDB->deleteTable('table99')); + $this->assertEquals( + array('table1'), + $oDB->getListOfTables() + ); + + $this->assertTrue($oDB->tableExists('table1')); + $this->assertFalse($oDB->tableExists('table99')); + $this->assertFalse($oDB->tableExists(null)); + + $this->assertEmpty($oDB->getListOfIndices()); + $oDB->exec('CREATE UNIQUE INDEX table1_index ON table1 (id)'); + $this->assertEquals( + array('table1_index'), + $oDB->getListOfIndices() + ); + $this->assertEmpty($oDB->getListOfIndices('table2')); + } + + # select queries + { + $oDB->exec( + "INSERT INTO table1 VALUES (1, 'Berlin', 'Germany'), (2, 'Paris', 'France')" + ); + + $this->assertEquals( + array( + array('city' => 'Berlin'), + array('city' => 'Paris') + ), + $oDB->getAll('SELECT city FROM table1') + ); + $this->assertEquals( + array(), + $oDB->getAll('SELECT city FROM table1 WHERE id=999') + ); + + + $this->assertEquals( + array('id' => 1, 'city' => 'Berlin', 'country' => 'Germany'), + $oDB->getRow('SELECT * FROM table1 WHERE id=1') + ); + $this->assertEquals( + false, + $oDB->getRow('SELECT * FROM table1 WHERE id=999') + ); + + + $this->assertEquals( + array('Berlin', 'Paris'), + $oDB->getCol('SELECT city FROM table1') + ); + $this->assertEquals( + array(), + $oDB->getCol('SELECT city FROM table1 WHERE id=999') + ); + + $this->assertEquals( + 'Berlin', + $oDB->getOne('SELECT city FROM table1 WHERE id=1') + ); + $this->assertEquals( + null, + $oDB->getOne('SELECT city FROM table1 WHERE id=999') + ); + + $this->assertEquals( + array('Berlin' => 'Germany', 'Paris' => 'France'), + $oDB->getAssoc('SELECT city, country FROM table1') + ); + $this->assertEquals( + array(), + $oDB->getAssoc('SELECT city, country FROM table1 WHERE id=999') + ); + } + } } diff --git a/utils/check_import_finished.php b/utils/check_import_finished.php index b81cace1..4529c693 100755 --- a/utils/check_import_finished.php +++ b/utils/check_import_finished.php @@ -28,7 +28,7 @@ function isReverseOnlyInstallation() echo 'Checking database got created ... '; -if ($oDB->databaseExists()) { +if ($oDB->checkConnection()) { $print_success(); } else { $print_fail(); From a5d0657d9b7a7d975082d2d65a20918c1ca5b108 Mon Sep 17 00:00:00 2001 From: marc tobias Date: Sun, 26 Apr 2020 03:33:15 +0200 Subject: [PATCH 2/2] lonvia PR feedback --- .travis.yml | 2 +- lib/DB.php | 35 +++++++++++------------------------ test/README.md | 4 ++-- test/php/Nominatim/DBTest.php | 14 +++++++++++--- 4 files changed, 25 insertions(+), 30 deletions(-) diff --git a/.travis.yml b/.travis.yml index dd973aec..f5344742 100644 --- a/.travis.yml +++ b/.travis.yml @@ -25,7 +25,7 @@ script: - cd $TRAVIS_BUILD_DIR/ - if [[ $TEST_SUITE == "tests" ]]; then phpcs --report-width=120 . ; fi - cd $TRAVIS_BUILD_DIR/test/php - - if [[ $TEST_SUITE == "tests" ]]; then UNIT_TEST_DSN='pgsql:dbname=nominatim_unit_tests' /usr/bin/phpunit ./ ; fi + - if [[ $TEST_SUITE == "tests" ]]; then /usr/bin/phpunit ./ ; fi - cd $TRAVIS_BUILD_DIR/test/bdd - # behave --format=progress3 api - if [[ $TEST_SUITE == "tests" ]]; then behave -DREMOVE_TEMPLATE=1 --format=progress3 db ; fi diff --git a/lib/DB.php b/lib/DB.php index 5915b2e8..38b3e27e 100644 --- a/lib/DB.php +++ b/lib/DB.php @@ -251,7 +251,7 @@ class DB } /** - * Deletes a table. Returns true on success. Returns true if the table didn't exist. + * Deletes a table. Returns true if deleted or didn't exist. * * @param string $sTableName * @@ -405,29 +405,16 @@ END; */ public static function generateDSN($aInfo) { - $sDSN = 'pgsql:'; - if (isset($aInfo['host'])) { - $sDSN .= 'host=' . $aInfo['host'] . ';'; - } elseif (isset($aInfo['hostspec'])) { - $sDSN .= 'host=' . $aInfo['hostspec'] . ';'; - } - if (isset($aInfo['port'])) { - $sDSN .= 'port=' . $aInfo['port'] . ';'; - } - if (isset($aInfo['dbname'])) { - $sDSN .= 'dbname=' . $aInfo['dbname'] . ';'; - } elseif (isset($aInfo['database'])) { - $sDSN .= 'dbname=' . $aInfo['database'] . ';'; - } - if (isset($aInfo['user'])) { - $sDSN .= 'user=' . $aInfo['user'] . ';'; - } elseif (isset($aInfo['username'])) { - $sDSN .= 'user=' . $aInfo['username'] . ';'; - } - if (isset($aInfo['password'])) { - $sDSN .= 'password=' . $aInfo['password'] . ';'; - } - $sDSN = preg_replace('/;$/', '', $sDSN); + $sDSN = sprintf( + 'pgsql:host=%s;port=%s;dbname=%s;user=%s;password=%s;', + $aInfo['host'] ?? $aInfo['hostspec'] ?? '', + $aInfo['port'] ?? '', + $aInfo['dbname'] ?? $aInfo['database'] ?? '', + $aInfo['user'] ?? '', + $aInfo['password'] ?? '' + ); + $sDSN = preg_replace('/\b\w+=;/', '', $sDSN); + $sDSN = preg_replace('/;\Z/', '', $sDSN); return $sDSN; } diff --git a/test/README.md b/test/README.md index 5de08759..1f7a33ca 100644 --- a/test/README.md +++ b/test/README.md @@ -51,8 +51,8 @@ To execute the test suite run It will read phpunit.xml which points to the library, test path, bootstrap strip and set other parameters. -The database set by `UNIT_TEST_DSN` will be deleted and recreated. Not setting -it will skip some tests as pending, but not fail the tests. +It will use (and destroy) a local database 'nominatim_unit_tests'. You can set +a different connection string with e.g. UNIT_TEST_DSN='pgsql:dbname=foo_unit_tests'. BDD Functional Tests ==================== diff --git a/test/php/Nominatim/DBTest.php b/test/php/Nominatim/DBTest.php index e4791975..1991f6f1 100644 --- a/test/php/Nominatim/DBTest.php +++ b/test/php/Nominatim/DBTest.php @@ -128,11 +128,19 @@ class DBTest extends \PHPUnit\Framework\TestCase public function testAgainstDatabase() { - if (getenv('UNIT_TEST_DSN') == false) $this->markTestSkipped('UNIT_TEST_DSN not set'); + $unit_test_dsn = getenv('UNIT_TEST_DSN') != false ? + getenv('UNIT_TEST_DSN') : + 'pgsql:dbname=nominatim_unit_tests'; + + $this->assertRegExp( + '/unit_test/', + $unit_test_dsn, + 'Test database will get destroyed, thus should have a name like unit_test to be safe' + ); ## Create the database. { - $aDSNParsed = \Nominatim\DB::parseDSN(getenv('UNIT_TEST_DSN')); + $aDSNParsed = \Nominatim\DB::parseDSN($unit_test_dsn); $sDbname = $aDSNParsed['database']; $aDSNParsed['database'] = 'postgres'; @@ -142,7 +150,7 @@ class DBTest extends \PHPUnit\Framework\TestCase $oDB->exec('CREATE DATABASE ' . $sDbname); } - $oDB = new \Nominatim\DB(getenv('UNIT_TEST_DSN')); + $oDB = new \Nominatim\DB($unit_test_dsn); $oDB->connect(); $this->assertTrue(