momentohq / client-sdk-java

Official Java SDK for Momento Serverless Cache
Apache License 2.0
13 stars 5 forks source link

feat: storage get response 2.0 #375

Closed malandis closed 2 months ago

malandis commented 2 months ago
  1. Split GetResponse.Success into GetResponse.Found and, in the event the item isn't found, GetResponse.NotFound. This sheds the optional on value.
  2. Add a valueWhenFound shortcut accessor on the interface that returns a MomentoOptional<StorageValue>.
  3. Throw exceptions when StorageValue::get* are called on the wrong type. This sheds the Optional on those responses. Return a MomentoOptional which allows for both optional behavior and custom error messages. A careful user can still switch on StorageValue::getType for exhaustiveness checks.
  4. Add GetResponse::asFound which returns GetResponseFound in the event of a GetResponse.Found otherwise throws an exception. Will add this on demand per user feedback in order to keep the API slim.
  5. Introduce MomentoOptional for the convenience of a parameter-less orElseThrow method. The orElseThrow method will throw a ClientSdkException when the optional is empty with an informative error message. This makes using GetResponse::found slightly less long winded.
malandis commented 2 months ago

Leaving this as draft for not but open for design/ergonomics review.

cprice404 commented 2 months ago

@kvcache we spent some time discussing / playing around with ideas based on your feedback (thank you) and decided to stick with the possibility of adding the "throw-y" versions in the future. We did change the happy-path code route to expose valueWhenFound to give access to the value directly, rather than having to go through the found() chain. This sheds one level of accessor calls and puts us pretty much on par with SDKs like dynamo (though with more null safety).

We are going to go ahead and merge and release these changes so we'll at least be closer to the finish line if some users start testing this week.

If you have a better idea for that name, or other things you still feel strongly about, let us know. We would have preferred to call that just value() but the return type collides with the one from GetResponse.Found.value() so this was what we came up with.