Problem/Motivation

I work on a site which requires us to use ArcGIS and store the results. Per their terms of service, this requires an account with their service in order to provide a token. In order to support this, I have submitted a PR to the Geocoder PHP project (https://github.com/geocoder-php/Geocoder/pull/1090) to update the ArcGISOnline provider class/package.

Proposed resolution

To support this in Drupal I have updated the schema and created a ConfigurableProviderUsingHandlerWithAdapterBase Provider Plugin so that the credentials necessary for token creation can be entered in the config form. This also caches the token for the valid period. The attached is a patch to add these to the Geocoder Drupal project once the ArcGISOnline provider library has been updated/released.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

wsantell created an issue. See original summary.

wsantell’s picture

The PR has been merged and the ArcGIS Provider has been updated to 4.2.0 on both GitHub and Packagist.

itamair’s picture

Status: Active » Postponed (maintainer needs more info)

Thanks @wsantell ... great work (you did on this https://github.com/geocoder-php/Geocoder/pull/1090).
Your patch applies cleanly. But there is something that I would make sure & more clear.
This patch is creating a new arcgisonlinetoken geocoder provider, that looks alternative to the existing arcgisonline one.
I didn't inspect the arcgisonline services specifications, so I ask you.
Is your new arcgisonlinetoken adding support to a NEW (and alternative) more pro arcgisonline geocoding service that requires token/clinet_id and client_secret?
Has the previous existing arcgisonline provider still reason to exist/be present?

itamair’s picture

Status: Postponed (maintainer needs more info) » Needs work

Ah ok. I got it. They can both co-exist ...

But I think that you did some confusion in the code.
Why you are mentioning the token and it seems the arcgisonlinetoken provider settings are requesting client_id and client_secret (both required)?
And why here: https://github.com/geocoder-php/Geocoder/blob/master/src/Provider/ArcGIS...
is stating the following:

You can use the static token method on the provider to create a client which uses your valid ArcGIS Online token:

$httpClient = new \Http\Adapter\Guzzle6\Client();

// Client ID is required. Private key is optional.
$provider = \Geocoder\Provider\ArcGISList\ArcGISList::token($httpClient, 'your-token');

$result = $geocoder->geocodeQuery(GeocodeQuery::create('Buckingham Palace, London'));

It seems that you did a little mix (and mess) with code taken from the GoogleMaps provider, from here:
https://github.com/geocoder-php/Geocoder/tree/master/src/Provider/Google...
and from the geocoder_provider.configuration.googlemaps_business geocoder.schema, from here:
https://git.drupalcode.org/project/geocoder/-/blob/8.x-3.x/config/schema...

Definitely it seems to me that you need to interact again with the geocoder-php/Geocoder maintainer to correct this Readm.me file:
https://github.com/geocoder-php/Geocoder/blob/master/src/Provider/ArcGIS...

and Need Work to properly correct this new drupal Geocoder arcgisonlinetoken provider settings,
that is asking both Client ID and Client Secret (as required) instead of a Token value ...

wsantell’s picture

@itamair,

It does look like I need to update this comment on the ArcGISOnline provider readme:
// Client ID is required. Private key is optional.

However, the rest of that instruction is correct for calling ArcGISList::token. I did not explain the process for obtaining a token since the link to the instructions is provided in the readme, but to summarize you must provide a Client ID and Client Secret to obtain a token which is only valid for a period of time no longer than 14 days (default is 2 hours).

In Drupal, if the config form generated by the ArcGISOnlineToken class being added in this patch were used to store the token, the user would need to manually request a new token and enter it on this form every time the token expired. Instead, it works as follows:

  1. User stores the Client ID and Client Secret on the config form
  2. When Drupal attempts to use the ArcGISOnlineToken handler, it retrieves the cached token (if any exists)
  3. If the token is expired or the cache is empty, the Client ID and Client Secret are used to request and store a new token with an expiration time based on the expires_in value
  4. The token is passed to the ArcGISList::token method

Please let me know if there are still any points which are unclear or you feel I'm mistaken on.

wsantell’s picture

@itamair,

I have updated the README documentation in my fork:
https://github.com/wantell/Geocoder/tree/arcGISupdate-1089/src/Provider/...

Before I submit this to the main repo, does this documentation update resolve your questions from the perspective of this Drupal project? Do you think it would also help if I modified the client_id description, and added one for client_secret, to more clearly state the relation between them and the token? For reference this is the description in the initial patch:

+      description: '<a href="https://developers.arcgis.com/rest/geocode/api-reference/geocoding-authenticate-a-request.htm" target="blank">Authenticate a request to the ArcGIS World Geocoding Service</a>'

Thanks in advance for any feedback, and sorry for the slow response. Work is less chaotic now and hopefully I can respond to any requested changes quickly.

wsantell’s picture

@itamair,

Updated documentation has been merged into the Geocoder project:
https://github.com/geocoder-php/Geocoder/pull/1105

Please let me know if you need any other work on this patch.

ekes’s picture

@wsantell Thanks for this. I was just wondering how the token handling would work!

My test is against a free account, so I can only get as far as generating a valid token, and using it, to get the error (valid token - but not authorized to store results), which is correct.

During testing it does correctly use the oAuth credentials to get a token. The cached token is checked every time the provider has been loaded. And after expires it was refreshed.

While I was doing this I did a bit of coding standards cleanup:-

There were several services already in the Plugin so we can use the geocoder cache

< +    $cache = \Drupal::cache()->get('arcgis-online-token');
---
> +    $cache = $this->cacheBackend->get('arcgis-online-token');

The plugin has the messenger trait. I've also translated the output, it has the translation trait too. In the example below I just made the message a bit simpler as I couldn't think how to make the previous easily translatable.

< +      \Drupal::messenger()->addError(implode(' ', $errors));
---
> +      $this->messenger()->addError($this->t("Require geocoder authentication credentials are missing."));

There's a Guzzle 6 compatible HttpClient that geocoder provides as a service. I've not looked I guess it's 6 as that's what should be passed on to geocoder-php.

< +      $response = \Drupal::httpClient()
< +        ->get($url, ['headers' => ['Accept' => 'text/plain']]);
---
> +      $request = new Request('GET', $url);
> +      $response = $this->httpAdapter->sendRequest($request);

Coding standards like use statements

< +      $json = \Drupal\Component\Serialization\Json::decode($data);
---
> +      $json = Json::decode($data);

And this one's looking forward REQUEST_TIME is deprecated.

< +        REQUEST_TIME + intval($json['expires_in'])
---
> +        \Drupal::time()->getRequestTime() + intval($json['expires_in'])

Last one this is correct for SendRequest as far as I understand it (use statement is Http\Client\Exception\TransferException)

< +    catch (RequestException $e) { 
< +      \Drupal::messenger()->addError('An error occurred during the request.');
---
> +    catch (TransferException $e) {
> +      $this->messenger()->addError($this->t('An error occurred during the request.'));

But other than those niggly things that it makes sense here.

ekes’s picture

Status: Needs work » Needs review
itamair’s picture

Ok. Sincerely speaking I don't have now time to test and review the #8 patch @ekes, but I can see your coding standards are quite good, so I am inclined to trust the functionalities of this new arcgistoken geocoder_provider.
But to commit this I need a full RTBC here ...

itamair’s picture

For now I am adding a new patch that slightly cleans the #8 patch one (better doc for the new ArcGISOnlineToken provider class (and better patch naming complying with drupal patch naming conventions).

itamair’s picture

Title: Code to support ArcGIS provider update » Code to support ArcGIS provider update (with Token)