From 1d4bc84754d5be2c901840a5a7b6607ebcda5e5a Mon Sep 17 00:00:00 2001 From: Hannes Papenberg Date: Sat, 25 Jul 2026 20:11:40 +0200 Subject: [PATCH 1/2] Lazily instantiate API package objects so PHP 8.2 doesn't complain --- src/Github.php | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/Github.php b/src/Github.php index 632c54e4..d5db8927 100644 --- a/src/Github.php +++ b/src/Github.php @@ -53,6 +53,42 @@ class Github */ protected $client; + /* + * Lazily instantiated API package objects, declared here so PHP 8.2+ does not treat + * them as deprecated dynamic properties when __get() populates them. + */ + private ?Package\Activity $activity = null; + + private ?Package\Authorization $authorization = null; + + private ?Package\Data $data = null; + + private ?Package\Emojis $emojis = null; + + private ?Package\Gists $gists = null; + + private ?Package\Gitignore $gitignore = null; + + private ?Package\Graphql $graphql = null; + + private ?Package\Issues $issues = null; + + private ?Package\Markdown $markdown = null; + + private ?Package\Meta $meta = null; + + private ?Package\Orgs $orgs = null; + + private ?Package\Pulls $pulls = null; + + private ?Package\Repositories $repositories = null; + + private ?Package\Search $search = null; + + private ?Package\Users $users = null; + + private ?Package\Zen $zen = null; + /** * Constructor. * From 133cbaa363a360ce16a56e2a58225421925babc9 Mon Sep 17 00:00:00 2001 From: Hannes Papenberg Date: Thu, 12 Nov 2026 23:40:18 +0100 Subject: [PATCH 2/2] Cache the package objects in an array instead of properties Declaring private properties on Github only fixed the top level: the sub-packages (Repositories::$forks, Issues::$comments, ...) were still created as dynamic properties by AbstractPackage::__get(), and every new package needed another property. The private properties also broke assigning a replacement object such as a mock ($github->issues = $mock), which failed with "Cannot modify private property". Keep the objects in a private array in both __get() methods and add __set(), __isset() and __unset() so that objects can still be replaced. Co-Authored-By: Claude Opus 5.5 --- Tests/PackageTest.php | 72 +++++++++++++++++++++++++++++++ docs/overview.md | 14 ++++++ src/AbstractPackage.php | 61 ++++++++++++++++++++++++-- src/Github.php | 95 ++++++++++++++++++++++++----------------- 4 files changed, 199 insertions(+), 43 deletions(-) diff --git a/Tests/PackageTest.php b/Tests/PackageTest.php index c86e3c42..c0949ad9 100755 --- a/Tests/PackageTest.php +++ b/Tests/PackageTest.php @@ -7,6 +7,8 @@ namespace Joomla\Github\Tests; use Joomla\Github\Github; +use Joomla\Github\Package\Issues; +use Joomla\Github\Package\Repositories\Forks; use Joomla\Github\Tests\Stub\GitHubTestCase; /** @@ -65,4 +67,74 @@ public function testGetInvalid() $this->object->repositories->INVALID; } + + /** + * Tests that the lazily created objects are cached without dynamic properties + * + * @return void + */ + public function testGetCachesWithoutDynamicProperties() + { + $errors = []; + + set_error_handler( + static function (int $errno, string $errstr) use (&$errors): bool { + $errors[] = $errstr; + + return true; + } + ); + + try { + $repositories = $this->object->repositories; + $forks = $this->object->repositories->forks; + + $this->assertSame($repositories, $this->object->repositories); + $this->assertSame($forks, $this->object->repositories->forks); + } finally { + restore_error_handler(); + } + + $this->assertSame([], $errors); + } + + /** + * Tests that API objects can be replaced, e.g. with mocks + * + * @return void + */ + public function testSetReplacesObjects() + { + $issues = $this->createStub(Issues::class); + $forks = $this->createStub(Forks::class); + + $this->object->issues = $issues; + $this->object->repositories->forks = $forks; + + $this->assertSame($issues, $this->object->issues); + $this->assertSame($forks, $this->object->repositories->forks); + } + + /** + * Tests the magic __isset and __unset methods + * + * @return void + */ + public function testIssetAndUnset() + { + $this->assertFalse(isset($this->object->issues)); + $this->assertFalse(isset($this->object->repositories->forks)); + + $issues = $this->object->issues; + $forks = $this->object->repositories->forks; + + $this->assertTrue(isset($this->object->issues)); + $this->assertTrue(isset($this->object->repositories->forks)); + + unset($this->object->issues, $this->object->repositories->forks); + + $this->assertFalse(isset($this->object->issues)); + $this->assertNotSame($issues, $this->object->issues); + $this->assertNotSame($forks, $this->object->repositories->forks); + } } diff --git a/docs/overview.md b/docs/overview.md index cfd53634..b777f0fc 100644 --- a/docs/overview.md +++ b/docs/overview.md @@ -1,3 +1,17 @@ # Github API package TODO + +## Package objects + +`Github` creates the package objects (`$github->issues`, `$github->repositories`, …) and their sub-packages +(`$github->repositories->forks`) on first access and reuses them afterwards. They can be replaced, for example with a +mock in a test: + +```php +$github->issues = $issuesMock; +$github->repositories->forks = $forksMock; +``` + +`isset($github->issues)` tells whether the object has been created or set; `unset($github->issues)` makes the next +access create a new one. diff --git a/src/AbstractPackage.php b/src/AbstractPackage.php index 71266b7c..5da93f01 100644 --- a/src/AbstractPackage.php +++ b/src/AbstractPackage.php @@ -19,6 +19,16 @@ */ abstract class AbstractPackage extends AbstractGithubObject { + /** + * The lazily created API objects, keyed by name. + * + * Kept in an array instead of dynamic properties, which are deprecated since PHP 8.2. + * + * @var array + * @since __DEPLOY_VERSION__ + */ + private array $packages = []; + /** * Constructor. * @@ -47,6 +57,10 @@ public function __construct(Registry $options = null, Http $client = null) */ public function __get($name) { + if (isset($this->packages[$name])) { + return $this->packages[$name]; + } + $class = '\\Joomla\\Github\\Package\\' . $this->package . '\\' . ucfirst($name); if (class_exists($class) == false) { @@ -60,10 +74,49 @@ public function __get($name) ); } - if (isset($this->$name) == false) { - $this->$name = new $class($this->options, $this->client); - } + return $this->packages[$name] = new $class($this->options, $this->client); + } - return $this->$name; + /** + * Magic method to replace an API object, e.g. with a mock in tests + * + * @param string $name Name of the property + * @param mixed $value The object to use + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + public function __set($name, $value) + { + $this->packages[$name] = $value; + } + + /** + * Magic method to check whether an API object has been created or set + * + * @param string $name Name of the property + * + * @return boolean + * + * @since __DEPLOY_VERSION__ + */ + public function __isset($name) + { + return isset($this->packages[$name]); + } + + /** + * Magic method to remove an API object, so that it is created again on the next access + * + * @param string $name Name of the property + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + public function __unset($name) + { + unset($this->packages[$name]); } } diff --git a/src/Github.php b/src/Github.php index d5db8927..cb626d43 100644 --- a/src/Github.php +++ b/src/Github.php @@ -53,41 +53,15 @@ class Github */ protected $client; - /* - * Lazily instantiated API package objects, declared here so PHP 8.2+ does not treat - * them as deprecated dynamic properties when __get() populates them. + /** + * The lazily created API objects, keyed by name. + * + * Kept in an array instead of dynamic properties, which are deprecated since PHP 8.2. + * + * @var array + * @since __DEPLOY_VERSION__ */ - private ?Package\Activity $activity = null; - - private ?Package\Authorization $authorization = null; - - private ?Package\Data $data = null; - - private ?Package\Emojis $emojis = null; - - private ?Package\Gists $gists = null; - - private ?Package\Gitignore $gitignore = null; - - private ?Package\Graphql $graphql = null; - - private ?Package\Issues $issues = null; - - private ?Package\Markdown $markdown = null; - - private ?Package\Meta $meta = null; - - private ?Package\Orgs $orgs = null; - - private ?Package\Pulls $pulls = null; - - private ?Package\Repositories $repositories = null; - - private ?Package\Search $search = null; - - private ?Package\Users $users = null; - - private ?Package\Zen $zen = null; + private array $packages = []; /** * Constructor. @@ -126,19 +100,62 @@ public function __construct(Registry $options = null, Http $client = null) */ public function __get($name) { + if (isset($this->packages[$name])) { + return $this->packages[$name]; + } + $class = 'Joomla\\Github\\Package\\' . ucfirst($name); if (class_exists($class)) { - if (isset($this->$name) == false) { - $this->$name = new $class($this->options, $this->client); - } - - return $this->$name; + return $this->packages[$name] = new $class($this->options, $this->client); } throw new \InvalidArgumentException(sprintf('Argument %s produced an invalid class name: %s', $name, $class)); } + /** + * Magic method to replace an API object, e.g. with a mock in tests + * + * @param string $name Name of the property + * @param mixed $value The object to use + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + public function __set($name, $value) + { + $this->packages[$name] = $value; + } + + /** + * Magic method to check whether an API object has been created or set + * + * @param string $name Name of the property + * + * @return boolean + * + * @since __DEPLOY_VERSION__ + */ + public function __isset($name) + { + return isset($this->packages[$name]); + } + + /** + * Magic method to remove an API object, so that it is created again on the next access + * + * @param string $name Name of the property + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + public function __unset($name) + { + unset($this->packages[$name]); + } + /** * Get an option from the GitHub instance. *