-
Notifications
You must be signed in to change notification settings - Fork 24
Batch 12 of #2285, converting shredding-related ErrorCodeV1 entries
#2322
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
base: main
Are you sure you want to change the base?
Changes from all commits
c6b473e
348226d
b484ee4
68bb2d0
4f568de
276a2b0
51b5c5b
f57f9b1
7746144
d59e4c3
72d474e
f9d2645
ae4e795
fb4af3b
9c8a0c8
6ec6cdd
35bfa03
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,6 @@ | |
| import io.smallrye.config.ConfigMapping; | ||
| import io.smallrye.config.WithConverter; | ||
| import io.smallrye.config.WithDefault; | ||
| import io.stargate.sgv2.jsonapi.exception.ErrorCodeV1; | ||
| import io.stargate.sgv2.jsonapi.service.provider.ApiModelSupport; | ||
| import jakarta.annotation.Nullable; | ||
| import jakarta.inject.Inject; | ||
|
|
@@ -207,7 +206,10 @@ public static ValidationType fromString(String type) { | |
| } else if (type.equals("options")) { | ||
| return OPTIONS; | ||
| } | ||
| throw ErrorCodeV1.INVALID_PARAMETER_VALIDATION_TYPE.toApiException(type); | ||
| throw new IllegalArgumentException( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One-off failure not to be directly exposed: caller will wrap/convert validation problems in more centralized way.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am guessing we only need this stuff because of the EGW and the trouble we have with configs ? |
||
| "Invalid `ValidationType` value ('" | ||
| + type | ||
| + "') for `EmbeddingProvidersConfig`: expected either 'numericRange' or 'options'"); | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,12 +6,13 @@ | |
| import io.quarkus.runtime.annotations.RegisterForReflection; | ||
| import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.JsonType; | ||
| import io.stargate.sgv2.jsonapi.config.constants.DocumentConstants; | ||
| import io.stargate.sgv2.jsonapi.exception.ErrorCodeV1; | ||
| import io.stargate.sgv2.jsonapi.exception.DocumentException; | ||
| import io.stargate.sgv2.jsonapi.exception.JsonApiException; | ||
| import io.stargate.sgv2.jsonapi.service.shredding.DocRowIdentifer; | ||
| import io.stargate.sgv2.jsonapi.util.JsonUtil; | ||
| import java.math.BigDecimal; | ||
| import java.util.Date; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.UUID; | ||
|
|
||
|
|
@@ -78,20 +79,26 @@ static DocumentId fromJson(JsonNode node) { | |
| return fromExtensionType(extType, valueNode); | ||
| } | ||
| } | ||
| throw ErrorCodeV1.SHRED_BAD_DOCID_TYPE.toApiException( | ||
| "unrecognized JSON extension type '%s'", node.fieldNames().next()); | ||
| } | ||
| throw ErrorCodeV1.SHRED_BAD_DOCID_TYPE.toApiException( | ||
| "Document Id must be a JSON String, Number, Boolean, EJSON-Encoded Date Object or NULL instead got %s: %s", | ||
| node.getNodeType(), node.toString()); | ||
| throw DocumentException.Code.SHRED_BAD_DOCID_TYPE.get( | ||
| Map.of( | ||
| "errorMessage", | ||
| "unrecognized JSON extension type '%s'".formatted(node.fieldNames().next()))); | ||
| } | ||
| throw DocumentException.Code.SHRED_BAD_DOCID_TYPE.get( | ||
| Map.of( | ||
| "errorMessage", | ||
| "Document Id must be a JSON String, Number, Boolean, EJSON-Encoded Date Object or null instead got %s: %s" | ||
| .formatted(JsonUtil.nodeTypeAsString(node), node.toString()))); | ||
| } | ||
|
|
||
| static DocumentId fromDatabase(int typeId, String documentIdAsText) { | ||
| JsonType type = DocumentConstants.KeyTypeId.getJsonType(typeId); | ||
| if (type == null) { | ||
| throw ErrorCodeV1.SHRED_BAD_DOCID_TYPE.toApiException( | ||
| "Document Id must be a JSON String(1), Number(2), Boolean(3), NULL(4) or Date(5) instead got %d", | ||
| typeId); | ||
| throw DocumentException.Code.SHRED_BAD_DOCID_TYPE.get( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is an error when the DB has corrupted / invalid data, the errors above are from a request. So there may be some confusion here, it's not a REQUEST family error (which DocumentException is) it is a SERVER family error. It would be something like DatabaseException.Code.UNEXPECTED_DOCUMENT_ID_TYPE - which does not exists. Do we want to let this through and I can followup ? Same for all in this function
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! Yes, agreed, I blindly made it |
||
| Map.of( | ||
| "errorMessage", | ||
| "Document Id must be a JSON String(1), Number(2), Boolean(3), null(4) or Date(5) instead got %d" | ||
| .formatted(typeId))); | ||
| } | ||
| switch (type) { | ||
| case BOOLEAN -> { | ||
|
|
@@ -101,9 +108,11 @@ static DocumentId fromDatabase(int typeId, String documentIdAsText) { | |
| case "false": | ||
| return fromBoolean(false); | ||
| } | ||
| throw ErrorCodeV1.SHRED_BAD_DOCID_TYPE.toApiException( | ||
| "Document Id type Boolean stored as invalid String '%s' (must be 'true' or 'false')", | ||
| documentIdAsText); | ||
| throw DocumentException.Code.SHRED_BAD_DOCID_TYPE.get( | ||
| Map.of( | ||
| "errorMessage", | ||
| "Document Id type Boolean stored as invalid String '%s' (must be 'true' or 'false')" | ||
| .formatted(documentIdAsText))); | ||
| } | ||
| case NULL -> { | ||
| return fromNull(); | ||
|
|
@@ -112,9 +121,11 @@ static DocumentId fromDatabase(int typeId, String documentIdAsText) { | |
| try { | ||
| return fromNumber(new BigDecimal(documentIdAsText)); | ||
| } catch (NumberFormatException e) { | ||
| throw ErrorCodeV1.SHRED_BAD_DOCID_TYPE.toApiException( | ||
| "Document Id type Number stored as invalid String '%s' (not a valid Number)", | ||
| documentIdAsText); | ||
| throw DocumentException.Code.SHRED_BAD_DOCID_TYPE.get( | ||
| Map.of( | ||
| "errorMessage", | ||
| "Document Id type Number stored as invalid String '%s' (not a valid Number)" | ||
| .formatted(documentIdAsText))); | ||
| } | ||
| } | ||
| case STRING -> { | ||
|
|
@@ -125,13 +136,16 @@ static DocumentId fromDatabase(int typeId, String documentIdAsText) { | |
| long ts = Long.parseLong(documentIdAsText); | ||
| return fromTimestamp(ts); | ||
| } catch (NumberFormatException e) { | ||
| throw ErrorCodeV1.SHRED_BAD_DOCID_TYPE.toApiException( | ||
| "Document Id type Date stored as invalid String '%s' (needs to be Number)", | ||
| documentIdAsText); | ||
| throw DocumentException.Code.SHRED_BAD_DOCID_TYPE.get( | ||
| Map.of( | ||
| "errorMessage", | ||
| "Document Id type Date stored as invalid String '%s' (needs to be Number)" | ||
| .formatted(documentIdAsText))); | ||
| } | ||
| } | ||
| } | ||
| throw ErrorCodeV1.SHRED_BAD_DOCID_TYPE.toApiException(); | ||
| throw DocumentException.Code.SHRED_BAD_DOCID_TYPE.get( | ||
| Map.of("errorMessage", "unknown `JsonType`: '%s'".formatted(type))); | ||
| } | ||
|
|
||
| static DocumentId fromBoolean(boolean key) { | ||
|
|
@@ -150,7 +164,8 @@ static DocumentId fromNumber(BigDecimal key) { | |
| static DocumentId fromString(String key) { | ||
| key = Objects.requireNonNull(key); | ||
| if (key.isEmpty()) { | ||
| throw ErrorCodeV1.SHRED_BAD_DOCID_EMPTY_STRING.toApiException(); | ||
| throw DocumentException.Code.SHRED_BAD_DOCID_VALUE.get( | ||
| Map.of("errorMessage", "empty String not allowed")); | ||
| } | ||
| return new StringId(key); | ||
| } | ||
|
|
@@ -172,7 +187,7 @@ static DocumentId fromExtensionType(JsonExtensionType extType, JsonNode valueNod | |
| Object rawId = JsonUtil.extractExtendedValueUnwrapped(extType, valueNode); | ||
| return new ExtensionTypeId(extType, String.valueOf(rawId)); | ||
| } catch (JsonApiException e) { | ||
| throw ErrorCodeV1.SHRED_BAD_DOCID_TYPE.toApiException(e.getMessage()); | ||
| throw DocumentException.Code.SHRED_BAD_DOCID_TYPE.get(Map.of("errorMessage", e.getMessage())); | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we dont have any BAD error codes in V2 errors - we can let them through and then I can fix in a follow up ? as in they take some thinking do we want to do that now ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, postpone for now to give us more time to think it through.