feat: enhance identifier handling with new generated model #64
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,8 @@ | ||
| package org.cardanofoundation.signify.app.aiding; | ||
|
|
||
| import org.cardanofoundation.signify.generated.keria.model.Identifier; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public record IdentifierListResponse(int start, int end, int total, List<Identifier> aids) { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| package org.cardanofoundation.signify.app.aiding; | ||
|
|
||
| import com.fasterxml.jackson.core.JsonParser; | ||
| import com.fasterxml.jackson.databind.DeserializationContext; | ||
| import com.fasterxml.jackson.databind.JsonNode; | ||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import com.fasterxml.jackson.databind.deser.std.StdDeserializer; | ||
| import com.fasterxml.jackson.databind.node.ObjectNode; | ||
| import org.cardanofoundation.signify.generated.keria.model.KeyStateRecord; | ||
| import org.cardanofoundation.signify.app.config.GeneratedModelConfig; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| /** | ||
| * Allows kt/nt fields to arrive as arrays/objects and coerces them to strings for the generated model. | ||
| */ | ||
| public class KeyStateRecordDeserializer extends StdDeserializer<KeyStateRecord> { | ||
|
|
||
| private static final ObjectMapper delegate = GeneratedModelConfig.baseMapper(); | ||
|
|
||
| public KeyStateRecordDeserializer() { | ||
| super(KeyStateRecord.class); | ||
| } | ||
|
|
||
| @Override | ||
| public KeyStateRecord deserialize(JsonParser p, DeserializationContext ctxt) throws IOException { | ||
| ObjectMapper mapper = (ObjectMapper) p.getCodec(); | ||
| ObjectNode node = mapper.readTree(p); | ||
|
|
||
| coerceToString(node, "kt"); | ||
| coerceToString(node, "nt"); | ||
|
Collaborator
There was a problem hiding this comment. Do we really ever get ints back from the API? I thought it was always strings. (I know the API for creating an identifier accepts both though, which is different)
Collaborator
Author
There was a problem hiding this comment. We hit a parse failure when
Collaborator
There was a problem hiding this comment. Ah yes, but it's an array of strings. I think @Sotatek-Patrick-Vu might be fixing this, or have fixed it. We should return the string array, not a string.
Collaborator
Author
There was a problem hiding this comment. updated in
Collaborator
There was a problem hiding this comment. Right but is it possible to receive a numeric response from the API? I don't think it is (may be a mistake in Patrick's generated models)
Collaborator
There was a problem hiding this comment. @Sotatek-DucPhung What does the generator generate in Java types for this, when using the up to date API spec from Patrick?
Collaborator
Author
There was a problem hiding this comment. It's
Collaborator
There was a problem hiding this comment. But is this before or after getting Patrick's fixes? he needs to re-open a new PR: cardano-foundation/keria#16
Collaborator
Author
There was a problem hiding this comment. Anw, i have add logic to handle the String array deserilizer, so the e2e test is passed
Collaborator
There was a problem hiding this comment. This is incorrect, we are not meant to have strings of JSON arrays. We are meant to have arrays. Please work with Patrick to get the OpenAPI changes needed to update the generated model and have the correct format. |
||
|
|
||
| return delegate.treeToValue(node, KeyStateRecord.class); | ||
| } | ||
|
|
||
| private void coerceToString(ObjectNode node, String field) { | ||
| JsonNode value = node.get(field); | ||
| if (value == null) { | ||
| return; | ||
| } | ||
| // If already a string, leave it | ||
| if (value.isTextual()) { | ||
| return; | ||
| } | ||
| // If it's an array, convert to JSON string representation | ||
| if (value.isArray()) { | ||
| node.put(field, value.toString()); | ||
| return; | ||
| } | ||
| // If it's a number, convert to string | ||
| if (value.isNumber()) { | ||
| node.put(field, value.asText()); | ||
| return; | ||
| } | ||
| throw new IllegalArgumentException("Unexpected type for field '" + field + "': " + value.getNodeType()); | ||
| } | ||
| } | ||
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.
Pointer to https://github.com/cardano-foundation/cf-signify-java/pull/59/files#r2604004041
Should be resolved before this PR is merged!
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.
@Sotatek-DucPhung Please check my comment there: #59 (comment)
Those changes MUST be fixed in this PR before merging.