-
Notifications
You must be signed in to change notification settings - Fork 62
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
fix(uuid): UUID regexes to support all-or-none '-' separator #113
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #113 +/- ##
==========================================
+ Coverage 81.48% 81.52% +0.03%
==========================================
Files 12 12
Lines 2031 2035 +4
==========================================
+ Hits 1655 1659 +4
Misses 297 297
Partials 79 79
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
wouldn't it be better to just try to parse the UUID at this point? |
Yes. I've looked at google's uuid Parsing and it sure is faster... The problem is that they support their own edge cases etc, so they do not recommend it for validation... So yes you're right. I've just leant toward the easy implementation path... I can give a shot to a more direct approach. I am just afraid that then people will ask for more of that kind of rewrite... |
@casualjim if we are okay to support Microsoft's proprietary extension to UUID, the faster, direct way is to use google/uuid.Parse and check the UUID's type. go/nogo? :) |
I vote for google/uuid. Seems like it's the most used uuid library for golang anyway. |
@casualjim OK. I'll do that today, it is not hard. TTYL. |
works as expected ~ x15-20 faster: goos: linux
goarch: amd64
pkg: github.com/go-openapi/strfmt
cpu: AMD Ryzen 7 5800X 8-Core Processor
BenchmarkIsUUID/IsUUID_-_google.uuid-16 532049923 22.62 ns/op 0 B/op 0 allocs/op
BenchmarkIsUUID/IsUUID_-_regexp-16 33907857 396.1 ns/op 0 B/op 0 allocs/op
BenchmarkIsUUID/IsUUIDv3_-_google.uuid-16 422999581 28.40 ns/op 0 B/op 0 allocs/op
BenchmarkIsUUID/IsUUIDv3_-_regexp-16 35107362 385.0 ns/op 0 B/op 0 allocs/op
BenchmarkIsUUID/IsUUIDv4_-_google.uuid-16 432093879 27.62 ns/op 0 B/op 0 allocs/op
BenchmarkIsUUID/IsUUIDv4_-_regexp-16 34144864 373.8 ns/op 0 B/op 0 allocs/op
BenchmarkIsUUID/IsUUIDv5_-_google.uuid-16 435730776 28.49 ns/op 0 B/op 0 allocs/op
BenchmarkIsUUID/IsUUIDv5_-_regexp-16 32326554 377.1 ns/op 0 B/op 0 allocs/op
PASS
ok github.com/go-openapi/strfmt 112.502s Further, it gives us the ability to extend easily to uuid v6 and v7. But that story shall also be told... |
This PR changes the UUID validation to support either UUIDs with the expected number of separators or no separator at all. Under the hood, UUID validation no longer relies on regular expressions (exported regexp patterns are marked as deprecated) but on github.com/google/uuid. This brings a significant performance improvement on validation (~ 15-20 times faster). Notice that some non-standard UUID schemes as well as UUID v6 and v7 now pass "IsUUID". * contributes go-swagger/go-swagger#2878 Signed-off-by: Frederic BIDON <[email protected]>
12fa10e
to
b556c74
Compare
* onboards strfmt fix (go-openapi/strfmt#113): replaced regexp-based uuid validations by google/uuid * fixes go-swagger#2878 Signed-off-by: Frederic BIDON <[email protected]>
* onboards strfmt fix (go-openapi/strfmt#113): replaced regexp-based uuid validations by google/uuid * fixes #2878 Signed-off-by: Frederic BIDON <[email protected]>
This PR changes the UUID validation to support either
UUIDs with the expected number of separators or no separator at all.
Under the hood, UUID validation no longer relies on regular expressions
(exported regexp patterns are marked as deprecated) but on
github.com/google/uuid. This brings a significant performance
improvement on validation (~ 15-20 times faster).
Notice that some non-standard UUID schemes as well as UUID v6 and v7
now pass "IsUUID".
Signed-off-by: Frederic BIDON [email protected]