From: Patrick Figel <pfigel@greenpeace.org>
Date: Fri, 25 Jan 2019 15:35:01 +0000 (+0100)
Subject: dev/core#664 - CRM/Logging - Add indexes when updating log schema
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=8d369f66465e1802a49f71b6e0ae8c4cd137ebc5;p=civicrm-core.git

dev/core#664 - CRM/Logging - Add indexes when updating log schema

This change causes indexes set by the "alterLogTables" hook to be
added regardless of whether the schema was changed.
---

diff --git a/CRM/Logging/Schema.php b/CRM/Logging/Schema.php
index c540c93fbf..c75652fa78 100644
--- a/CRM/Logging/Schema.php
+++ b/CRM/Logging/Schema.php
@@ -299,7 +299,7 @@ AND    (TABLE_NAME LIKE 'log_civicrm_%' $nonStandardTableNameString )
    * Update log tables structure.
    *
    * This function updates log tables to have the log_conn_id type of varchar
-   * and also implements any engine change to INNODB defined by the hooks.
+   * and also implements the engine change defined by the hook (i.e. INNODB).
    *
    * Note changing engine & adding hook-defined indexes, but not changing back
    * to ARCHIVE if engine has not been deliberately set (by hook) and not dropping
@@ -312,15 +312,15 @@ AND    (TABLE_NAME LIKE 'log_civicrm_%' $nonStandardTableNameString )
       $tableSpec = $this->logTableSpec[$mainTable];
       if (isset($tableSpec['engine']) && strtoupper($tableSpec['engine']) != $this->getEngineForLogTable($logTable)) {
         $alterSql[] = "ENGINE=" . $tableSpec['engine'] . " " . CRM_Utils_Array::value('engine_config', $tableSpec);
-        if (!empty($tableSpec['indexes'])) {
-          $indexes = $this->getIndexesForTable($logTable);
-          foreach ($tableSpec['indexes'] as $indexName => $indexSpec) {
-            if (!in_array($indexName, $indexes)) {
-              if (is_array($indexSpec)) {
-                $indexSpec = implode(" , ", $indexSpec);
-              }
-              $alterSql[] = "ADD INDEX {$indexName}($indexSpec)";
+      }
+      if (!empty($tableSpec['indexes'])) {
+        $indexes = $this->getIndexesForTable($logTable);
+        foreach ($tableSpec['indexes'] as $indexName => $indexSpec) {
+          if (!in_array($indexName, $indexes)) {
+            if (is_array($indexSpec)) {
+              $indexSpec = implode(" , ", $indexSpec);
             }
+            $alterSql[] = "ADD INDEX {$indexName}($indexSpec)";
           }
         }
       }
diff --git a/tests/phpunit/api/v3/LoggingTest.php b/tests/phpunit/api/v3/LoggingTest.php
index 0d373b93b3..7ff702a307 100644
--- a/tests/phpunit/api/v3/LoggingTest.php
+++ b/tests/phpunit/api/v3/LoggingTest.php
@@ -157,6 +157,11 @@ class api_v3_LoggingTest extends CiviUnitTestCase {
     $this->assertEquals(array(), $spec['civicrm_contact']);
     $this->callAPISuccess('System', 'updatelogtables', array());
     $this->checkINNODBLogTableCreated();
+    // Check if API creates new indexes when they're added by hook
+    $this->hookClass->setHook('civicrm_alterLogTables', [$this, 'innodbLogTableSpecNewIndex']);
+    $this->callAPISuccess('System', 'updatelogtables', array());
+    $this->checkINNODBLogTableCreated();
+    $this->assertContains('KEY `index_log_user_id` (`log_user_id`)', $this->checkLogTableCreated());
   }
 
   /**
@@ -200,6 +205,24 @@ class api_v3_LoggingTest extends CiviUnitTestCase {
     );
   }
 
+  /**
+   * Set log engine to InnoDB and add one index
+   *
+   * @param array $logTableSpec
+   */
+  public function innodbLogTableSpecNewIndex(&$logTableSpec) {
+    $logTableSpec['civicrm_contact'] = array(
+      'engine' => 'InnoDB',
+      'engine_config' => 'ROW_FORMAT=COMPRESSED KEY_BLOCK_SIZE=4',
+      'indexes' => array(
+        'index_id' => 'id',
+        'index_log_conn_id' => 'log_conn_id',
+        'index_log_date' => 'log_date',
+        'index_log_user_id' => 'log_user_id', // new index
+      ),
+    );
+  }
+
   /**
    * Check the log tables were created and look OK.
    */