systopia / de.systopia.osm

CiviCRM OpenStreetmap GeoCoding-Provider
2 stars 17 forks source link

PHP 8.1 compatibility - don't pass stdObject as array to array_key_exists #28

Closed MegaphoneJon closed 1 year ago

MegaphoneJon commented 1 year ago

Per the PHP docs on array_key_exists:

Note:

    For backward compatibility reasons, array_key_exists() will also return true if key is a property defined within an object given as array. This behaviour is deprecated as of PHP 7.4.0, and removed as of PHP 8.0.0.

    To check whether a property exists in an object, [property_exists()](https://www.php.net/manual/en/function.property-exists.php) should be used.

decode_json() will return an array instead of a stdObject if you pass TRUE as the second argument.

Without this fix, you'll get this error:

php TypeError: array_key_exists(): Argument #2 ($array) must be of type array, stdClass given in CRM_Utils_Geocode_OpenStreetMapCoding::format() (line 169 of /var/www/mysite/web/sites/all/civicrm/extensions/de.systopia.osm/CRM/Utils/Geocode/OpenStreetMapCoding.php) #0 /var/www/mysite/vendor/civicrm/civicrm-core/CRM/Core/BAO/Address.php(1312): CRM_Utils_Geocode_OpenStreetMapCoding::format()

(Ignore the line number in my error - I'm using an outdated version but the problem exists on the master branch also)

bjendres commented 1 year ago

Thanks @MegaphoneJon

bjendres commented 1 year ago

I have reverted this, because it simply doesn't work in my setup (PHP 7.4).

@MegaphoneJon Could you have a another look and that change? Keep in mind that there's a cache when testing...

olivierh65 commented 1 year ago

I've checked $json = json_decode($string, TRUE); with PHP 7.4 and PHP 8.1. After clearing the cacje (drush cr), there is no problem with both PHP version.

It should be nice to merge again.

bjendres commented 1 year ago

I've checked $json = json_decode($string, TRUE); with PHP 7.4 and PHP 8.1. After clearing the cacje (drush cr), there is no problem with both PHP version.

That's weird, because it definitely worked without the patch, and then didn't work with the patch.

I'll have another look.

olivierh65 commented 1 year ago

Without the patch, with PHP 8.1.13 and after cleared the cache, I get this error systematically : `[09-Dec-2022 20:51:05 Europe/Paris] TypeError: array_key_exists(): Argument #2 ($array) must be of type array, stdClass given in C:\Applis\wamp64\www\dev9\web\sites\default\files\civicrm\ext\de.systopia.osm\CRM\Utils\Geocode\OpenStreetMapCoding.php on line 217 #0 C:\Applis\wamp64\www\dev9\web\sites\default\files\civicrm\ext\de.systopia.osm\CRM\Utils\Geocode\OpenStreetMapCoding.php(136): CRM_Utils_Geocode_OpenStreetMapCoding::makeRequest('https://nominat...')

1 C:\Applis\wamp64\www\dev9\vendor\civicrm\civicrm-core\CRM\Core\BAO\Address.php(1312): CRM_Utils_Geocode_OpenStreetMapCoding::format(Array)

.....)`

I understood why there is a 'random' behavior. The function will first consult the cache and return a previously retrieved json.

For the patch to work, it is necessary to make other changes: return [ 'geo_code_1' => (float) substr($json[0]->lat, 0, 12), 'geo_code_2' => (float) substr($json[0]->lon, 0, 12), ]; must be changed to return [ 'geo_code_1' => (float) substr($json[0]['lat'], 0, 12), 'geo_code_2' => (float) substr($json[0]['lon'], 0, 12), ]; ($json[0] is now an array et no more an object).