Adds UUID Support #989

kamronbatman · 2016-09-20T21:59:13Z

Adds UUID version 1 through 5 support. Based on #970.

kamronbatman · 2016-09-20T23:45:01Z

I added proper bracket support, bracket/separator match support and support for options in the form of { versions: ['uuidv1','uuidv2','uuidv3','uuidv4','uuidv5'] }.
If no options are specified then it will validate exactly like the previous GUID check where brackets and separators are optional.

Help with testing would be appreciated! Thanks!

eight-molecules · 2016-09-22T13:14:36Z

In your tests, did you generate those UUIDs or copy a single one and modify its versioning? This comes up because doing so means you may be validating out-of-spec UUIDs (Mostly thinking about v1 and v2 since they rely on semi-fixed seed data).

DavidTPate · 2016-09-22T14:01:20Z

From what I'm seeing within RFC 4122 it wouldn't matter how these are generated as they all validate to the same style (with the exception of the version number).

Here's the ABNF from page 4:

      UUID                   = time-low "-" time-mid "-"
                               time-high-and-version "-"
                               clock-seq-and-reserved
                               clock-seq-low "-" node
      time-low               = 4hexOctet
      time-mid               = 2hexOctet
      time-high-and-version  = 2hexOctet
      clock-seq-and-reserved = hexOctet
      clock-seq-low          = hexOctet
      node                   = 6hexOctet
      hexOctet               = hexDigit hexDigit
      hexDigit =
            "0" / "1" / "2" / "3" / "4" / "5" / "6" / "7" / "8" / "9" /
            "a" / "b" / "c" / "d" / "e" / "f" /
            "A" / "B" / "C" / "D" / "E" / "F"

Based on that document the validation of the output doesn't change from what I'm seeing but how that output is generated (ie. with MAC address, timestamps, etc.) is what changes. They still come out to the same format (with the exception of the version number).

DavidTPate · 2016-09-22T14:37:48Z

lib/string.js

+            }
+
+            // Matching braces
+            if (brackets[results[1]] !== results[11]) {


I'm not sure if they are eluding me or what but I'm not seeing any tests for mismatched braces.

DavidTPate · 2016-09-22T14:38:04Z

lib/string.js

            }

-            return this.createError('string.guid', { value }, state, options);
+            // Matching separators
+            if (results[3] !== results[5] || results[3] !== results[7] || results[3] !== results[9]) {


I wasn't able to find any tests for mismatches separators (not sure if they are hiding from me).

kamronbatman · 2016-09-22T17:58:44Z

test/string.js

@@ -2983,7 +2983,74 @@ describe('string', () => {
                ['b4b2fb69c6244e5eb0698e0c6ec66618', true],
                ['{283B67B2-430F-4E6F-97E6-19041992-C1B0}', false, null, '"value" must be a valid GUID'],
                ['{D1A5279D-B27D-4CD4-A05E-EFDD53D08E8D', false, null, '"value" must be a valid GUID'],
-                ['D1A5279D-B27D-4CD4-A05E-EFDD53D08E8D}', false, null, '"value" must be a valid GUID']
+                ['D1A5279D-B27D-4CD4-A05E-EFDD53D08E8D}', false, null, '"value" must be a valid GUID'],


These are the tests I added for mismatched braces and separators.
The existing test above this (line 2985) is also a test for that.

Do you have tests for hyphenated UUIDs without brackets? These are technically valid also, though they require all hyphens to be included.

Sorry, @kamronbatman I was meaning ones like [D1A5279D-B27D-4CD4-A05E-EFDD53D08E8D} where the braces don't match. I definitely saw the ones your referenced above.

@gelliott181 There is already a test for that: line 2978. I could add them to each of the UUID versions, but it would technically be duplicative because of how the code is written. Let me know what you think.

@DavidTPate I see what you are saying. I can definitely add them in but based on how the code was written it is duplicatable. No brackets mixed with brackets are considered the same as mixing other types of brackets (parenths, braces, etc).

I would expect the tests to verify that the functionality is working as expected, not that the code is working the way it was written.

Ok, fair enough.

@gelliott181 and @DavidTPate I updated/added more tests. Let me know what you two think.

Thanks!

DavidTPate · 2016-09-23T17:38:18Z

LGTM

kamronbatman · 2016-09-23T17:47:24Z

@DavidTPate I added support for string value of the versions option. Also updated the API reference.

kamronbatman · 2016-09-23T17:49:11Z

@Marsup I think this might be ready to merge! 🎉

kamronbatman · 2016-09-23T21:34:44Z

@DavidTPate @gelliott181 and @Marsup - I updated the options, added assertions, and segregated the tests to follow in the way other Joi features were written. Should still be good in terms of tests/coverage.

Marsup · 2016-09-27T21:47:51Z

lib/string.js

+            versions = Hoek.unique(versions);
+        }
+
+        const regex = /^([\[{\(]?)([0-9A-F]{8})([:-]?)([0-9A-F]{4})([:-]?)([0-9A-F]{4})([:-]?)([0-9A-F]{4})([:-]?)([0-9A-F]{12})([\]}\)]?)$/i;

        return this._test('guid', undefined, function (value, state, options) {


Please add the options here instead of leaving undefined.

I think I fixed it appropriately. Let me know.

Marsup · 2016-09-27T21:58:34Z

lib/string.js

+            'uuidv5': '5'
+        };
+
+        let versions;


I'd rather have a const here and check for length later.

I was following line 186. Not sure what you would suggest otherwise.

Oh I see, I guess the code can't always be consistent everywhere :)

ok, so are you thinking to move the checks to inside of the test callback? I am sorry for being confused on your thoughts.

I was suggesting a const versions = [], but I guess we won't be able to do the unique then, I don't think it matters much.

I think I changed it to what you were asking. Let me know if that works and I will resquash.

Marsup · 2016-09-28T21:44:23Z

Are you able to squash all those commits as well ?

kamronbatman · 2016-09-28T21:58:01Z

I just squashed it. And yes, I did not make a const because of the unique check.

DavidTPate · 2016-09-29T13:24:20Z

@Marsup Not sure if you know about it or just prefer it be done differently but wanted to point out that you can now squash on merge for a PR in GitHub (you can also rebase too as of this week prior to merge). You can see the announcement for more details.

Marsup · 2016-09-29T13:40:23Z

I know about it, they did it wrong. I want squashed commits but I still want the merge commit, none of the options work for me.

DavidTPate · 2016-09-29T13:50:56Z

Roger that, @Marsup it definitely doesn't add a merge commit like you point out. Just wanted to see if it would work for you.

Marsup · 2016-09-29T17:16:58Z

Thanks a lot for the work y'all, I'll release in a bit as I need to add another small thing to it.

eight-molecules · 2016-09-29T17:34:27Z

Nice PR @kamronbatman, thanks for taking my minor change so much further!

kamronbatman mentioned this pull request Sep 20, 2016

Alias String.guid() with String.uuid() #970

Closed

DavidTPate reviewed Sep 22, 2016

View reviewed changes

kamronbatman commented Sep 22, 2016

View reviewed changes

Marsup requested changes Sep 27, 2016

View reviewed changes

Adds UUID support to Joi

7371add

kamronbatman added 2 commits September 28, 2016 16:16

[Fix] Fixes version array and changes it to a const.

20caf4b

[Fix] Fixes UUID version array length check.

fcac4ce

Marsup added the feature New functionality or improvement label Sep 29, 2016

Marsup self-assigned this Sep 29, 2016

Marsup merged commit 1dd3d94 into hapijs:master Sep 29, 2016

Marsup added this to the 9.1.0 milestone Sep 29, 2016

shockey mentioned this pull request Oct 3, 2019

Validation of uuid without dashes swagger-api/swagger-ui#5281

Closed

kamronbatman deleted the uuid branch January 9, 2020 08:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds UUID Support #989

Adds UUID Support #989

kamronbatman commented Sep 20, 2016 •

edited

Loading

kamronbatman commented Sep 20, 2016

eight-molecules commented Sep 22, 2016

DavidTPate commented Sep 22, 2016 •

edited

Loading

DavidTPate Sep 22, 2016

DavidTPate Sep 22, 2016

kamronbatman Sep 22, 2016

eight-molecules Sep 22, 2016

DavidTPate Sep 22, 2016

kamronbatman Sep 23, 2016 •

edited

Loading

DavidTPate Sep 23, 2016

kamronbatman Sep 23, 2016

kamronbatman Sep 23, 2016

DavidTPate commented Sep 23, 2016

kamronbatman commented Sep 23, 2016

kamronbatman commented Sep 23, 2016

kamronbatman commented Sep 23, 2016 •

edited

Loading

Marsup Sep 27, 2016

kamronbatman Sep 28, 2016

Marsup Sep 27, 2016

kamronbatman Sep 28, 2016

Marsup Sep 28, 2016

kamronbatman Sep 28, 2016

Marsup Sep 28, 2016

kamronbatman Sep 28, 2016

Marsup commented Sep 28, 2016

kamronbatman commented Sep 28, 2016

DavidTPate commented Sep 29, 2016

Marsup commented Sep 29, 2016 •

edited

Loading

DavidTPate commented Sep 29, 2016

Marsup commented Sep 29, 2016

eight-molecules commented Sep 29, 2016

Adds UUID Support #989

Adds UUID Support #989

Conversation

kamronbatman commented Sep 20, 2016 • edited Loading

kamronbatman commented Sep 20, 2016

eight-molecules commented Sep 22, 2016

DavidTPate commented Sep 22, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamronbatman Sep 23, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidTPate commented Sep 23, 2016

kamronbatman commented Sep 23, 2016

kamronbatman commented Sep 23, 2016

kamronbatman commented Sep 23, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Marsup commented Sep 28, 2016

kamronbatman commented Sep 28, 2016

DavidTPate commented Sep 29, 2016

Marsup commented Sep 29, 2016 • edited Loading

DavidTPate commented Sep 29, 2016

Marsup commented Sep 29, 2016

eight-molecules commented Sep 29, 2016

kamronbatman commented Sep 20, 2016 •

edited

Loading

DavidTPate commented Sep 22, 2016 •

edited

Loading

kamronbatman Sep 23, 2016 •

edited

Loading

kamronbatman commented Sep 23, 2016 •

edited

Loading

Marsup commented Sep 29, 2016 •

edited

Loading