Skip to content
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

fix collection not exist #612

Merged
merged 1 commit into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public enum ErrorCode {
COMMAND_ACCEPTS_NO_OPTIONS("Command accepts no options"),

CONCURRENCY_FAILURE("Unable to complete transaction due to concurrent transactions"),

COLLECTION_NOT_EXIST("Collection does not exist, collection name: "),
DATASET_TOO_BIG("Response data set too big to be sorted, add more filters"),

DOCUMENT_ALREADY_EXISTS("Document already exists with the given _id"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import io.grpc.StatusRuntimeException;
import io.smallrye.mutiny.Uni;
import io.stargate.sgv2.jsonapi.exception.ErrorCode;
import io.stargate.sgv2.jsonapi.exception.JsonApiException;
import java.time.Duration;

/** Caches the vector enabled status for the namespace */
Expand Down Expand Up @@ -41,14 +42,24 @@ protected Uni<CollectionSettings> getCollectionProperties(String collectionName)
.transformToUni(
(result, error) -> {
if (null != error) {
// collection does not exist
if (error instanceof RuntimeException rte
tatu-at-datastax marked this conversation as resolved.
Show resolved Hide resolved
&& rte.getMessage()
.startsWith(ErrorCode.INVALID_COLLECTION_NAME.getMessage())) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not wrong, but in my experience it is usually best practice to check against the error code or some other constant other than the error message, because (1) error messages may change, (2) it is possible that there may be multiple types of errors that start with the same message. Not a blocker, just figured I'd point this out.

return Uni.createFrom()
.failure(
new JsonApiException(
ErrorCode.COLLECTION_NOT_EXIST,
ErrorCode.COLLECTION_NOT_EXIST
.getMessage()
.concat(collectionName)));
}

// ignoring the error and return false. This will be handled while trying to
// execute the query
if ((error instanceof StatusRuntimeException sre
&& (sre.getStatus().getCode() == io.grpc.Status.Code.NOT_FOUND
|| sre.getStatus().getCode() == io.grpc.Status.Code.INVALID_ARGUMENT))
|| (error instanceof RuntimeException rte
&& rte.getMessage()
.startsWith(ErrorCode.INVALID_COLLECTION_NAME.getMessage()))) {
&& (sre.getStatus().getCode() == io.grpc.Status.Code.NOT_FOUND
|| sre.getStatus().getCode() == io.grpc.Status.Code.INVALID_ARGUMENT))) {
return Uni.createFrom()
.item(new CollectionSettings(collectionName, false, 0, null, null, null));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ public void regularError() {
endsWith(
"INVALID_ARGUMENT: table %s.%s does not exist"
.formatted(namespaceName, "badCollection")),
endsWith("INVALID_ARGUMENT: table %s does not exist".formatted("badCollection")));
endsWith("INVALID_ARGUMENT: table %s does not exist".formatted("badCollection")),
endsWith(
"Collection does not exist, collection name: %s".formatted("badCollection")));
given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
.contentType(ContentType.JSON)
Expand All @@ -101,7 +103,7 @@ public void regularError() {
.body("errors", is(notNullValue()))
.body("errors[0].message", is(not(blankString())))
.body("errors[0].message", anyOf)
.body("errors[0].exceptionClass", is("StatusRuntimeException"));
.body("errors[0].exceptionClass", is("JsonApiException"));
}

@Test
Expand Down