add UUID1, UUID3, UUID4 and UUID5 types

[Gr1N]

No description provided.

[codecov]

Codecov Report

Merging #167 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #167   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           9      9           
  Lines         938    950   +12     
  Branches      206    207    +1     
=====================================
+ Hits          938    950   +12

[samuelcolvin]

Given that there are only 4 times, should it would be cleaner to just have 4 tyes: Uuid1, Uuid2, ...

Rather than having to implement conuuid

[Gr1N]

You're right!

[Gr1N]

@samuelcolvin, updated, please review

[samuelcolvin]

Looking good, but I think we can make it simpler.

[samuelcolvin]

I think adding this is unnecessarily confusing. The 5 types UUID, UUID1, UUID3, UUID4 and UUID5 are sufficient.

[samuelcolvin]

Then you can remove the duplicated logic in ConstrainedUUID and uuid_version_validator

[Gr1N]

Done

[samuelcolvin]

It'll be much clearer to read if you show how these were generated, eg.

uuid=uuid.uuid4(),
uuid_v1=uuid.uuid1(),
....
uuid_v=uuid.uuid5(uuid.NAMESPACE_DNS, 'example.com')

[samuelcolvin]

same in tests.

[Gr1N]

Done

[samuelcolvin]

ConstrainedUUID could lead to confusion, I think we can remove it altogether.

I think best to just have

class UUID1(UUID):
    _required_version = 1

...

Then change uuid_validator:

def uuid_validator(v) -> UUID:
    if isinstance(v, str):
        v = UUID(v)
    elif isinstance(v, (bytes, bytearray)):
        v = UUID(v.decode())
    elif not isinstance(v, UUID):
        raise ValueError(f'str, byte or native UUID type expected not {type(v)}')
    required_version = getattr(v, '_required_version', None)
    if required_version and required_version != v.version:
        ...
    return v

[Gr1N]

This is a good idea, but I think this is impossible to implement (or I don't know how :). As I see in my tests in uuid_validator v is instance of UUID type, not UUID1 or UUID2...

Validator implementation:

def uuid_validator(v) -> UUID:
    if isinstance(v, str):
        v = UUID(v)
    elif isinstance(v, (bytes, bytearray)):
        v = UUID(v.decode())
    elif not isinstance(v, UUID):
        raise ValueError(f'str, byte or native UUID type expected not {type(v)}')

    required_version = getattr(v, '_required_version', None)
    if required_version and v.version != required_version:
        raise ValueError(f'uuid version {required_version} expected, not {v.version}')

    return v

And test:

class UUIDModel(BaseModel):
    a: UUID1
    b: UUID3
    c: UUID4
    d: UUID5


def test_uuid_validation():
    a = uuid.uuid1()
    b = uuid.uuid3(uuid.NAMESPACE_DNS, 'python.org')
    c = uuid.uuid4()
    d = uuid.uuid5(uuid.NAMESPACE_DNS, 'python.org')

    m = UUIDModel(a=a, b=b, c=c, d=d)
    assert m.dict() == {
        'a': a,
        'b': b,
        'c': c,
        'd': d,
    }

    with pytest.raises(ValidationError) as exc_info:
        UUIDModel(a=d, b=c, c=b, d=a)
    assert exc_info.value.message == '4 errors validating input'

[Gr1N]

Done! :)

[Gr1N]

@samuelcolvin please review again!

[samuelcolvin]

Looks good, thank you very much. Sorry for the mistake in my comment code!