fix: storage SDK orElseThrow should take a supplier (#383)

The `orElseThrow` signature of Java Optionals requires a single
arg, `supplier`, that is a lambda function returning the exception
that the user wishes to have thrown if the optional is empty.
In our current storage client code we have an empty-args signature,
which would be surprising to Java users familiar with Optionals.
This commit updates the signature to accept a supplier.

Makefile

@@ -1,3 +1,6 @@
+.PHONY: all
+all: precommit
+
 .PHONY: clean
 ## Clean the project
 clean:
@@ -34,7 +37,7 @@ lint:
 
 .PHONY: precommit
 ## Run the precommit checks
-precommit: format lint test
+precommit: format lint build test
 
 .PHONY: help
 # See <https://gist.github.com/klmr/575726c7e05d8780505a> for explanation.

momento-sdk/src/intTest/java/momento/sdk/storage/DataTests.java

@@ -80,7 +80,12 @@ void storeKeyNotFound() {
     assertThat(response).isInstanceOf(GetResponse.NotFound.class);
     assert response.valueWhenFound().isEmpty();
     assert response instanceof GetResponse.NotFound;
-    assertThrows(ClientSdkException.class, response.valueWhenFound()::orElseThrow);
+    assertThrows(ClientSdkException.class, response.valueWhenFound()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> {
+          response.valueWhenFound().orElseThrow(() -> new RuntimeException("derp"));
+        });
   }
 
   @Test

momento-sdk/src/main/java/momento/sdk/responses/storage/StorageValue.java

@@ -1,5 +1,6 @@
 package momento.sdk.responses.storage;
 
+import java.util.function.Supplier;
 import momento.sdk.utils.MomentoOptional;
 
 /**
@@ -55,8 +56,8 @@ public StorageItemType getType() {
    * Get the value as a byte array.
    *
    * @return the value as an optional byte array. If the value is not a byte array, an empty
-   *     optional will be returned. Call {@link MomentoOptional#orElseThrow()} to short circuit the
-   *     operation and throw an exception.
+   *     optional will be returned. Call {@link MomentoOptional#orElseThrow(Supplier)} to short
+   *     circuit the operation and throw an exception.
    */
   public MomentoOptional<byte[]> getByteArray() {
     if (itemType != StorageItemType.BYTE_ARRAY) {
@@ -69,8 +70,8 @@ public MomentoOptional<byte[]> getByteArray() {
    * Get the value as a string.
    *
    * @return the value as an optional string. If the value is not a string, an empty optional will
-   *     be returned. Call {@link MomentoOptional#orElseThrow()} to short circuit the operation and
-   *     throw an exception.
+   *     be returned. Call {@link MomentoOptional#orElseThrow(Supplier)} to short circuit the
+   *     operation and throw an exception.
    */
   public MomentoOptional<String> getString() {
     if (itemType != StorageItemType.STRING) {
@@ -83,8 +84,8 @@ public MomentoOptional<String> getString() {
    * Get the value as a long.
    *
    * @return the value as an optional long. If the value is not a long, an empty optional will be
-   *     returned. Call {@link MomentoOptional#orElseThrow()} to short circuit the operation and
-   *     throw an exception.
+   *     returned. Call {@link MomentoOptional#orElseThrow(Supplier)} to short circuit the operation
+   *     and throw an exception.
    */
   public MomentoOptional<Long> getLong() {
     if (itemType != StorageItemType.LONG) {
@@ -98,8 +99,8 @@ public MomentoOptional<Long> getLong() {
    * Get the value as a double.
    *
    * @return the value as an optional double. If the value is not a double, an empty optional will
-   *     be returned. Call {@link MomentoOptional#orElseThrow()} to short circuit the operation and
-   *     throw an exception.
+   *     be returned. Call {@link MomentoOptional#orElseThrow(Supplier)} to short circuit the
+   *     operation and throw an exception.
    */
   public MomentoOptional<Double> getDouble() {
     if (itemType != StorageItemType.DOUBLE) {

momento-sdk/src/main/java/momento/sdk/utils/MomentoOptional.java

@@ -1,6 +1,7 @@
 package momento.sdk.utils;
 
 import java.util.Optional;
+import java.util.function.Supplier;
 import momento.sdk.exceptions.ClientSdkException;
 
 /**
@@ -64,16 +65,20 @@ public static <T> MomentoOptional<T> empty(String onEmptyExceptionMessage) {
    * @return the value.
    */
   public T get() {
-    return this.orElseThrow();
+    return optional.orElseThrow(() -> new ClientSdkException(onEmptyExceptionMessage));
   }
 
   /**
    * Gets the value of the optional or throws an exception if the optional is empty.
    *
    * @return the value.
    */
-  public T orElseThrow() {
-    return optional.orElseThrow(() -> new ClientSdkException(onEmptyExceptionMessage));
+  public <X extends Throwable> T orElseThrow(Supplier<? extends X> exceptionSupplier) throws X {
+    if (this.isEmpty()) {
+      throw exceptionSupplier.get();
+    } else {
+      return this.get();
+    }
   }
 
   /**

momento-sdk/src/test/java/momento/sdk/responses/storage/GetResponseTest.java

@@ -12,29 +12,65 @@
 public class GetResponseTest {
   @Test
   public void testGetResponseFoundWorksOnTheRightType() {
-    GetResponse.Found response = GetResponse.Found.of(new byte[] {0, 1, 2, 3});
+    final GetResponse.Found response = GetResponse.Found.of(new byte[] {0, 1, 2, 3});
     assert response.value().getByteArray().get().length == 4;
-    assertThrows(ClientSdkException.class, response.value().getString()::orElseThrow);
-    assertThrows(ClientSdkException.class, response.value().getLong()::orElseThrow);
-    assertThrows(ClientSdkException.class, response.value().getDouble()::orElseThrow);
+    assertThrows(ClientSdkException.class, response.value().getString()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> response.value().getString().orElseThrow(() -> new RuntimeException("derp")));
+    assertThrows(ClientSdkException.class, response.value().getLong()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> response.value().getLong().orElseThrow(() -> new RuntimeException("derp")));
+    assertThrows(ClientSdkException.class, response.value().getDouble()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> response.value().getDouble().orElseThrow(() -> new RuntimeException("derp")));
 
-    response = GetResponse.Found.of("string");
-    assertThrows(ClientSdkException.class, response.value().getByteArray()::orElseThrow);
-    assert response.value().getString().get().equals("string");
-    assertThrows(ClientSdkException.class, response.value().getLong()::orElseThrow);
-    assertThrows(ClientSdkException.class, response.value().getDouble()::orElseThrow);
+    final GetResponse.Found response2 = GetResponse.Found.of("string");
+    assertThrows(ClientSdkException.class, response2.value().getByteArray()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> response2.value().getByteArray().orElseThrow(() -> new RuntimeException("derp")));
+    assert response2.value().getString().get().equals("string");
+    assertThrows(ClientSdkException.class, response2.value().getLong()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> response2.value().getLong().orElseThrow(() -> new RuntimeException("derp")));
+    assertThrows(ClientSdkException.class, response2.value().getDouble()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> response2.value().getDouble().orElseThrow(() -> new RuntimeException("derp")));
 
-    response = GetResponse.Found.of(42L);
-    assertThrows(ClientSdkException.class, response.value().getByteArray()::orElseThrow);
-    assertThrows(ClientSdkException.class, response.value().getString()::orElseThrow);
-    assert response.value().getLong().get() == 42L;
-    assertThrows(ClientSdkException.class, response.value().getDouble()::orElseThrow);
+    final GetResponse.Found response3 = GetResponse.Found.of(42L);
+    assertThrows(ClientSdkException.class, response3.value().getByteArray()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> response3.value().getByteArray().orElseThrow(() -> new RuntimeException("derp")));
+    assertThrows(ClientSdkException.class, response3.value().getString()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> response3.value().getString().orElseThrow(() -> new RuntimeException("derp")));
+    assert response3.value().getLong().get() == 42L;
+    assertThrows(ClientSdkException.class, response3.value().getDouble()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> response3.value().getDouble().orElseThrow(() -> new RuntimeException("derp")));
 
-    response = GetResponse.Found.of(3.14);
-    assertThrows(ClientSdkException.class, response.value().getByteArray()::orElseThrow);
-    assertThrows(ClientSdkException.class, response.value().getString()::orElseThrow);
-    assertThrows(ClientSdkException.class, response.value().getLong()::orElseThrow);
-    assert response.value().getDouble().get() == 3.14;
+    final GetResponse.Found response4 = GetResponse.Found.of(3.14);
+    assertThrows(ClientSdkException.class, response4.value().getByteArray()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> response4.value().getByteArray().orElseThrow(() -> new RuntimeException("derp")));
+    assertThrows(ClientSdkException.class, response4.value().getString()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> response4.value().getString().orElseThrow(() -> new RuntimeException("derp")));
+    assertThrows(ClientSdkException.class, response4.value().getLong()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> response4.value().getLong().orElseThrow(() -> new RuntimeException("derp")));
+    assert response4.value().getDouble().get() == 3.14;
   }
 
   @Test
@@ -62,6 +98,9 @@ public void testConvenienceMethodsOnGetResponse() {
                 new MomentoTransportErrorDetails(
                     new MomentoGrpcErrorDetails(Status.Code.NOT_FOUND, "not found"))));
     assert error.valueWhenFound().isEmpty();
-    assertThrows(ClientSdkException.class, error.valueWhenFound()::orElseThrow);
+    assertThrows(ClientSdkException.class, error.valueWhenFound()::get);
+    assertThrows(
+        RuntimeException.class,
+        () -> error.valueWhenFound().orElseThrow(() -> new RuntimeException("derp")));
   }
 }