Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal (needs some cleaning up) for uniquely naming codecs #60

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

clintonmead
Copy link

I occasionally have Autodocodec definitions like this:

instance (Autodocodec.HasCodec a) => Autodocodec.HasCodec (Alice a) where
  codec = Autodocodec.object "Alice" Autodocodec.objectCodec

instance (Autodocodec.HasCodec a) => Autodocodec.HasObjectCodec (Alice a) where
  objectCodec = ...

deriving via Autodocodec (Alice a) instance (Autodocodec.HasCodec a) => ToSchema (Alice a)

but the problem is, each one of those schemas is going to have the name "Alice". That is, Alice Int is going to have the same schema name as Alice Char, namely they're both going to be called "Alice".

So then I use servant-openapi to generate the OpenAPI JSON.

But the problem is, if seems like servant-openapi gets two types with the same name, it just overwrites the latter with the former, which means all the references to the former are replaced by the latter, resulting in the wrong schema definitions and any generated code from this OpenAPI not matching the ToJSON/FromJSON serialiser definitions.

Admittedly I think this behaviour of servant-openapi just silently ignoring and overwriting clashing names is far from ideal, but in anycase, even if servant-openapi did behaviour better in these cases and actually error, one would still need to differentiate between these different types.

So what resolve this issue is something like this:

instance (Autodocodec.HasCodec a) => Autodocodec.HasCodec (Alice a) where
  codec = Autodocodec.object ("Alice" <> nameOfHasCodec @a) Autodocodec.objectCodec

(note the nameOfHasCodec @a)

in this PR I have given a proposed implementation of nameOfCodec, and nameOfHasCodec, which takes a type parameter instead of an actual codec. What I'm looking for is:

  1. For name clashes to be mostly avoided (I don't mind if they're still technically possible, as long as they tend not to occur unless you seek them out/do something weird).
  2. For explicitly given names to codecs (like via object) to be respected.
  3. For names to be at least somewhat human readable.

So I've came up with a quick implementation in this PR. The actual naming convention is customisable via a record, but I've given an example convention. I imagine in most real scenarios one will find an ObjectOfCodec or ReferenceCodec somewhere in the codec chain which will give an explicit name, but we need to cover all cases. Also in these cases laziness means we don't even calculate a generated name (as fromMaybe is lazy).

Let me know what you think, and whether you think sort of thing is appropriate for the Autodocodec library, or whether you'd rather have it separate? I know this PR needs some work but I'd just thought I'd sound you out on your thoughts before going further.

@NorfairKing
Copy link
Owner

@clintonmead I'll need to have a closer look later, but at first glance I think this may be better suited for autodocodec-openapi than for autodocodec.

@clintonmead
Copy link
Author

@NorfairKing you may be right. No rush, I probably won't have time for at least a few weeks to do some serious work on it, but I thought I throw it up when the idea was fresh in my head.

@NorfairKing
Copy link
Owner

nix-ci run

Copy link

nix-ci-app bot commented Feb 11, 2025

NixCI started a suite, more details are available here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants