Not the required data is displayed for the new user, because the data is taken from the cache of the previous user

Question

I made a custom module that displays the weather in a particular city.

But I got these comments after the code review:

1. Interesting question, what happens to your cache data, if the site first comes to a person from the city of London, and then Paris?

As I understand it, it means that a person from Paris, when he enters the site, will see the weather in London, because it will be taken from the cache. But I put the data in the cache so that there are not too many requests, I made a request once, put the data in the cache, and the next time I took the data from the cache.

2. In the small function, you are calling the http://ip-api.com/json/ endpoint twice. What happens when the site is visited by a thousand people per minute?

Here, I do not understand what the problem is. If it meant that the connection limit to the resource would be exhausted, then how to solve this problem? In the getCity() function, put the data in the cache in the same way as I do in the build() function? But then the same problem comes up as in the first remark, if a person from another city visits the site, then the data from the cache (name of the city of London) will be taken and not the name of his real city. How then to be?

Can you please tell me what needs to be changed in my code? Below I will write a slightly reduced code for my php file. Full version here: https://phpsandbox.io/n/sweet-forest-1lew-1wmof

// ....
use Drupal\Core\Cache\CacheBackendInterface;
use GuzzleHttp\Client;

//....

  public function getCity() {

    $ip = '193.62.157.66';  // static because for testing

    try {
      $response_ip = $this->httpClient->get('http://ip-api.com/json/' . $ip);
      $response_data_ip = $response_ip->getBody();
      $data_ip = json_decode($response_data_ip);

      if ($data_ip->status == 'success') {
        return $data_ip->city;
      }
      else {
        return $this->configFactory->get('sydneypro_weather.settings')->get('weather_city');
      }

    }
    catch (RequestException $e) {
      return FALSE;
    }

  }

  public function build() {
    $client = $this->httpClient;
    $api_key = $this->configFactory->get('sydneypro_weather.settings')->get('weather_api_key');
    $cid = 'sydneypro_weather';
    $weather_config = $this->configFactory->get('sydneypro_weather.settings');

    if (!$weather_config) {
      $this->logger->get('sydneypro_weather')->error('Config "sydneypro_weather.settings" is missing4');
      return [];
    }

    if (empty($api_key) || empty($this->getCity())) {
      return [
        '#type' => 'markup',
        '#markup' => $this->t('Please enter your API key and City in the Admin panel to see the weather'),
      ];
    }

    try {
      if ($cache = $this->cacheBackend->get($cid)) {
        $data = $cache->data;
      }
      else {
        $response = $client->get('http://api.openweathermap.org/data/2.5/weather?q=' . $this->getCity() . ',&appid=' . $api_key . '&units=metric');
        $response_data = $response->getBody();
        $data = json_decode($response_data);
        $this->cacheBackend->set($cid, $data, $this->time->getRequestTime() + 21600);
      }

      $build = [
        '#theme' => 'weather_block',
        '#data' => $data,
        '#attached' => [
          'library' => [
            'sydneypro_weather/sydneypro_weather',
          ],
        ],
      ];

      return $build;

    }
  // ....

score 2 · Accepted Answer · answered Apr 10 '22 at 10:14

You need to display some data according to a given city, so you should cache the data on a per-city basis, using a cache id that allows you to retrieve data for a specific city. In other words, $cid must contain a city name or identifier.
```
$city = $this->getCity();
$cid = 'sydneypro_weather:' . $city;
```
Using a variable for $city prevents getCity() to be called twice. Also you could set a cache that maps IP adresses to their corresponding city but it might not be a good idea as the number of (possible) distinct adresses populating the cache table could be very high.
Another approach would be to get the region/city of the user from his browser using javascript and/or cookies, and call the api only for those that does not share their location.

Not the required data is displayed for the new user, because the data is taken from the cache of the previous user

1 Answers1