-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add missing v1 ops #99
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
5248d63
feat: add data operations for v1
pgautier404 219b59b
fix: remove unneeded imports
pgautier404 65e38f9
chore: add InRegion configuration
pgautier404 ce2861d
chore: add keyExists/keysExist
pgautier404 91ebf47
fix: add validation for keys
pgautier404 1a803f9
add dictionary accessor to keysExist success response
pgautier404 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,25 +5,22 @@ | |
|
||
use Cache_client\_DeleteRequest; | ||
use Cache_client\_DictionaryDeleteRequest; | ||
use Cache_client\_DictionaryDeleteRequest\All; | ||
use Cache_client\_DictionaryFetchRequest; | ||
use Cache_client\_DictionaryFieldValuePair; | ||
use Cache_client\_DictionaryGetRequest; | ||
use Cache_client\_DictionaryIncrementRequest; | ||
use Cache_client\_DictionarySetRequest; | ||
use Cache_client\_GetRequest; | ||
use Cache_client\_ListEraseRequest; | ||
use Cache_client\_KeysExistRequest; | ||
use Cache_client\_ListFetchRequest; | ||
use Cache_client\_ListLengthRequest; | ||
use Cache_client\_ListPopBackRequest; | ||
use Cache_client\_ListPopFrontRequest; | ||
use Cache_client\_ListPushBackRequest; | ||
use Cache_client\_ListPushFrontRequest; | ||
use Cache_client\_ListRange; | ||
use Cache_client\_ListRemoveRequest; | ||
use Cache_client\_SetDifferenceRequest; | ||
use Cache_client\_SetDifferenceRequest\_Subtrahend; | ||
use Cache_client\_SetDifferenceRequest\_Subtrahend\_Identity; | ||
use Cache_client\_SetDifferenceRequest\_Subtrahend\_Set; | ||
use Cache_client\_SetFetchRequest; | ||
use Cache_client\_SetIfNotExistsRequest; | ||
|
@@ -36,9 +33,6 @@ | |
use Momento\Cache\CacheOperationTypes\CacheDeleteResponse; | ||
use Momento\Cache\CacheOperationTypes\CacheDeleteResponseError; | ||
use Momento\Cache\CacheOperationTypes\CacheDeleteResponseSuccess; | ||
use Momento\Cache\CacheOperationTypes\CacheDictionaryDeleteResponse; | ||
use Momento\Cache\CacheOperationTypes\CacheDictionaryDeleteResponseError; | ||
use Momento\Cache\CacheOperationTypes\CacheDictionaryDeleteResponseSuccess; | ||
use Momento\Cache\CacheOperationTypes\CacheDictionaryFetchResponse; | ||
use Momento\Cache\CacheOperationTypes\CacheDictionaryFetchResponseError; | ||
use Momento\Cache\CacheOperationTypes\CacheDictionaryFetchResponseHit; | ||
|
@@ -70,9 +64,12 @@ | |
use Momento\Cache\CacheOperationTypes\CacheGetResponseError; | ||
use Momento\Cache\CacheOperationTypes\CacheGetResponseHit; | ||
use Momento\Cache\CacheOperationTypes\CacheGetResponseMiss; | ||
use Momento\Cache\CacheOperationTypes\CacheListEraseResponse; | ||
use Momento\Cache\CacheOperationTypes\CacheListEraseResponseError; | ||
use Momento\Cache\CacheOperationTypes\CacheListEraseResponseSuccess; | ||
use Momento\Cache\CacheOperationTypes\CacheKeyExistsResponse; | ||
use Momento\Cache\CacheOperationTypes\CacheKeyExistsResponseError; | ||
use Momento\Cache\CacheOperationTypes\CacheKeyExistsResponseSuccess; | ||
use Momento\Cache\CacheOperationTypes\CacheKeysExistResponse; | ||
use Momento\Cache\CacheOperationTypes\CacheKeysExistResponseError; | ||
use Momento\Cache\CacheOperationTypes\CacheKeysExistResponseSuccess; | ||
use Momento\Cache\CacheOperationTypes\CacheListFetchResponse; | ||
use Momento\Cache\CacheOperationTypes\CacheListFetchResponseError; | ||
use Momento\Cache\CacheOperationTypes\CacheListFetchResponseHit; | ||
|
@@ -100,9 +97,6 @@ | |
use Momento\Cache\CacheOperationTypes\CacheSetAddElementResponse; | ||
use Momento\Cache\CacheOperationTypes\CacheSetAddElementResponseError; | ||
use Momento\Cache\CacheOperationTypes\CacheSetAddElementResponseSuccess; | ||
use Momento\Cache\CacheOperationTypes\CacheSetDeleteResponse; | ||
use Momento\Cache\CacheOperationTypes\CacheSetDeleteResponseError; | ||
use Momento\Cache\CacheOperationTypes\CacheSetDeleteResponseSuccess; | ||
use Momento\Cache\CacheOperationTypes\CacheSetFetchResponse; | ||
use Momento\Cache\CacheOperationTypes\CacheSetFetchResponseError; | ||
use Momento\Cache\CacheOperationTypes\CacheSetFetchResponseHit; | ||
|
@@ -122,7 +116,6 @@ | |
use Momento\Cache\Errors\UnknownError; | ||
use Momento\Config\IConfiguration; | ||
use Momento\Requests\CollectionTtl; | ||
use Momento\Requests\CollectionTtlFactory; | ||
use Momento\Utilities\_ErrorConverter; | ||
use Psr\Log\LoggerAwareInterface; | ||
use Psr\Log\LoggerInterface; | ||
|
@@ -133,9 +126,9 @@ | |
use function Momento\Utilities\validateFields; | ||
use function Momento\Utilities\validateFieldsKeys; | ||
use function Momento\Utilities\validateItems; | ||
use function Momento\Utilities\validateKeys; | ||
use function Momento\Utilities\validateListName; | ||
use function Momento\Utilities\validateOperationTimeout; | ||
use function Momento\Utilities\validateRange; | ||
use function Momento\Utilities\validateSetName; | ||
use function Momento\Utilities\validateTruncateSize; | ||
use function Momento\Utilities\validateTtl; | ||
|
@@ -288,6 +281,34 @@ public function delete(string $cacheName, string $key): CacheDeleteResponse | |
return new CacheDeleteResponseSuccess(); | ||
} | ||
|
||
public function keysExist(string $cacheName, array $keys) : CacheKeysExistResponse | ||
{ | ||
try { | ||
validateCacheName($cacheName); | ||
validateKeys($keys); | ||
$keysExistRequest = new _KeysExistRequest(); | ||
$keysExistRequest->setCacheKeys($keys); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to validate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great callout, I'll add that, thanks! |
||
$call = $this->grpcManager->client->KeysExist( | ||
$keysExistRequest, ["cache"=> [$cacheName]], ["timeout" => $this->timeout] | ||
); | ||
$response = $this->processCall($call); | ||
} catch (SdkError $e) { | ||
return new CacheKeysExistResponseError($e); | ||
} catch (Exception $e) { | ||
return new CacheKeysExistResponseError(new UnknownError($e->getMessage())); | ||
} | ||
return new CacheKeysExistResponseSuccess($response, $keys); | ||
} | ||
|
||
public function keyExists(string $cacheName, string $key) : CacheKeyExistsResponse | ||
{ | ||
$response = $this->keysExist($cacheName, [$key]); | ||
if ($response instanceof CacheKeysExistResponseError) { | ||
return new CacheKeyExistsResponseError($response->innerException()); | ||
} | ||
return new CacheKeyExistsResponseSuccess($response->asSuccess()->exists()[0]); | ||
} | ||
|
||
public function listFetch(string $cacheName, string $listName): CacheListFetchResponse | ||
{ | ||
try { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
<?php | ||
declare(strict_types=1); | ||
|
||
namespace Momento\Config\Configurations; | ||
|
||
use Momento\Config\Configuration; | ||
use Momento\Config\Transport\StaticGrpcConfiguration; | ||
use Momento\Config\Transport\StaticTransportStrategy; | ||
use Momento\Logging\ILoggerFactory; | ||
use Momento\Logging\NullLoggerFactory; | ||
|
||
class InRegion extends Configuration | ||
{ | ||
|
||
public static function latest(?ILoggerFactory $loggerFactory = null): Laptop | ||
{ | ||
$loggerFactory = $loggerFactory ?? new NullLoggerFactory(); | ||
$grpcConfig = new StaticGrpcConfiguration(1100); | ||
$transportStrategy = new StaticTransportStrategy(null, $grpcConfig, $loggerFactory); | ||
return new Laptop($loggerFactory, $transportStrategy); | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, up to you: i am imagining that for most of the SDKs we will want to provide a dictionary accessor for this. If we do it as an array like this, it's useful if the caller wants to do a
.every
or.all
check, but otherwise they have to loop over this and their original fields list in parallel to check values and it's kinda janky.I feel like as an end user it's a much nicer experience if I can get back a dictionary with the fields as keys.
we can always add this later, so definitely not a blocker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with that, but I was going off the SDK API spec and it explicitly calls for a list of booleans. For that reason, the golang SDK is returning []bool for this too. I totally agree that a dictionary is a better UX choice. I'm happy to change this up to return a dictionary (which because this is PHP will still be called and typed "an array") but we should think about adding
existsMap
or similar to the gloang SDK as well. And we should obviously correct the spec.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need the list of booleans. but i think adding a map too is nice. you can see how we did it in the .NET SDK if you like. And yes if you wouldn't mind updating the spec that would be great, thank you!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and adding it to go at some point would be great too but it's an additive change so we can do it whenever IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a "dictionary" accessor in 1a803f9 and updated the spec to reference
existsDictionaryString
andexistsDictionaryBytes
accessors. @cprice404 I opted to omit a "Bool" suffix on those accessors but feel free to change that if you prefer the more verbose version. Because PHP doesn't differentiate, I just added the accessor in this SDK asexistsDictionary
.