geocoder-php / GeocoderLaravel

Geocoder service provider for Laravel
http://geocoder-php.org/GeocoderLaravel/
MIT License
701 stars 102 forks source link

Empty collection is cached #106

Closed ftrudeau closed 6 years ago

ftrudeau commented 6 years ago

General Information

GeocoderLaravel Version: 4.0.2 Laravel Version: 5.5.19 PHP Version: PHP 7.1.9-1+ubuntu16.04.1+deb.sury.org+1 Operating System and Version: Ubuntu 16.04.3 LTS

Issue Description

Empty collection area cached.

Steps to Replicate

We are using an API key that is included in other of our projects, and were caped to 2,500 request per day. It seams that after this, the API silently returns an empty result, that is still cached by this package. Correct behavior would be to raise an exception that could be trapped, and/or at the very least, not cache an empty collection.

mikebronner commented 6 years ago

@ftrudeau Thanks for the report. The returning an error bit would have to be addressed by the underlying library https://github.com/geocoder-php/Geocoder. However, you are right in that an empty result should not be cached. I will update that. Thanks for bringing that to my attention! :)

mikebronner commented 6 years ago

Fixed in v4.0.3

HamzaB-coder commented 4 years ago

General Information GeocoderLaravel Version: ^4.1 Laravel Version: 5.8.* PHP Version:>=^7.1.3 Operating System and Version: Windows 10 with Laravel

Issue Description I always receiving an Empty Collection.

Collection {#363 ▼ #items: [] }

Steps to Replicate use Geocoder\Laravel\Facades\Geocoder;

$result = Geocoder::geocode('Los Angeles, CA')->get();

dd($result);

Google Place API Key is provided, Any suggestions?

mikebronner commented 4 years ago

@HamzaB-coder if you dd() a get result, it will always show empty. Instead try:

$result = Geocoder::geocode('Los Angeles, CA')->json();
dd($result);
HamzaB-coder commented 4 years ago

tried this but didnt work .. i got this error "Call to undefined method Geocoder\Laravel\ProviderAndDumperAggregator::json()"

mikebronner commented 4 years ago

Sorry, ->toJson() instead of ->json.

HamzaB-coder commented 4 years ago

Here is my code

` $result = app('geocoder')->geocode('Los Angeles, CA')->json();

    dd($result);`

This is the facade am using use Geocoder\Laravel\ProviderAndDumperAggregator as Geocoder;

This am using as provider in app.php Geocoder\Laravel\Providers\GeocoderService::class,

This is my geocoder.php file

<?php

use Geocoder\Provider\Chain\Chain;
use Geocoder\Provider\GeoPlugin\GeoPlugin;
use Geocoder\Provider\GoogleMaps\GoogleMaps;
use Http\Client\Curl\Client;

return [
    'cache' => [

        /*
        |-----------------------------------------------------------------------
        | Cache Store
        |-----------------------------------------------------------------------
        |
        | Specify the cache store to use for caching. The value "null" will use
        | the default cache store specified in /config/cache.php file.
        |
        | Default: null
        |
        */

        "store" => "geocode",

        /*
        |-----------------------------------------------------------------------
        | Cache Duration
        |-----------------------------------------------------------------------
        |
        | Specify the cache duration in minutes. The default approximates a
        | "forever" cache, but there are certain issues with Laravel's forever
        | caching methods that prevent us from using them in this project.
        |
        | Default: 9999999 (integer)
        |
        */

        'duration' => 9999999,
    ],

    /*
    |---------------------------------------------------------------------------
    | Providers
    |---------------------------------------------------------------------------
    |
    | Here you may specify any number of providers that should be used to
    | perform geocaching operations. The `chain` provider is special,
    | in that it can contain multiple providers that will be run in
    | the sequence listed, should the previous provider fail. By
    | default the first provider listed will be used, but you
    | can explicitly call subsequently listed providers by
    | alias: `app('geocoder')->using('google_maps')`.
    |
    | Please consult the official Geocoder documentation for more info.
    | https://github.com/geocoder-php/Geocoder#providers
    |
    */
    'providers' => [
        Chain::class => [
            GoogleMaps::class => [
                env('GOOGLE_MAPS_LOCALE','en_US'),
                env('GOOGLE_MAPS_API_KEY'),
            ],
            GeoPlugin::class  => [],
        ],
    ],

    /*
    |---------------------------------------------------------------------------
    | Adapter
    |---------------------------------------------------------------------------
    |
    | You can specify which PSR-7-compliant HTTP adapter you would like to use.
    | There are multiple options at your disposal: CURL, Guzzle, and others.
    |
    | Please consult the official Geocoder documentation for more info.
    | https://github.com/geocoder-php/Geocoder#usage
    |
    | Default: Client::class (FQCN for CURL adapter)
    |
    */
    'adapter'  => Client::class,

    /*
    |---------------------------------------------------------------------------
    | Reader
    |---------------------------------------------------------------------------
    |
    | You can specify a reader for specific providers, like GeoIp2, which
    | connect to a local file-database. The reader should be set to an
    | instance of the required reader class.
    |
    | Please consult the official Geocoder documentation for more info.
    | https://github.com/geocoder-php/geoip2-provider
    |
    | Default: null
    |
    */
    'reader' => null,

];

This is the package am using in composer.json "toin0u/geocoder-laravel": "^4.1"

This is my Database.php code

 'redis' => [

        'client' => env('REDIS_CLIENT', 'predis'),

        'options' => [
            'cluster' => env('REDIS_CLUSTER', 'predis'),
            'prefix' => Str::slug(env('APP_NAME', 'laravel'), '_') . '_database',
        ],

        'default' => [
            'host' => env('REDIS_HOST', '127.0.0.1'),
            'password' => env('REDIS_PASSWORD', null),
            'port' => env('REDIS_PORT', 6379),
            'database' => env('REDIS_DB', 0),
        ],

        'cache' => [
            'host' => env('REDIS_HOST', '127.0.0.1'),
            'password' => env('REDIS_PASSWORD', null),
            'port' => env('REDIS_PORT', 6379),
            'database' => env('REDIS_CACHE_DB', 1),
        ],

        "geocode-cache" => [ // choose an appropriate name
            'host' => env('REDIS_HOST', '192.168.10.10'),
            'password' => env('REDIS_PASSWORD', null),
            'port' => env('REDIS_PORT', 6379),
            'database' => 1, // be sure this number differs from your other redis databases
        ],

    ],
HamzaB-coder commented 4 years ago

Sorry, ->toJson() instead of ->json. Tried this also but got this error "Return value of Geocoder\Laravel\ProviderAndDumperAggregator::toJson() must be of the type string, null returned"

mikebronner commented 4 years ago

@HamzaB-coder Thanks, I'll take a look and see if I can reproduce this issue. Let's continue the conversation in #176