From 9e949f84b6535d111af4fbc1b1c6ed6342158b5a Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Thu, 9 Mar 2023 17:31:35 +1300 Subject: [PATCH] Fix cache clearing when import table is changed --- .../Event/Subscriber/ImportSubscriber.php | 108 +++++++++++++++--- .../tests/phpunit/CiviApiImportTest.php | 24 +++- 2 files changed, 112 insertions(+), 20 deletions(-) diff --git a/ext/civiimport/Civi/Api4/Event/Subscriber/ImportSubscriber.php b/ext/civiimport/Civi/Api4/Event/Subscriber/ImportSubscriber.php index 4e21ed880b..2ad00eeba2 100644 --- a/ext/civiimport/Civi/Api4/Event/Subscriber/ImportSubscriber.php +++ b/ext/civiimport/Civi/Api4/Event/Subscriber/ImportSubscriber.php @@ -19,6 +19,7 @@ use Civi\Api4\UserJob; use Civi\Api4\Utils\CoreUtil; use Civi\Core\Event\PostEvent; use Civi\Core\Event\GenericHookEvent; +use Civi\Core\Event\PreEvent; use Civi\Core\Service\AutoService; use CRM_Core_DAO_AllCoreTables; use Civi\Api4\Import; @@ -41,6 +42,7 @@ class ImportSubscriber extends AutoService implements EventSubscriberInterface { public static function getSubscribedEvents(): array { return [ 'hook_civicrm_post' => 'on_hook_civicrm_post', + 'hook_civicrm_pre' => 'on_hook_civicrm_pre', 'civi.api4.entityTypes' => 'on_civi_api4_entityTypes', 'civi.api.authorize' => [['onApiAuthorize', Events::W_EARLY]], 'civi.afform.get' => 'on_civi_afform_get', @@ -52,6 +54,8 @@ class ImportSubscriber extends AutoService implements EventSubscriberInterface { * Register each valid import as an entity * * @param \Civi\Core\Event\GenericHookEvent $event + * + * @noinspection PhpUnused */ public static function on_civi_api4_entityTypes(GenericHookEvent $event): void { $importEntities = Civi\BAO\Import::getImportTables(); @@ -96,36 +100,73 @@ class ImportSubscriber extends AutoService implements EventSubscriberInterface { } } + /** + * Callback for hook_civicrm_pre(). + * + * @noinspection PhpUnused + */ + public function on_hook_civicrm_pre(PreEvent $event): void { + if ($event->entity === 'UserJob' && $event->action === 'edit') { + if ($this->isTableChange($event)) { + $this->flushEntityMetadata(); + } + } + } + + /** + * Get the import table from event data. + * + * @param \Civi\Core\Event\GenericHookEvent $event + * + * @return string|null + */ + private function getImportTableFromEvent(GenericHookEvent $event): ?string { + if (isset($event->object)) { + $metadata = json_decode($event->object->metadata, TRUE); + if (!is_array($metadata)) { + return NULL; + } + return $metadata['DataSource']['table_name'] ?: NULL; + } + return $event->params['metadata']['DataSource']['table_name'] ?: NULL; + } + /** * Callback for hook_civicrm_post(). */ public function on_hook_civicrm_post(PostEvent $event): void { if ($event->entity === 'UserJob') { - try { - $exists = Entity::get(FALSE)->addWhere('name', '=', 'Import_' . $event->id)->selectRowCount()->execute()->count(); - if (!$exists || $event->action === 'delete') { - // Flush entities cache key so our new Import will load as an entity. - unset(Civi::$statics['civiimport_tables']); - Civi::cache('metadata')->delete('api4.entities.info'); - Civi::cache('metadata')->delete('civiimport_tables'); - CRM_Core_DAO_AllCoreTables::flush(); - Managed::reconcile(FALSE)->setModules(['civiimport'])->execute(); - } - } - catch (\CRM_Core_Exception $e) { - // Log & move on. - \Civi::log()->warning('Failed to flush cache on UserJob clear', ['exception' => $e]); - return; + if ($event->action === 'delete' || ($this->getImportTableFromEvent($event) && !$this->ImportEntityExists($event))) { + $this->flushEntityMetadata(); } } } + /** + * Is the update changing the associated temp table. + * + * @param \Civi\Core\Event\GenericHookEvent $event + * + * @return bool + * @noinspection PhpDocMissingThrowsInspection + * @noinspection PhpUnhandledExceptionInspection + */ + private function isTableChange(GenericHookEvent $event): bool { + $newTable = $this->getImportTableFromEvent($event); + $userJob = UserJob::get(FALSE) + ->addWhere('id', '=', $event->id) + ->addSelect('metadata')->execute()->first(); + $savedTable = $userJob['metadata']['DataSource']['table_name'] ?? NULL; + return $newTable !== $savedTable; + } + /** * @param \Civi\API\Event\AuthorizeEvent $event * API authorization event. * * @throws \CRM_Core_Exception * @throws \Civi\API\Exception\UnauthorizedException + * @noinspection PhpUnused */ public function onApiAuthorize(AuthorizeEvent $event): void { $apiRequest = $event->getApiRequest(); @@ -148,7 +189,7 @@ class ImportSubscriber extends AutoService implements EventSubscriberInterface { * @noinspection PhpUnused */ public static function on_civi_afform_get(GenericHookEvent $event): void { - // We're only providing afforms of type 'search' + // We're only providing form builder forms of type 'search' if ($event->getTypes && !in_array('search', $event->getTypes, TRUE)) { return; } @@ -203,4 +244,39 @@ class ImportSubscriber extends AutoService implements EventSubscriberInterface { return $forms; } + /** + * Flush entities cache key so our new Import will load as an entity. + */ + protected function flushEntityMetadata(): void { + try { + unset(Civi::$statics['civiimport_tables']); + Civi::cache('metadata')->delete('api4.entities.info'); + Civi::cache('metadata')->delete('civiimport_tables'); + CRM_Core_DAO_AllCoreTables::flush(); + Managed::reconcile(FALSE)->setModules(['civiimport'])->execute(); + } + catch (\CRM_Core_Exception $e) { + // Log & move on. + \Civi::log()->warning('Failed to flush cache on UserJob clear', ['exception' => $e]); + return; + } + } + + /** + * Does the pseudo-entity for the import exist yet. + * + * @param \Civi\Core\Event\PostEvent $event + * + * @return int + * @noinspection PhpDocMissingThrowsInspection + * @noinspection PhpUnhandledExceptionInspection + */ + protected function ImportEntityExists(PostEvent $event): int { + return Entity::get(FALSE) + ->addWhere('name', '=', 'Import_' . $event->id) + ->selectRowCount() + ->execute() + ->count(); + } + } diff --git a/ext/civiimport/tests/phpunit/CiviApiImportTest.php b/ext/civiimport/tests/phpunit/CiviApiImportTest.php index eb5b158a39..6ea0655167 100644 --- a/ext/civiimport/tests/phpunit/CiviApiImportTest.php +++ b/ext/civiimport/tests/phpunit/CiviApiImportTest.php @@ -40,6 +40,9 @@ class CiviApiImportTest extends TestCase implements HeadlessInterface, HookInter ->apply(); } + /** + * @throws \Civi\Core\Exception\DBQueryException + */ public function tearDown():void { CRM_Core_DAO::executeQuery('DROP TABLE IF EXISTS abc'); parent::tearDown(); @@ -52,7 +55,7 @@ class CiviApiImportTest extends TestCase implements HeadlessInterface, HookInter */ public function testApiActions():void { $this->createUserJobTable(); - $userJobID = UserJob::create()->setValues([ + $userJobParameters = [ 'metadata' => [ 'DataSource' => ['table_name' => 'abc', 'column_headers' => ['External Identifier', 'Amount Given', 'Date Received', 'Financial Type', 'In honor']], 'submitted_values' => [ @@ -73,7 +76,8 @@ class CiviApiImportTest extends TestCase implements HeadlessInterface, HookInter ], 'status_id:name' => 'draft', 'job_type' => 'contribution_import', - ])->execute()->first()['id']; + ]; + $userJobID = UserJob::create()->setValues($userJobParameters)->execute()->first()['id']; $importFields = Import::getFields($userJobID)->execute(); $this->assertEquals('abc', $importFields[0]['table_name']); $this->assertEquals('_id', $importFields[0]['column_name']); @@ -135,13 +139,25 @@ class CiviApiImportTest extends TestCase implements HeadlessInterface, HookInter $imported = Import::import($userJobID)->addWhere('_id', '=', $rowID)->setLimit(1)->execute()->first(); $this->assertEquals('ERROR', $imported['_status']); $this->assertEquals('No matching Contact found', $imported['_status_message']); + + // Update the table with a new table name & check the api still works. + // This relies on the change in table name being detected & caches being + // flushed. + CRM_Core_DAO::executeQuery('DROP TABLE abc'); + $this->createUserJobTable('xyz'); + $userJobParameters['metadata']['DataSource']['table_name'] = 'xyz'; + UserJob::update(FALSE)->addWhere('id', '=', $userJobID)->setValues($userJobParameters)->execute(); + // This is our new table, with nothing in it, but we if we api-call & don't get an exception so we are winning. + Import::get($userJobID)->setSelect(['external_identifier', 'amount_given', '_status'])->addWhere('_id', '=', $rowID)->execute()->first(); } /** * Create a table for our Import api. + * + * @throws \Civi\Core\Exception\DBQueryException */ - private function createUserJobTable(): void { - CRM_Core_DAO::executeQuery("CREATE TABLE IF NOT EXISTS `abc` ( + private function createUserJobTable($tableName = 'abc'): void { + CRM_Core_DAO::executeQuery("CREATE TABLE IF NOT EXISTS `" . $tableName . "` ( `external_identifier` text DEFAULT NULL, `amount_given` text DEFAULT NULL, `receive_date` text DEFAULT NULL, -- 2.25.1