From 7f1a780ca77dd63cd1cd9fbf535d6e14e8529689 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Mon, 23 Dec 2019 09:47:10 +1300
Subject: [PATCH] Fix misleading error message on deadlock

We were seeing error messages like
API_Exception : financial_type_id is not valid : Donation
when a deadlock was hit. The error made not sense as 'Donation' DID exist and did not help us
to identify the issue as a deadlock. It's still helpful with a deadlock to know if
a real constraint violation is hit as per https://lab.civicrm.org/dev/core/issues/1481 sometimes
a disturbing amount of queries have been rolled back.

Let's fix to handle pseudoconstants here & give a useful message.
The previously converted params did not get to this point but we can re-do that work in this low
volume function rather than do a lot of re-thinking
---
 api/v3/utils.php                   | 13 +++++++++----
 tests/phpunit/api/v3/UtilsTest.php | 22 ++++++++++++++++++++++
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/api/v3/utils.php b/api/v3/utils.php
index 9e6350d202..42b7732c78 100644
--- a/api/v3/utils.php
+++ b/api/v3/utils.php
@@ -1639,7 +1639,7 @@ function _civicrm_api3_validate_foreign_keys($entity, $action, &$params, $fields
     if (!empty($fieldInfo['FKClassName'])) {
       if (!empty($params[$fieldName])) {
         foreach ((array) $params[$fieldName] as $fieldValue) {
-          _civicrm_api3_validate_constraint($fieldValue, $fieldName, $fieldInfo);
+          _civicrm_api3_validate_constraint($fieldValue, $fieldName, $fieldInfo, $entity);
         }
       }
       elseif (!empty($fieldInfo['required'])) {
@@ -1731,16 +1731,21 @@ function _civicrm_api3_getValidDate($dateValue, $fieldName, $fieldType) {
  *
  * @param mixed $fieldValue
  * @param string $fieldName
- *   Uniquename of field being checked.
+ *   Unique name of field being checked.
  * @param array $fieldInfo
  *   Array of fields from getfields function.
+ * @param string $entity
  *
  * @throws \API_Exception
  */
-function _civicrm_api3_validate_constraint(&$fieldValue, &$fieldName, &$fieldInfo) {
+function _civicrm_api3_validate_constraint($fieldValue, $fieldName, $fieldInfo, $entity) {
   $daoName = $fieldInfo['FKClassName'];
+  $fieldInfo = [$fieldName => $fieldInfo];
+  $params = [$fieldName => $fieldValue];
+  _civicrm_api3_validate_fields($entity, NULL, $params, $fieldInfo);
+  /* @var CRM_Core_DAO $dao*/
   $dao = new $daoName();
-  $dao->id = $fieldValue;
+  $dao->id = $params[$fieldName];
   $dao->selectAdd();
   $dao->selectAdd('id');
   if (!$dao->find()) {
diff --git a/tests/phpunit/api/v3/UtilsTest.php b/tests/phpunit/api/v3/UtilsTest.php
index e57ab231bb..f11fa585cc 100644
--- a/tests/phpunit/api/v3/UtilsTest.php
+++ b/tests/phpunit/api/v3/UtilsTest.php
@@ -428,6 +428,8 @@ class api_v3_UtilsTest extends CiviUnitTestCase {
 
   /**
    * CRM-20892 Add Tests of new timestamp checking function
+   *
+   * @throws \CRM_Core_Exception
    */
   public function testTimeStampChecking() {
     CRM_Core_DAO::executeQuery("INSERT INTO civicrm_mailing (id, modified_date) VALUES (25, '2016-06-30 12:52:52')");
@@ -437,4 +439,24 @@ class api_v3_UtilsTest extends CiviUnitTestCase {
     $this->callAPISuccess('Mailing', 'delete', ['id' => 25]);
   }
 
+  /**
+   * Test that the foreign key constraint test correctly interprets pseudoconstants.
+   *
+   * @throws \CRM_Core_Exception
+   * @throws \API_Exception
+   */
+  public function testKeyConstraintCheck() {
+    $fieldInfo = $this->callAPISuccess('Contribution', 'getfields', [])['values']['financial_type_id'];
+    _civicrm_api3_validate_constraint(1, 'financial_type_id', $fieldInfo, 'Contribution');
+    _civicrm_api3_validate_constraint('Donation', 'financial_type_id', $fieldInfo, 'Contribution');
+    try {
+      _civicrm_api3_validate_constraint('Blah', 'financial_type_id', $fieldInfo, 'Contribution');
+    }
+    catch (API_Exception $e) {
+      $this->assertEquals("'Blah' is not a valid option for field financial_type_id", $e->getMessage());
+      return;
+    }
+    $this->fail('Last function call should have thrown an exception');
+  }
+
 }
-- 
2.25.1