geocoder-php / GeocoderLaravel

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

Localization solution avoids caching #159

Open mattmant opened 5 years ago

mattmant commented 5 years ago

General Information

GeocoderLaravel Version: latest Laravel Version: 6.x PHP Version: 7.2.4 Operating System and Version: Ubuntu 18.04.1

Issue Description

Following the instructions provided by https://github.com/geocoder-php/GeocoderLaravel/issues/131 or https://github.com/geocoder-php/GeocoderLaravel/issues/128 you can successfully get localized results, but those methods have the side effect of bypassing ProviderAndDumperAggregator and so avoiding geocoding cache.

mikebronner commented 5 years ago

Thanks, I will look into this. Can you provide an example that I can use as a test-case?

mattmant commented 5 years ago

First implementation I tried setting italian locale by configuring config/geocoder.php:

'cache' => [
        'store' => 'geocode',
        'duration' => 9999999
    ],
...
'providers' => [
        Chain::class => [
            GoogleMaps::class => [
                'it-IT',
                env('GOOGLE_MAPS_API_KEY'),
            ],
            GeoPlugin::class  => [],
        ],
    ],

but it doesn't affect the query result, indeed app('geocoder')->reverse(44.5266777, 11.3213184)->get()->first()->getFormattedAddress(); still returns english result:

Via della Selva Pescarola, 42, 40131 Bologna BO, Italy

That behavior would probably deserve a specific issue ticket (???).

As you can see, I configured caching on Redis and it successfully work.


Second implementation So I found #131 and #128 and followed them by implementing:

$provider = app('geocoder')->getProvider();
$geocoder = new \Geocoder\StatefulGeocoder($provider, 'it');
$result = $geocoder->reverse(44.5266777, 11.3213184)->first();

and finally I get a localized result:

Via della Selva Pescarola, 42, 40131 Bologna BO, Italia

The side effect of this implementation is that the result won't be cached.


I'm not sure but I think it's something concerning the ProviderAndDumperAggregator:

  1. in the first implementation, the reverse method is called on the ProviderAndDumperAggregator that is responsible of the caching layer;
  2. in the second implementation, the reverse method is called on \Geocoder\StatefulGeocoder that doesn't provide caching layer.
gpanos commented 4 years ago

Run into this today i can confirm that since the ProviderAndDumperAggregator is not used there is no caching layer. Is this the recommended way to localize the results or maybe there is an alternative approach? Thanks a lot!

mikebronner commented 4 years ago

If you are not using the ProviderAndDumperAggregator class, you are essentially bypassing this entire package. You might as well use Geocoder's php library directly. What is the use-case for bypassing things like this, and not instead configuring all your providers in the config file?

gpanos commented 4 years ago

Hey thanks for your reply. My use case is pretty simple: Localize the results to the user selected language. The only way that I was able to achieve this was by following the steps described in #131 Maybe I am missing something here but couldn't figure out a better way. Maybe simply localization is not supported in this package but i would love to hear back from you if i am overseeing something. As a side note i was able to achieve caching with localization by installing and utilizing the https://github.com/geocoder-php/cache-provider but as you said this simply means that I am using the php geocoder library directly.

        $cachedProvider = new ProviderCache(
            $geocoder->getProvider(),
            app('cache')->store(config('geocoder.cache.store')),
            config('geocoder.cache.duration')
        );

        $this->geocoder = new StatefulGeocoder($cachedProvider);

Maybe it's worth considering refactoring the caching layer to make use of the ProviderCache? Thanks again

mikebronner commented 4 years ago

I was taking a look at how to implement caching by locale. At this time I don't have a solution yet. My recommendation would be to use ->doNotCache() on this queries for the time being.