Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

dev/core#2079 Eliminate unused query on CRM_Core_BAO_CustomQuery::_construct #18668

Merged
merged 1 commit into from Oct 12, 2020
Merged

dev/core#2079 Eliminate unused query on CRM_Core_BAO_CustomQuery::_construct #18668

merged 1 commit into from Oct 12, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Oct 3, 2020 •
edited

Overview

This cleans up unnecessary queries in Contact.get api when custom data is involved. As mentioned _options is deprecated
but still used - more accurately the work wasn't done at the time to determine whether it was.

This time I've gone through and clarified all the places it potentially might have been used

https://github.com/civicrm/civicrm-core/pulls?q=+is%3Apr+dev%2Fcore%232079+

Before

Extraneous queries on every apiv3 or BAO-query object contact.get call, if custom fields involved

After

poof

Technical Details

Easier to do the grep work to confirm once the preliminaries are closed

Only 3 times where this is constructed

Screen Shot 2020-10-06 at 8 55 26 PM

Only one of which accesses _options

Screen Shot 2020-10-06 at 8 56 24 PM

CRM_Profile_Selector_Listings is the only place that accesses them -

I have grepped universe & checked all references to ->_options - there are a lot but looking at them I am convinced only 3 relate to this variable

TechToThePeople/civisualize#111 & CRM_Profile_Selector_Listings & CRM_Contact_Selector - this PR removes the 2 in core - below is an analysis of the remaining results from searching core for ->_options

Targets    
Occurrences of '->_options' in Directory /Users/eileenmcnaughton/buildkit/build/dmaster/sites/all/modules/civicrm
Found Occurrences  (142 usages found)    
civicrm  (129 usages found)    
Specific to dedupe form CRM/Contact/Form  (1 usage found)
Specific to dedupe form DedupeRules.php  (1 usage found)
Specific to dedupe form 43 $this->_options = CRM_Core_SelectValues::getDedupeRuleTypes();
Relates to products/premiums CRM/Contribute/Form  (5 usages found)
Relates to products/premiums AdditionalInfo.php  (3 usages found)
Relates to products/premiums 52 $form->_options = $sel2;
Relates to products/premiums 345 if (empty($params['product_option']) && !empty($form->_options[$productDAO->id])) {
Relates to products/premiums 346 $params['product_option'] = $form->_options[$productDAO->id][$productOptionID];
Relates to products/premiums Contribution.php  (2 usages found)
Relates to products/premiums 390 $options = $this->_options[$this->_productDAO->product_id] ?? "";
Relates to products/premiums 1638 $this->_premiumID, $this->_options
DB query options CRM/Core  (4 usages found)
DB query options DAO.php  (2 usages found)
DB query options 451 $this->_setDBOptions($this->_options);
DB query options 2912 $this->_options = $options;
Relates to QF Form.php  (2 usages found)
Relates to QF 1005 $this->_options = $options;
Relates to QF 2546 $targetField->_options = [];
Relates to QF CRM/Core/QuickForm  (3 usages found)
Relates to QF GroupMultiSelect.php  (3 usages found)
Relates to QF 66 foreach ($this->_options as $option) {
Relates to QF 151 $options = count($this->_options);
Relates to QF 156 foreach ($this->_options as $option) {
Relates to QF CRM/Event/Form  (1 usage found)
Relates to QF Registration.php  (1 usage found)
Relates to QF 1165 foreach ($element->_options as $option) {
Reports options CRM/Report  (5 usages found)
Reports options Form.php  (5 usages found)
Reports options 1027 foreach ($this->_options as $fieldName => $field) {
Reports options 1499 if (!empty($this->_options)) {
Reports options 1503 foreach ($this->_options as $fieldName => $field) {
Reports options 1518 if (!empty($this->_options) &&
Reports options 1528 $this->assign('otherOptions', $this->_options);
Reports options CRM/Report/Form/Case  (1 usage found)
Reports options Detail.php  (1 usage found)
Reports options 282 $this->_options = [
Reports options CRM/Report/Form/Event  (2 usages found)
Reports options ParticipantListCount.php  (1 usage found)
Reports options 314 $this->_options = [
Reports options ParticipantListing.php  (1 usage found)
Reports options 360 $this->_options = array(
Relates to QF packages/ORIGINAL/HTML/QuickForm  (48 usages found)
Relates to QF advmultiselect.php  (9 usages found)
Relates to QF 609 $unselected_count = count($this->_options);
Relates to QF 613 foreach ($this->_options as $option) {
Relates to QF 735 $options           = count($this->_options);
Relates to QF 740 foreach ($this->_options as $option) {
Relates to QF 1138 foreach ($this->_options as $k => $v) {
Relates to QF 1141 $this->_options[$k]['attr']['disabled'] = 'disabled';
Relates to QF 1143 unset($this->_options[$k]['attr']['disabled']);
Relates to QF 1164 foreach ($this->_options as $k => $v) {
Relates to QF 1166 $options[] = $this->_options[$k]['attr']['value'];
Relates to QF date.php  (22 usages found)
Relates to QF 291 $this->_options['language'] = isset($this->_locale[$value])? $value: 'en';
Relates to QF 292 } elseif (isset($this->_options[$name])) {
Relates to QF 293 if (is_array($value) && is_array($this->_options[$name])) {
Relates to QF 294 $this->_options[$name] = @array_merge($this->_options[$name], $value);
Relates to QF 296 $this->_options[$name] = $value;
Relates to QF 310 $locale    =& $this->_locale[$this->_options['language']];
Relates to QF 312 for ($i = 0, $length = strlen($this->_options['format']); $i < $length; $i++) {
Relates to QF 313 $sign = $this->_options['format']{$i};
Relates to QF 345 $this->_options['minYear'],
Relates to QF 346 $this->_options['maxYear'],
Relates to QF 347 $this->_options['minYear'] > $this->_options['maxYear']? -1: 1
Relates to QF 352 $this->_options['minYear'],
Relates to QF 353 $this->_options['maxYear'],
Relates to QF 354 $this->_options['minYear'] > $this->_options['maxYear']? -1: 1
Relates to QF 369 $options = $this->_createOptionList(0, 59, $this->_options['optionIncrement']['i']);
Relates to QF 372 $options = $this->_createOptionList(0, 59, $this->_options['optionIncrement']['s']);
Relates to QF 400 if (!is_array($this->_options['addEmptyOption']) && $this->_options['addEmptyOption'] ||
Relates to QF 401 is_array($this->_options['addEmptyOption']) && !empty($this->_options['addEmptyOption'][$sign])) {
Relates to QF 404 if (is_array($this->_options['emptyOptionText']) && !empty($this->_options['emptyOptionText'][$sign])) {
Relates to QF 405 $options = array($this->_options['emptyOptionValue'] => $this->_options['emptyOptionText'][$sign]) + $options;
Relates to QF 407 $options = array($this->_options['emptyOptionValue'] => $this->_options['emptyOptionText']) + $options;
Relates to QF 448 if (0 == strcmp($str, $this->_options['emptyOptionValue'])) {
Relates to QF hierselect.php  (9 usages found)
Relates to QF 157 $this->_options = $options;
Relates to QF 160 $this->_nbElements = count($this->_options);
Relates to QF 165 $totalNbElements = count($this->_options);
Relates to QF 189 $this->_options[0] = $array;
Relates to QF 212 $this->_options[1] = $array;
Relates to QF 246 $select->_options = array();
Relates to QF 443 $jsParts[] = $this->_convertArrayToJavascript($this->_options[$i]);
Relates to QF 456 $values[] = $this->_elements[$key]->getMultiple() || empty($this->_elements[$key]->_options[0])?
Relates to QF 458 $this->_elements[$key]->_options[0]['attr']['value'];
Relates to QF select.php  (8 usages found)
Relates to QF 319 $this->_options[] = array('text' => $text, 'attr' => $attributes);
Relates to QF 497 foreach ($this->_options as $option) {
Relates to QF 524 for ($i = 0, $optCount = count($this->_options); $i < $optCount; $i++) {
Relates to QF 525 if (0 == strcmp($val, $this->_options[$i]['attr']['value'])) {
Relates to QF 526 $value[$key] = $this->_options[$i]['text'];
Relates to QF 568 if (is_array($value) && !empty($this->_options)) {
Relates to QF 571 for ($i = 0, $optCount = count($this->_options); $i < $optCount; $i++) {
Relates to QF 572 if (0 == strcmp($v, $this->_options[$i]['attr']['value'])) {
Relates to QF packages/HTML/QuickForm  (54 usages found)
Relates to QF advmultiselect.php  (9 usages found)
Relates to QF 609 $unselected_count = count($this->_options);
Relates to QF 613 foreach ($this->_options as $option) {
Relates to QF 735 $options           = count($this->_options);
Relates to QF 740 foreach ($this->_options as $option) {
Relates to QF 1137 foreach ($this->_options as $k => $v) {
Relates to QF 1140 $this->_options[$k]['attr']['disabled'] = 'disabled';
Relates to QF 1142 unset($this->_options[$k]['attr']['disabled']);
Relates to QF 1163 foreach ($this->_options as $k => $v) {
Relates to QF 1165 $options[] = $this->_options[$k]['attr']['value'];
Relates to QF autocomplete.php  (3 usages found)
Relates to QF 109 $this->_options = array_values($options);
Relates to QF 248 for ($i = 0; $i < count($this->_options); $i++) {
Relates to QF 249 $js .= $arrayName . '[' . $i . "] = '" . strtr($this->_options[$i], $jsEscape) . "';\n";
Relates to QF date.php  (23 usages found)
Relates to QF 134 $this->_options['maxYear'] = date('Y');
Relates to QF 139 if (isset($this->_options[$name])) {
Relates to QF 140 if (is_array($value) && is_array($this->_options[$name])) {
Relates to QF 141 $this->_options[$name] = @array_merge($this->_options[$name], $value);
Relates to QF 143 $this->_options[$name] = $value;
Relates to QF 159 for ($i = 0, $length = strlen($this->_options['format']); $i < $length; $i++) {
Relates to QF 160 $sign = $this->_options['format']{$i};
Relates to QF 211 $this->_options['minYear'],
Relates to QF 212 $this->_options['maxYear'],
Relates to QF 213 $this->_options['minYear'] > $this->_options['maxYear']? -1: 1
Relates to QF 220 $this->_options['minYear'],
Relates to QF 221 $this->_options['maxYear'],
Relates to QF 222 $this->_options['minYear'] > $this->_options['maxYear']? -1: 1
Relates to QF 243 $options = $this->_createOptionList(0, 59, $this->_options['optionIncrement']['i']);
Relates to QF 248 $options = $this->_createOptionList(0, 59, $this->_options['optionIncrement']['s']);
Relates to QF 282 if (!is_array($this->_options['addEmptyOption']) && $this->_options['addEmptyOption'] ||
Relates to QF 283 is_array($this->_options['addEmptyOption']) && !empty($this->_options['addEmptyOption'][$sign])) {
Relates to QF 286 if (is_array($this->_options['emptyOptionText']) && !empty($this->_options['emptyOptionText'][$sign])) {
Relates to QF 287 $text = $emptyText ? $emptyText : $this->_options['emptyOptionText'][$sign];
Relates to QF 288 $options = array($this->_options['emptyOptionValue'] => $text) + $options;
Relates to QF 290 $text = $emptyText ? $emptyText : $this->_options['emptyOptionText'];
Relates to QF 291 $options = array($this->_options['emptyOptionValue'] => $text) + $options;
Relates to QF 342 if (0 == strcmp($str, $this->_options['emptyOptionValue'])) {
Relates to QF hiddenselect.php  (2 usages found)
Relates to QF 91 for ($i = 0, $optCount = count($this->_options); $i < $optCount; $i++) {
Relates to QF 92 if ($val == $this->_options[$i]['attr']['value']) {
Relates to QF hierselect.php  (10 usages found)
Relates to QF 146 $this->_options = $options;
Relates to QF 149 $this->_nbElements = count($this->_options);
Relates to QF 154 $totalNbElements = count($this->_options);
Relates to QF 178 $this->_options[0] = $array;
Relates to QF 200 $this->_options[1] = $array;
Relates to QF 232 if (isset($this->_options[$key])) {
Relates to QF 233 if ((empty($arrayKeys)) || CRM_Utils_Array::pathIsset($this->_options[$key], $arrayKeys)) {
Relates to QF 234 $array = empty($arrayKeys) ? $this->_options[$key] : CRM_Utils_Array::pathGet($this->_options[$key], $arrayKeys);
Relates to QF 237 $select->_options = array();
Relates to QF 314 $this->_setJSArray($this->_jsArrayName, $this->_options[$i], $js);
Relates to QF select.php  (7 usages found)
Relates to QF 319 $this->_options[] = array('text' => $text, 'attr' => $attributes);
Relates to QF 497 foreach ($this->_options as $option) {
Relates to QF 525 foreach ($this->_options as $oKey => $oVal ) {
Relates to QF 526 if (0 == strcmp($val, $this->_options[$oKey]['attr']['value'])) {
Relates to QF 572 if (is_array($value) && !empty($this->_options)) {
Relates to QF 575 for ($i = 0, $optCount = count($this->_options); $i < $optCount; $i++) {
Relates to QF 576 if (0 == strcmp($v, $this->_options[$i]['attr']['value'])) {
Relates to QF packages/HTML/Template  (5 usages found)
Relates to QF IT.php  (5 usages found)
Relates to QF 399 if (array_key_exists($option, $this->_options)) {
Relates to QF 400 $this->_options[$option] = $value;
Relates to QF 474 if ($this->_options['preserve_data']) {
Relates to QF 559 if ($this->_options['use_preg']) {
Relates to QF 569 if ($this->_options['preserve_data']) {
Relates to QF PHP  (11 usages found)
Relates to QF vendor/pear/log/Log  (11 usages found)
Relates to QF mcal.php  (2 usages found)
Relates to QF 83 $this->_options = $conf['options'];
Relates to QF 95 $this->_password, $this->_options);
Relates to QF mdb2.php  (3 usages found)
Relates to QF 124 $this->_options = $conf['options'];
Relates to QF 146 $this->_db = &MDB2::singleton($conf['singleton'], $this->_options);
Relates to QF 165 $this->_db = &MDB2::connect($this->_dsn, $this->_options);
Relates to QF sql.php  (2 usages found)
Relates to QF 137 $this->_options = $conf['options'];
Relates to QF 174 $this->_db = &DB::connect($this->_dsn, $this->_options);
Relates to QF sqlite.php  (4 usages found)
Relates to QF 82 $this->_options[$k] = $opt;
Relates to QF 105 if (empty($this->_options['persistent'])) {
Relates to QF 112 if ($this->_db = $connectFunction($this->_options['filename'],
Relates to QF 113 (int)$this->_options['mode'],
Relates to QF Usage in comments  (1 usage found)
Relates to QF civicrm  (1 usage found)
Relates to QF CRM/Report  (1 usage found)
Relates to QF Form.php  (1 usage found)
Relates to QF 1496 * Add options defined in $this->_options to the report.
Relates to QF Usage in string constants  (1 usage found)
Relates to QF civicrm  (1 usage found)
Relates to QF packages/ORIGINAL/HTML/QuickForm  (1 usage found)
Relates to QF hierselect.php  (1 usage found)
Relates to QF 243 $array = eval("return isset($this->_options[{$key}]{$toLoad})? $this->_options[{$key}]{$toLoad}: null;");

@civibot
Copy link

civibot bot commented Oct 3, 2020

(Standard links)

@civibot civibot bot added the master label Oct 3, 2020
@eileenmcnaughton eileenmcnaughton changed the title dev/core#2079 Eliminate unused query on CRM_Core_BAO_CustomQuery::_co… dev/core#2079 Eliminate unused query on CRM_Core_BAO_CustomQuery::_construct Oct 3, 2020
eileenmcnaughton added a commit to eileenmcnaughton/civisualize that referenced this pull request Oct 7, 2020
@eileenmcnaughton
Remove unused $options that is proposed for removal upstream
3a14e4c 
civicrm/civicrm-core#18668
@eileenmcnaughton eileenmcnaughton mentioned this pull request Oct 7, 2020
Merged
@eileenmcnaughton
dev/core#2079 Eliminate unused query on CRM_Core_BAO_CustomQuery::_co…
6cba576 
…nstruct

This cleans up unnecessary queries in Contact.get api when custom data is involved. As mentioned _options is deprecated
but still used - more accurately the work wasn't done at the time to determine whether it was.

This time I've gone through and clarified all the places it potentially might have been used

https://github.com/civicrm/civicrm-core/pulls?q=+is%3Apr+dev%2Fcore%232079+
@eileenmcnaughton
Copy link
Contributor Author

@colemanw @seamuslee001 @monishdeb I feel like I've dug pretty deep into this - per above & I think I've made the case that about 4 years back (when @monishdeb & I did some cleanup) this should have been removed & can go now - can one of you look?

@monishdeb
Copy link
Member

Agree with the cleanup. Checked on local, esp places where CRM_Contact_BAO_Query::apiQuery() is called and it looks good to me. Merging now.

@monishdeb monishdeb merged commit 48eb3e7 into civicrm:master Oct 12, 2020
@eileenmcnaughton eileenmcnaughton deleted the no_opt branch October 12, 2020 20:23
@eileenmcnaughton
Copy link
Contributor Author

thanks @monishdeb nice to have the code gone & after the rc is cut makes sense I guess

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
master
Projects
None yet
Milestone
No milestone
2 participants
@eileenmcnaughton @monishdeb