From a8cfed84a6c56ae647edaa74d7a326efea202912 Mon Sep 17 00:00:00 2001 From: Nate Anderson Date: Mon, 21 Oct 2024 14:52:05 -0700 Subject: [PATCH] feat: add flags for toggling inclusivity on fetchByScore scores (#238) Add $inclusiveMin and $inclusiveMax to sortedSetFetchByScore to allow callers to toggle whether the min and max scores should be inclusive or exclusive. They default to inclusive. Add tests for the flags. Change the behavior for an unrecognized sort order from defaulting to ascending to returning an error. Make $order not nullable. There isn't a reason for it to be null and it has a default argument. --- src/Cache/CacheClient.php | 42 ++++++++------- src/Cache/Internal/ScsDataClient.php | 17 +++--- src/Utilities/_DataValidation.php | 11 +++- tests/Cache/CacheClientTest.php | 81 +++++++++++++++++++++++----- 4 files changed, 113 insertions(+), 38 deletions(-) diff --git a/src/Cache/CacheClient.php b/src/Cache/CacheClient.php index 20e003f..fa1b3a8 100644 --- a/src/Cache/CacheClient.php +++ b/src/Cache/CacheClient.php @@ -1965,13 +1965,16 @@ public function sortedSetFetchByRank(string $cacheName, string $sortedSetName, ? * * @param string $cacheName Name of the cache that contains the set. * @param string $sortedSetName The sorted set to fetch. - * @param ?float $minScore The minimum score (inclusive) of the - * elements to fetch. Defaults to negative infinity. - * @param ?float $maxScore The maximum score (inclusive) of the - * elements to fetch. Defaults to positive infinity. - * @param ?int $order The order to fetch the elements in. Defaults to Ascending. - * Use SORT_ASC for ascending order, or SORT_DESC for descending order. - * Any other values will be treated as ascending + * @param ?float $minScore The minimum score of the elements to fetch. + * Defaults to negative infinity. + * @param bool $inclusiveMin Whether to include elements with a score equal to $minScore in the length. + * Defaults to true. + * @param ?float $maxScore The maximum score of the elements to fetch. + * Defaults to positive infinity. + * @param bool $inclusiveMax Whether to include elements with a score equal to $maxScore in the length. + * Defaults to true. + * @param int $order The order to fetch the elements in. Defaults to Ascending. + * Must be SORT_ASC for ascending order, or SORT_DESC for descending order. * @param ?int $offset The number of elements to skip before * returning the first element. Defaults to 0. Note: this is not the rank of * the first element to return, but the number of elements of the result set @@ -1999,9 +2002,9 @@ public function sortedSetFetchByRank(string $cacheName, string $sortedSetName, ? * we implicitly wait for completion of the request on destruction of the * response future. */ - public function sortedSetFetchByScoreAsync(string $cacheName, string $sortedSetName, ?float $minScore = null, ?float $maxScore = null, ?int $order = SORT_ASC, ?int $offset = null, ?int $count = null): ResponseFuture + public function sortedSetFetchByScoreAsync(string $cacheName, string $sortedSetName, ?float $minScore = null, bool $inclusiveMin = true, ?float $maxScore = null, bool $inclusiveMax = true, int $order = SORT_ASC, ?int $offset = null, ?int $count = null): ResponseFuture { - return $this->getNextDataClient()->sortedSetFetchByScore($cacheName, $sortedSetName, $minScore, $maxScore, $order, $offset, $count); + return $this->getNextDataClient()->sortedSetFetchByScore($cacheName, $sortedSetName, $minScore, $inclusiveMin, $maxScore, $inclusiveMax, $order, $offset, $count); } /** @@ -2009,13 +2012,16 @@ public function sortedSetFetchByScoreAsync(string $cacheName, string $sortedSetN * * @param string $cacheName Name of the cache that contains the set. * @param string $sortedSetName The sorted set to fetch. - * @param ?float $minScore The minimum score (inclusive) of the - * elements to fetch. Defaults to negative infinity. - * @param ?float $maxScore The maximum score (inclusive) of the - * elements to fetch. Defaults to positive infinity. - * @param ?int $order The order to fetch the elements in. Defaults to Ascending. - * Use SORT_ASC for ascending order, or SORT_DESC for descending order. - * Any other values will be treated as ascending + * @param ?float $minScore The minimum score of the elements to fetch. + * Defaults to negative infinity. + * @param bool $inclusiveMin Whether to include elements with a score equal to $minScore in the length. + * Defaults to true. + * @param ?float $maxScore The maximum score of the elements to fetch. + * Defaults to positive infinity. + * @param bool $inclusiveMax Whether to include elements with a score equal to $maxScore in the length. + * Defaults to true. + * @param int $order The order to fetch the elements in. Defaults to Ascending. + * Must be SORT_ASC for ascending order, or SORT_DESC for descending order. * @param ?int $offset The number of elements to skip before * returning the first element. Defaults to 0. Note: this is not the rank of * the first element to return, but the number of elements of the result set @@ -2036,9 +2042,9 @@ public function sortedSetFetchByScoreAsync(string $cacheName, string $sortedSetN * } * */ - public function sortedSetFetchByScore(string $cacheName, string $sortedSetName, ?float $minScore = null, ?float $maxScore = null, ?int $order = SORT_ASC, ?int $offset = null, ?int $count = null): SortedSetFetchResponse + public function sortedSetFetchByScore(string $cacheName, string $sortedSetName, ?float $minScore = null, bool $inclusiveMin = true, ?float $maxScore = null, bool $inclusiveMax = true, int $order = SORT_ASC, ?int $offset = null, ?int $count = null): SortedSetFetchResponse { - return $this->sortedSetFetchByScoreAsync($cacheName, $sortedSetName, $minScore, $maxScore, $order, $offset, $count)->wait(); + return $this->sortedSetFetchByScoreAsync($cacheName, $sortedSetName, $minScore, $inclusiveMin, $maxScore, $inclusiveMax, $order, $offset, $count)->wait(); } /** diff --git a/src/Cache/Internal/ScsDataClient.php b/src/Cache/Internal/ScsDataClient.php index 95e77bb..acf0c28 100644 --- a/src/Cache/Internal/ScsDataClient.php +++ b/src/Cache/Internal/ScsDataClient.php @@ -242,6 +242,7 @@ use function Momento\Utilities\validateSetName; use function Momento\Utilities\validateSortedSetElements; use function Momento\Utilities\validateSortedSetName; +use function Momento\Utilities\validateSortedSetOrder; use function Momento\Utilities\validateSortedSetRanks; use function Momento\Utilities\validateSortedSetScores; use function Momento\Utilities\validateSortedSetValues; @@ -1755,12 +1756,13 @@ function () use ($call): SortedSetIncrementScoreResponse { /** * @return ResponseFuture */ - public function sortedSetFetchByRank(string $cacheName, string $sortedSetName, ?int $startRank = 0, ?int $endRank = null, ?int $order = SORT_ASC): ResponseFuture + public function sortedSetFetchByRank(string $cacheName, string $sortedSetName, ?int $startRank = 0, ?int $endRank = null, int $order = SORT_ASC): ResponseFuture { try { validateCacheName($cacheName); validateSortedSetName($sortedSetName); validateSortedSetRanks($startRank, $endRank); + validateSortedSetOrder($order); $sortedSetFetchRequest = new _SortedSetFetchRequest(); $sortedSetFetchRequest->setSetName($sortedSetName); @@ -1817,12 +1819,13 @@ function () use ($call): SortedSetFetchResponse { /** * @return ResponseFuture */ - public function sortedSetFetchByScore(string $cacheName, string $sortedSetName, ?float $minScore = null, ?float $maxScore = null, ?int $order = SORT_ASC, ?int $offset = null, ?int $count = null): ResponseFuture + public function sortedSetFetchByScore(string $cacheName, string $sortedSetName, ?float $minScore = null, bool $inclusiveMin = true, ?float $maxScore = null, bool $inclusiveMax = true, int $order = SORT_ASC, ?int $offset = null, ?int $count = null): ResponseFuture { try { validateCacheName($cacheName); validateSortedSetName($sortedSetName); validateSortedSetScores($minScore, $maxScore); + validateSortedSetOrder($order); $sortedSetFetchRequest = new _SortedSetFetchRequest(); $sortedSetFetchRequest->setSetName($sortedSetName); @@ -1835,7 +1838,7 @@ public function sortedSetFetchByScore(string $cacheName, string $sortedSetName, } else { $byScore->setMinScore(new _SortedSetFetchRequest\_ByScore\_Score([ 'score' => $minScore, - 'exclusive' => false, + 'exclusive' => !$inclusiveMin, ])); } if (is_null($maxScore)) { @@ -1843,7 +1846,7 @@ public function sortedSetFetchByScore(string $cacheName, string $sortedSetName, } else { $byScore->setMaxScore(new _SortedSetFetchRequest\_ByScore\_Score([ 'score' => $maxScore, - 'exclusive' => false, + 'exclusive' => !$inclusiveMax, ])); } @@ -1862,10 +1865,10 @@ public function sortedSetFetchByScore(string $cacheName, string $sortedSetName, $sortedSetFetchRequest->setByScore($byScore); - if (is_null($order) || $order >= SORT_ASC) { - $sortedSetFetchRequest->setOrder(_SortedSetFetchRequest\Order::ASCENDING); - } else { + if ($order == SORT_DESC) { $sortedSetFetchRequest->setOrder(_SortedSetFetchRequest\Order::DESCENDING); + } else { + $sortedSetFetchRequest->setOrder(_SortedSetFetchRequest\Order::ASCENDING); } $call = $this->grpcManager->client->SortedSetFetch( diff --git a/src/Utilities/_DataValidation.php b/src/Utilities/_DataValidation.php index 684f221..956eb19 100644 --- a/src/Utilities/_DataValidation.php +++ b/src/Utilities/_DataValidation.php @@ -202,7 +202,7 @@ function validateSortedSetElements(array $elements): void throw new InvalidArgumentError("Sorted set score must be a float"); } - validateValueName($value); + validateNullOrEmpty($value, "Sorted set value"); } } } @@ -233,3 +233,12 @@ function validateSortedSetScores(?float $minScore, ?float $maxScore): void } } } + +if (!function_exists('validateSortedSetOrder')) { + function validateSortedSetOrder(int $order): void + { + if ($order != SORT_ASC && $order != SORT_DESC) { + throw new InvalidArgumentError("Sorted set sort order must be SORT_ASC or SORT_DESC"); + } + } +} diff --git a/tests/Cache/CacheClientTest.php b/tests/Cache/CacheClientTest.php index 408efff..4baeee4 100644 --- a/tests/Cache/CacheClientTest.php +++ b/tests/Cache/CacheClientTest.php @@ -3097,6 +3097,14 @@ public function testSortedSetFetchByRankWithNegativeStartRankLessThanEndRank_Thr $this->assertEquals(MomentoErrorCode::INVALID_ARGUMENT_ERROR, $response->asError()->errorCode()); } + public function testSortedSetFetchByRankWithBadOrder_ThrowsException() + { + $sortedSetName = uniqid(); + $response = $this->client->sortedSetFetchByRank($this->TEST_CACHE_NAME, $sortedSetName, null, null, 1234); + $this->assertNotNull($response->asError(), "Expected error but got: $response"); + $this->assertEquals(MomentoErrorCode::INVALID_ARGUMENT_ERROR, $response->asError()->errorCode()); + } + public function testSortedSetFetchByScore_HappyPath() { $sortedSetName = uniqid(); @@ -3127,7 +3135,7 @@ public function testSortedSetFetchByScore_HappyPath() $this->assertSame($expectedElements, $fetchedElements, "The fetched elements did not match the expected elements."); // full array descending - $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, null, null, SORT_DESC); + $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, null, true, null, true, SORT_DESC); $this->assertNull($response->asError(), "Error occurred while fetching sorted set '$sortedSetName'"); $this->assertNotNull($response->asHit(), "Expected a hit but got: $response"); @@ -3136,7 +3144,7 @@ public function testSortedSetFetchByScore_HappyPath() $this->assertSame($expectedElements, $fetchedElements, "The fetched elements did not match the expected elements."); - // limit by min score + // limit by min score inclusive $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, 1.1); $this->assertNull($response->asError(), "Error occurred while fetching sorted set '$sortedSetName'"); $this->assertNotNull($response->asHit(), "Expected a hit but got: $response"); @@ -3150,8 +3158,22 @@ public function testSortedSetFetchByScore_HappyPath() $this->assertSame($expectedElements, $fetchedElements, "The fetched elements did not match the expected elements."); - // limit by max score - $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, null, 3.0); + // limit by min score exclusive + $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, 1.0, false); + $this->assertNull($response->asError(), "Error occurred while fetching sorted set '$sortedSetName'"); + $this->assertNotNull($response->asHit(), "Expected a hit but got: $response"); + + $fetchedElements = $response->asHit()->valuesArray(); + $expectedElements = [ + "bar" => 2.0, + "baz" => 3.0, + "qux" => 4.0, + ]; + + $this->assertSame($expectedElements, $fetchedElements, "The fetched elements did not match the expected elements."); + + // limit by max score inclusive + $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, null, true, 3.0); $this->assertNull($response->asError(), "Error occurred while fetching sorted set '$sortedSetName'"); $this->assertNotNull($response->asHit(), "Expected a hit but got: $response"); @@ -3164,8 +3186,35 @@ public function testSortedSetFetchByScore_HappyPath() $this->assertSame($expectedElements, $fetchedElements, "The fetched elements did not match the expected elements."); - // limit by min and max score - $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, 1.1, 3.1); + // limit by max score exclusive + $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, null, true, 4.0, false); + $this->assertNull($response->asError(), "Error occurred while fetching sorted set '$sortedSetName'"); + $this->assertNotNull($response->asHit(), "Expected a hit but got: $response"); + + $fetchedElements = $response->asHit()->valuesArray(); + $expectedElements = [ + "foo" => 1.0, + "bar" => 2.0, + "baz" => 3.0, + ]; + + $this->assertSame($expectedElements, $fetchedElements, "The fetched elements did not match the expected elements."); + + // limit by min and max score inclusive + $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, 1.1, true, 3.1); + $this->assertNull($response->asError(), "Error occurred while fetching sorted set '$sortedSetName'"); + $this->assertNotNull($response->asHit(), "Expected a hit but got: $response"); + + $fetchedElements = $response->asHit()->valuesArray(); + $expectedElements = [ + "bar" => 2.0, + "baz" => 3.0, + ]; + + $this->assertSame($expectedElements, $fetchedElements, "The fetched elements did not match the expected elements."); + + // limit by min and max score exclusive + $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, 1.0, false, 4.0, false); $this->assertNull($response->asError(), "Error occurred while fetching sorted set '$sortedSetName'"); $this->assertNotNull($response->asHit(), "Expected a hit but got: $response"); @@ -3178,7 +3227,7 @@ public function testSortedSetFetchByScore_HappyPath() $this->assertSame($expectedElements, $fetchedElements, "The fetched elements did not match the expected elements."); // descending with min and max score - $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, 1.1, 3.0, SORT_DESC); + $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, 1.1, true, 3.0, true, SORT_DESC); $this->assertNull($response->asError(), "Error occurred while fetching sorted set '$sortedSetName'"); $this->assertNotNull($response->asHit(), "Expected a hit but got: $response"); @@ -3191,7 +3240,7 @@ public function testSortedSetFetchByScore_HappyPath() $this->assertSame($expectedElements, $fetchedElements, "The fetched elements did not match the expected elements."); // skip elements with offset - $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, null, null, null, 2); + $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, null, true, null, true, SORT_ASC, 2); $this->assertNull($response->asError(), "Error occurred while fetching sorted set '$sortedSetName'"); $this->assertNotNull($response->asHit(), "Expected a hit but got: $response"); @@ -3204,7 +3253,7 @@ public function testSortedSetFetchByScore_HappyPath() $this->assertSame($expectedElements, $fetchedElements, "The fetched elements did not match the expected elements."); // limit by min score and skip by offset - $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, 1.1, null, null, 2); + $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, 1.1, true, null, true, SORT_ASC, 2); $this->assertNull($response->asError(), "Error occurred while fetching sorted set '$sortedSetName'"); $this->assertNotNull($response->asHit(), "Expected a hit but got: $response"); @@ -3216,7 +3265,7 @@ public function testSortedSetFetchByScore_HappyPath() $this->assertSame($expectedElements, $fetchedElements, "The fetched elements did not match the expected elements."); // limit by count - $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, null, null, null, null, 2); + $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, null, true, null, true, SORT_ASC, null, 2); $this->assertNull($response->asError(), "Error occurred while fetching sorted set '$sortedSetName'"); $this->assertNotNull($response->asHit(), "Expected a hit but got: $response"); @@ -3229,7 +3278,7 @@ public function testSortedSetFetchByScore_HappyPath() $this->assertSame($expectedElements, $fetchedElements, "The fetched elements did not match the expected elements."); // limit by count to 0 - $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, null, null, null, null, 0); + $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, null, true, null, true, SORT_ASC, null, 0); $this->assertNull($response->asError(), "Error occurred while fetching sorted set '$sortedSetName'"); $this->assertNotNull($response->asHit(), "Expected a hit but got: $response"); @@ -3261,7 +3310,15 @@ public function testSortedSetFetchByScoreWithMinScoreLargerThanMaxScore_ThrowsEx $sortedSetName = uniqid(); $minScore = 100.0; $maxScore = 1.0; - $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, $minScore, $maxScore); + $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, $minScore, true, $maxScore); + $this->assertNotNull($response->asError(), "Expected error but got: $response"); + $this->assertEquals(MomentoErrorCode::INVALID_ARGUMENT_ERROR, $response->asError()->errorCode()); + } + + public function testSortedSetFetchByScoreWithBadOrder_ThrowsException() + { + $sortedSetName = uniqid(); + $response = $this->client->sortedSetFetchByScore($this->TEST_CACHE_NAME, $sortedSetName, null, true, null, true, 1234); $this->assertNotNull($response->asError(), "Expected error but got: $response"); $this->assertEquals(MomentoErrorCode::INVALID_ARGUMENT_ERROR, $response->asError()->errorCode()); }