Skip to content
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

support patterns from string descriptors #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

collintime
Copy link

@collintime collintime commented Oct 18, 2022

Hello!

Great work with this fork!

To my surprise, 42Crunch recently indicated to me that some uuid/guid schemas were not setting patterns in the openapi schema. Have you ever considered something like my proposed change? The string().uuid().regex(/not-at-all-uuid-compatible/) possibility does bother me a little.

Please let me know what you think. Thanks for all your work on this.

Edit: Also curious if you've had any contact with the original author. It would be great to have a place to create issues etc.

@sdrubolo
Copy link
Owner

sdrubolo commented Jan 4, 2023

Hi, thank you to point this out! Since those are built-in formats they shouldn't need any pattern to be specified (because if you provide the pattern you could omit the format) but I do not see any harm and I may include it in the code base.
However, I think the translation should be reviewed a little in particular for the case you were pointing out. In my opinion we should return a

allOf : [ { type : "string", format : "uuid"}, { type : "string", pattern : "not-at-all-uuid-compatible" } ]
because it is more like an AND (all regular expressions must be valid). Therefore I would need some additional thought before merging this and apply the needed changes

Finally, about the original author I do not have any contact. I've just forked that project and developed all the additional features

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.

2 participants