Skip to content

Conversation

JohanCodinha
Copy link
Contributor

No description provided.

Both StringSchema and JsonSchema (with "string" type) use a new transform-string helper. This enables structured handling of formats like uuid, date, and email, while preserving support for maxLength, pattern, and enumerations.
"date-time" string?
"password" string?
"email" string?
"uri" string?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could use uri?

Copy link
Owner

Choose a reason for hiding this comment

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

can we test it a bit in a reitit server to see what type the coercion produces?

(case fmt
"uuid" uuid?
"binary" string?
"byte" string?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

bytes?

Copy link
Owner

Choose a reason for hiding this comment

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

same for this, test coercion in the server

@lispyclouds
Copy link
Owner

thanks for this, will have a look later today or by tomorrow!

@lispyclouds
Copy link
Owner

@john-shaffer can i ask a favour of you to maybe test these changes with the 3.1.0 samples you have too? this would greatly help me out. @JohanCodinha can we also add the yamls to the repo too? we should run the full parse test i feel.

@JohanCodinha
Copy link
Contributor Author

Have a look here, let me know if you like that way of testing I can add it here too.

@lispyclouds
Copy link
Owner

@JohanCodinha yes, lets get some of the yamls in here too like that.

"ipv6" string?
string?))

(defn transform-string
Copy link
Owner

Choose a reason for hiding this comment

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

we can make this private

"uuid" uuid?
"binary" string?
"byte" string?
"date" string?
Copy link
Owner

Choose a reason for hiding this comment

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

date and date-time can use inst?

Copy link
Owner

Choose a reason for hiding this comment

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

maybe this needs a transform date-time fn too when #11 comes in

Copy link
Owner

Choose a reason for hiding this comment

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

We have #11 merged now, can use the date schemas from that.

@john-shaffer
Copy link
Contributor

@john-shaffer can i ask a favour of you to maybe test these changes with the 3.1.0 samples you have too? this would greatly help me out. @JohanCodinha can we also add the yamls to the repo too? we should run the full parse test i feel.

Sure. I tested the changes in my app with good results. Not only does this patch work well, it pinpointed a bug in my app.

@lispyclouds
Copy link
Owner

That's awesome to hear @john-shaffer

@john-shaffer
Copy link
Contributor

john-shaffer commented Jan 24, 2025

Regarding date and date-time formats, my app handles this by parsing the strings that come in from reitit. AFAIK all {:type "string"} objects come in to the handlers as Strings regardless of the format.

@lispyclouds
Copy link
Owner

lispyclouds commented Jan 24, 2025

right, so with this change if we use inst? the coercion would change it to an Instant if the format is date or date-time. reitit can then check the string for you too when it gets it from the api

@john-shaffer
Copy link
Contributor

Thanks, I'm not very familiar with malli coercion. I tested with "date-time" inst? and the results were as expected. date-time values are passed to handlers as java.util.Dates, and Dates returned from handlers are coerced to strings in the correct format.

@lispyclouds
Copy link
Owner

@JohanCodinha i think if you could add in the tests, I can follow up with the rest of the changes myself 😄

@lispyclouds lispyclouds merged commit 898c429 into lispyclouds:main Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants