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 (https://github.com/pydantic/pydantic/pull/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 https://github.com/mike-oakley/openapi-pydantic/pull/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#10692https://github.com/pydantic/pydantic/pull/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 GitHubhttps://github.com/mike-oakley/openapi-pydantic/pull/50#issuecomment-2514178252, or unsubscribehttps://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.