Skip to content
This repository has been archived by the owner on Dec 12, 2024. It is now read-only.

Update currency representation #129

Merged
merged 4 commits into from
Jan 17, 2024
Merged

Update currency representation #129

merged 4 commits into from
Jan 17, 2024

Conversation

diehuxx
Copy link

@diehuxx diehuxx commented Jan 12, 2024

Addresses #114, see also spec change TBD54566975/tbdex#206 and equivalent js PR TBD54566975/tbdex-js#134

There are a few places where we are not validating data upon Message/resource creation or sending. In those places, i added JSON Schema validations. These additional checks helped me find default data and test data that needed to be updated.

Copy link

codecov bot commented Jan 12, 2024

Codecov Report

Merging #129 (6474f8c) into main (fe212cb) will increase coverage by 0.25%.
The diff coverage is 78.78%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #129      +/-   ##
==========================================
+ Coverage   78.39%   78.64%   +0.25%     
==========================================
  Files          30       30              
  Lines         671      693      +22     
  Branches       65       65              
==========================================
+ Hits          526      545      +19     
- Misses        111      114       +3     
  Partials       34       34              
Components Coverage Δ
protocol 85.09% <66.66%> (-0.44%) ⬇️
httpclient 82.97% <100.00%> (+0.24%) ⬆️

* @throws Exception if validation fails, including a list of validation errors.
*/
fun validateMessage(message: Message) {
val mapper = ObjectMapper()
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use the jsonMapper from Json.kt instead of reinstantiating

Copy link
Author

Choose a reason for hiding this comment

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

done

* @throws Exception if validation fails, including a list of validation errors.
*/
fun validateData(data: Data, messageKind: String) {
val mapper = ObjectMapper()
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto: You can use the jsonMapper from Json.kt instead of reinstantiating

Copy link
Author

Choose a reason for hiding this comment

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

done

protocol/src/main/resources/definitions.json Show resolved Hide resolved
@diehuxx diehuxx merged commit 7bd0e32 into main Jan 17, 2024
3 of 5 checks passed
@diehuxx diehuxx deleted the currency-199 branch January 17, 2024 00:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants