Bug when use chain and GeoIp2 at first position with cache

[aetiv]

GeoIp2 return empty collection and save it in cache after that next providers(custom) load from cache empty collection instead of using his code

  $result = json_decode($this->executeQuery($address));

        if (null === $result) {
            return new AddressCollection([]);
        }

$result = json_decode(''); //NULL

private function executeQuery(string $address): string
    {
        $uri = sprintf('file://geoip?%s', $address);

        try {
            $result = $this->adapter->getContent($uri);
        } catch (AddressNotFoundException $e) {
            return '';
        } catch (AuthenticationException $e) {
            throw new InvalidCredentials(
                $e->getMessage(),
                $e->getCode(),
                $e
            );
        } catch (OutOfQueriesException $e) {
            throw new QuotaExceeded(
                $e->getMessage(),
                $e->getCode(),
                $e
            );
        }

        return $result;
    }

I think there should not be return '';, but there should be fatal

 public function geocodeQuery(GeocodeQuery $query): Collection
    {
        foreach ($this->providers as $provider) {
            try {
                $result = $provider->geocodeQuery($query);
                if (!$result->isEmpty()) {
                    return $result;
                }
            } catch (\Throwable $e) {
                $this->log(
                    'alert',
                    sprintf('Provider "%s" could geocode address: "%s".', $provider->getName(), $query->getText()),
                    ['exception' => $e]
                );
            }
        }

        return new AddressCollection();
    }

\Geocoder\Provider\Chain\Chain::geocodeQuery now always return empty collectioon \Geocoder\Model\AddressCollection();

[Nyholm]

I think you are correct. Is that how other providers do?

[aetiv]

For example GoogleMaps - https://github.com/geocoder-php/Geocoder/blob/80da39e5b4049e00540564fdfe8c2ac94383b86c/src/Provider/GoogleMaps/GoogleMaps.php#L196

method validateResponse() - https://github.com/geocoder-php/Geocoder/blob/80da39e5b4049e00540564fdfe8c2ac94383b86c/src/Provider/GoogleMaps/GoogleMaps.php#L400

Throw FATAL if json incorrect, please see

$json = json_decode($content);

        // API error
        if (!isset($json)) {
            throw InvalidServerResponse::create($url);
        }

        if ('REQUEST_DENIED' === $json->status && 'The provided API key is invalid.' === $json->error_message) {
            throw new InvalidCredentials(sprintf('API key is invalid %s', $url));
        }

        if ('REQUEST_DENIED' === $json->status) {
            throw new InvalidServerResponse(
                sprintf('API access denied. Request: %s - Message: %s', $url, $json->error_message)
            );
        }

        // you are over your quota
        if ('OVER_QUERY_LIMIT' === $json->status) {
            throw new QuotaExceeded(sprintf('Daily quota exceeded %s', $url));
        }

return empty AddressCollection()

// no result
        if (!isset($json->results) || !count($json->results) || 'OK' !== $json->status) {
            return new AddressCollection([]);
        }

Ok, all providers return empty collection when result is empty, but in combination with cache, this empty collection save to cache and next providers return empty collection from cache.

Maybe no need to save in cache empty collection, but this is incorrect if we use only one provider without Chain and with cache. This you need decide how to solve this problem, but now \Geocoder\Provider\Chain\Chain::geocodeQuery with cache incorrect works( sorry for my English )

[atymic]

@aetiv

Ok, all providers return empty collection when result is empty, but in combination with cache, this empty collection save to cache and next providers return empty collection from cache.

Maybe no need to save in cache empty collection, but this is incorrect if we use only one provider without Chain and with cache. This you need decide how to solve this problem, but now \Geocoder\Provider\Chain\Chain::geocodeQuery with cache incorrect works( sorry for my English )

Sorry, I'm a bit confused what you mean here. When using the chain provider and the first provider (geoip2) returns an empty result, it will proceed to the next one due to the if (!$result->isEmpty()) check.

If all of the providers return an empty result, this will be cached. There is some discussion relating to this in https://github.com/geocoder-php/Geocoder/issues/904