Refactored and fixed cache, closes #9, thanks @goranata for reporting!

cmfcmf · cmfcmf · commit c2db0e28e382 · 2014-02-25T16:44:59.000+01:00
diff --git a/Cmfcmf/OpenWeatherMap.php b/Cmfcmf/OpenWeatherMap.php
@@ -16,6 +16,7 @@
 
 namespace Cmfcmf;
 
+use Cmfcmf\OpenWeatherMap\AbstractCache;
 use Cmfcmf\OpenWeatherMap\CurrentWeather;
 use Cmfcmf\OpenWeatherMap\Exception as OWMException;
 use Cmfcmf\OpenWeatherMap\Fetcher\CurlFetcher;
@@ -77,20 +78,19 @@ class OpenWeatherMap
      * Constructs the OpenWeatherMap object.
      *
      * @param null|FetcherInterface $fetcher The interface to fetch the data from OpenWeatherMap. Defaults to
-     *                                       CurlFetcher() if if cURL is available. Otherwise defaults to
+     *                                       CurlFetcher() if cURL is available. Otherwise defaults to
      *                                       FileGetContentsFetcher() using 'file_get_contents()'.
-     * @param bool|string           $cacheClass If set to false, caching is disabled. If this is a valid class
-     *                                          extending Cmfcmf\OpenWeatherMap\Util\Cache, caching will be enabled.
-     *                                          Default false.
+     * @param bool|string           $cacheClass If set to false, caching is disabled. Otherwise this must be a class
+     *                                       extending AbstractCache. Defaults to false.
      * @param int                   $seconds How long weather data shall be cached. Default 10 minutes.
      *
      * @throws \Exception If $cache is neither false nor a valid callable extending Cmfcmf\OpenWeatherMap\Util\Cache.
      * @api
      */
     public function __construct($fetcher = null, $cacheClass = false, $seconds = 600)
     {
-        if ($cacheClass !== false && !class_exists($cacheClass)) {
-            throw new \Exception("Class $cacheClass does not exist.");
+        if ($cacheClass !== false && !($cacheClass instanceof AbstractCache)) {
+            throw new \Exception("The cache class must implement the FetcherInterface!");
         }
         if (!is_numeric($seconds)) {
             throw new \Exception("\$seconds must be numeric.");
@@ -348,7 +348,7 @@ public function getRawWeatherData($query, $units = 'imperial', $lang = 'en', $ap
     {
         $url = $this->buildUrl($query, $units, $lang, $appid, $mode, $this->weatherUrl);
 
-        return $this->cacheOrFetchResult('weather', $query, $units, $lang, $mode, $url);
+        return $this->cacheOrFetchResult($url);
     }
 
     /**
@@ -394,7 +394,7 @@ public function getRawHourlyForecastData($query, $units = 'imperial', $lang = 'e
     {
         $url = $this->buildUrl($query, $units, $lang, $appid, $mode, $this->weatherHourlyForecastUrl);
 
-        return $this->cacheOrFetchResult('hourlyForecast', $query, $units, $lang, $mode, $url);
+        return $this->cacheOrFetchResult($url);
     }
 
     /**
@@ -445,7 +445,7 @@ public function getRawDailyForecastData($query, $units = 'imperial', $lang = 'en
         }
         $url = $this->buildUrl($query, $units, $lang, $appid, $mode, $this->weatherDailyForecastUrl) . "&cnt=$cnt";
 
-        return $this->cacheOrFetchResult('dailyForecast', $query, $units, $lang, $mode, $url);
+        return $this->cacheOrFetchResult($url);
     }
 
     /**
@@ -515,7 +515,7 @@ public function getRawWeatherHistory($query, \DateTime $start, $endOrCount = 1,
             $queryUrl .= "&APPID=$appid";
         }
 
-        return $this->cacheOrFetchResult('weatherHistory', $query, $units, $lang, $type, $queryUrl);
+        return $this->cacheOrFetchResult($queryUrl);
     }
 
     /**
@@ -532,17 +532,17 @@ public function getRawWeatherHistory($query, \DateTime $start, $endOrCount = 1,
      *
      * @internal
      */
-    private function cacheOrFetchResult($type, $query, $units, $lang, $mode, $url)
+    private function cacheOrFetchResult($url)
     {
         if ($this->cacheClass !== false) {
             /** @var \Cmfcmf\OpenWeatherMap\AbstractCache $cache */
-            $cache = new $this->cacheClass;
+            $cache = $this->cacheClass;
             $cache->setSeconds($this->seconds);
-            if ($cache->isCached($type, $query, $units, $lang, $mode)) {
-                return $cache->getCached($type, $query, $units, $lang, $mode);
+            if ($cache->isCached($url)) {
+                return $cache->getCached($url);
             }
             $result = $this->fetcher->fetch($url);
-            $cache->setCached($type, $result, $query, $units, $lang, $mode);
+            $cache->setCached($url, $result);
         } else {
             $result = $this->fetcher->fetch($url);
         }
diff --git a/Cmfcmf/OpenWeatherMap/AbstractCache.php b/Cmfcmf/OpenWeatherMap/AbstractCache.php
@@ -29,46 +29,32 @@ abstract class AbstractCache
     /**
      * Checks whether a cached weather data is available.
      *
-     * @param string           $type The type of the cached data. Can be either 'weather', 'hourlyForecast' or 'dailyForecast'.
-     * @param array|int|string $query The query parameters used.
-     * @param string           $units The units requested.
-     * @param string           $lang The language requested.
-     * @param string           $mode The mode requested ('xml' or'json').
+     * @param string $url The unique url of the cached content.
      *
-     * @return \DateTime|bool A \DateTime object containing the time when the weather information was last updated, false if no
-     * cached information is available.
+     * @return bool False if no cached information is available, otherwise true.
      *
-     * @note This is not the time when the weather was cached, but the {@link Weather::$lastUpdate} value of the cached weather.
-     * @note You need to check here if a cached result is outdated. Return false in that case.
+     * You need to check if a cached result is outdated here. Return false in that case.
      */
-    public abstract function isCached($type, $query, $units, $lang, $mode);
+    abstract public function isCached($url);
 
     /**
      * Returns cached weather data.
      *
-     * @param string           $type The type of the cached data. Can be either 'weather', 'hourlyForecast' or 'dailyForecast'.
-     * @param array|int|string $query The query parameters used.
-     * @param string           $units The units requested.
-     * @param string           $lang The language requested.
-     * @param string           $mode The mode requested ('xml' or'json').
+     * @param string $url The unique url of the cached content.
      *
      * @return string|bool The cached data if it exists, false otherwise.
      */
-    public abstract function getCached($type, $query, $units, $lang, $mode);
+    abstract public function getCached($url);
 
     /**
      * Saves cached weather data.
      *
-     * @param string           $type The type of the cached data. Can be either 'weather', 'hourlyForecast' or 'dailyForecast'.
-     * @param string           $content The weather data to cache.
-     * @param array|int|string $query The query parameters used.
-     * @param string           $units The units requested.
-     * @param string           $lang The language requested.
-     * @param string           $mode The mode requested ('xml' or'json').
+     * @param string $url     The unique url of the cached content.
+     * @param string $content The weather data to cache.
      *
      * @return bool True on success, false on failure.
      */
-    public abstract function setCached($type, $content, $query, $units, $lang, $mode);
+    abstract public function setCached($url, $content);
 
     /**
      * Set after how much seconds the cache shall expire.
diff --git a/Examples/Cache.php b/Examples/Cache.php
@@ -32,34 +32,48 @@
  */
 class ExampleCache extends AbstractCache
 {
+    private function urlToPath($url)
+    {
+        $tmp = sys_get_temp_dir();
+        $dir = $tmp . DIRECTORY_SEPARATOR . "OpenWeatherMapPHPAPI";
+        if (!is_dir($dir)) {
+            mkdir($dir);
+        }
+
+        $path = $dir . DIRECTORY_SEPARATOR . md5($url);
+
+        return $path;
+    }
+
     /**
      * @inheritdoc
      */
-    public function isCached($type, $query, $units, $lang, $mode)
+    public function isCached($url)
     {
-        echo "Checking cache for $type $query $units $lang $mode …<br />";
+        $path = $this->urlToPath($url);
+        if (!file_exists($path) || filectime($path) + $this->seconds < time()) {
+            echo "Weather data is NOT cached!\n";
+            return false;
+        }
 
-        return false;
+        echo "Weather data is cached!\n";
+        return true;
     }
 
     /**
      * @inheritdoc
      */
-    public function getCached($type, $query, $units, $lang, $mode)
+    public function getCached($url)
     {
-        echo "Get cache for $type $query $units $lang $mode …<br />";
-
-        return false;
+        return file_get_contents($this->urlToPath($url));
     }
 
     /**
      * @inheritdoc
      */
-    public function setCached($type, $content, $query, $units, $lang, $mode)
+    public function setCached($url, $content)
     {
-        echo "Set cache for $type $query $units $lang $mode … ({$this->seconds} seconds)<br />";
-
-        return false;
+        file_put_contents($this->urlToPath($url), $content);
     }
 }
 
@@ -69,8 +83,8 @@ public function setCached($type, $content, $query, $units, $lang, $mode)
 // Units (can be 'metric' or 'imperial' [default]):
 $units = 'metric';
 
-// Example 1: Use your own cache implementation. See the example_cache file.
-$owm = new OpenWeatherMap('ExampleCache', 100);
+// Example 1: Use your own cache implementation. Cache for 10 seconds only in this example.
+$owm = new OpenWeatherMap(null, new ExampleCache(), 10);
 
 $weather = $owm->getWeather('Berlin', $units, $lang);
 echo "EXAMPLE 1<hr />\n\n\n";

Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@`
`16`	`16`
`17`	`17`	`namespace Cmfcmf;`
`18`	`18`
	`19`	`+use Cmfcmf\OpenWeatherMap\AbstractCache;`
`19`	`20`	`use Cmfcmf\OpenWeatherMap\CurrentWeather;`
`20`	`21`	`use Cmfcmf\OpenWeatherMap\Exception as OWMException;`
`21`	`22`	`use Cmfcmf\OpenWeatherMap\Fetcher\CurlFetcher;`
`@@ -77,20 +78,19 @@ class OpenWeatherMap`
`77`	`78`	`* Constructs the OpenWeatherMap object.`
`78`	`79`	`*`
`79`	`80`	`* @param null\|FetcherInterface $fetcher The interface to fetch the data from OpenWeatherMap. Defaults to`
`80`		`- * CurlFetcher() if if cURL is available. Otherwise defaults to`
	`81`	`+ * CurlFetcher() if cURL is available. Otherwise defaults to`
`81`	`82`	`* FileGetContentsFetcher() using 'file_get_contents()'.`
`82`		`- * @param bool\|string $cacheClass If set to false, caching is disabled. If this is a valid class`
`83`		`- * extending Cmfcmf\OpenWeatherMap\Util\Cache, caching will be enabled.`
`84`		`- * Default false.`
	`83`	`+ * @param bool\|string $cacheClass If set to false, caching is disabled. Otherwise this must be a class`
	`84`	`+ * extending AbstractCache. Defaults to false.`
`85`	`85`	`* @param int $seconds How long weather data shall be cached. Default 10 minutes.`
`86`	`86`	`*`
`87`	`87`	`* @throws \Exception If $cache is neither false nor a valid callable extending Cmfcmf\OpenWeatherMap\Util\Cache.`
`88`	`88`	`* @api`
`89`	`89`	`*/`
`90`	`90`	`public function __construct($fetcher = null, $cacheClass = false, $seconds = 600)`
`91`	`91`	`{`
`92`		`- if ($cacheClass !== false && !class_exists($cacheClass)) {`
`93`		`- throw new \Exception("Class $cacheClass does not exist.");`
	`92`	`+ if ($cacheClass !== false && !($cacheClass instanceof AbstractCache)) {`
	`93`	`+ throw new \Exception("The cache class must implement the FetcherInterface!");`
`94`	`94`	`}`
`95`	`95`	`if (!is_numeric($seconds)) {`
`96`	`96`	`throw new \Exception("\$seconds must be numeric.");`
`@@ -348,7 +348,7 @@ public function getRawWeatherData($query, $units = 'imperial', $lang = 'en', $ap`
`348`	`348`	`{`
`349`	`349`	`$url = $this->buildUrl($query, $units, $lang, $appid, $mode, $this->weatherUrl);`
`350`	`350`
`351`		`- return $this->cacheOrFetchResult('weather', $query, $units, $lang, $mode, $url);`
	`351`	`+ return $this->cacheOrFetchResult($url);`
`352`	`352`	`}`
`353`	`353`
`354`	`354`	`/**`
`@@ -394,7 +394,7 @@ public function getRawHourlyForecastData($query, $units = 'imperial', $lang = 'e`
`394`	`394`	`{`
`395`	`395`	`$url = $this->buildUrl($query, $units, $lang, $appid, $mode, $this->weatherHourlyForecastUrl);`
`396`	`396`
`397`		`- return $this->cacheOrFetchResult('hourlyForecast', $query, $units, $lang, $mode, $url);`
	`397`	`+ return $this->cacheOrFetchResult($url);`
`398`	`398`	`}`
`399`	`399`
`400`	`400`	`/**`
`@@ -445,7 +445,7 @@ public function getRawDailyForecastData($query, $units = 'imperial', $lang = 'en`
`445`	`445`	`}`
`446`	`446`	`$url = $this->buildUrl($query, $units, $lang, $appid, $mode, $this->weatherDailyForecastUrl) . "&cnt=$cnt";`
`447`	`447`
`448`		`- return $this->cacheOrFetchResult('dailyForecast', $query, $units, $lang, $mode, $url);`
	`448`	`+ return $this->cacheOrFetchResult($url);`
`449`	`449`	`}`
`450`	`450`
`451`	`451`	`/**`
`@@ -515,7 +515,7 @@ public function getRawWeatherHistory($query, \DateTime $start, $endOrCount = 1,`
`515`	`515`	`$queryUrl .= "&APPID=$appid";`
`516`	`516`	`}`
`517`	`517`
`518`		`- return $this->cacheOrFetchResult('weatherHistory', $query, $units, $lang, $type, $queryUrl);`
	`518`	`+ return $this->cacheOrFetchResult($queryUrl);`
`519`	`519`	`}`
`520`	`520`
`521`	`521`	`/**`
`@@ -532,17 +532,17 @@ public function getRawWeatherHistory($query, \DateTime $start, $endOrCount = 1,`
`532`	`532`	`*`
`533`	`533`	`* @internal`
`534`	`534`	`*/`
`535`		`- private function cacheOrFetchResult($type, $query, $units, $lang, $mode, $url)`
	`535`	`+ private function cacheOrFetchResult($url)`
`536`	`536`	`{`
`537`	`537`	`if ($this->cacheClass !== false) {`
`538`	`538`	`/** @var \Cmfcmf\OpenWeatherMap\AbstractCache $cache */`
`539`		`- $cache = new $this->cacheClass;`
	`539`	`+ $cache = $this->cacheClass;`
`540`	`540`	`$cache->setSeconds($this->seconds);`
`541`		`- if ($cache->isCached($type, $query, $units, $lang, $mode)) {`
`542`		`- return $cache->getCached($type, $query, $units, $lang, $mode);`
	`541`	`+ if ($cache->isCached($url)) {`
	`542`	`+ return $cache->getCached($url);`
`543`	`543`	`}`
`544`	`544`	`$result = $this->fetcher->fetch($url);`
`545`		`- $cache->setCached($type, $result, $query, $units, $lang, $mode);`
	`545`	`+ $cache->setCached($url, $result);`
`546`	`546`	`} else {`
`547`	`547`	`$result = $this->fetcher->fetch($url);`
`548`	`548`	`}`