feature #907 [3.x] Re-worked pagination to not mutate the api classes (GrahamCampbell)

This PR was squashed before being merged into the 3.0.x-dev branch.

Discussion
----------

This re-works pagination to avoid mutating the API classes. It took a few attempts to get the implementation details right. Let me explain why I chose to use clone:

1. If we were to use `new static(...)`, then I would have had to have made the abstract api constructor final. This could cause problems for people mocking the api classes, and also limits the possibility to thunk up arguments to the api classes, say if we want to introduce an api class which takes an extra parameter which is always used (this is common place in the gitlab fork of this library).
2. Is there another way to do things without making the constructor final or using clone? Well, we could have every api object implement perPage for themselves. This is kinda cumbersome though, and I don't think we want to do this.

---

This PR partially addresses #904.

So, I think cloning then setting the private property, within the abstract api class is the way to go, and nicely separates responsibilities. Classes that extend the abstract api class cannot mutate the property, and outsiders also cannot either. They have to call perPage which gives a fresh instance (one we've cloned).

Commits
-------

54c652c Re-worked pagination to not mutate the api classes
4ba3e48 Avoid needing getPerPage and perPage
decf105 Restored test coverage

Author: GrahamCampbell

composer.json

@@ -26,7 +26,8 @@
         "psr/cache": "^1.0",
         "psr/http-client-implementation": "^1.0",
         "psr/http-factory-implementation": "^1.0",
-        "psr/http-message": "^1.0"
+        "psr/http-message": "^1.0",
+        "symfony/polyfill-php80": "^1.17"
     },
     "require-dev": {
         "symfony/cache": "^5.1.8",

lib/Github/Api/AbstractApi.php

@@ -6,79 +6,59 @@
 use Github\HttpClient\Message\ResponseMediator;
 
 /**
- * Abstract class for Api classes.
- *
  * @author Joseph Bielawski <[email protected]>
+ * @author Graham Campbell <[email protected]>
  */
-abstract class AbstractApi implements ApiInterface
+abstract class AbstractApi
 {
     /**
-     * The client.
+     * The client instance.
      *
      * @var Client
      */
-    protected $client;
+    private $client;
 
     /**
-     * The requested page (GitHub pagination).
+     * The per page parameter.
      *
-     * @var null|int
+     * @var int|null
      */
-    private $page;
+    private $perPage;
 
     /**
-     * Number of items per page (GitHub pagination).
+     * Create a new API instance.
      *
-     * @var null|int
-     */
-    protected $perPage;
-
-    /**
      * @param Client $client
+     *
+     * @return void
      */
     public function __construct(Client $client)
     {
         $this->client = $client;
     }
 
-    public function configure()
-    {
-    }
-
-    /**
-     * @return null|int
-     */
-    public function getPage()
-    {
-        return $this->page;
-    }
-
     /**
-     * @param null|int $page
+     * Get the client instance.
+     *
+     * @return Client
      */
-    public function setPage($page)
+    protected function getClient()
     {
-        $this->page = (null === $page ? $page : (int) $page);
-
-        return $this;
+        return $this->client;
     }
 
     /**
-     * @return null|int
+     * Get the API version.
+     *
+     * @return string
      */
-    public function getPerPage()
+    protected function getApiVersion()
     {
-        return $this->perPage;
+        return $this->client->getApiVersion();
     }
 
-    /**
-     * @param null|int $perPage
-     */
-    public function setPerPage($perPage)
+    public function configure()
     {
-        $this->perPage = (null === $perPage ? $perPage : (int) $perPage);
-
-        return $this;
     }
 
     /**
@@ -92,12 +72,10 @@ public function setPerPage($perPage)
      */
     protected function get($path, array $parameters = [], array $requestHeaders = [])
     {
-        if (null !== $this->page && !isset($parameters['page'])) {
-            $parameters['page'] = $this->page;
-        }
         if (null !== $this->perPage && !isset($parameters['per_page'])) {
             $parameters['per_page'] = $this->perPage;
         }
+
         if (array_key_exists('ref', $parameters) && null === $parameters['ref']) {
             unset($parameters['ref']);
         }

lib/Github/Api/ApiInterface.php

@@ -35,15 +35,15 @@ public function update(array $params)
      */
     public function emails()
     {
-        return new Emails($this->client);
+        return new Emails($this->getClient());
     }
 
     /**
      * @return Followers
      */
     public function follow()
     {
-        return new Followers($this->client);
+        return new Followers($this->getClient());
     }
 
     public function followers($page = 1)
@@ -71,23 +71,23 @@ public function issues(array $params = [], $includeOrgIssues = true)
      */
     public function keys()
     {
-        return new PublicKeys($this->client);
+        return new PublicKeys($this->getClient());
     }
 
     /**
      * @return Notifications
      */
     public function notifications()
     {
-        return new Notifications($this->client);
+        return new Notifications($this->getClient());
     }
 
     /**
      * @return Memberships
      */
     public function memberships()
     {
-        return new Memberships($this->client);
+        return new Memberships($this->getClient());
     }
 
     /**
@@ -147,15 +147,15 @@ public function repositories($type = 'owner', $sort = 'full_name', $direction =
      */
     public function watchers()
     {
-        return new Watchers($this->client);
+        return new Watchers($this->getClient());
     }
 
     /**
      * @return Starring
      */
     public function starring()
     {
-        return new Starring($this->client);
+        return new Starring($this->getClient());
     }
 
     /**

lib/Github/Api/CurrentUser.php

@@ -26,7 +26,7 @@ class Starring extends AbstractApi
     public function configure($bodyType = null)
     {
         if ('star' === $bodyType) {
-            $this->acceptHeaderValue = sprintf('application/vnd.github.%s.star+json', $this->client->getApiVersion());
+            $this->acceptHeaderValue = sprintf('application/vnd.github.%s.star+json', $this->getApiVersion());
         }
 
         return $this;

lib/Github/Api/CurrentUser/Starring.php

@@ -22,30 +22,30 @@ class Enterprise extends AbstractApi
      */
     public function stats()
     {
-        return new Stats($this->client);
+        return new Stats($this->getClient());
     }
 
     /**
      * @return License
      */
     public function license()
     {
-        return new License($this->client);
+        return new License($this->getClient());
     }
 
     /**
      * @return ManagementConsole
      */
     public function console()
     {
-        return new ManagementConsole($this->client);
+        return new ManagementConsole($this->getClient());
     }
 
     /**
      * @return UserAdmin
      */
     public function userAdmin()
     {
-        return new UserAdmin($this->client);
+        return new UserAdmin($this->getClient());
     }
 }

lib/Github/Api/Enterprise.php

@@ -29,7 +29,7 @@ public function configure($bodyType = null)
             $bodyType = 'raw';
         }
 
-        $this->acceptHeaderValue = sprintf('application/vnd.github.%s.%s+json', $this->client->getApiVersion(), $bodyType);
+        $this->acceptHeaderValue = sprintf('application/vnd.github.%s.%s+json', $this->getApiVersion(), $bodyType);
 
         return $this;
     }

lib/Github/Api/Gist/Comments.php

@@ -32,7 +32,7 @@ public function configure($bodyType = null)
             $bodyType = 'raw';
         }
 
-        $this->acceptHeaderValue = sprintf('application/vnd.github.%s.%s', $this->client->getApiVersion(), $bodyType);
+        $this->acceptHeaderValue = sprintf('application/vnd.github.%s.%s', $this->getApiVersion(), $bodyType);
 
         return $this;
     }
@@ -177,6 +177,6 @@ public function unstar($id)
      */
     public function comments()
     {
-        return new Comments($this->client);
+        return new Comments($this->getClient());
     }
 }

lib/Github/Api/Gists.php

@@ -22,38 +22,38 @@ class GitData extends AbstractApi
      */
     public function blobs()
     {
-        return new Blobs($this->client);
+        return new Blobs($this->getClient());
     }
 
     /**
      * @return Commits
      */
     public function commits()
     {
-        return new Commits($this->client);
+        return new Commits($this->getClient());
     }
 
     /**
      * @return References
      */
     public function references()
     {
-        return new References($this->client);
+        return new References($this->getClient());
     }
 
     /**
      * @return Tags
      */
     public function tags()
     {
-        return new Tags($this->client);
+        return new Tags($this->getClient());
     }
 
     /**
      * @return Trees
      */
     public function trees()
     {
-        return new Trees($this->client);
+        return new Trees($this->getClient());
     }
 }

lib/Github/Api/GitData.php

@@ -26,7 +26,7 @@ class Blobs extends AbstractApi
     public function configure($bodyType = null)
     {
         if ('raw' === $bodyType) {
-            $this->acceptHeaderValue = sprintf('application/vnd.github.%s.raw', $this->client->getApiVersion());
+            $this->acceptHeaderValue = sprintf('application/vnd.github.%s.raw', $this->getApiVersion());
         }
 
         return $this;