Move relationship return properties to the processor class to fix leakage related...
authoreileen <emcnaughton@wikimedia.org>
Fri, 20 Jul 2018 03:56:03 +0000 (15:56 +1200)
committereileen <emcnaughton@wikimedia.org>
Sun, 22 Jul 2018 22:08:02 +0000 (10:08 +1200)
CRM/Export/BAO/Export.php
CRM/Export/BAO/ExportProcessor.php
tests/phpunit/CRM/Export/BAO/ExportTest.php

index 478942d0b1207775a675d071b74039dfb40605bb..8822076944f387cdaaf4a7e0ae78e6eff8cfdfbc 100644 (file)
@@ -41,8 +41,6 @@ class CRM_Export_BAO_Export {
   // CRM-7675
   const EXPORT_ROW_COUNT = 100000;
 
-  protected static $relationshipReturnProperties = [];
-
   /**
    * Key representing the head of household in the relationship array.
    *
@@ -70,66 +68,6 @@ class CRM_Export_BAO_Export {
    */
   protected static $relationshipTypes = [];
 
-  /**
-   * @param $value
-   * @param $locationTypeFields
-   * @param $relationshipTypes
-   *
-   * @return array
-   */
-  protected static function setRelationshipReturnProperties($value, $locationTypeFields, $relationshipTypes) {
-    $locationTypes = CRM_Core_PseudoConstant::get('CRM_Core_DAO_Address', 'location_type_id');
-    $relPhoneTypeId = $relIMProviderId = NULL;
-    if (!empty($value[2])) {
-      $relationField = CRM_Utils_Array::value(2, $value);
-      if (trim(CRM_Utils_Array::value(3, $value))) {
-        $relLocTypeId = CRM_Utils_Array::value(3, $value);
-      }
-      else {
-        $relLocTypeId = 'Primary';
-      }
-
-      if ($relationField == 'phone') {
-        $relPhoneTypeId = CRM_Utils_Array::value(4, $value);
-      }
-      elseif ($relationField == 'im') {
-        $relIMProviderId = CRM_Utils_Array::value(4, $value);
-      }
-    }
-    elseif (!empty($value[4])) {
-      $relationField = CRM_Utils_Array::value(4, $value);
-      $relLocTypeId = CRM_Utils_Array::value(5, $value);
-      if ($relationField == 'phone') {
-        $relPhoneTypeId = CRM_Utils_Array::value(6, $value);
-      }
-      elseif ($relationField == 'im') {
-        $relIMProviderId = CRM_Utils_Array::value(6, $value);
-      }
-    }
-    if (in_array($relationField, $locationTypeFields) && is_numeric($relLocTypeId)) {
-      if ($relPhoneTypeId) {
-        self::$relationshipReturnProperties[$relationshipTypes]['location'][$locationTypes[$relLocTypeId]]['phone-' . $relPhoneTypeId] = 1;
-      }
-      elseif ($relIMProviderId) {
-        self::$relationshipReturnProperties[$relationshipTypes]['location'][$locationTypes[$relLocTypeId]]['im-' . $relIMProviderId] = 1;
-      }
-      else {
-        self::$relationshipReturnProperties[$relationshipTypes]['location'][$locationTypes[$relLocTypeId]][$relationField] = 1;
-      }
-    }
-    else {
-      self::$relationshipReturnProperties[$relationshipTypes][$relationField] = 1;
-    }
-    return array($relationField);
-  }
-
-  /**
-   * @return array
-   */
-  public static function getRelationshipReturnProperties() {
-    return self::relationshipReturnProperties;
-  }
-
   /**
    * Get default return property for export based on mode
    *
@@ -326,22 +264,6 @@ class CRM_Export_BAO_Export {
     if ($fields) {
       //construct return properties
       $locationTypes = CRM_Core_PseudoConstant::get('CRM_Core_DAO_Address', 'location_type_id');
-      $locationTypeFields = array(
-        'street_address',
-        'supplemental_address_1',
-        'supplemental_address_2',
-        'supplemental_address_3',
-        'city',
-        'postal_code',
-        'postal_code_suffix',
-        'geo_code_1',
-        'geo_code_2',
-        'state_province',
-        'country',
-        'phone',
-        'email',
-        'im',
-      );
 
       foreach ($fields as $key => $value) {
         $fieldName = CRM_Utils_Array::value(1, $value);
@@ -350,9 +272,7 @@ class CRM_Export_BAO_Export {
         }
 
         if ($processor->isRelationshipTypeKey($fieldName) && (!empty($value[2]) || !empty($value[4]))) {
-          self::setRelationshipReturnProperties($value, $locationTypeFields, $fieldName);
-          // @todo we can later not add this to this array but maintain a separate array.
-          $returnProperties = array_merge($returnProperties, self::$relationshipReturnProperties);
+          $returnProperties[$fieldName] = $processor->setRelationshipReturnProperties($value, $fieldName);
         }
         elseif (is_numeric(CRM_Utils_Array::value(2, $value))) {
           $locTypeId = $value[2];
index cfdc0f5881d5dc93b2c2c5b4e49efcee20ad8304..8efc7102551c7b3893d8c16b8ef9d7bb61103633 100644 (file)
@@ -81,6 +81,13 @@ class CRM_Export_BAO_ExportProcessor {
    */
   protected $relationshipTypes = [];
 
+  /**
+   * Array of properties to retrieve for relationships.
+   *
+   * @var array
+   */
+  protected $relationshipReturnProperties = [];
+
   /**
    * CRM_Export_BAO_ExportProcessor constructor.
    *
@@ -334,7 +341,11 @@ class CRM_Export_BAO_ExportProcessor {
   public function getDefaultReturnProperties() {
     $returnProperties = [];
     $fields = CRM_Contact_BAO_Contact::exportableFields('All', TRUE, TRUE);
-    $skippedFields = ($this->getQueryMode() === CRM_Contact_BAO_Query::MODE_CONTACTS) ? [] : ['groups', 'tags', 'notes'];
+    $skippedFields = ($this->getQueryMode() === CRM_Contact_BAO_Query::MODE_CONTACTS) ? [] : [
+      'groups',
+      'tags',
+      'notes'
+    ];
 
     foreach ($fields as $key => $var) {
       if ($key && (substr($key, 0, 6) != 'custom') && !in_array($key, $skippedFields)) {
@@ -345,4 +356,85 @@ class CRM_Export_BAO_ExportProcessor {
     return $returnProperties;
   }
 
+  /**
+   * Add the field to relationship return properties & return it.
+   *
+   * This function is doing both setting & getting which is yuck but it is an interim
+   * refactor.
+   *
+   * @param array $value
+   * @param string $relationshipKey
+   *
+   * @return array
+   */
+  public function setRelationshipReturnProperties($value, $relationshipKey) {
+    $locationTypes = CRM_Core_PseudoConstant::get('CRM_Core_DAO_Address', 'location_type_id');
+    $relPhoneTypeId = $relIMProviderId = NULL;
+    if (!empty($value[2])) {
+      $relationField = CRM_Utils_Array::value(2, $value);
+      if (trim(CRM_Utils_Array::value(3, $value))) {
+        $relLocTypeId = CRM_Utils_Array::value(3, $value);
+      }
+      else {
+        $relLocTypeId = 'Primary';
+      }
+
+      if ($relationField == 'phone') {
+        $relPhoneTypeId = CRM_Utils_Array::value(4, $value);
+      }
+      elseif ($relationField == 'im') {
+        $relIMProviderId = CRM_Utils_Array::value(4, $value);
+      }
+    }
+    elseif (!empty($value[4])) {
+      $relationField = CRM_Utils_Array::value(4, $value);
+      $relLocTypeId = CRM_Utils_Array::value(5, $value);
+      if ($relationField == 'phone') {
+        $relPhoneTypeId = CRM_Utils_Array::value(6, $value);
+      }
+      elseif ($relationField == 'im') {
+        $relIMProviderId = CRM_Utils_Array::value(6, $value);
+      }
+    }
+    if (in_array($relationField, $this->getValidLocationFields()) && is_numeric($relLocTypeId)) {
+      if ($relPhoneTypeId) {
+        $this->relationshipReturnProperties[$relationshipKey]['location'][$locationTypes[$relLocTypeId]]['phone-' . $relPhoneTypeId] = 1;
+      }
+      elseif ($relIMProviderId) {
+        $this->relationshipReturnProperties[$relationshipKey]['location'][$locationTypes[$relLocTypeId]]['im-' . $relIMProviderId] = 1;
+      }
+      else {
+        $this->relationshipReturnProperties[$relationshipKey]['location'][$locationTypes[$relLocTypeId]][$relationField] = 1;
+      }
+    }
+    else {
+      $this->relationshipReturnProperties[$relationshipKey][$relationField] = 1;
+    }
+    return $this->relationshipReturnProperties[$relationshipKey];
+  }
+
+  /**
+   * Get the default location fields to request.
+   *
+   * @return array
+   */
+  public function getValidLocationFields() {
+    return [
+      'street_address',
+      'supplemental_address_1',
+      'supplemental_address_2',
+      'supplemental_address_3',
+      'city',
+      'postal_code',
+      'postal_code_suffix',
+      'geo_code_1',
+      'geo_code_2',
+      'state_province',
+      'country',
+      'phone',
+      'email',
+      'im',
+    ];
+  }
+
 }
index 3ec50e160a04d3057b7d599d15a010e5a123fc7e..1a0418413a47ef52980850525be449fada2602d5 100644 (file)
@@ -101,7 +101,7 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
       array('Contribution', 'trxn_id'),
     );
 
-    list($tableName, $sqlColumns) = CRM_Export_BAO_Export::exportComponents(
+    list($tableName) = CRM_Export_BAO_Export::exportComponents(
       TRUE,
       $this->contributionIDs,
       array(),
@@ -595,7 +595,7 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
         }
       }
     }
-    list($tableName) = $this->doExport($fields, $this->contactIDs[0]);
+    list($tableName, $sqlColumns) = $this->doExport($fields, $this->contactIDs[0]);
 
     $dao = CRM_Core_DAO::executeQuery('SELECT * FROM ' . $tableName);
     while ($dao->fetch()) {
@@ -611,9 +611,6 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
       }
     }
 
-    // early return for now until we solve a leakage issue.
-    return;
-
     $this->assertEquals([
       'billing_im_provider' => 'billing_im_provider text',
       'billing_im_screen_name' => 'billing_im_screen_name text',
@@ -775,7 +772,7 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
     ));
 
     //export and merge contacts with same address
-    list($tableName, $sqlColumns) = CRM_Export_BAO_Export::exportComponents(
+    list($tableName) = CRM_Export_BAO_Export::exportComponents(
       TRUE,
       array($contactA['id'], $contactB['id']),
       array(),
@@ -841,7 +838,11 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
   }
 
   /**
+   * Do a CiviCRM export.
+   *
    * @param $selectedFields
+   * @param int $id
+   *
    * @return array
    */
   protected function doExport($selectedFields, $id) {
@@ -995,7 +996,7 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
   /**
    * Get basic return properties.
    *
-   * @bool $isContactMode
+   * @param bool $isContactMode
    *   Are we in contact mode or not
    *
    * @return array