From 4b2085093eeb576120c9c02185b2c6162c2a1a33 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 1 Jun 2021 18:21:49 +1200 Subject: [PATCH] [Ref] replace direct calls to enable logging with calls to the setting --- .../CRM/Core/BAO/SchemaHandlerTest.php | 5 +- tests/phpunit/CRM/Logging/LoggingTest.php | 43 +++---- tests/phpunit/CRM/Logging/SchemaTest.php | 118 ++++++++++-------- 3 files changed, 86 insertions(+), 80 deletions(-) diff --git a/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php b/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php index 85b42f4998..273338a5dc 100644 --- a/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php +++ b/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php @@ -336,13 +336,12 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { /** * Test that columns are dropped */ - public function testDropColumn() { + public function testDropColumn(): void { CRM_Core_DAO::executeQuery('DROP TABLE IF EXISTS `civicrm_test_drop_column`'); CRM_Core_DAO::executeQuery('CREATE TABLE `civicrm_test_drop_column` (`id` int(10), `col1` varchar(255), `col2` varchar(255))'); // test with logging enabled to ensure log triggers don't break anything - $schema = new CRM_Logging_Schema(); - $schema->enableLogging(); + Civi::settings()->set('logging', TRUE); $alterParams = [ 'table_name' => 'civicrm_test_drop_column', diff --git a/tests/phpunit/CRM/Logging/LoggingTest.php b/tests/phpunit/CRM/Logging/LoggingTest.php index cd1d64c750..2a92319f55 100644 --- a/tests/phpunit/CRM/Logging/LoggingTest.php +++ b/tests/phpunit/CRM/Logging/LoggingTest.php @@ -7,67 +7,64 @@ class CRM_Logging_LoggingTest extends CiviUnitTestCase { public function tearDown(): void { + Civi::settings()->set('logging', FALSE); CRM_Core_I18n_Schema::makeSinglelingual('en_US'); $logging = new CRM_Logging_Schema(); $logging->dropAllLogTables(); - \Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; parent::tearDown(); } /** * Test creating logging schema when database is in multilingual mode. */ - public function testMultilingualLogging() { + public function testMultilingualLogging(): void { CRM_Core_I18n_Schema::makeMultilingual('en_US'); - $logging = new CRM_Logging_Schema(); - $logging->enableLogging(); - $value = CRM_Core_DAO::singleValueQuery("SELECT id FROM log_civicrm_contact LIMIT 1", [], FALSE, FALSE); + Civi::settings()->set('logging', TRUE); + $value = CRM_Core_DAO::singleValueQuery('SELECT id FROM log_civicrm_contact LIMIT 1', [], FALSE, FALSE); $this->assertNotNull($value, 'Logging not enabled successfully'); - $logging->disableLogging(); } /** * Test creating logging schema when database is in multilingual mode. * Also test altering a multilingual table. */ - public function testMultilingualAlterSchemaLogging() { + public function testMultilingualAlterSchemaLogging(): void { CRM_Core_I18n_Schema::makeMultilingual('en_US'); + Civi::settings()->set('logging', TRUE); $logging = new CRM_Logging_Schema(); - $logging->enableLogging(); - $value = CRM_Core_DAO::singleValueQuery("SELECT id FROM log_civicrm_contact LIMIT 1", [], FALSE, FALSE); + $value = CRM_Core_DAO::singleValueQuery('SELECT id FROM log_civicrm_contact LIMIT 1', [], FALSE, FALSE); $this->assertNotNull($value, 'Logging not enabled successfully'); CRM_Core_DAO::executeQuery("ALTER TABLE `civicrm_option_value` ADD COLUMN `logging_test` INT DEFAULT '0'", [], FALSE, NULL, FALSE, FALSE); CRM_Core_I18n_Schema::rebuildMultilingualSchema(['en_US']); - \Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; + Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; $logging->fixSchemaDifferencesFor('civicrm_option_value'); Civi::service('sql_triggers')->rebuild('civicrm_option_value'); - $query = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE `log_civicrm_option_value`", [], TRUE, NULL, FALSE, FALSE); + $query = CRM_Core_DAO::executeQuery('SHOW CREATE TABLE `log_civicrm_option_value`', [], TRUE, NULL, FALSE, FALSE); $query->fetch(); $create = explode("\n", $query->Create_Table); // MySQL may return "DEFAULT 0" or "DEFAULT '0'" depending on version $this->assertTrue( - in_array(" `logging_test` int(11) DEFAULT '0'", $create) - || in_array(" `logging_test` int(11) DEFAULT 0", $create) - || in_array(" `logging_test` int DEFAULT '0'", $create) - || in_array(" `logging_test` int DEFAULT 0", $create) + in_array(" `logging_test` int(11) DEFAULT '0'", $create, TRUE) + || in_array(' `logging_test` int(11) DEFAULT 0', $create, TRUE) + || in_array(" `logging_test` int DEFAULT '0'", $create, TRUE) + || in_array(' `logging_test` int DEFAULT 0', $create, TRUE) ); - CRM_Core_DAO::executeQuery("ALTER TABLE `civicrm_option_value` DROP COLUMN `logging_test`", [], FALSE, NULL, FALSE, FALSE); - $query = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE `log_civicrm_option_value`", [], TRUE, NULL, FALSE, FALSE); + CRM_Core_DAO::executeQuery('ALTER TABLE `civicrm_option_value` DROP COLUMN `logging_test`', [], FALSE, NULL, FALSE, FALSE); + $query = CRM_Core_DAO::executeQuery('SHOW CREATE TABLE `log_civicrm_option_value`', [], TRUE, NULL, FALSE, FALSE); $query->fetch(); $domain = new CRM_Core_DAO_Domain(); $domain->find(TRUE); $locales = explode(CRM_Core_DAO::VALUE_SEPARATOR, $domain->locales); - \Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; + Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; CRM_Core_I18n_Schema::rebuildMultilingualSchema($locales); $logging->fixSchemaDifferencesFor('civicrm_option_value'); Civi::service('sql_triggers')->rebuild('civicrm_option_value'); $this->assertTrue( - in_array(" `logging_test` int(11) DEFAULT '0'", $create) - || in_array(" `logging_test` int(11) DEFAULT 0", $create) - || in_array(" `logging_test` int DEFAULT '0'", $create) - || in_array(" `logging_test` int DEFAULT 0", $create) + in_array(" `logging_test` int(11) DEFAULT '0'", $create, TRUE) + || in_array(' `logging_test` int(11) DEFAULT 0', $create, TRUE) + || in_array(" `logging_test` int DEFAULT '0'", $create, TRUE) + || in_array(' `logging_test` int DEFAULT 0', $create, TRUE) ); - $logging->disableLogging(); } } diff --git a/tests/phpunit/CRM/Logging/SchemaTest.php b/tests/phpunit/CRM/Logging/SchemaTest.php index e438ab80db..70e014bd9f 100644 --- a/tests/phpunit/CRM/Logging/SchemaTest.php +++ b/tests/phpunit/CRM/Logging/SchemaTest.php @@ -16,19 +16,21 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { /** * Clean up after test. * + * @throws \API_Exception * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception */ public function tearDown(): void { $schema = new CRM_Logging_Schema(); - $schema->disableLogging(); + $schema->dropAllLogTables(); + Civi::settings()->set('logging', FALSE); $this->databaseVersion = NULL; parent::tearDown(); $this->quickCleanup(['civicrm_contact'], TRUE); - $schema->dropAllLogTables(); - CRM_Core_DAO::executeQuery("DROP TABLE IF EXISTS civicrm_test_table"); - CRM_Core_DAO::executeQuery("DROP TABLE IF EXISTS civicrm_test_column_info"); - CRM_Core_DAO::executeQuery("DROP TABLE IF EXISTS civicrm_test_length_change"); - CRM_Core_DAO::executeQuery("DROP TABLE IF EXISTS civicrm_test_enum_change"); + CRM_Core_DAO::executeQuery('DROP TABLE IF EXISTS civicrm_test_table'); + CRM_Core_DAO::executeQuery('DROP TABLE IF EXISTS civicrm_test_column_info'); + CRM_Core_DAO::executeQuery('DROP TABLE IF EXISTS civicrm_test_length_change'); + CRM_Core_DAO::executeQuery('DROP TABLE IF EXISTS civicrm_test_enum_change'); } /** @@ -36,7 +38,7 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { * * @return array */ - public function queryExamples() { + public function queryExamples(): array { $examples = []; $examples[] = ["`modified_date` timestamp NULL DEFAULT current_timestamp() ON UPDATE current_timestamp() COMMENT 'When the mailing (or closely related entity) was created or modified or deleted.'", "`modified_date` timestamp NULL COMMENT 'When the mailing (or closely related entity) was created or modified or deleted.'"]; $examples[] = ["`modified_date` timestamp NULL DEFAULT current_timestamp ON UPDATE current_timestamp COMMENT 'When the mailing (or closely related entity) was created or modified or deleted.'", "`modified_date` timestamp NULL COMMENT 'When the mailing (or closely related entity) was created or modified or deleted.'"]; @@ -55,10 +57,9 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { /** * Test log tables are created as InnoDB by default */ - public function testLogEngine() { - $schema = new CRM_Logging_Schema(); - $schema->enableLogging(); - $log_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE log_civicrm_acl"); + public function testLogEngine(): void { + Civi::settings()->set('logging', TRUE); + $log_table = CRM_Core_DAO::executeQuery('SHOW CREATE TABLE log_civicrm_acl'); while ($log_table->fetch()) { $this->assertRegexp('/ENGINE=InnoDB/', $log_table->Create_Table); } @@ -67,11 +68,10 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { /** * Test that the log table engine can be changed via hook to e.g. MyISAM */ - public function testHookLogEngine() { + public function testHookLogEngine(): void { $this->hookClass->setHook('civicrm_alterLogTables', [$this, 'alterLogTables']); - $schema = new CRM_Logging_Schema(); - $schema->enableLogging(); - $log_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE log_civicrm_acl"); + Civi::settings()->set('logging', TRUE); + $log_table = CRM_Core_DAO::executeQuery('SHOW CREATE TABLE log_civicrm_acl'); while ($log_table->fetch()) { $this->assertRegexp('/ENGINE=MyISAM/', $log_table->Create_Table); } @@ -80,7 +80,7 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { /** * Tests that choosing to ignore a custom table does not result in e-notices. */ - public function testIgnoreCustomTableByHook() { + public function testIgnoreCustomTableByHook(): void { $group = $this->customGroupCreate(); Civi::settings()->set('logging', TRUE); $this->hookClass->setHook('civicrm_alterLogTables', [$this, 'noCustomTables']); @@ -92,7 +92,7 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { * * @param array $logTableSpec */ - public function noCustomTables(&$logTableSpec) { + public function noCustomTables(&$logTableSpec): void { foreach (array_keys($logTableSpec) as $index) { if (substr($index, 0, 14) === 'civicrm_value_') { unset($logTableSpec[$index]); @@ -105,12 +105,12 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { * * @throws \Exception */ - public function testArchiveEngineConversion() { + public function testArchiveEngineConversion(): void { + Civi::settings()->set('logging', TRUE); $schema = new CRM_Logging_Schema(); - $schema->enableLogging(); // change table to ARCHIVE - CRM_Core_DAO::executeQuery("ALTER TABLE log_civicrm_acl ENGINE ARCHIVE"); - $log_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE log_civicrm_acl"); + CRM_Core_DAO::executeQuery('ALTER TABLE log_civicrm_acl ENGINE ARCHIVE'); + $log_table = CRM_Core_DAO::executeQuery('SHOW CREATE TABLE log_civicrm_acl'); while ($log_table->fetch()) { $this->assertRegexp('/ENGINE=ARCHIVE/', $log_table->Create_Table); } @@ -167,8 +167,9 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { test_id int(10) unsigned NOT NULL AUTO_INCREMENT, PRIMARY KEY (`test_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci"); + + Civi::settings()->set('logging', TRUE); $schema = new CRM_Logging_Schema(); - $schema->enableLogging(); $diffs = $schema->columnsWithDiffSpecs("civicrm_test_table", "log_civicrm_test_table"); // Test that just having a non id named column with Auto Increment doesn't create diffs $this->assertTrue(empty($diffs['MODIFY'])); @@ -176,34 +177,35 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { $this->assertTrue(empty($diffs['OBSOLETE'])); // Check we can add a primary key to the log table and it will not be treated as obsolete. - CRM_Core_DAO::executeQuery(" + CRM_Core_DAO::executeQuery(' ALTER TABLE log_civicrm_test_table ADD COLUMN `log_id` bigint(20) unsigned NOT NULL AUTO_INCREMENT, ADD PRIMARY KEY (`log_id`) - "); - \Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; + '); + Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; $diffs = $schema->columnsWithDiffSpecs('civicrm_test_table', "log_civicrm_test_table"); - $this->assertTrue(empty($diffs['OBSOLETE'])); + $this->assertEmpty($diffs['OBSOLETE']); CRM_Core_DAO::executeQuery("ALTER TABLE civicrm_test_table ADD COLUMN test_varchar varchar(255) DEFAULT NULL"); - \Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; + Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; // Check that it still picks up new columns added. $diffs = $schema->columnsWithDiffSpecs("civicrm_test_table", "log_civicrm_test_table"); $this->assertTrue(!empty($diffs['ADD'])); $this->assertTrue(empty($diffs['MODIFY'])); $this->assertTrue(empty($diffs['OBSOLETE'])); + unset(Civi::$statics['CRM_Logging_Schema']); $schema->fixSchemaDifferences(); - CRM_Core_DAO::executeQuery("ALTER TABLE civicrm_test_table CHANGE COLUMN test_varchar test_varchar varchar(400) DEFAULT NULL"); + CRM_Core_DAO::executeQuery('ALTER TABLE civicrm_test_table CHANGE COLUMN test_varchar test_varchar varchar(400) DEFAULT NULL'); // Check that it properly picks up modifications to columns. - \Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; + unset(Civi::$statics['CRM_Logging_Schema']); $diffs = $schema->columnsWithDiffSpecs("civicrm_test_table", "log_civicrm_test_table"); - $this->assertTrue(!empty($diffs['MODIFY'])); - $this->assertTrue(empty($diffs['ADD'])); - $this->assertTrue(empty($diffs['OBSOLETE'])); + $this->assertNotEmpty($diffs['MODIFY']); + $this->assertEmpty($diffs['ADD']); + $this->assertEmpty($diffs['OBSOLETE']); $schema->fixSchemaDifferences(); - CRM_Core_DAO::executeQuery("ALTER TABLE civicrm_test_table CHANGE COLUMN test_varchar test_varchar varchar(300) DEFAULT NULL"); + CRM_Core_DAO::executeQuery('ALTER TABLE civicrm_test_table CHANGE COLUMN test_varchar test_varchar varchar(300) DEFAULT NULL'); // Check that when we reduce the size of column that the log table doesn't shrink as well. - \Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; - $diffs = $schema->columnsWithDiffSpecs("civicrm_test_table", "log_civicrm_test_table"); + Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; + $diffs = $schema->columnsWithDiffSpecs('civicrm_test_table', "log_civicrm_test_table"); $this->assertTrue(empty($diffs['MODIFY'])); $this->assertTrue(empty($diffs['ADD'])); $this->assertTrue(empty($diffs['OBSOLETE'])); @@ -215,7 +217,7 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { public function testTriggerInfo() { $info = []; $schema = new CRM_Logging_Schema(); - $schema->enableLogging(); + Civi::settings()->set('logging', TRUE); $schema->triggerInfo($info, 'civicrm_group'); // should have 3 triggers (insert/update/delete) $this->assertCount(3, $info); @@ -232,7 +234,10 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { } } - public function testColumnInfo() { + /** + * @throws \CiviCRM_API3_Exception + */ + public function testColumnInfo(): void { CRM_Core_DAO::executeQuery("CREATE TABLE `civicrm_test_column_info` ( test_id int(10) unsigned NOT NULL AUTO_INCREMENT, test_varchar varchar(42) NOT NULL, @@ -243,10 +248,11 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { test_date date DEFAULT NULL, PRIMARY KEY (`test_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci"); + Civi::settings()->set('logging', TRUE); + $schema = new CRM_Logging_Schema(); - $schema->enableLogging(); $schema->updateLogTableSchema(['updateChangedEngineConfig' => FALSE, 'forceEngineMigration' => FALSE]); - $ci = \Civi::$statics['CRM_Logging_Schema']['columnSpecs']['civicrm_test_column_info']; + $ci = Civi::$statics['CRM_Logging_Schema']['columnSpecs']['civicrm_test_column_info']; $this->assertEquals('test_id', $ci['test_id']['COLUMN_NAME']); $this->assertEquals('int', $ci['test_id']['DATA_TYPE']); @@ -293,15 +299,15 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { test_decimal decimal(20,2) NULL, PRIMARY KEY (`test_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci"); + Civi::settings()->set('logging', TRUE); $schema = new CRM_Logging_Schema(); - $schema->enableLogging(); CRM_Core_DAO::executeQuery( "ALTER TABLE civicrm_test_length_change CHANGE COLUMN test_integer test_integer int(6) NULL, CHANGE COLUMN test_decimal test_decimal decimal(22,2) NULL" ); $schema->fixSchemaDifferences(); - $ci = \Civi::$statics['CRM_Logging_Schema']['columnSpecs']; + $ci = Civi::$statics['CRM_Logging_Schema']['columnSpecs']; // length should increase if (!$this->isMySQL8()) { $this->assertEquals(6, $ci['log_civicrm_test_length_change']['test_integer']['LENGTH']); @@ -313,7 +319,7 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { CHANGE COLUMN test_decimal test_decimal decimal(20,2) NULL" ); $schema->fixSchemaDifferences(); - $ci = \Civi::$statics['CRM_Logging_Schema']['columnSpecs']; + $ci = Civi::$statics['CRM_Logging_Schema']['columnSpecs']; // length should not decrease if (!$this->isMySQL8()) { $this->assertEquals(6, $ci['log_civicrm_test_length_change']['test_integer']['LENGTH']); @@ -321,28 +327,32 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { $this->assertEquals('22,2', $ci['log_civicrm_test_length_change']['test_decimal']['LENGTH']); } - public function testEnumChange() { + /** + * Test changing the enum. + */ + public function testEnumChange(): void { CRM_Core_DAO::executeQuery("CREATE TABLE `civicrm_test_enum_change` ( test_id int(10) unsigned NOT NULL AUTO_INCREMENT, test_enum enum('A','B','C') NULL, PRIMARY KEY (`test_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci"); + Civi::settings()->set('logging', TRUE); $schema = new CRM_Logging_Schema(); - $schema->enableLogging(); CRM_Core_DAO::executeQuery("ALTER TABLE civicrm_test_enum_change CHANGE COLUMN test_enum test_enum enum('A','B','C','D') NULL"); - \Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; + Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = []; $schema->fixSchemaDifferences(); - $ci = \Civi::$statics['CRM_Logging_Schema']['columnSpecs']; + $ci = Civi::$statics['CRM_Logging_Schema']['columnSpecs']; // new enum value should be included $this->assertEquals("'A','B','C','D'", $ci['civicrm_test_enum_change']['test_enum']['ENUM_VALUES']); } /** * Test editing a custom field + * + * @throws \CRM_Core_Exception */ - public function testCustomFieldEdit() { - $schema = new CRM_Logging_Schema(); - $schema->enableLogging(); + public function testCustomFieldEdit(): void { + Civi::settings()->set('logging', TRUE); $customGroup = $this->entityCustomGroupWithSingleFieldCreate('Contact', 'ContactTest.php'); // get the custom group table name @@ -367,6 +377,7 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { $this->callAPISuccess('custom_field', 'create', $params); // update logging schema + $schema = new CRM_Logging_Schema(); $schema->fixSchemaDifferences(); // verify @@ -379,9 +390,8 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { * Test creating a table with SchemaHandler::createTable when logging * is enabled. */ - public function testCreateTableWithLogging() { - $schema = new CRM_Logging_Schema(); - $schema->enableLogging(); + public function testCreateTableWithLogging(): void { + Civi::settings()->set('logging', TRUE); CRM_Core_BAO_SchemaHandler::createTable([ 'name' => 'civicrm_test_table', @@ -422,7 +432,7 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { $this->assertStringContainsString('FOREIGN KEY (`activity_id`) REFERENCES `civicrm_activity` (`id`) ON DELETE CASCADE', $dao->Create_Table); // Check log table. - $dao = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE log_civicrm_test_table"); + $dao = CRM_Core_DAO::executeQuery('SHOW CREATE TABLE log_civicrm_test_table'); $dao->fetch(); $this->assertStringNotContainsString('AUTO_INCREMENT', $dao->Create_Table); // This seems debatable whether `id` should lose its NOT NULL status @@ -438,7 +448,7 @@ class CRM_Logging_SchemaTest extends CiviUnitTestCase { * * @return bool */ - protected function isMySQL8() { + protected function isMySQL8(): bool { return (bool) (version_compare($this->databaseVersion, '8.0.19', '>=') && stripos($this->databaseVersion, 'mariadb') === FALSE); } -- 2.25.1