From f98e96e5eb5041f5b0fcf00717e164e79c8b397f Mon Sep 17 00:00:00 2001 From: Michael Landis Date: Tue, 9 Jul 2024 12:30:54 -0700 Subject: [PATCH 1/2] refactor: storage tests should use a single test store and base Refactors the storage tests to use a base class similar to the one just introduced for cache. Handles store set up and tear down uniformly. --- .../momento/sdk/BaseStorageTestClass.java | 55 ++++++++++++++++ .../momento/sdk/storage/ControlTests.java | 65 ++++++------------ .../java/momento/sdk/storage/DataTests.java | 66 ++++++------------- .../sdk/PreviewStorageClientBuilder.java | 2 +- 4 files changed, 97 insertions(+), 91 deletions(-) create mode 100644 momento-sdk/src/intTest/java/momento/sdk/BaseStorageTestClass.java diff --git a/momento-sdk/src/intTest/java/momento/sdk/BaseStorageTestClass.java b/momento-sdk/src/intTest/java/momento/sdk/BaseStorageTestClass.java new file mode 100644 index 00000000..1172da5d --- /dev/null +++ b/momento-sdk/src/intTest/java/momento/sdk/BaseStorageTestClass.java @@ -0,0 +1,55 @@ +package momento.sdk; + +import java.time.Duration; +import java.util.UUID; +import momento.sdk.auth.CredentialProvider; +import momento.sdk.config.StorageConfigurations; +import momento.sdk.responses.storage.CreateStoreResponse; +import momento.sdk.responses.storage.DeleteStoreResponse; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; + +public class BaseStorageTestClass { + protected static final Duration TEN_SECONDS = Duration.ofSeconds(10); + protected static CredentialProvider credentialProvider; + protected static PreviewStorageClient storageClient; + protected static String storeName; + + @BeforeAll + static void beforeAll() { + credentialProvider = CredentialProvider.fromEnvVar("TEST_AUTH_TOKEN"); + storageClient = + new PreviewStorageClientBuilder() + .withCredentialProvider(credentialProvider) + .withConfiguration(StorageConfigurations.Laptop.latest()) + .build(); + storeName = testStoreName(); + ensureTestStoreExists(storeName); + } + + @AfterAll + static void afterAll() { + cleanupTestStore(storeName); + storageClient.close(); + } + + protected static void ensureTestStoreExists(String storeName) { + CreateStoreResponse response = storageClient.createStore(storeName).join(); + if (response instanceof CreateStoreResponse.Error) { + throw new RuntimeException( + "Failed to test create store: " + ((CreateStoreResponse.Error) response).getMessage()); + } + } + + public static void cleanupTestStore(String storeName) { + DeleteStoreResponse response = storageClient.deleteStore(storeName).join(); + if (response instanceof DeleteStoreResponse.Error) { + throw new RuntimeException( + "Failed to test delete store: " + ((DeleteStoreResponse.Error) response).getMessage()); + } + } + + public static String testStoreName() { + return "java-integration-test-default-" + UUID.randomUUID(); + } +} diff --git a/momento-sdk/src/intTest/java/momento/sdk/storage/ControlTests.java b/momento-sdk/src/intTest/java/momento/sdk/storage/ControlTests.java index 1cc31383..09e8cf26 100644 --- a/momento-sdk/src/intTest/java/momento/sdk/storage/ControlTests.java +++ b/momento-sdk/src/intTest/java/momento/sdk/storage/ControlTests.java @@ -3,8 +3,7 @@ import static momento.sdk.TestUtils.randomString; import static org.assertj.core.api.Assertions.assertThat; -import java.time.Duration; -import momento.sdk.BaseTestClass; +import momento.sdk.BaseStorageTestClass; import momento.sdk.PreviewStorageClient; import momento.sdk.auth.CredentialProvider; import momento.sdk.config.StorageConfigurations; @@ -16,45 +15,23 @@ import momento.sdk.responses.storage.DeleteStoreResponse; import momento.sdk.responses.storage.ListStoresResponse; import org.assertj.core.api.InstanceOfAssertFactories; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -public class ControlTests extends BaseTestClass { - private static PreviewStorageClient client; - - public static final Duration TEN_SECONDS = Duration.ofSeconds(10); - - @BeforeAll - static void setup() { - client = - new PreviewStorageClient( - CredentialProvider.fromEnvVar("MOMENTO_API_KEY"), - StorageConfigurations.Laptop.latest()); - - // TODO re-using this name - client.createStore(System.getenv("TEST_CACHE_NAME")).join(); - } - - @AfterAll - static void tearDown() { - client.close(); - } - +public class ControlTests extends BaseStorageTestClass { @Test public void returnsAlreadyExistsWhenCreatingExistingStore() { // TODO externalize this // TODO rename env var to something broader like TEST_RESOURCE_NAME final String existingStore = System.getenv("TEST_CACHE_NAME"); - assertThat(client.createStore(existingStore)) + assertThat(storageClient.createStore(existingStore)) .succeedsWithin(TEN_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(CreateStoreResponse.AlreadyExists.class)); } @Test public void returnsNotFoundWhenDeletingUnknownStore() { - assertThat(client.deleteStore(randomString("name"))) + assertThat(storageClient.deleteStore(randomString("name"))) .succeedsWithin(TEN_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(DeleteStoreResponse.Error.class)) .satisfies(error -> assertThat(error).hasCauseInstanceOf(StoreNotFoundException.class)); @@ -64,12 +41,12 @@ public void returnsNotFoundWhenDeletingUnknownStore() { public void listsStoresHappyPath() { final String storeName = randomString("name"); - assertThat(client.createStore(storeName)) + assertThat(storageClient.createStore(storeName)) .succeedsWithin(TEN_SECONDS) .isInstanceOf(CreateStoreResponse.Success.class); try { - assertThat(client.listStores()) + assertThat(storageClient.listStores()) .succeedsWithin(TEN_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(ListStoresResponse.Success.class)) .satisfies( @@ -77,11 +54,11 @@ public void listsStoresHappyPath() { assertThat(success.getStores()) .anyMatch(storeInfo -> storeInfo.getName().equals(storeName))); - final ListStoresResponse response = client.listStores().join(); + final ListStoresResponse response = storageClient.listStores().join(); assertThat(response).isInstanceOf(ListStoresResponse.Success.class); } finally { // cleanup - assertThat(client.deleteStore(storeName)) + assertThat(storageClient.deleteStore(storeName)) .succeedsWithin(TEN_SECONDS) .isInstanceOf(DeleteStoreResponse.Success.class); } @@ -89,7 +66,7 @@ public void listsStoresHappyPath() { @Test public void returnsBadRequestForEmptyStoreName() { - assertThat(client.createStore(" ")) + assertThat(storageClient.createStore(" ")) .succeedsWithin(TEN_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(CreateStoreResponse.Error.class)) .satisfies(error -> assertThat(error).hasCauseInstanceOf(BadRequestException.class)); @@ -97,12 +74,12 @@ public void returnsBadRequestForEmptyStoreName() { @Test public void throwsValidationExceptionForNullStoreName() { - assertThat(client.createStore(null)) + assertThat(storageClient.createStore(null)) .succeedsWithin(TEN_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(CreateStoreResponse.Error.class)) .satisfies(error -> assertThat(error).hasCauseInstanceOf(InvalidArgumentException.class)); - assertThat(client.deleteStore(null)) + assertThat(storageClient.deleteStore(null)) .succeedsWithin(TEN_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(DeleteStoreResponse.Error.class)) .satisfies(error -> assertThat(error).hasCauseInstanceOf(InvalidArgumentException.class)); @@ -113,25 +90,25 @@ public void deleteSucceeds() { final String storeName = randomString("name"); try { - assertThat(client.createStore(storeName)) + assertThat(storageClient.createStore(storeName)) .succeedsWithin(TEN_SECONDS) .isInstanceOf(CreateStoreResponse.Success.class); - assertThat(client.createStore(storeName)) + assertThat(storageClient.createStore(storeName)) .succeedsWithin(TEN_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(CreateStoreResponse.AlreadyExists.class)); - assertThat(client.deleteStore(storeName)) + assertThat(storageClient.deleteStore(storeName)) .succeedsWithin(TEN_SECONDS) .isInstanceOf(DeleteStoreResponse.Success.class); - assertThat(client.deleteStore(storeName)) + assertThat(storageClient.deleteStore(storeName)) .succeedsWithin(TEN_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(DeleteStoreResponse.Error.class)) .satisfies(error -> assertThat(error).hasCauseInstanceOf(StoreNotFoundException.class)); } finally { // Just in case the second create or delete fails - client.deleteStore(storeName).join(); + storageClient.deleteStore(storeName).join(); } } @@ -144,23 +121,23 @@ public void returnsErrorForBadToken() { + "s76573jnajhjjjhjdhnndy"; final CredentialProvider badTokenProvider = CredentialProvider.fromString(badToken); - try (final PreviewStorageClient client = + try (final PreviewStorageClient storageClient = new PreviewStorageClient( CredentialProvider.fromString(badToken), - StorageConfigurations.Laptop.latest()) /*CacheClient.builder( + StorageConfigurations.Laptop.latest()) /*CacheStorageClient.builder( badTokenProvider, Configurations.Laptop.latest(), Duration.ofSeconds(10)) .build()*/) { - assertThat(client.createStore(storeName)) + assertThat(storageClient.createStore(storeName)) .succeedsWithin(TEN_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(CreateStoreResponse.Error.class)) .satisfies(error -> assertThat(error).hasCauseInstanceOf(AuthenticationException.class)); - assertThat(client.deleteStore(storeName)) + assertThat(storageClient.deleteStore(storeName)) .succeedsWithin(TEN_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(DeleteStoreResponse.Error.class)) .satisfies(error -> assertThat(error).hasCauseInstanceOf(AuthenticationException.class)); - assertThat(client.listStores()) + assertThat(storageClient.listStores()) .succeedsWithin(TEN_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(ListStoresResponse.Error.class)) .satisfies(error -> assertThat(error).hasCauseInstanceOf(AuthenticationException.class)); diff --git a/momento-sdk/src/intTest/java/momento/sdk/storage/DataTests.java b/momento-sdk/src/intTest/java/momento/sdk/storage/DataTests.java index 3db2e4cc..289cb66f 100644 --- a/momento-sdk/src/intTest/java/momento/sdk/storage/DataTests.java +++ b/momento-sdk/src/intTest/java/momento/sdk/storage/DataTests.java @@ -4,52 +4,26 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; -import momento.sdk.BaseTestClass; -import momento.sdk.PreviewStorageClient; -import momento.sdk.auth.CredentialProvider; -import momento.sdk.config.StorageConfigurations; +import momento.sdk.BaseStorageTestClass; import momento.sdk.exceptions.ClientSdkException; import momento.sdk.exceptions.StoreNotFoundException; import momento.sdk.responses.storage.DeleteResponse; import momento.sdk.responses.storage.GetResponse; import momento.sdk.responses.storage.PutResponse; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -public class DataTests extends BaseTestClass { - private static PreviewStorageClient client; - - // TODO can set to the same value as the cache tests - // TODO rename env var for clarity to TEST_RESOURCE_NAME or similar - private final String storeName = System.getenv("TEST_CACHE_NAME"); - - @BeforeAll - static void setup() { - client = - new PreviewStorageClient( - CredentialProvider.fromEnvVar("MOMENTO_API_KEY"), - StorageConfigurations.Laptop.latest()); - - client.createStore(System.getenv("TEST_CACHE_NAME")).join(); - } - - @AfterAll - static void tearDown() { - client.close(); - } - +public class DataTests extends BaseStorageTestClass { @Test void getReturnsValueAsStringAfterPut() { final String key = randomString("key"); final String value = randomString("value"); // Successful Set - final PutResponse putResponse = client.put(storeName, key, value).join(); + final PutResponse putResponse = storageClient.put(storeName, key, value).join(); assertThat(putResponse).isInstanceOf(PutResponse.Success.class); // Successful Get - final GetResponse getResponse = client.get(storeName, key).join(); + final GetResponse getResponse = storageClient.get(storeName, key).join(); assertThat(getResponse).isInstanceOf(GetResponse.Found.class); assertThat(getResponse.valueWhenFound().get().getString().get()).isEqualTo(value); } @@ -60,11 +34,11 @@ void getReturnsValueAsByteArrayAfterPut() { final String value = randomString("value"); // Successful Set - final PutResponse putResponse = client.put(storeName, key, value.getBytes()).join(); + final PutResponse putResponse = storageClient.put(storeName, key, value.getBytes()).join(); assertThat(putResponse).isInstanceOf(PutResponse.Success.class); // Successful Get - final GetResponse getResponse = client.get(storeName, key).join(); + final GetResponse getResponse = storageClient.get(storeName, key).join(); assertThat(getResponse).isInstanceOf(GetResponse.Found.class); assertThat(getResponse.valueWhenFound().get().getByteArray().get()).isEqualTo(value.getBytes()); } @@ -75,11 +49,11 @@ void getReturnsValueAsLongAfterPut() { final long value = 42L; // Successful Set - final PutResponse putResponse = client.put(storeName, key, value).join(); + final PutResponse putResponse = storageClient.put(storeName, key, value).join(); assertThat(putResponse).isInstanceOf(PutResponse.Success.class); // Successful Get - final GetResponse getResponse = client.get(storeName, key).join(); + final GetResponse getResponse = storageClient.get(storeName, key).join(); assertThat(getResponse).isInstanceOf(GetResponse.Found.class); assertThat(getResponse.valueWhenFound().get().getLong().get()).isEqualTo(value); } @@ -90,11 +64,11 @@ void getReturnsValueAsDoubleAfterPut() { final double value = 3.14; // Successful Set - final PutResponse putResponse = client.put(storeName, key, value).join(); + final PutResponse putResponse = storageClient.put(storeName, key, value).join(); assertThat(putResponse).isInstanceOf(PutResponse.Success.class); // Successful Get - final GetResponse getResponse = client.get(storeName, key).join(); + final GetResponse getResponse = storageClient.get(storeName, key).join(); assertThat(getResponse).isInstanceOf(GetResponse.Found.class); assertThat(getResponse.valueWhenFound().get().getDouble().get()).isEqualTo(value); } @@ -102,7 +76,7 @@ void getReturnsValueAsDoubleAfterPut() { @Test void storeKeyNotFound() { // Get key that was not set - final GetResponse response = client.get(storeName, randomString("key")).join(); + final GetResponse response = storageClient.get(storeName, randomString("key")).join(); assertThat(response).isInstanceOf(GetResponse.NotFound.class); assert response.valueWhenFound().isEmpty(); assert response instanceof GetResponse.NotFound; @@ -113,11 +87,11 @@ void storeKeyNotFound() { public void badStoreNameReturnsError() { final String storeName = randomString("name"); - final GetResponse getResponse = client.get(storeName, "").join(); + final GetResponse getResponse = storageClient.get(storeName, "").join(); assertThat(getResponse).isInstanceOf(GetResponse.Error.class); assertThat(((GetResponse.Error) getResponse)).hasCauseInstanceOf(StoreNotFoundException.class); - final PutResponse putResponse = client.put(storeName, "", "").join(); + final PutResponse putResponse = storageClient.put(storeName, "", "").join(); assertThat(putResponse).isInstanceOf(PutResponse.Error.class); assertThat(((PutResponse.Error) putResponse)).hasCauseInstanceOf(StoreNotFoundException.class); } @@ -126,8 +100,8 @@ public void badStoreNameReturnsError() { public void allowEmptyKeyValuesOnGet() throws Exception { final String emptyKey = ""; final String emptyValue = ""; - client.put(storeName, emptyKey, emptyValue).get(); - final GetResponse response = client.get(storeName, emptyKey).get(); + storageClient.put(storeName, emptyKey, emptyValue).get(); + final GetResponse response = storageClient.get(storeName, emptyKey).get(); assertThat(response).isInstanceOf(GetResponse.Found.class); assert response.valueWhenFound().get().getString().get().isEmpty(); } @@ -137,15 +111,15 @@ public void deleteHappyPath() throws Exception { final String key = "key"; final String value = "value"; - client.put(storeName, key, value).get(); - final GetResponse getResponse = client.get(storeName, key).get(); + storageClient.put(storeName, key, value).get(); + final GetResponse getResponse = storageClient.get(storeName, key).get(); assertThat(getResponse).isInstanceOf(GetResponse.Found.class); assertThat(getResponse.valueWhenFound().get().getString().get()).isEqualTo(value); - final DeleteResponse deleteResponse = client.delete(storeName, key).get(); + final DeleteResponse deleteResponse = storageClient.delete(storeName, key).get(); assertThat(deleteResponse).isInstanceOf(DeleteResponse.Success.class); - final GetResponse getAfterDeleteResponse = client.get(storeName, key).get(); + final GetResponse getAfterDeleteResponse = storageClient.get(storeName, key).get(); assert getAfterDeleteResponse.valueWhenFound().isEmpty(); assert getAfterDeleteResponse instanceof GetResponse.NotFound; } @@ -154,7 +128,7 @@ public void deleteHappyPath() throws Exception { public void deleteNonExistentKey() throws Exception { final String key = randomString("key"); - final DeleteResponse deleteResponse = client.delete(storeName, key).get(); + final DeleteResponse deleteResponse = storageClient.delete(storeName, key).get(); assertThat(deleteResponse).isInstanceOf(DeleteResponse.Success.class); } } diff --git a/momento-sdk/src/main/java/momento/sdk/PreviewStorageClientBuilder.java b/momento-sdk/src/main/java/momento/sdk/PreviewStorageClientBuilder.java index cf61dfba..f5b601d6 100644 --- a/momento-sdk/src/main/java/momento/sdk/PreviewStorageClientBuilder.java +++ b/momento-sdk/src/main/java/momento/sdk/PreviewStorageClientBuilder.java @@ -49,7 +49,7 @@ public PreviewStorageClientBuilder withConfiguration( * * @return the client. */ - public IPreviewStorageClient build() { + public PreviewStorageClient build() { if (credentialProvider == null) { credentialProvider = new EnvVarCredentialProvider("MOMENTO_API_KEY"); } From 234da3b2751f5a43b10e90b1053474106b78ca68 Mon Sep 17 00:00:00 2001 From: Michael Landis Date: Tue, 9 Jul 2024 12:35:44 -0700 Subject: [PATCH 2/2] fix: existing store test --- .../src/intTest/java/momento/sdk/storage/ControlTests.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/momento-sdk/src/intTest/java/momento/sdk/storage/ControlTests.java b/momento-sdk/src/intTest/java/momento/sdk/storage/ControlTests.java index 09e8cf26..abdf134c 100644 --- a/momento-sdk/src/intTest/java/momento/sdk/storage/ControlTests.java +++ b/momento-sdk/src/intTest/java/momento/sdk/storage/ControlTests.java @@ -20,9 +20,7 @@ public class ControlTests extends BaseStorageTestClass { @Test public void returnsAlreadyExistsWhenCreatingExistingStore() { - // TODO externalize this - // TODO rename env var to something broader like TEST_RESOURCE_NAME - final String existingStore = System.getenv("TEST_CACHE_NAME"); + final String existingStore = storeName; assertThat(storageClient.createStore(existingStore)) .succeedsWithin(TEN_SECONDS)