Merge pull request #24 from benrowe/releases/build-5

Releases/build 5

.env.example

@@ -2,4 +2,7 @@ APP_ENV=dev
 
 LASTFM_KEY=
 LASTFM_SECRET=
-LASTFM_PAGINATION_LIMIT=5
+LASTFM_PAGINATION_LIMIT=5
+
+CACHE_DRIVER=file
+CACHE_FILE_PATH=storage/cache

.gitignore

@@ -1,4 +1,5 @@
 /vendor/
 .env
 .idea
-/storage/views/
+/storage/views/
+/storage/cache/

app/Exceptions/HttpException.php

@@ -4,6 +4,11 @@
 
 use Exception;
 
+/**
+ * HttpException
+ *
+ * @package App\Exceptions
+ */
 class HttpException extends Exception
 {
 }

app/Exceptions/RuntimeException.php

@@ -0,0 +1,16 @@
+<?php
+
+namespace App\Exceptions;
+
+use RuntimeException as Exception;
+
+/**
+ * RuntimeException
+ *
+ * Represents a simple runtime Exception that is localised to this application
+ *
+ * @package App\Exceptions
+ */
+class RuntimeException extends Exception
+{
+}

app/Http/Controllers/AbstractController.php

@@ -64,6 +64,7 @@ protected function hasRequestParam($key)
      * @param string $method
      * @param array $params
      * @return string|array|ResponseInterface
+     * @todo Invoke action through container for DI in controller::action*()
      */
     public static function __callStatic($method, $params)
     {
@@ -82,7 +83,7 @@ public static function __callStatic($method, $params)
      *
      * @param string $path
      * @param array  $params
-     * @return \Psr\Http\Message\StreamInterface
+     * @return ResponseInterface
      */
     protected function view($path, array $params = [])
     {

app/Http/Controllers/ArtistController.php

@@ -5,14 +5,27 @@
 use App\Exceptions\HttpException;
 use App\Services\LastFm\Response\Artist;
 
+/**
+ * ArtistController
+ *
+ * @package App\Http\Controllers
+ */
 class ArtistController extends AbstractController
 {
+    /**
+     * Display details for the selected artist
+     *
+     * @param string $id music brains id
+     * @return \Psr\Http\Message\ResponseInterface
+     */
     public function actionView($id)
     {
         return $this->view('artist', ['artist' => $this->getModel($id)]);
     }
 
     /**
+     * Load the artist model based on the supplied id
+     *
      * @param string $id
      * @return Artist
      * @throws HttpException

app/Http/Controllers/DefaultController.php

@@ -5,6 +5,7 @@
 use App\Models\Country;
 use App\Models\Forms\SearchForm;
 
+
 /**
  * Class DefaultController
  *
@@ -26,7 +27,7 @@ public function actionIndex()
         return $this->view('default', [
             'countries' => Country::all(),
             'model' => $searchForm,
-            'results' => $searchForm->isSearchable() ? $searchForm->results() : null,
+            'results' => $searchForm->isSearchable() ? $searchForm->results($this->getRequestParam('page', 1)) : null,
         ]);
     }
 }

app/Http/routes.php

@@ -3,6 +3,8 @@
 use App\Http\Controllers\ArtistController;
 use App\Http\Controllers\DefaultController;
 
+
+
 /**
  * @var \League\Route\RouteCollection $route
  */

app/Models/AbstractModel.php

@@ -12,13 +12,25 @@ abstract class AbstractModel
 {
     protected $attributes = [];
 
+    /**
+     * AbstractModel constructor.
+     * Initialise the concrete model with the supplied attributes
+     *
+     * @param null $attributes
+     */
     public function __construct($attributes = null)
     {
         if (is_array($attributes)) {
             $this->attributes = $attributes;
         }
     }
 
+    /**
+     * Fill the model with the array of attributes, in addition to any existing attribute values
+     * Any existing values which matching keys will be overridden.
+     *
+     * @param array $attributes
+     */
     public function fill(array $attributes)
     {
         $this->attributes = array_merge($this->attributes, $attributes);

app/Models/Country.php

@@ -4,9 +4,15 @@
 
 use App\Exceptions\InvalidParamException;
 
+/**
+ * Country
+ *
+ * @package App\Models
+ */
 class Country
 {
     const DATA_FILE = 'resources/data/countries.json';
+
     /**
      * Raw country data
      *

app/Models/Forms/SearchForm.php

@@ -5,8 +5,12 @@
 use App\Models\AbstractModel;
 use App\Models\Country;
 use App\Services\LastFm\Client;
+use App\Services\LastFm\Contracts\ResultSet;
 
 /**
+ * SearchForm model
+ * Represents the state of the search form and allows a result set to be produced for the top artist
+ * as per the attributes
  *
  * @property Country country
  * @package App\Models
@@ -23,20 +27,21 @@ class SearchForm extends AbstractModel
     /**
      * Calculate the result set based on the current state of the object
      *
+     * @param int $page
      * @return ResultSet
      */
-    public function results()
+    public function results($page = 1)
     {
         $client  = $this->getLastFmApi();
 
-        return $client->geo->topArtists($this->country()->name);
+        return $client->geo->topArtists($this->country()->name, ['page' => $page]);
     }
 
     /**
      * Check the state of the SearchForm model to determine if a search could
      * return possible results
      *
-     * @return boolean [description]
+     * @return boolean
      */
     public function isSearchable()
     {

app/Services/LastFm/Api/Artist.php

@@ -65,7 +65,7 @@ public function search(SearchRequest $request, array $params = []): ResultSet
      * Find the artist by their id
      *
      * @param string $artistRef either the musicbrainz id or the artist name
-     * @return ArtistResponse|bool false when no artist found
+     * @return AbstractResponse|bool false when no artist found
      * @throws Exception
      */
     public function find($artistRef)
@@ -99,8 +99,7 @@ public function find($artistRef)
      */
     public function topTracks($artistRef, array $params = [])
     {
-        $params = $this->buildArtistParams($artistRef);
-        $params = array_merge($params, $params);
+        $params = array_merge($params, $this->buildArtistParams($artistRef));
         try {
             $response = $this->client->request(
                 'artist.gettoptracks',
@@ -122,7 +121,7 @@ public function topTracks($artistRef, array $params = [])
     }
 
     /**
-     * Build the arist api params based on the type of data being provided
+     * Build the artist api params based on the type of data being provided
      *
      * @param string $artistRef
      * @return array

app/Services/LastFm/Api/Geo.php

@@ -3,7 +3,7 @@
 namespace App\Services\LastFm\Api;
 
 use App\Services\LastFm\Client;
-use App\Services\LastFm\Response\AbstractResponse;
+use App\Services\LastFm\Contracts\ResultSet;
 use App\Services\LastFm\Response\Artist as ArtistResponse;
 
 /**
@@ -12,6 +12,7 @@
  */
 class Geo
 {
+    use TraitSupportsPagintation;
     /**
      * @var Client
      */
@@ -32,8 +33,11 @@ public function __construct(Client $client)
     public function topArtists($countryCode, array $params = [])
     {
         $params = array_merge($params, ['country' => $countryCode]);
-        $response = $this->client->request('geo.gettopartists', $params);
-
-        return AbstractResponse::makeResultSet($this->client, $response, ArtistResponse::class, 'topartists.artist');
+        return $this->buildPaginationResultSet(
+            'geo.gettopartists',
+            $params,
+            ArtistResponse::class,
+            'topartists.artist'
+        );
     }
 }

app/Services/LastFm/Api/TraitSupportsPagintation.php

@@ -0,0 +1,73 @@
+<?php
+
+namespace App\Services\LastFm\Api;
+
+use App\Services\LastFm\Response\AbstractResponse;
+
+/**
+ * Provides paginated result set functionality to the lastfm api components
+ *
+ * @package App\Services\LastFm\Api
+ */
+trait TraitSupportsPagintation
+{
+    private function buildPaginationParams($params)
+    {
+        if (!isset($params['limit'])) {
+            $params['limit'] = $this->client->getOption('pagination.limit', 5);
+        }
+        if (!isset($params['page']) || $params['page'] < 1) {
+            $params['page'] = 1;
+        }
+
+        return $params;
+    }
+
+    /**
+     * @param $methodName
+     * @param $params
+     * @param $type
+     * @param $key
+     * @return \App\Services\LastFm\Response\ResultSet
+     */
+    private function buildPaginationResultSet($methodName, $params, $type, $key)
+    {
+        $params = $this->buildPaginationParams($params);
+        $response = $this->client->request($methodName, $params);
+
+        /**
+         * !important
+         * see method note below
+         */
+        $response = $this->fixLastFmPaginationBug($response, $key);
+
+        return AbstractResponse::makeResultSet($this->client, $response, $type, $key);
+    }
+
+    /**
+     * Note: LastFM's pagination mechanism is very broken! Requesting a limit & page does
+     * not reply with the expected result, thus the following is a client side fix to ensure the result set is
+     * correct.
+     * The problem is the limit is ignored, and the result includes previous pages!
+     * If they ever fix this problem, this 'hack' should not effect the result set
+     *
+     * @param array $response
+     * @param string $key
+     * @return array
+     */
+    private function fixLastFmPaginationBug(array $response, $key): array
+    {
+        $data = array_pull($response, $key);
+        if (count($data) > $this->client->getOption('pagination.limit', 5)) {
+            $data = array_slice($data, $this->client->getOption('pagination.limit', 5) * -1);
+        }
+
+        array_set(
+            $response,
+            $key,
+            $data
+        );
+
+        return $response;
+    }
+}