From 4fc40d1490b6a8093594ad254582c0767a7bd646 Mon Sep 17 00:00:00 2001 From: Michael Landis Date: Thu, 20 Jun 2024 13:52:02 -0700 Subject: [PATCH 1/5] refactor: NotFoundException -> CacheNotFoundException --- .../java/momento/sdk/CacheClientTest.java | 10 ++--- .../momento/sdk/CacheControlPlaneTest.java | 6 +-- .../java/momento/sdk/CacheDataPlaneTest.java | 6 +-- .../java/momento/sdk/SortedSetTest.java | 38 +++++++++---------- .../java/momento/sdk/storage/DataTests.java | 6 +-- ...ption.java => CacheNotFoundException.java} | 5 ++- .../CacheServiceExceptionMapper.java | 2 +- .../responses/storage/GetResponseTest.java | 4 +- 8 files changed, 39 insertions(+), 38 deletions(-) rename momento-sdk/src/main/java/momento/sdk/exceptions/{NotFoundException.java => CacheNotFoundException.java} (84%) diff --git a/momento-sdk/src/intTest/java/momento/sdk/CacheClientTest.java b/momento-sdk/src/intTest/java/momento/sdk/CacheClientTest.java index f981cf68..0787c3af 100644 --- a/momento-sdk/src/intTest/java/momento/sdk/CacheClientTest.java +++ b/momento-sdk/src/intTest/java/momento/sdk/CacheClientTest.java @@ -14,8 +14,8 @@ import momento.sdk.auth.StringCredentialProvider; import momento.sdk.config.Configurations; import momento.sdk.exceptions.AuthenticationException; +import momento.sdk.exceptions.CacheNotFoundException; import momento.sdk.exceptions.InvalidArgumentException; -import momento.sdk.exceptions.NotFoundException; import momento.sdk.exceptions.ServerUnavailableException; import momento.sdk.responses.cache.DeleteResponse; import momento.sdk.responses.cache.GetBatchResponse; @@ -148,7 +148,7 @@ public void shouldReturnNotFoundWhenCacheToFlushDoesNotExist() { assertThat(target.flushCache(randomString("name"))) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(CacheFlushResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test @@ -643,7 +643,7 @@ public void getBatchFailsWithNonExistentCache() { assertThat(target.getBatch(randomString("cache"), items)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(GetBatchResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test @@ -662,7 +662,7 @@ public void setBatchFailsWithNonExistentCache() { assertThat(target.setBatch(randomString("cache"), items)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SetBatchResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test @@ -702,6 +702,6 @@ public void setBatchStringBytessFailsWithNonExistentCache() { assertThat(target.setBatchStringBytes(randomString("cache"), items)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SetBatchResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } } diff --git a/momento-sdk/src/intTest/java/momento/sdk/CacheControlPlaneTest.java b/momento-sdk/src/intTest/java/momento/sdk/CacheControlPlaneTest.java index 5eeec19a..9c5d2781 100644 --- a/momento-sdk/src/intTest/java/momento/sdk/CacheControlPlaneTest.java +++ b/momento-sdk/src/intTest/java/momento/sdk/CacheControlPlaneTest.java @@ -10,8 +10,8 @@ import momento.sdk.exceptions.AlreadyExistsException; import momento.sdk.exceptions.AuthenticationException; import momento.sdk.exceptions.BadRequestException; +import momento.sdk.exceptions.CacheNotFoundException; import momento.sdk.exceptions.InvalidArgumentException; -import momento.sdk.exceptions.NotFoundException; import momento.sdk.responses.cache.control.CacheCreateResponse; import momento.sdk.responses.cache.control.CacheDeleteResponse; import momento.sdk.responses.cache.control.CacheListResponse; @@ -82,7 +82,7 @@ public void returnsNotFoundWhenDeletingUnknownCache() { assertThat(target.deleteCache(randomString("name"))) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(CacheDeleteResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test @@ -149,7 +149,7 @@ public void deleteSucceeds() { assertThat(target.deleteCache(cacheName)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(CacheDeleteResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test diff --git a/momento-sdk/src/intTest/java/momento/sdk/CacheDataPlaneTest.java b/momento-sdk/src/intTest/java/momento/sdk/CacheDataPlaneTest.java index de4e20ee..e45ea64f 100644 --- a/momento-sdk/src/intTest/java/momento/sdk/CacheDataPlaneTest.java +++ b/momento-sdk/src/intTest/java/momento/sdk/CacheDataPlaneTest.java @@ -8,7 +8,7 @@ import momento.sdk.auth.StringCredentialProvider; import momento.sdk.config.Configurations; import momento.sdk.exceptions.AuthenticationException; -import momento.sdk.exceptions.NotFoundException; +import momento.sdk.exceptions.CacheNotFoundException; import momento.sdk.exceptions.TimeoutException; import momento.sdk.responses.cache.DeleteResponse; import momento.sdk.responses.cache.GetResponse; @@ -101,11 +101,11 @@ public void nonExistentCacheNameReturnsErrorOnGetOrSet() { final GetResponse getResponse = client.get(cacheName, "").join(); assertThat(getResponse).isInstanceOf(GetResponse.Error.class); - assertThat(((GetResponse.Error) getResponse)).hasCauseInstanceOf(NotFoundException.class); + assertThat(((GetResponse.Error) getResponse)).hasCauseInstanceOf(CacheNotFoundException.class); final SetResponse setResponse = client.set(cacheName, "", "", Duration.ofSeconds(10)).join(); assertThat(setResponse).isInstanceOf(SetResponse.Error.class); - assertThat(((SetResponse.Error) setResponse)).hasCauseInstanceOf(NotFoundException.class); + assertThat(((SetResponse.Error) setResponse)).hasCauseInstanceOf(CacheNotFoundException.class); } @Test diff --git a/momento-sdk/src/intTest/java/momento/sdk/SortedSetTest.java b/momento-sdk/src/intTest/java/momento/sdk/SortedSetTest.java index 1bab2b46..7aca1554 100644 --- a/momento-sdk/src/intTest/java/momento/sdk/SortedSetTest.java +++ b/momento-sdk/src/intTest/java/momento/sdk/SortedSetTest.java @@ -14,8 +14,8 @@ import java.util.Map; import java.util.Set; import momento.sdk.config.Configurations; +import momento.sdk.exceptions.CacheNotFoundException; import momento.sdk.exceptions.InvalidArgumentException; -import momento.sdk.exceptions.NotFoundException; import momento.sdk.requests.CollectionTtl; import momento.sdk.responses.SortOrder; import momento.sdk.responses.cache.sortedset.ScoredElement; @@ -166,14 +166,14 @@ public void sortedSetPutElementReturnsErrorWithNonexistentCacheName() { assertThat(client.sortedSetPutElement(randomString("cache"), sortedSetName, "element", 1.0)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetPutElementResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); assertThat( client.sortedSetPutElement( randomString("cache"), sortedSetName, "element".getBytes(), 1.0)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetPutElementResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test @@ -279,7 +279,7 @@ public void sortedSetPutElementsReturnsErrorWithNonexistentCacheName() { randomString("cache"), sortedSetName, Collections.singletonMap("element", 1.0))) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetPutElementsResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); assertThat( client.sortedSetPutElementsByteArray( @@ -288,7 +288,7 @@ public void sortedSetPutElementsReturnsErrorWithNonexistentCacheName() { Collections.singletonMap("element".getBytes(), 1.0))) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetPutElementsResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test @@ -434,7 +434,7 @@ public void sortedSetFetchByRankReturnsErrorWithNonexistentCacheName() { assertThat(client.sortedSetFetchByRank(randomString("cache"), sortedSetName)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetFetchResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test @@ -558,7 +558,7 @@ public void sortedSetFetchByScoreReturnsErrorWithNonexistentCacheName() { randomString("cache"), sortedSetName, null, null, null, 0, 100)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetFetchResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test @@ -627,14 +627,14 @@ public void sortedSetGetRankReturnsErrorWithNonexistentCacheName() { randomString("cache"), sortedSetName, "element", SortOrder.ASCENDING)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetGetRankResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); assertThat( client.sortedSetGetRank( randomString("cache"), sortedSetName, "element".getBytes(), SortOrder.ASCENDING)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetGetRankResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test @@ -743,12 +743,12 @@ public void sortedSetGetScoreReturnsErrorWithNonexistentCacheName() { assertThat(client.sortedSetGetScore(randomString("cache"), sortedSetName, "element")) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetGetScoreResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); assertThat(client.sortedSetGetScore(randomString("cache"), sortedSetName, "element".getBytes())) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetGetScoreResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test @@ -848,14 +848,14 @@ public void sortedSetGetScoresReturnsErrorWithNonexistentCacheName() { randomString("cache"), sortedSetName, Collections.singleton("element"))) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetGetScoresResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); assertThat( client.sortedSetGetScoresByteArray( randomString("cache"), sortedSetName, Collections.singleton("element".getBytes()))) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetGetScoresResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test @@ -986,14 +986,14 @@ public void sortedSetIncrementScoreReturnsErrorWithNonexistentCacheName() { assertThat(client.sortedSetIncrementScore(randomString("cache"), sortedSetName, "element", 1.0)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetIncrementScoreResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); assertThat( client.sortedSetIncrementScore( randomString("cache"), sortedSetName, "element".getBytes(), 1.0)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetIncrementScoreResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test @@ -1106,14 +1106,14 @@ public void sortedSetRemoveElementReturnsErrorWithNonexistentCacheName() { assertThat(client.sortedSetRemoveElement(randomString("cache"), sortedSetName, "element")) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetRemoveElementResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); assertThat( client.sortedSetRemoveElement( randomString("cache"), sortedSetName, "element".getBytes())) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetRemoveElementResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test @@ -1226,14 +1226,14 @@ public void sortedSetRemoveElementsReturnsErrorWithNonexistentCacheName() { randomString("cache"), sortedSetName, Collections.emptySet())) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetRemoveElementsResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); assertThat( client.sortedSetRemoveElementsByteArray( randomString("cache"), sortedSetName, Collections.emptySet())) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(SortedSetRemoveElementsResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class)); + .satisfies(error -> assertThat(error).hasCauseInstanceOf(CacheNotFoundException.class)); } @Test 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 516cc0f8..818ee18a 100644 --- a/momento-sdk/src/intTest/java/momento/sdk/storage/DataTests.java +++ b/momento-sdk/src/intTest/java/momento/sdk/storage/DataTests.java @@ -7,7 +7,7 @@ import momento.sdk.PreviewStorageClient; import momento.sdk.auth.CredentialProvider; import momento.sdk.config.StorageConfigurations; -import momento.sdk.exceptions.NotFoundException; +import momento.sdk.exceptions.CacheNotFoundException; import momento.sdk.responses.storage.DeleteResponse; import momento.sdk.responses.storage.GetResponse; import momento.sdk.responses.storage.PutResponse; @@ -112,11 +112,11 @@ public void badStoreNameReturnsError() { final GetResponse getResponse = client.get(storeName, "").join(); assertThat(getResponse).isInstanceOf(GetResponse.Error.class); - assertThat(((GetResponse.Error) getResponse)).hasCauseInstanceOf(NotFoundException.class); + assertThat(((GetResponse.Error) getResponse)).hasCauseInstanceOf(CacheNotFoundException.class); final PutResponse putResponse = client.put(storeName, "", "").join(); assertThat(putResponse).isInstanceOf(PutResponse.Error.class); - assertThat(((PutResponse.Error) putResponse)).hasCauseInstanceOf(NotFoundException.class); + assertThat(((PutResponse.Error) putResponse)).hasCauseInstanceOf(CacheNotFoundException.class); } @Test diff --git a/momento-sdk/src/main/java/momento/sdk/exceptions/NotFoundException.java b/momento-sdk/src/main/java/momento/sdk/exceptions/CacheNotFoundException.java similarity index 84% rename from momento-sdk/src/main/java/momento/sdk/exceptions/NotFoundException.java rename to momento-sdk/src/main/java/momento/sdk/exceptions/CacheNotFoundException.java index 728a1d3f..4433c7ad 100644 --- a/momento-sdk/src/main/java/momento/sdk/exceptions/NotFoundException.java +++ b/momento-sdk/src/main/java/momento/sdk/exceptions/CacheNotFoundException.java @@ -3,7 +3,7 @@ import momento.sdk.internal.MomentoTransportErrorDetails; /** Requested resource or the resource on which an operation was requested doesn't exist. */ -public class NotFoundException extends MomentoServiceException { +public class CacheNotFoundException extends MomentoServiceException { private static final String MESSAGE = "A cache with the specified name does not exist. To resolve this error, " @@ -15,7 +15,8 @@ public class NotFoundException extends MomentoServiceException { * @param cause the cause. * @param transportErrorDetails details about the request and error. */ - public NotFoundException(Throwable cause, MomentoTransportErrorDetails transportErrorDetails) { + public CacheNotFoundException( + Throwable cause, MomentoTransportErrorDetails transportErrorDetails) { super( MomentoErrorCode.NOT_FOUND_ERROR, completeMessage(transportErrorDetails), diff --git a/momento-sdk/src/main/java/momento/sdk/exceptions/CacheServiceExceptionMapper.java b/momento-sdk/src/main/java/momento/sdk/exceptions/CacheServiceExceptionMapper.java index 2a8625ae..f38d1dda 100644 --- a/momento-sdk/src/main/java/momento/sdk/exceptions/CacheServiceExceptionMapper.java +++ b/momento-sdk/src/main/java/momento/sdk/exceptions/CacheServiceExceptionMapper.java @@ -88,7 +88,7 @@ public static SdkException convert(Throwable e, Metadata metadata) { } else if (errorCause.contains("Store with name")) { return new StoreNotFoundException(grpcException, errorDetails); } else { - return new NotFoundException(grpcException, errorDetails); + return new CacheNotFoundException(grpcException, errorDetails); } case ALREADY_EXISTS: return new AlreadyExistsException(grpcException, errorDetails); diff --git a/momento-sdk/src/test/java/momento/sdk/responses/storage/GetResponseTest.java b/momento-sdk/src/test/java/momento/sdk/responses/storage/GetResponseTest.java index 9133e7ab..b21c1e31 100644 --- a/momento-sdk/src/test/java/momento/sdk/responses/storage/GetResponseTest.java +++ b/momento-sdk/src/test/java/momento/sdk/responses/storage/GetResponseTest.java @@ -1,7 +1,7 @@ package momento.sdk.responses.storage; import io.grpc.Status; -import momento.sdk.exceptions.NotFoundException; +import momento.sdk.exceptions.CacheNotFoundException; import momento.sdk.internal.MomentoGrpcErrorDetails; import momento.sdk.internal.MomentoTransportErrorDetails; import org.junit.jupiter.api.Test; @@ -55,7 +55,7 @@ public void testConvenienceMethodsOnGetResponse() { // TODO distinguish store not found from key not found GetResponse.Error error = new GetResponse.Error( - new NotFoundException( + new CacheNotFoundException( new Exception(), new MomentoTransportErrorDetails( new MomentoGrpcErrorDetails(Status.Code.NOT_FOUND, "not found")))); From 59edeffab1bfaa48f778d7d7a42a858e38302451 Mon Sep 17 00:00:00 2001 From: Michael Landis Date: Thu, 20 Jun 2024 13:52:52 -0700 Subject: [PATCH 2/5] refactor: `AlreadyExistsException` -> `CacheExistsException` --- .../intTest/java/momento/sdk/CacheControlPlaneTest.java | 8 +++++--- .../src/main/java/momento/sdk/StorageControlClient.java | 4 ++-- ...stsException.java => CacheAlreadyExistsException.java} | 4 ++-- .../sdk/exceptions/CacheServiceExceptionMapper.java | 2 +- 4 files changed, 10 insertions(+), 8 deletions(-) rename momento-sdk/src/main/java/momento/sdk/exceptions/{AlreadyExistsException.java => CacheAlreadyExistsException.java} (90%) diff --git a/momento-sdk/src/intTest/java/momento/sdk/CacheControlPlaneTest.java b/momento-sdk/src/intTest/java/momento/sdk/CacheControlPlaneTest.java index 9c5d2781..5e6fb673 100644 --- a/momento-sdk/src/intTest/java/momento/sdk/CacheControlPlaneTest.java +++ b/momento-sdk/src/intTest/java/momento/sdk/CacheControlPlaneTest.java @@ -7,9 +7,9 @@ import java.time.Duration; import momento.sdk.auth.CredentialProvider; import momento.sdk.config.Configurations; -import momento.sdk.exceptions.AlreadyExistsException; import momento.sdk.exceptions.AuthenticationException; import momento.sdk.exceptions.BadRequestException; +import momento.sdk.exceptions.CacheAlreadyExistsException; import momento.sdk.exceptions.CacheNotFoundException; import momento.sdk.exceptions.InvalidArgumentException; import momento.sdk.responses.cache.control.CacheCreateResponse; @@ -74,7 +74,8 @@ public void throwsAlreadyExistsWhenCreatingExistingCache() { assertThat(target.createCache(existingCache)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(CacheCreateResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(AlreadyExistsException.class)); + .satisfies( + error -> assertThat(error).hasCauseInstanceOf(CacheAlreadyExistsException.class)); } @Test @@ -140,7 +141,8 @@ public void deleteSucceeds() { assertThat(target.createCache(cacheName)) .succeedsWithin(FIVE_SECONDS) .asInstanceOf(InstanceOfAssertFactories.type(CacheCreateResponse.Error.class)) - .satisfies(error -> assertThat(error).hasCauseInstanceOf(AlreadyExistsException.class)); + .satisfies( + error -> assertThat(error).hasCauseInstanceOf(CacheAlreadyExistsException.class)); assertThat(target.deleteCache(cacheName)) .succeedsWithin(FIVE_SECONDS) diff --git a/momento-sdk/src/main/java/momento/sdk/StorageControlClient.java b/momento-sdk/src/main/java/momento/sdk/StorageControlClient.java index de537a34..644b5cfc 100644 --- a/momento-sdk/src/main/java/momento/sdk/StorageControlClient.java +++ b/momento-sdk/src/main/java/momento/sdk/StorageControlClient.java @@ -16,7 +16,7 @@ import javax.annotation.Nonnull; import momento.sdk.auth.CredentialProvider; import momento.sdk.config.StorageConfiguration; -import momento.sdk.exceptions.AlreadyExistsException; +import momento.sdk.exceptions.CacheAlreadyExistsException; import momento.sdk.exceptions.CacheServiceExceptionMapper; import momento.sdk.exceptions.SdkException; import momento.sdk.responses.storage.CreateStoreResponse; @@ -81,7 +81,7 @@ private CompletableFuture sendCreateStore(String storeName) final Function failure = e -> { final SdkException sdkException = CacheServiceExceptionMapper.convert(e); - if (sdkException instanceof AlreadyExistsException) { + if (sdkException instanceof CacheAlreadyExistsException) { return new CreateStoreResponse.AlreadyExists(); } return new CreateStoreResponse.Error(CacheServiceExceptionMapper.convert(e)); diff --git a/momento-sdk/src/main/java/momento/sdk/exceptions/AlreadyExistsException.java b/momento-sdk/src/main/java/momento/sdk/exceptions/CacheAlreadyExistsException.java similarity index 90% rename from momento-sdk/src/main/java/momento/sdk/exceptions/AlreadyExistsException.java rename to momento-sdk/src/main/java/momento/sdk/exceptions/CacheAlreadyExistsException.java index 90c0fa6f..cd117a61 100644 --- a/momento-sdk/src/main/java/momento/sdk/exceptions/AlreadyExistsException.java +++ b/momento-sdk/src/main/java/momento/sdk/exceptions/CacheAlreadyExistsException.java @@ -3,7 +3,7 @@ import momento.sdk.internal.MomentoTransportErrorDetails; /** A resource already exists. */ -public class AlreadyExistsException extends MomentoServiceException { +public class CacheAlreadyExistsException extends MomentoServiceException { private static final String MESSAGE = "A cache with the specified name already exists. To resolve this error, " @@ -15,7 +15,7 @@ public class AlreadyExistsException extends MomentoServiceException { * @param cause the cause. * @param transportErrorDetails details about the request and error. */ - public AlreadyExistsException( + public CacheAlreadyExistsException( Throwable cause, MomentoTransportErrorDetails transportErrorDetails) { super( MomentoErrorCode.ALREADY_EXISTS_ERROR, diff --git a/momento-sdk/src/main/java/momento/sdk/exceptions/CacheServiceExceptionMapper.java b/momento-sdk/src/main/java/momento/sdk/exceptions/CacheServiceExceptionMapper.java index f38d1dda..5baa6a83 100644 --- a/momento-sdk/src/main/java/momento/sdk/exceptions/CacheServiceExceptionMapper.java +++ b/momento-sdk/src/main/java/momento/sdk/exceptions/CacheServiceExceptionMapper.java @@ -91,7 +91,7 @@ public static SdkException convert(Throwable e, Metadata metadata) { return new CacheNotFoundException(grpcException, errorDetails); } case ALREADY_EXISTS: - return new AlreadyExistsException(grpcException, errorDetails); + return new CacheAlreadyExistsException(grpcException, errorDetails); case UNKNOWN: return new UnknownServiceException(grpcException, errorDetails); From dcaa0f46276e45127a7b2bb870604ce4d25fa8e7 Mon Sep 17 00:00:00 2001 From: Michael Landis Date: Thu, 20 Jun 2024 13:58:53 -0700 Subject: [PATCH 3/5] refactor: specialize `AlreadyExistsException` to cache and store --- .../momento/sdk/StorageControlClient.java | 4 +-- .../CacheAlreadyExistsException.java | 2 +- .../CacheServiceExceptionMapper.java | 7 ++-- .../StoreAlreadyExistsException.java | 34 +++++++++++++++++++ 4 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 momento-sdk/src/main/java/momento/sdk/exceptions/StoreAlreadyExistsException.java diff --git a/momento-sdk/src/main/java/momento/sdk/StorageControlClient.java b/momento-sdk/src/main/java/momento/sdk/StorageControlClient.java index 644b5cfc..7d9e6728 100644 --- a/momento-sdk/src/main/java/momento/sdk/StorageControlClient.java +++ b/momento-sdk/src/main/java/momento/sdk/StorageControlClient.java @@ -16,9 +16,9 @@ import javax.annotation.Nonnull; import momento.sdk.auth.CredentialProvider; import momento.sdk.config.StorageConfiguration; -import momento.sdk.exceptions.CacheAlreadyExistsException; import momento.sdk.exceptions.CacheServiceExceptionMapper; import momento.sdk.exceptions.SdkException; +import momento.sdk.exceptions.StoreAlreadyExistsException; import momento.sdk.responses.storage.CreateStoreResponse; import momento.sdk.responses.storage.DeleteStoreResponse; import momento.sdk.responses.storage.ListStoresResponse; @@ -81,7 +81,7 @@ private CompletableFuture sendCreateStore(String storeName) final Function failure = e -> { final SdkException sdkException = CacheServiceExceptionMapper.convert(e); - if (sdkException instanceof CacheAlreadyExistsException) { + if (sdkException instanceof StoreAlreadyExistsException) { return new CreateStoreResponse.AlreadyExists(); } return new CreateStoreResponse.Error(CacheServiceExceptionMapper.convert(e)); diff --git a/momento-sdk/src/main/java/momento/sdk/exceptions/CacheAlreadyExistsException.java b/momento-sdk/src/main/java/momento/sdk/exceptions/CacheAlreadyExistsException.java index cd117a61..3ec7fe14 100644 --- a/momento-sdk/src/main/java/momento/sdk/exceptions/CacheAlreadyExistsException.java +++ b/momento-sdk/src/main/java/momento/sdk/exceptions/CacheAlreadyExistsException.java @@ -10,7 +10,7 @@ public class CacheAlreadyExistsException extends MomentoServiceException { + "either delete the existing cache and make a new one, or use a different name."; /** - * Constructs an AlreadyExistsException with a cause and error details. + * Constructs a CacheAlreadyExistsException with a cause and error details. * * @param cause the cause. * @param transportErrorDetails details about the request and error. diff --git a/momento-sdk/src/main/java/momento/sdk/exceptions/CacheServiceExceptionMapper.java b/momento-sdk/src/main/java/momento/sdk/exceptions/CacheServiceExceptionMapper.java index 5baa6a83..c3b91f43 100644 --- a/momento-sdk/src/main/java/momento/sdk/exceptions/CacheServiceExceptionMapper.java +++ b/momento-sdk/src/main/java/momento/sdk/exceptions/CacheServiceExceptionMapper.java @@ -91,8 +91,11 @@ public static SdkException convert(Throwable e, Metadata metadata) { return new CacheNotFoundException(grpcException, errorDetails); } case ALREADY_EXISTS: - return new CacheAlreadyExistsException(grpcException, errorDetails); - + if (errorCause.contains("Store with name")) { + return new StoreAlreadyExistsException(grpcException, errorDetails); + } else { + return new CacheAlreadyExistsException(grpcException, errorDetails); + } case UNKNOWN: return new UnknownServiceException(grpcException, errorDetails); diff --git a/momento-sdk/src/main/java/momento/sdk/exceptions/StoreAlreadyExistsException.java b/momento-sdk/src/main/java/momento/sdk/exceptions/StoreAlreadyExistsException.java new file mode 100644 index 00000000..cc0c9ea7 --- /dev/null +++ b/momento-sdk/src/main/java/momento/sdk/exceptions/StoreAlreadyExistsException.java @@ -0,0 +1,34 @@ +package momento.sdk.exceptions; + +import momento.sdk.internal.MomentoTransportErrorDetails; + +/** A resource already exists. */ +public class StoreAlreadyExistsException extends MomentoServiceException { + + private static final String MESSAGE = + "A store with the specified name already exists. To resolve this error, " + + "either delete the existing store and make a new one, or use a different name."; + + /** + * Constructs an StoreAlreadyExistsException with a cause and error details. + * + * @param cause the cause. + * @param transportErrorDetails details about the request and error. + */ + public StoreAlreadyExistsException( + Throwable cause, MomentoTransportErrorDetails transportErrorDetails) { + super( + MomentoErrorCode.ALREADY_EXISTS_ERROR, + completeMessage(transportErrorDetails), + cause, + transportErrorDetails); + } + + private static String completeMessage(MomentoTransportErrorDetails transportErrorDetails) { + return transportErrorDetails + .getGrpcErrorDetails() + .getCacheName() + .map(s -> MESSAGE + " Cache name: " + s) + .orElse(MESSAGE); + } +} From 7b94f1f5c60f87e0d92f98e2bb13dae4d8eb07bb Mon Sep 17 00:00:00 2001 From: Michael Landis Date: Thu, 20 Jun 2024 14:02:45 -0700 Subject: [PATCH 4/5] chore: retain old exceptions for backwards compatibility --- .../exceptions/AlreadyExistsException.java | 19 +++++++++++++++++++ .../sdk/exceptions/NotFoundException.java | 18 ++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 momento-sdk/src/main/java/momento/sdk/exceptions/AlreadyExistsException.java create mode 100644 momento-sdk/src/main/java/momento/sdk/exceptions/NotFoundException.java diff --git a/momento-sdk/src/main/java/momento/sdk/exceptions/AlreadyExistsException.java b/momento-sdk/src/main/java/momento/sdk/exceptions/AlreadyExistsException.java new file mode 100644 index 00000000..1ba78dd8 --- /dev/null +++ b/momento-sdk/src/main/java/momento/sdk/exceptions/AlreadyExistsException.java @@ -0,0 +1,19 @@ +package momento.sdk.exceptions; + +import momento.sdk.internal.MomentoTransportErrorDetails; + +/** A resource already exists. */ +@Deprecated // Use CacheAlreadyExistsException instead +public class AlreadyExistsException extends CacheAlreadyExistsException { + + /** + * Constructs an AlreadyExistsException with a cause and error details. + * + * @param cause the cause. + * @param transportErrorDetails details about the request and error. + */ + public AlreadyExistsException( + Throwable cause, MomentoTransportErrorDetails transportErrorDetails) { + super(cause, transportErrorDetails); + } +} diff --git a/momento-sdk/src/main/java/momento/sdk/exceptions/NotFoundException.java b/momento-sdk/src/main/java/momento/sdk/exceptions/NotFoundException.java new file mode 100644 index 00000000..db2e76f7 --- /dev/null +++ b/momento-sdk/src/main/java/momento/sdk/exceptions/NotFoundException.java @@ -0,0 +1,18 @@ +package momento.sdk.exceptions; + +import momento.sdk.internal.MomentoTransportErrorDetails; + +/** Requested resource or the resource on which an operation was requested doesn't exist. */ +@Deprecated // Use CacheNotFoundException instead +public class NotFoundException extends CacheNotFoundException { + + /** + * Constructs a NotFoundException with a cause and error details. + * + * @param cause the cause. + * @param transportErrorDetails details about the request and error. + */ + public NotFoundException(Throwable cause, MomentoTransportErrorDetails transportErrorDetails) { + super(cause, transportErrorDetails); + } +} From a85b6455e36023ef952326b93ca017eab67ed6cd Mon Sep 17 00:00:00 2001 From: Michael Landis Date: Thu, 20 Jun 2024 14:42:43 -0700 Subject: [PATCH 5/5] fix: update test to use correct class --- .../java/momento/sdk/responses/storage/GetResponseTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/momento-sdk/src/test/java/momento/sdk/responses/storage/GetResponseTest.java b/momento-sdk/src/test/java/momento/sdk/responses/storage/GetResponseTest.java index b21c1e31..72356e8a 100644 --- a/momento-sdk/src/test/java/momento/sdk/responses/storage/GetResponseTest.java +++ b/momento-sdk/src/test/java/momento/sdk/responses/storage/GetResponseTest.java @@ -1,7 +1,7 @@ package momento.sdk.responses.storage; import io.grpc.Status; -import momento.sdk.exceptions.CacheNotFoundException; +import momento.sdk.exceptions.StoreNotFoundException; import momento.sdk.internal.MomentoGrpcErrorDetails; import momento.sdk.internal.MomentoTransportErrorDetails; import org.junit.jupiter.api.Test; @@ -55,7 +55,7 @@ public void testConvenienceMethodsOnGetResponse() { // TODO distinguish store not found from key not found GetResponse.Error error = new GetResponse.Error( - new CacheNotFoundException( + new StoreNotFoundException( new Exception(), new MomentoTransportErrorDetails( new MomentoGrpcErrorDetails(Status.Code.NOT_FOUND, "not found"))));