Python|Ruby - QueryParametersMapper not getting generated

[andreaTP]

I "think" this is the root cause of this issue:

Apicurio/apicurio-registry#3465

We have an OpenAPI spec file that defines a QueryParameter with a field named ifExists, the mangled name in the corresponding dataclass is therefore if_exists.

I found that, in theory, there should be a generated Mapper, but I cannot find it in the generated sources 😞 , nor I can find any evidence of the generation of the get_query_parameter (which, I believe, should return the mapping) that is in the abstractions library.

I'm looking for some hint on how this is supposed to work all together 🙂 and what should be fixed to have it working as expected.

cc. @baywet

[andreaTP]

I'm looking closer at this issue, so far I found:

• QueryParameters have been "excluded" from the name mangling in the first place, but the Python Writers are eagerly calling .ToSnakeCase() - I think that this approach might generate confusion and breaks the separation of concerns as all the other languages names mangling is performed in the Refiners - but it's not the "real" issue
• I think that the generated method QueryParametersMapper is never written for all of the languages (at least the same issue is present in Ruby e.g.)

[baywet]

Thanks for reporting this bug and for already starting a PR about it by the time I got to this notification 😅

[andreaTP]

Quick follow-up, I was thinking twice about this:

the Python Writers are eagerly calling .ToSnakeCase() - I think that this approach might generate confusion and breaks the separation of concerns as all the other languages' names mangling is performed in the Refiners

and I believe that this is a design issue, some of the Refiners phases are taking into consideration the fact that a name is escaped or not but here we are breaking the assumption.
Applying the manipulation in the Writers results in logic to be executed that, I believe, should happen in the Refiner.

wdyt @baywet ?

[baywet]

I think you're right. Normalization should happen before the writers. We've had so many issues caused by the fact two writers were not normalizing the same way and references being broken.
The thing I'm a bit worried about is how this will play with the convention service and scalar types.
But yeah if you want to explore building a generic normalizing function in the common refiners and remove normalization from writers, I'd support it.

Long term will need to enable concurrency for the refiners as they have been the performance bottleneck for a while now. If we enabled that without causing race conditions we'd unblock significant performance gains...

[andreaTP]

Opened #2842 for tracking the normalization issue.

Regarding:

Long term will need to enable concurrency for the refiners as they have been the performance bottleneck for a while now. If we enabled that without causing race conditions we'd unblock significant performance gains...

at the moment I think that this is a long long shot, I would not even attempt to go there until #2442 is fixed.