fix: issue#728 pydantic_core._pydantic_core.Url object is not iterable

[tomohirohiratsuka]

Hi,
To fix #728 which causing below errors, I've added Url type in encoder.
I'd appreciate if you check it.
Thank you.

ValueError: [TypeError("'pydantic_core._pydantic_core.Url' object is not iterable"), TypeError('vars() argument must have __dict__ attribute')]

[gsakkis]

@roman-right before fixing this please merge #584. It's been open for 4 months and I already rebased and fixed the conflicts once, I probably won't bother again.

[roman-right]

Hi @tomohirohiratsuka
Sorry, I've produced a merge conflict here. Please let me know, if you can resolve it, otherwise I'll pick it up.

[tomohirohiratsuka]

@roman-right
Hi, I've resolved conflicts, please check it.
Thank you.

[roman-right]

Hey @tomohirohiratsuka ,
It looks like a few checks failed. Could you please take a look?

[roman-right]

I think this one is not available for Pydantic V1. Can you please use a condition with IS_PYDANTIC_V2?

[tomohirohiratsuka]

Thanks, I'll change.

[tomohirohiratsuka]

To make it conditional, changing Mapping to MutableMapping is acceptable ?

DEFAULT_CUSTOM_ENCODERS: MutableMapping[type, SingleArgCallable] = {
    ipaddress.IPv4Address: str,
    ipaddress.IPv4Interface: str,
    ipaddress.IPv4Network: str,
    ipaddress.IPv6Address: str,
    ipaddress.IPv6Interface: str,
    ipaddress.IPv6Network: str,
    pathlib.PurePath: str,
    pydantic.SecretBytes: pydantic.SecretBytes.get_secret_value,
    pydantic.SecretStr: pydantic.SecretStr.get_secret_value,
    datetime.timedelta: operator.methodcaller("total_seconds"),
    enum.Enum: operator.attrgetter("value"),
    Link: operator.attrgetter("ref"),
    bytes: bson.Binary,
    decimal.Decimal: bson.Decimal128,
    uuid.UUID: bson.Binary.from_uuid,
    re.Pattern: bson.Regex.from_native,
}
if IS_PYDANTIC_V2:
    from pydantic_core import Url
    DEFAULT_CUSTOM_ENCODERS[Url] = str

[tomohirohiratsuka]

@roman-right
Hi, I've fixed the test case though it is a not perfectly clean since it has if condition inside the test.
Since pydantic v1, v2 url transform logic is changed like below.
I've confirmed beanie can handle both v1 same as before, v2 to fix bug.

Could you check changes?

from pydantic import BaseModel, HttpUrl


class HttpUrlModel(BaseModel):
	url: HttpUrl

pydantic v1

from main import HttpUrlModel


def test_debug():

	model = HttpUrlModel(url="https://www.google.com")
	assert model.url == "https://www.google.com" # True
	assert isinstance(model.url, str) # True

pydantic v2

from pydantic_core import Url

from main import HttpUrlModel


def test_debug():

	model = HttpUrlModel(url="https://www.google.com")
	# assert model.url == "https://www.google.com" # This is not true in v2
	# assert isinstance(model.url, str) # This is not true in v2
	assert isinstance(model.url, Url) # This is true in v2, Url object is a new object in V2.

[roman-right]

It is better to use IS_PYDANTIC_V2 variable here

[tomohirohiratsuka]

Thanks, I'll use it.

[roman-right]

Please check, if Pydantic V1 has a class for URL inherited from str. I'll check it too

[tomohirohiratsuka]

@roman-right

Yes it has AnyUrl class.
Is it better to assert AnyUrl instead of str ?

def test_debug():

	model = HttpUrlModel(url="https://www.google.com")
	assert model.url == "https://www.google.com"
	assert isinstance(model.url, AnyUrl) # True since HttpUrl(AnyHttpUrl(AnyUrl(str)))

[roman-right]

Yes, it is better, as it will display, that it was deserialized.
Hm. Will not HttpUrl work for both cases here then?

[roman-right]

Or maybe I'm missing the point of the test

[tomohirohiratsuka]

Yes, it is better, as it will display, that it was deserialized.
Hm. Will not HttpUrl work for both cases here then?

No it won't work since v1 use class inheritance but v2 uses subscripted generics for HttpUrl.

v2 networks.py

AnyUrl = Url
"""A type that will accept any http or https URL."""
HttpUrl = Annotated[Url, UrlConstraints(max_length=2083, allowed_schemes=['http', 'https'])]

v2 _pydantic_core.py

class Url(object):

v1 networks.py

class AnyUrl(str):

But if we use AnyUrl as assertion it will pass both version without if condition though it hides the base class of instance.

async def test_url():
    doc = DocumentWithHttpUrlField(url_field="https://example.com")
    assert isinstance(doc.url_field, AnyUrl)
    await doc.save()

    new_doc = await DocumentWithHttpUrlField.find_one(
        DocumentWithHttpUrlField.id == doc.id
    )

    assert isinstance(new_doc.url_field, AnyUrl)
    assert new_doc.url_field == doc.url_field

I think this depends on beanie's policy what we should assert exactly.

[roman-right]

I think we have to be sure that it was encoded correctly. How it was decoded doesn't matter that much.
I'd check how the encoder encodes the document and would check if the output is correct. This way, we will be sure that it was stored in the db correctly. How it will be parsed is more Pydantic's responsibility. What do you think?

[tomohirohiratsuka]

Yes, I agree.
I've added a test for encoding.
Saving/Retrieving test case is remained to confirm it works in practice.

[roman-right]

@tomohirohiratsuka
Having different tests for different Pydantic versions is ok. I left a few comments, please take a look

[roman-right]

Merged! Thank you for the PR. It will be published tomorrow.