From 68989e7178aaff4b9e0a39f903d75d745f308a26 Mon Sep 17 00:00:00 2001 From: eileen Date: Sun, 23 Jun 2019 10:58:20 +1200 Subject: [PATCH] [test] convert export test to handle exception rather than early return This is part of switching to testing the eventual output rather than the temp table created. In most cases the temp table is not truly needed so testing the real output makes more sense --- CRM/Core/Exception/PrematureExitException.php | 15 +++++ CRM/Export/BAO/Export.php | 39 ++++++----- CRM/Export/BAO/ExportProcessor.php | 25 +++++++ CRM/Utils/System.php | 6 +- tests/phpunit/CRM/Export/BAO/ExportTest.php | 65 ++++++++++++------- tests/phpunit/CiviTest/CiviUnitTestCase.php | 33 ++++++++++ 6 files changed, 141 insertions(+), 42 deletions(-) diff --git a/CRM/Core/Exception/PrematureExitException.php b/CRM/Core/Exception/PrematureExitException.php index 3b0f574d16..854b233481 100644 --- a/CRM/Core/Exception/PrematureExitException.php +++ b/CRM/Core/Exception/PrematureExitException.php @@ -41,4 +41,19 @@ */ class CRM_Core_Exception_PrematureExitException extends RuntimeException { + /** + * Construct the exception. Note: The message is NOT binary safe. + * + * @link https://php.net/manual/en/exception.construct.php + * + * @param string $message [optional] The Exception message to throw. + * @param array $errorData + * @param int $error_code + * @param throwable $previous [optional] The previous throwable used for the exception chaining. + */ + public function __construct($message = "", $errorData = [], $error_code = 0, throwable $previous = NULL) { + parent::__construct($message, $error_code, $previous); + $this->errorData = $errorData + ['error_code' => $error_code]; + } + } diff --git a/CRM/Export/BAO/Export.php b/CRM/Export/BAO/Export.php index fcdac5163a..207fea2218 100644 --- a/CRM/Export/BAO/Export.php +++ b/CRM/Export/BAO/Export.php @@ -434,7 +434,7 @@ INSERT INTO {$componentTable} SELECT distinct gc.contact_id FROM civicrm_group_c $headerRows = $processor->getHeaderRows(); $sqlColumns = $processor->getSQLColumns(); - $exportTempTable = self::createTempTable($sqlColumns); + $processor->setTemporaryTable(self::createTempTable($sqlColumns)); $limitReached = FALSE; while (!$limitReached) { @@ -460,7 +460,7 @@ INSERT INTO {$componentTable} SELECT distinct gc.contact_id FROM civicrm_group_c // output every $tempRowCount rows if ($count % $tempRowCount == 0) { - self::writeDetailsToTable($exportTempTable, $componentDetails, $sqlColumns); + self::writeDetailsToTable($processor, $componentDetails, $sqlColumns); $componentDetails = []; } } @@ -470,32 +470,37 @@ INSERT INTO {$componentTable} SELECT distinct gc.contact_id FROM civicrm_group_c $offset += $rowCount; } - if ($exportTempTable) { - self::writeDetailsToTable($exportTempTable, $componentDetails, $sqlColumns); + if ($processor->getTemporaryTable()) { + self::writeDetailsToTable($processor, $componentDetails, $sqlColumns); // do merge same address and merge same household processing if ($mergeSameAddress) { - self::mergeSameAddress($exportTempTable, $sqlColumns, $exportParams); + self::mergeSameAddress($processor, $sqlColumns, $exportParams); } // call export hook - CRM_Utils_Hook::export($exportTempTable, $headerRows, $sqlColumns, $exportMode, $componentTable, $ids); + $table = $processor->getTemporaryTable(); + CRM_Utils_Hook::export($table, $headerRows, $sqlColumns, $exportMode, $componentTable, $ids); + if ($table !== $processor->getTemporaryTable()) { + CRM_Core_Error::deprecatedFunctionWarning('altering the export table in the hook is deprecated (in some flows the table itself will be)'); + $processor->setTemporaryTable($table); + } // In order to be able to write a unit test against this function we need to suppress // the csv writing. In future hopefully the csv writing & the main processing will be in separate functions. if (empty($exportParams['suppress_csv_for_testing'])) { - self::writeCSVFromTable($exportTempTable, $headerRows, $sqlColumns, $processor); + self::writeCSVFromTable($headerRows, $sqlColumns, $processor); } else { // return tableName sqlColumns headerRows in test context - return [$exportTempTable, $sqlColumns, $headerRows, $processor]; + return [$processor->getTemporaryTable(), $sqlColumns, $headerRows, $processor]; } // delete the export temp table and component table - $sql = "DROP TABLE IF EXISTS {$exportTempTable}"; + $sql = "DROP TABLE IF EXISTS " . $processor->getTemporaryTable(); CRM_Core_DAO::executeQuery($sql); CRM_Core_DAO::reenableFullGroupByMode(); - CRM_Utils_System::civiExit(); + CRM_Utils_System::civiExit(0, ['processor' => $processor]); } else { CRM_Core_DAO::reenableFullGroupByMode(); @@ -594,11 +599,12 @@ INSERT INTO {$componentTable} SELECT distinct gc.contact_id FROM civicrm_group_c } /** - * @param string $tableName + * @param \CRM_Export_BAO_ExportProcessor $processor * @param $details * @param $sqlColumns */ - public static function writeDetailsToTable($tableName, $details, $sqlColumns) { + public static function writeDetailsToTable($processor, $details, $sqlColumns) { + $tableName = $processor->getTemporaryTable(); if (empty($details)) { return; } @@ -679,11 +685,12 @@ VALUES $sqlValueString } /** - * @param string $tableName + * @param \CRM_Export_BAO_ExportProcessor $processor * @param $sqlColumns * @param array $exportParams */ - public static function mergeSameAddress($tableName, &$sqlColumns, $exportParams) { + public static function mergeSameAddress($processor, &$sqlColumns, $exportParams) { + $tableName = $processor->getTemporaryTable(); // check if any records are present based on if they have used shared address feature, // and not based on if city / state .. matches. $sql = " @@ -961,12 +968,12 @@ WHERE id IN ( $deleteIDString ) } /** - * @param $exportTempTable * @param $headerRows * @param $sqlColumns * @param \CRM_Export_BAO_ExportProcessor $processor */ - public static function writeCSVFromTable($exportTempTable, $headerRows, $sqlColumns, $processor) { + public static function writeCSVFromTable($headerRows, $sqlColumns, $processor) { + $exportTempTable = $processor->getTemporaryTable(); $exportMode = $processor->getExportMode(); $writeHeader = TRUE; $offset = 0; diff --git a/CRM/Export/BAO/ExportProcessor.php b/CRM/Export/BAO/ExportProcessor.php index 25e61c07c3..8034f215a2 100644 --- a/CRM/Export/BAO/ExportProcessor.php +++ b/CRM/Export/BAO/ExportProcessor.php @@ -143,6 +143,31 @@ class CRM_Export_BAO_ExportProcessor { */ protected $outputSpecification = []; + /** + * Name of a temporary table created to hold the results. + * + * Current decision making on when to create a temp table is kinda bad so this might change + * a bit as it is reviewed but basically we need a temp table or similar to calculate merging + * addresses. Merging households is handled in php. We create a temp table even when we don't need them. + * + * @var string + */ + protected $temporaryTable; + + /** + * @return string + */ + public function getTemporaryTable(): string { + return $this->temporaryTable; + } + + /** + * @param string $temporaryTable + */ + public function setTemporaryTable(string $temporaryTable) { + $this->temporaryTable = $temporaryTable; + } + /** * CRM_Export_BAO_ExportProcessor constructor. * diff --git a/CRM/Utils/System.php b/CRM/Utils/System.php index e14e92dfb6..69db26034d 100644 --- a/CRM/Utils/System.php +++ b/CRM/Utils/System.php @@ -1396,12 +1396,12 @@ class CRM_Utils_System { * @param int $status * (optional) Code with which to exit. * - * @throws \CRM_Core_PrematureExit_Exception + * @param array $testParameters */ - public static function civiExit($status = 0) { + public static function civiExit($status = 0, $testParameters = []) { if (CIVICRM_UF === 'UnitTests') { - throw new CRM_Core_Exception_PrematureExitException('civiExit called'); + throw new CRM_Core_Exception_PrematureExitException('civiExit called', $testParameters); } if ($status > 0) { http_response_code(500); diff --git a/tests/phpunit/CRM/Export/BAO/ExportTest.php b/tests/phpunit/CRM/Export/BAO/ExportTest.php index ad9eb22559..6fd7f72284 100644 --- a/tests/phpunit/CRM/Export/BAO/ExportTest.php +++ b/tests/phpunit/CRM/Export/BAO/ExportTest.php @@ -28,7 +28,6 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { */ protected $activityIDs = []; - /** * Contribution IDs created for testing. * @@ -45,7 +44,20 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { protected $locationTypes = []; + /** + * Processor generated in test. + * + * @var \CRM_Export_BAO_ExportProcessor + */ + protected $processor; + + /** + * Cleanup data. + * + * @throws \Exception + */ public function tearDown() { + $this->quickCleanUpFinancialEntities(); $this->quickCleanup([ 'civicrm_contact', 'civicrm_email', @@ -56,11 +68,15 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { 'civicrm_case_contact', 'civicrm_case_activity', ]); - $this->quickCleanUpFinancialEntities(); + if (!empty($this->locationTypes)) { $this->callAPISuccess('LocationType', 'delete', ['id' => $this->locationTypes['Whare Kai']['id']]); $this->callAPISuccess('LocationType', 'create', ['id' => $this->locationTypes['Main']['id'], 'name' => 'Main']); } + if ($this->processor && $this->processor->getTemporaryTable()) { + // delete the export temp table + CRM_Core_DAO::executeQuery("DROP TABLE IF EXISTS " . $this->processor->getTemporaryTable()); + } parent::tearDown(); } @@ -68,29 +84,32 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { * Basic test to ensure the exportComponents function completes without error. * * @throws \CRM_Core_Exception + * @throws \League\Csv\Exception */ public function testExportComponentsNull() { - list($tableName) = CRM_Export_BAO_Export::exportComponents( - TRUE, - [], - [], - NULL, - NULL, - NULL, - CRM_Export_Form_Select::CONTACT_EXPORT, - NULL, - NULL, - FALSE, - FALSE, - [ - 'exportOption' => 1, - 'suppress_csv_for_testing' => TRUE, - ] - ); - - // delete the export temp table and component table - $sql = "DROP TABLE IF EXISTS {$tableName}"; - CRM_Core_DAO::executeQuery($sql); + $this->startCapturingOutput(); + try { + list($tableName) = CRM_Export_BAO_Export::exportComponents( + TRUE, + [], + [], + NULL, + NULL, + NULL, + CRM_Export_Form_Select::CONTACT_EXPORT, + NULL, + NULL, + FALSE, + FALSE, + [ + 'exportOption' => 1, + ] + ); + } + catch (CRM_Core_Exception_PrematureExitException $e) { + } + $this->processor = $e->errorData['processor']; + ob_end_clean(); } /** diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 8a6419e12e..ff5625fc5d 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -27,6 +27,7 @@ */ use Civi\Payment\System; +use League\Csv\Reader; /** * Include class definitions @@ -3285,4 +3286,36 @@ AND ( TABLE_NAME LIKE 'civicrm_value_%' ) return civicrm_api3('option_value', 'create', $params); } + /** + * Start capturing browser output. + * + * The starts the process of browser output being captured, setting any variables needed for e-notice prevention. + */ + protected function startCapturingOutput() { + ob_start(); + $_SERVER['HTTP_USER_AGENT'] = 'unittest'; + } + + /** + * Stop capturing browser output and return as a csv. + * + * @param bool $isFirstRowHeaders + * + * @return \League\Csv\Reader + * + * @throws \League\Csv\Exception + */ + protected function captureOutputToCSV($isFirstRowHeaders = TRUE) { + $output = ob_get_flush(); + $stream = fopen('php://memory', 'r+'); + fwrite($stream, $output); + rewind($stream); + $csv = Reader::createFromString($output); + if ($isFirstRowHeaders) { + $csv->setHeaderOffset(0); + } + ob_clean(); + return $csv; + } + } -- 2.25.1