fix: replace invalid characters in $ref field with underscore.

[jfschneider]

used the regex from pydantic.json_schema.GenerateJSONSchema.normalize_name to replace all characters that are invalid reference names.

[mike-oakley]

Thanks @jfschneider! Would you mind adding a comment explaining the logic too, and a link to the pydantic source? Would also be great if you could add a test or two to cover this case 🙏🏼

[jfschneider]

Hi, sure, I can provide a link and an explanation. I also just noticed that there is a duplicate of the util.py in the v3_0 folder, so I probably should duplicate the fix there, too, and then put the tests in tests/util/test_utily.py and tests/v3_0/test_utily.py?

[jfschneider]

OK, that was a bit premature. I had a test that works, but just for either pydantic v2 orv1 (basically copied a test in util.py and just used a generic as response), not v1, which seems to use a different method for generic models:
v2: class GenericResponse(BaseModel, Generic[DataT])
vs
v1: class GenericResponse(GenericModel, Generic[DataT])
I admit I am a bit out of my depth here, I have no idea how I can include both variants in the test, except maybe some "if PYDANTIC_V2" conditions...?

[mike-oakley]

Yep exactly that, you can use the PYDANTIC_V2 flag to constrain certain tests to pydantic V1 or v2 if that helps seperate concerns/fixtures like here

[jfschneider]

Sooo, after a lot of trial and error, I managed to cobble up full tests for pydantic v1/v2. The tox run still fails, but not in my tests ("tests/util/test_pydantic_field.py:72: AssertionError"), and that also fails in a clean clone of the original repo, so I gather it has nothing to do with my changes...

I briefly thought about fixing the _construct_ref_obj function by instantiating the Pydantic GenerateJSONschema-class and calling the normalize_name-method, but decided against it as those classes are again different in Pydantic V1/V2 which would've required some further if-else and bloated code, so I just copied the regex from the normalize_name method, as I think it is rather unlikely to change often. It just replaces invalid characters for refnames with an underscore.
https://github.com/pydantic/pydantic/blob/aee6057378ccfec02126bf9c984a9b6d6b411777/pydantic/json_schema.py#L2031

[mike-oakley]

Ah yes, looks like the break was introduced in a recent Pydantic update (pydantic/pydantic#10692) - i'll address this separately 👍🏼

Could you add the comment with source/explanation however for the regex logic? 🙏🏼

[mike-oakley]

Test is fixed in #52 - if you rebase to main you should pick it up 👍🏼

[jfschneider]

Ah yes, looks like the break was introduced in a recent Pydantic update (pydantic/pydantic#10692) - i'll address this separately 👍🏼

Could you add the comment with source/explanation however for the regex logic? 🙏🏼

Do you mean add a comment in the function itself?

[mike-oakley]

Yep exactly, something inline by the regex to explain it a bit for future readers

…

________________________________ From: jfschneider ***@***.***> Sent: Tuesday, December 3, 2024 11:40:14 AM To: mike-oakley/openapi-pydantic ***@***.***> Cc: Mike Oakley ***@***.***>; Comment ***@***.***> Subject: Re: [mike-oakley/openapi-pydantic] fix: replace invalid characters in $ref field with underscore. (PR #50) Ah yes, looks like the break was introduced in a recent Pydantic update (pydantic/pydantic#10692<pydantic/pydantic#10692>) - i'll address this separately 👍🏼 Could you add the comment with source/explanation however for the regex logic? 🙏🏼 Do you mean add a comment in the function itself? — Reply to this email directly, view it on GitHub<#50 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AENE36D3U6XQEIESHW4BIET2DWDA5AVCNFSM6AAAAABRNCDE6OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMJUGE3TQMRVGI>. You are receiving this because you commented.Message ID: ***@***.***>

[jfschneider]

OK, added a docstring with an explanation. Tests were fine now.