From f4feabb83f6cb839f2d9eba85872af8641c6a4bc Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 20 Oct 2019 18:14:48 +0200 Subject: [PATCH] MDL-66968 php74: array_key_exists() for objects is deprecated Replace it for correct property_exists() when the element being inspected is a property of object/class. Amended and squased changes: - keep mongo unmodified. The information is array, hence correct. - fix a couple of messaging phpdocs that were incorrect. Amended take#2: - As far as mongo resturns BSONDocument that is ArrayObject, aka implements ArrayAccess, we have decided to explicitly cast results to array so existing array_key_exists() and other accesses will continue working the same. --- admin/message.php | 2 +- admin/repository.php | 4 ++-- cache/stores/mongodb/lib.php | 8 +++++++- competency/classes/api.php | 2 +- enrol/ldap/lib.php | 2 +- lib/grade/grade_category.php | 2 +- lib/outputcomponents.php | 2 +- lib/outputrenderers.php | 4 ++-- message/defaultoutputs.php | 2 +- message/renderer.php | 8 ++++---- mod/lesson/lib.php | 2 +- mod/quiz/lib.php | 2 +- user/filters/checkbox.php | 4 ++-- user/filters/cohort.php | 2 +- user/filters/courserole.php | 2 +- user/filters/globalrole.php | 2 +- user/filters/profilefield.php | 2 +- user/filters/select.php | 2 +- user/filters/simpleselect.php | 2 +- user/filters/text.php | 2 +- user/tests/privacy_test.php | 8 ++++---- 21 files changed, 36 insertions(+), 30 deletions(-) diff --git a/admin/message.php b/admin/message.php index 166acccaed3..52ef04e4d01 100644 --- a/admin/message.php +++ b/admin/message.php @@ -77,7 +77,7 @@ if (($form = data_submitted()) && confirm_sesskey()) { // Record the site preference. $preferences[$processor->name.'_provider_'.$componentprovidersetting] = $value; } - } else if (array_key_exists($componentprovidersetting, $form)) { + } else if (property_exists($form, $componentprovidersetting)) { // We must be processing loggedin or loggedoff checkboxes. Store // defained comma-separated processors as setting value. // Using array_filter eliminates elements set to 0 above. diff --git a/admin/repository.php b/admin/repository.php index 0c7998783a9..e677eb454be 100644 --- a/admin/repository.php +++ b/admin/repository.php @@ -123,13 +123,13 @@ if (($action == 'edit') || ($action == 'new')) { } $instanceoptionnames = repository::static_function($repository, 'get_instance_option_names'); if (!empty($instanceoptionnames)) { - if (array_key_exists('enablecourseinstances', $fromform)) { + if (property_exists($fromform, 'enablecourseinstances')) { $settings['enablecourseinstances'] = $fromform->enablecourseinstances; } else { $settings['enablecourseinstances'] = 0; } - if (array_key_exists('enableuserinstances', $fromform)) { + if (property_exists($fromform, 'enableuserinstances')) { $settings['enableuserinstances'] = $fromform->enableuserinstances; } else { diff --git a/cache/stores/mongodb/lib.php b/cache/stores/mongodb/lib.php index 2122b817a0a..20fe822870c 100644 --- a/cache/stores/mongodb/lib.php +++ b/cache/stores/mongodb/lib.php @@ -254,7 +254,13 @@ class cachestore_mongodb extends cache_store implements cache_is_configurable { } $result = $this->collection->findOne($key); - if ($result === null || !array_key_exists('data', $result)) { + // Note $result is really an object, BSONDocument extending ArrayObject, + // which implements ArrayAccess. That enables access to its information + // using square brackets and some array operations. But, it seems that + // it's not enough for array_key_exists() to operate on it. Hence, we + // are explicitly casting to array, after having checked that the operation + // doesn't incur into any performance penalty. + if ($result === null || !array_key_exists('data', (array)$result)) { return false; } $data = @unserialize($result['data']); diff --git a/competency/classes/api.php b/competency/classes/api.php index cc825050b8f..600f8f823b1 100644 --- a/competency/classes/api.php +++ b/competency/classes/api.php @@ -3839,7 +3839,7 @@ class api { if (!$userevidence->can_manage()) { throw new required_capability_exception($context, 'moodle/competency:userevidencemanage', 'nopermissions', ''); - } else if (array_key_exists('userid', $data) && $data->userid != $userevidence->get('userid')) { + } else if (property_exists($data, 'userid') && $data->userid != $userevidence->get('userid')) { throw new coding_exception('Can not change the userid of a user evidence.'); } diff --git a/enrol/ldap/lib.php b/enrol/ldap/lib.php index e6f41406c03..8c1e1b2c33b 100644 --- a/enrol/ldap/lib.php +++ b/enrol/ldap/lib.php @@ -460,7 +460,7 @@ class enrol_ldap_plugin extends enrol_plugin { // this is an odd array -- mix of hash and array -- $ldapmembers = array(); - if (array_key_exists('memberattribute_role'.$role->id, $this->config) + if (property_exists($this->config, 'memberattribute_role'.$role->id) && !empty($this->config->{'memberattribute_role'.$role->id}) && !empty($course[$this->config->{'memberattribute_role'.$role->id}])) { // May have no membership! diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index f9130cdec06..20c8ef04a09 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -2192,7 +2192,7 @@ class grade_category extends grade_object { $children_array = array(); foreach ($category->children as $sortorder=>$child) { - if (array_key_exists('itemtype', $child)) { + if (property_exists($child, 'itemtype')) { $grade_item = new grade_item($child, false); if (in_array($grade_item->itemtype, array('course', 'category'))) { diff --git a/lib/outputcomponents.php b/lib/outputcomponents.php index dd0834b3a31..cc041c5e0aa 100644 --- a/lib/outputcomponents.php +++ b/lib/outputcomponents.php @@ -228,7 +228,7 @@ class user_picture implements renderable { // only touch the DB if we are missing data and complain loudly... $needrec = false; foreach (self::$fields as $field) { - if (!array_key_exists($field, $user)) { + if (!property_exists($user, $field)) { $needrec = true; debugging('Missing '.$field.' property in $user object, this is a performance problem that needs to be fixed by a developer. ' .'Please use user_picture::fields() to get the full list of required fields.', DEBUG_DEVELOPER); diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php index 10124c0c3b3..df50b1a4850 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -1996,7 +1996,7 @@ class core_renderer extends renderer_base { $button = new single_button($url, $label, $method); foreach ((array)$options as $key=>$value) { - if (array_key_exists($key, $button)) { + if (property_exists($button, $key)) { $button->$key = $value; } else { $button->set_attribute($key, $value); @@ -2479,7 +2479,7 @@ class core_renderer extends renderer_base { public function user_picture(stdClass $user, array $options = null) { $userpicture = new user_picture($user); foreach ((array)$options as $key=>$value) { - if (array_key_exists($key, $userpicture)) { + if (property_exists($userpicture, $key)) { $userpicture->$key = $value; } } diff --git a/message/defaultoutputs.php b/message/defaultoutputs.php index 1dfff8d36e7..fe733d46e94 100644 --- a/message/defaultoutputs.php +++ b/message/defaultoutputs.php @@ -76,7 +76,7 @@ if (($form = data_submitted()) && confirm_sesskey()) { // record the site preference $preferences[$processor->name.'_provider_'.$componentprovidersetting] = $value; } - } else if (array_key_exists($componentprovidersetting, $form)) { + } else if (property_exists($form, $componentprovidersetting)) { // we must be processing loggedin or loggedoff checkboxes. Store // defained comma-separated processors as setting value. // Using array_filter eliminates elements set to 0 above diff --git a/message/renderer.php b/message/renderer.php index 9af97a28013..272f51b057e 100644 --- a/message/renderer.php +++ b/message/renderer.php @@ -42,7 +42,7 @@ class core_message_renderer extends plugin_renderer_base { * @param array $allprocessors array of objects containing all message processors * @param array $processors array of objects containing active message processors * @param array $providers array of objects containing message providers - * @param array $preferences array of objects containing current preferences + * @param stdClass $preferences object containing current preferences * @return string The text to render */ public function manage_messageoutput_settings($allprocessors, $processors, $providers, $preferences) { @@ -120,7 +120,7 @@ class core_message_renderer extends plugin_renderer_base { * * @param array $processors array of objects containing message processors * @param array $providers array of objects containing message providers - * @param array $preferences array of objects containing current preferences + * @param stdClass $preferences object containing current preferences * @return string The text to render */ public function manage_defaultmessageoutputs($processors, $providers, $preferences) { @@ -177,7 +177,7 @@ class core_message_renderer extends plugin_renderer_base { $preference = $processor->name.'_provider_'.$preferencebase; if ($providerdisabled) { $select = MESSAGE_DISALLOWED; - } else if (array_key_exists($preference, $preferences)) { + } else if (property_exists($preferences, $preference)) { $select = $preferences->{$preference}; } // dropdown menu @@ -192,7 +192,7 @@ class core_message_renderer extends plugin_renderer_base { $checked = true; } else if ($select == 'permitted') { $preference = 'message_provider_'.$preferencebase; - if (array_key_exists($preference, $preferences)) { + if (property_exists($preferences, $preference)) { $checked = (int)in_array($processor->name, explode(',', $preferences->{$preference})); } } diff --git a/mod/lesson/lib.php b/mod/lesson/lib.php index bda94f9efdb..1758dc3ff92 100644 --- a/mod/lesson/lib.php +++ b/mod/lesson/lib.php @@ -669,7 +669,7 @@ function lesson_grade_item_update($lesson, $grades=null) { require_once($CFG->libdir.'/gradelib.php'); } - if (array_key_exists('cmidnumber', $lesson)) { //it may not be always present + if (property_exists($lesson, 'cmidnumber')) { //it may not be always present $params = array('itemname'=>$lesson->name, 'idnumber'=>$lesson->cmidnumber); } else { $params = array('itemname'=>$lesson->name); diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index 84e0493875d..20fe537a6be 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -741,7 +741,7 @@ function quiz_grade_item_update($quiz, $grades = null) { require_once($CFG->dirroot . '/mod/quiz/locallib.php'); require_once($CFG->libdir . '/gradelib.php'); - if (array_key_exists('cmidnumber', $quiz)) { // May not be always present. + if (property_exists($quiz, 'cmidnumber')) { // May not be always present. $params = array('itemname' => $quiz->name, 'idnumber' => $quiz->cmidnumber); } else { $params = array('itemname' => $quiz->name); diff --git a/user/filters/checkbox.php b/user/filters/checkbox.php index c99a79cf438..f2b8f942183 100644 --- a/user/filters/checkbox.php +++ b/user/filters/checkbox.php @@ -100,12 +100,12 @@ class user_filter_checkbox extends user_filter_type { // Check if disable if options are set. if yes then don't add this.. if (!empty($this->disableelements) && is_array($this->disableelements)) { foreach ($this->disableelements as $disableelement) { - if (array_key_exists($disableelement, $formdata)) { + if (property_exists($formdata, $disableelement)) { return false; } } } - if (array_key_exists($field, $formdata) and $formdata->$field !== '') { + if (property_exists($formdata, $field) and $formdata->$field !== '') { return array('value' => (string)$formdata->$field); } return false; diff --git a/user/filters/cohort.php b/user/filters/cohort.php index be64fe5964a..1ea0fe1da12 100644 --- a/user/filters/cohort.php +++ b/user/filters/cohort.php @@ -92,7 +92,7 @@ class user_filter_cohort extends user_filter_type { $field = $this->_name; $operator = $field.'_op'; - if (array_key_exists($operator, $formdata)) { + if (property_exists($formdata, $operator)) { if ($formdata->$operator != 5 and $formdata->$field == '') { // No data - no change except for empty filter. return false; diff --git a/user/filters/courserole.php b/user/filters/courserole.php index 7aa793de152..03c1ac35bce 100644 --- a/user/filters/courserole.php +++ b/user/filters/courserole.php @@ -98,7 +98,7 @@ class user_filter_courserole extends user_filter_type { $role = $field .'_rl'; $category = $field .'_ct'; - if (array_key_exists($field, $formdata)) { + if (property_exists($formdata, $field)) { if (empty($formdata->$field) and empty($formdata->$role) and empty($formdata->$category)) { // Nothing selected. return false; diff --git a/user/filters/globalrole.php b/user/filters/globalrole.php index e205bbaa9fe..aca521d76fa 100644 --- a/user/filters/globalrole.php +++ b/user/filters/globalrole.php @@ -82,7 +82,7 @@ class user_filter_globalrole extends user_filter_type { public function check_data($formdata) { $field = $this->_name; - if (array_key_exists($field, $formdata) and !empty($formdata->$field)) { + if (property_exists($formdata, $field) and !empty($formdata->$field)) { return array('value' => (int)$formdata->$field); } return false; diff --git a/user/filters/profilefield.php b/user/filters/profilefield.php index 65723be4df3..a46030e05cf 100644 --- a/user/filters/profilefield.php +++ b/user/filters/profilefield.php @@ -122,7 +122,7 @@ class user_filter_profilefield extends user_filter_type { $operator = $field.'_op'; $profile = $field.'_fld'; - if (array_key_exists($profile, $formdata)) { + if (property_exists($formdata, $profile)) { if ($formdata->$operator < 5 and $formdata->$field === '') { return false; } diff --git a/user/filters/select.php b/user/filters/select.php index 75fdfda94b3..88e942a59e6 100644 --- a/user/filters/select.php +++ b/user/filters/select.php @@ -108,7 +108,7 @@ class user_filter_select extends user_filter_type { $field = $this->_name; $operator = $field.'_op'; - if (array_key_exists($field, $formdata) and !empty($formdata->$operator)) { + if (property_exists($formdata, $field) and !empty($formdata->$operator)) { return array('operator' => (int)$formdata->$operator, 'value' => (string)$formdata->$field); } diff --git a/user/filters/simpleselect.php b/user/filters/simpleselect.php index 639588b1526..04f2c3b5f0d 100644 --- a/user/filters/simpleselect.php +++ b/user/filters/simpleselect.php @@ -86,7 +86,7 @@ class user_filter_simpleselect extends user_filter_type { public function check_data($formdata) { $field = $this->_name; - if (array_key_exists($field, $formdata) and $formdata->$field !== '') { + if (property_exists($formdata, $field) and $formdata->$field !== '') { return array('value' => (string)$formdata->$field); } diff --git a/user/filters/text.php b/user/filters/text.php index fabea89c5a1..0fc4a684f3e 100644 --- a/user/filters/text.php +++ b/user/filters/text.php @@ -96,7 +96,7 @@ class user_filter_text extends user_filter_type { $field = $this->_name; $operator = $field.'_op'; - if (array_key_exists($operator, $formdata)) { + if (property_exists($formdata, $operator)) { if ($formdata->$operator != 5 and $formdata->$field == '') { // No data - no change except for empty filter. return false; diff --git a/user/tests/privacy_test.php b/user/tests/privacy_test.php index 8a1a9f8b1f3..cc5e6ed43aa 100644 --- a/user/tests/privacy_test.php +++ b/user/tests/privacy_test.php @@ -116,11 +116,11 @@ class core_user_privacy_testcase extends provider_testcase { $courserequestdata = (array) $writer->get_data([get_string('privacy:courserequestpath', 'user')]); $entry = array_shift($courserequestdata); // Make sure that the password is not exported. - $this->assertFalse(array_key_exists('password', $entry)); + $this->assertFalse(property_exists($entry, 'password')); // Check that some of the other fields are present. - $this->assertTrue(array_key_exists('fullname', $entry)); - $this->assertTrue(array_key_exists('shortname', $entry)); - $this->assertTrue(array_key_exists('summary', $entry)); + $this->assertTrue(property_exists($entry, 'fullname')); + $this->assertTrue(property_exists($entry, 'shortname')); + $this->assertTrue(property_exists($entry, 'summary')); // User details. $userdata = (array) $writer->get_data([]);