From c92957c54a0862775e7624ee55e62123bc9ce30d Mon Sep 17 00:00:00 2001 From: Anton Ukhanev Date: Thu, 6 Jan 2022 10:29:35 +0100 Subject: [PATCH 1/6] `SiteMeta` will now use site functions (#4) This removes the need for a failure-signifying value, as the mechanism is now different. --- src/Options/SiteMeta.php | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/Options/SiteMeta.php b/src/Options/SiteMeta.php index 9fba126..2859be6 100644 --- a/src/Options/SiteMeta.php +++ b/src/Options/SiteMeta.php @@ -26,19 +26,12 @@ class SiteMeta implements MutableContainerInterface */ protected $siteId; - /** - * @var mixed - */ - protected $default; - /** * @param int $siteId ID of the site. - * @param mixed $default The value that, if returned by WP, will indicate that the key is not found. */ - public function __construct(int $siteId, $default) + public function __construct(int $siteId) { $this->siteId = $siteId; - $this->default = $default; } /** @@ -114,7 +107,7 @@ public function set(string $key, $value): void public function unset(string $key): void { $siteId = $this->siteId; - $result = delete_network_option($siteId, $key); + $result = delete_site_meta($siteId, $key); if ($result === false) { throw new ContainerException( @@ -140,10 +133,22 @@ public function unset(string $key): void protected function getMeta(string $name) { $siteId = $this->siteId; - $default = $this->default; - $value = get_network_option($siteId, $name, $default); - - if ($value === $default) { + $value = get_site_meta($siteId, $name, false); + + /* + * There's no way to pass a control value that would be returned if not found. + * Due to the way meta retrieval works (via cache), it does not check for individual keys in the DB, + * but instead retrieves and caches them all. There's no additional check to return a special value + * that would reliably signify that the value is not found. Instead, it returns maybe unserialized value + * if it is found, and null otherwise. Therefore, null is the closest value to signifying its absence. + * https://github.com/WordPress/WordPress/blob/2699b3032a710335e47a4a3a9d3fd5e44c35bac0/wp-includes/meta.php#L665 + * + * Also, the value can be an empty string if site ID is not found, or false if invalid, according to docs. + * There is no way to determine the site specified is missing or if its ID is invalid, or if the empty string + * is the actual value, without checking for existence of the site with the ID. + * This is the concern of a factory instantiating this class. + */ + if ($value === null) { throw new UnexpectedValueException( $this->__( 'Meta key "%1$s" for blog #%2$d does not exist', @@ -169,7 +174,7 @@ protected function setMeta(string $name, $value): void { $siteId = $this->siteId; - $isSuccessful = update_network_option($siteId, $name, $value); + $isSuccessful = update_site_meta($siteId, $name, $value); if (!$isSuccessful) { $newValue = $this->getMeta($name); $isSuccessful = $value === $newValue; From 08db4404f7447c21c43bfa56c117f9aef797e6fe Mon Sep 17 00:00:00 2001 From: Anton Ukhanev Date: Thu, 6 Jan 2022 11:30:16 +0100 Subject: [PATCH 2/6] Update test --- tests/functional/Options/SiteMetaTest.php | 60 ++++++++++------------- 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/tests/functional/Options/SiteMetaTest.php b/tests/functional/Options/SiteMetaTest.php index f77237f..c0c7a61 100644 --- a/tests/functional/Options/SiteMetaTest.php +++ b/tests/functional/Options/SiteMetaTest.php @@ -60,9 +60,9 @@ public function testHasTrue() [$siteId, $default], null ); - $fnGetNetworkOption = Functions\expect('get_network_option') + $fnGetNetworkOption = Functions\expect('get_site_meta') ->times(1) - ->with($siteId, $optionName, $default) + ->with($siteId, $optionName, false) ->andReturn($optionValue); } @@ -89,15 +89,14 @@ public function testHasFalse() { $siteId = rand(1, 99); $optionName = uniqid('option-name'); - $default = uniqid('default-value'); $subject = $this->createSubject( - [$siteId, $default], + [$siteId], null ); - $fnGetNetworkOption = Functions\expect('get_network_option') + $fnGetNetworkOption = Functions\expect('get_site_meta') ->times(1) - ->with($siteId, $optionName, $default) - ->andReturn($default); + ->with($siteId, $optionName, false) + ->andReturn(null); } { @@ -154,14 +153,13 @@ public function testGet($optionValue) { $siteId = rand(1, 99); $optionName = uniqid('option-name'); - $default = uniqid('default'); $subject = $this->createSubject( - [$siteId, $default], + [$siteId], null ); - $fnGetNetworkOption = Functions\expect('get_network_option') + $fnGetNetworkOption = Functions\expect('get_site_meta') ->times(1) - ->with($siteId, $optionName, $default) + ->with($siteId, $optionName, false) ->andReturn($optionValue); } @@ -189,15 +187,14 @@ public function testGetNotFound() $siteId = rand(1, 99); $optionName = uniqid('option-name'); $optionValue = uniqid('option-value'); - $default = uniqid('default'); $subject = $this->createSubject( - [$siteId, $default], + [$siteId], null ); - $fnGetNetworkOption = Functions\expect('get_network_option') + $fnGetNetworkOption = Functions\expect('get_site_meta') ->times(1) - ->with($siteId, $optionName, $default) - ->andReturn($default); + ->with($siteId, $optionName, false) + ->andReturn(null); $this->expectException(NotFoundExceptionInterface::class); } @@ -227,12 +224,11 @@ public function testSet($optionValue) { $siteId = rand(1, 99); $optionName = uniqid('option-name'); - $default = uniqid('default'); $subject = $this->createSubject( - [$siteId, $default], + [$siteId], null ); - $fnUpdateNetworkOption = Functions\expect('update_network_option') + $fnUpdateNetworkOption = Functions\expect('update_site_meta') ->times(1) ->with($siteId, $optionName, $optionValue) ->andReturn(true); @@ -262,18 +258,17 @@ public function testSetSame($optionValue) { $siteId = rand(1, 99); $optionName = uniqid('option-name'); - $default = uniqid('default'); $subject = $this->createSubject( - [$siteId, $default], + [$siteId], null ); - $fnUpdateNetworkOption = Functions\expect('update_network_option') + $fnUpdateNetworkOption = Functions\expect('update_site_meta') ->times(1) ->with($siteId, $optionName, $optionValue) ->andReturn(false); - $fnGetNetworkOption = Functions\expect('get_network_option') + $fnGetNetworkOption = Functions\expect('get_site_meta') ->times(1) - ->with($siteId, $optionName, $default) + ->with($siteId, $optionName, false) ->andReturn($optionValue); } @@ -297,18 +292,17 @@ public function testSetFailure() $siteId = rand(1, 99); $optionName = uniqid('option-name'); $optionValue = uniqid('option-value'); - $default = uniqid('default'); $subject = $this->createSubject( - [$siteId, $default], + [$siteId], null ); - $fnUpdateNetworkOption = Functions\expect('update_network_option') + $fnUpdateNetworkOption = Functions\expect('update_site_meta') ->times(1) ->with($siteId, $optionName, $optionValue) ->andReturn(false); - $fnGetNetworkOption = Functions\expect('get_network_option') + $fnGetNetworkOption = Functions\expect('get_site_meta') ->times(1) - ->with($siteId, $optionName, $default) + ->with($siteId, $optionName, false) ->andReturn(uniqid('different-value')); $this->expectException(ContainerExceptionInterface::class); } @@ -335,10 +329,10 @@ public function testUnset() $blogId = rand(1, 99); $optionName = uniqid('option-name'); $subject = $this->createSubject( - [$blogId, uniqid('default-value')], + [$blogId], null ); - $fnDeleteNetworkOption = Functions\expect('delete_network_option') + $fnDeleteNetworkOption = Functions\expect('delete_site_meta') ->times(1) ->with($blogId, $optionName) ->andReturn(true); @@ -366,10 +360,10 @@ public function testUnsetFailure() $blogId = rand(1, 99); $optionName = uniqid('option-name'); $subject = $this->createSubject( - [$blogId, uniqid('default-value')], + [$blogId], null ); - $fnDeleteNetworkOption = Functions\expect('delete_network_option') + $fnDeleteNetworkOption = Functions\expect('delete_site_meta') ->times(1) ->with($blogId, $optionName) ->andReturn(false); From d6d6fc6dcbc1ee60dfa99e20dc0b892b7c6bbdd5 Mon Sep 17 00:00:00 2001 From: Anton Ukhanev Date: Thu, 6 Jan 2022 11:30:42 +0100 Subject: [PATCH 3/6] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 814e1fc..ad51c61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `BlogOptions` now accepts a `null` blog ID, which causes it to use the current site's options every time, regardless (#3). +### Fixed +- `SiteMeta` now uses correct site meta functions (#5). + ## [0.1.1-alpha1] - 2022-01-05 ### Changed - Factory callables now documented in Psalm (#2). From 8cff1e9a7006eb73b5d258a6888673a6e0eb7ec4 Mon Sep 17 00:00:00 2001 From: Anton Ukhanev Date: Thu, 6 Jan 2022 11:35:48 +0100 Subject: [PATCH 4/6] Allow null site ID for `SiteMeta` (#4) This allows the creation of a site meta container that always points to the current site. which is much simpler, and may be significantly more convenient to set up on a single site --- src/Options/SiteMeta.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Options/SiteMeta.php b/src/Options/SiteMeta.php index 2859be6..a1cd1bf 100644 --- a/src/Options/SiteMeta.php +++ b/src/Options/SiteMeta.php @@ -21,13 +21,11 @@ class SiteMeta implements MutableContainerInterface { use StringTranslatingTrait; - /** - * @var int - */ + /** @var int|null */ protected $siteId; /** - * @param int $siteId ID of the site. + * @param int|null $siteId ID of the site, or `null` to always use current site. */ public function __construct(int $siteId) { @@ -107,6 +105,7 @@ public function set(string $key, $value): void public function unset(string $key): void { $siteId = $this->siteId; + /** @psalm-suppress PossiblyNullArgument Actually allows null site ID */ $result = delete_site_meta($siteId, $key); if ($result === false) { @@ -133,6 +132,7 @@ public function unset(string $key): void protected function getMeta(string $name) { $siteId = $this->siteId; + /** @psalm-suppress PossiblyNullArgument Actually allows null site ID */ $value = get_site_meta($siteId, $name, false); /* @@ -174,6 +174,7 @@ protected function setMeta(string $name, $value): void { $siteId = $this->siteId; + /** @psalm-suppress PossiblyNullArgument Actually allows null site ID */ $isSuccessful = update_site_meta($siteId, $name, $value); if (!$isSuccessful) { $newValue = $this->getMeta($name); From b7596df051ffa5053c0a6b5179d486121c3af595 Mon Sep 17 00:00:00 2001 From: Anton Ukhanev Date: Thu, 6 Jan 2022 11:37:15 +0100 Subject: [PATCH 5/6] Improve changelog --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad51c61..f550d02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [[*next-version*]] - YYYY-MM-DD ### Added -- `BlogOptions` now accepts a `null` blog ID, - which causes it to use the current site's options every time, regardless (#3). +- `BlogOptions` now accepts a `null` blog ID, which causes it to use the current site's options every time (#3). ### Fixed - `SiteMeta` now uses correct site meta functions (#5). From ac6683d1864f3f74e968d4c711bf6e60fa1f1cdc Mon Sep 17 00:00:00 2001 From: Anton Ukhanev Date: Thu, 6 Jan 2022 11:37:35 +0100 Subject: [PATCH 6/6] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f550d02..e33b89e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [[*next-version*]] - YYYY-MM-DD ### Added - `BlogOptions` now accepts a `null` blog ID, which causes it to use the current site's options every time (#3). +- `SiteMeta` now accepts a `null` blog ID, which causes it to use the current site's meta every time (#5). ### Fixed - `SiteMeta` now uses correct site meta functions (#5).