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
Author
There was a problem hiding this comment. @Sotatek-Patrick-Vu could you please take a look for these field ? There was a problem hiding this comment. We are expecting
Collaborator
There was a problem hiding this comment. @Sotatek-DucPhung @Sotatek-PhongVu @Sotatek-Patrick-Vu It's also not generated correctly in Signify-TS: https://github.com/WebOfTrust/signify-ts/blob/main/src/types/keria-api-schema.ts#L363 This is referring to the Key State only. So I think there are missing changes in KERIA to generate this correctly in both Signify-TS and Signify-Java: cardano-foundation/keria#16
Collaborator
There was a problem hiding this comment. After fixing keria and here is the new type in signify-ts:
Collaborator
There was a problem hiding this comment. Indeed @Sotatek-Patrick-Vu if you can fix this PR to remove |
||
|
|
||
| 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.
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.
If it will take time to fix those, and we resolve the other comment in this thread first. We could defer fixing these until the next PR
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.
yep, sounds good to me