Torann / laravel-geoip

Determine the geographical location of website visitors based on their IP addresses.
http://lyften.com/projects/laravel-geoip
BSD 2-Clause "Simplified" License
2.09k stars 372 forks source link

Bugfix - not caching results propertly - missing set cache name when ip not passed directly #125

Open pixel-anovak opened 5 years ago

pixel-anovak commented 5 years ago

GeoIP.php

public function getLocation($ip = null)
    {
        // Get location data
        $this->location = $this->find($ip);

        // If ip is not passed in method retrive ip from location object
        if ( empty($ip) ) {
            $ip = $this->location->getAttribute('ip');
        }

        // Should cache location
        if ($this->shouldCache($ip, $this->location)) {
            $this->getCache()->set($ip, $this->location);
        }

        return $this->location;
    }

Here is bugfix for not caching results properly in laravel database cache.

iamvladshevchuk commented 9 months ago

Yeah, I want to reopen this issue:

public function getLocation($ip = null)
    {
        // Get location data
        $this->location = $this->find($ip);

        // Should cache location
         if ($this->shouldCache($this->location, $ip)) {
             $this->getCache()->set($ip, $this->location);
         }

         return $this->location;
     }

In the current code when $ip is not provided, it tries to cache the location with $ip as the key (which is null). Effectively, the caching doesn't work when you don't provide $ip. In case if you work with https://github.com/swayok/alternative-laravel-cache it even leads to errors, because it expects the cache key will be always present.

I guess, you meant:

public function getLocation($ip = null)
    {
        // Get location data
        $this->location = $this->find($ip);

        // Should cache location
         if ($this->shouldCache($this->location, $ip)) {
             $this->getCache()->set($this->remote_ip, $this->location);
         }

         return $this->location;
     }