If I save an entity that contains an unresolvable address in the field that is being geolocated (I tried 127.0.0.1) then I get the following exception:

Geocoder\Exception\CollectionIsEmpty in /home/cphp/data/container/75e146b4-4b3a-43a6-bb1f-e2724e4128f1/workspace/vendor/willdurand/geocoder/Model/AddressCollection.php:57

        Stack trace:
        #0 /home/cphp/data/container/75e146b4-4b3a-43a6-bb1f-e2724e4128f1/workspace/web/modules/contrib/geocoder/modules/geocoder_field/geocoder_field.module(238): Geocoder\Model\AddressCollection->first()
        #1 [internal function]: geocoder_field_entity_presave(Object(Drupal\node\Entity\Node))
CommentFileSizeAuthor
#6 2989600-6.patch2.36 KBitamair
#2 2989600-2.patch601 bytespfrenssen

Comments

pfrenssen created an issue. See original summary.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Active » Needs review
StatusFileSize
new 2989600-2.patch601 bytes

Here is a proposed patch, I haven't tested it yet. Will run test suite now but this takes 2 hours and by then I will be asleep :)

pfrenssen’s picture

pfrenssen
Location Sofia
commented

I've applied the patch in my project and started a test: https://app.continuousphp.com/git-hub/ec-europa/joinup-dev/build/587b10e...

pfrenssen’s picture

pfrenssen
Location Sofia
commented

OK test has turned green, it appears the exception is fixed.

itamair’s picture

itamair commented

Thanks @pfrenssen ... I am testing and reviewing all this for actually possible use cases ... coming back asap on this.

itamair’s picture

itamair commented
StatusFileSize
new 2989600-6.patch2.36 KB

Ok ... I tested this better, with different use cases.
Probably that exception is not thrown by every Geocoder Provider.
Instead correctly intercepted before with a false AddressCollection by others.

In particular I tested with google-maps and argis-online providers.
With them an exception will be catch for IP, and the $address_collection will be returned FALSE (boolean) from the Geocode->geocode operation, and not as an empty array.

So it seems to me that the #2 patch won't prevent from the following error:

Error: Call to a member function isEmpty() on boolean in geocoder_field_entity_presave() (line 223 of modules/custom/geocoder/modules/geocoder_field/geocoder_field.module).

Moreover, at the actual state of 3.x dev, that error / scenario will be thrown also if the end user chooses a geocoder plugin from the back end field definition or display formatter correspondent to a (geocoder-php) Geocoder/Provider that has not been added to the Drupal project/application.

This might happen as, at the moment, the geocoder-php providers are require-dev for the 3.x dev branch:

  • the list of choosable providers should be filtered in those UIs to "show providers that are installed", as it is being done in the SettingsForm (probably with a new specialized public method in the ProviderPluginManager service);
  • and may be the list of choosable providers should be ALL added as requirement/dependency for the Geocoder 3.x

(the above will be part of a new specific issue)

Thus I amended the previous patch with the following (to make it more robust/defensive) and added some better exceptions handlings to the Geocoder providers instances creations operations.

Going to add to dev ...

  • itamair committed 16da780 on 8.x-3.x authored by pfrenssen 
    Issue #2989600 by pfrenssen, itamair: Exception thrown when saving an...
itamair’s picture

itamair commented
Status: Needs review » Fixed
itamair’s picture

itamair commented
Related issues: +#2989996: Limit the selectable Geocoder providers to those effectively installed in the project.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.