aj-foster / open-api-generator

Open API code generator for Elixir
MIT License
97 stars 13 forks source link

Module name is incorrect when the name as acronym word #39

Closed wingyplus closed 4 months ago

wingyplus commented 7 months ago

My module has acronym word like PymServerClient.PagerOfGPDailySummary. The library at revision 377c923c5597e76445124cb6af749cdf3456b4b8 working correctly. But when bumping to the lastest main (11ed134aa3b8f6ebc469496ba79fe6c2d3585dd9 at this point), the module name renders to PymServerClient.PagerOfGpdailySummary.

I assume it is probably because of the word GP that produces incorrect results.

aj-foster commented 6 months ago

Hello there! I did make some changes to the way module names are handled in https://github.com/aj-foster/open-api-generator/commit/8052f69108694baac656185acc0003a32f32f8b2 (the commit after the one that worked for you), however I haven't figured out what went wrong yet. Could you share an example OpenAPI spec that generated the issue?

Thanks!

xadhoom commented 5 months ago

Same here, before 1.0.0-rc* modules names was reflecting openapi spec, now they're lowercased+camelized.

A minimal schema that exposes this issue can be:

components:
  schemas:
    TOTPCreate:
      additionalProperties: false
      description: |
        Object for a TOTP token config, with confirmation OTP.
      properties:
        otp:
          description: The OTP as obtained from the authenticator app/device
          type: string
        uri:
          description: The OTP Auth URI obtained from `TOTPRequest`
          type: string
        verification_code:
          description: The opaque signature obtained from `TOTPRequest`
          type: string
      required:
        - uri
        - otp
        - verification_code
      title: TOTPCreate
      type: object
info:
  title: Acme API
  version: '1'
openapi: 3.0.0
paths:
  /api/v1/me/totp:
    post:
      callbacks: {}
      description: |
        Saves a config obtained with `TOTPRequest`, along with a device generated
        OTP to confirm it's correctness.
      operationId: Controllers.ApiV1.TOTP.totp_save
      parameters: []
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/TOTPCreate'
        description: The TOTP config to save
        required: false
      responses:
        '201':
          description: TOTP request config saved
        '429':
          description: Too many requests, retry later
      summary: Saves a TOTP config
      tags: []
servers:
  - url: '{endpoint_url}'
    variables:
      endpoint_url:
        default: https://localhost:5443/

TOTPCreate is rendered as Totpcreate after library update.

xadhoom commented 5 months ago

I've tried playing a bit with the fun normalize_identifier/1 to allow sequences of uppercases like the following:

  def normalize_identifier(input) do
    input
    |> String.split(~r/(([^A-Za-z0-9]+)?([A-Z0-9]+(?![a-z0-9])))|(([^A-Za-z0-9]+)?([A-Z]+)?([a-z0-9]+)?)/, include_captures: true, trim: true)
    |> Enum.map_join("_", fn segment ->
      segment
      |> String.replace(~r/^[^A-Za-z]+/, "")
      |> String.replace(~r/[^A-Za-z0-9]+$/, "")
      |> maybe_downcase_segment()
    end)
  end

  defp maybe_downcase_segment(segment) do
    case String.upcase(segment)  do
      ^segment -> segment
      _ -> String.downcase(segment)
    end
  end

Which translates "PymServerClient.PagerOfGPDailySummary" in:

iex(1)> normalize_identifier("PymServerClient.PagerOfGPDailySummary")|>Macro.camelize()
"PymServerClientPagerOf_GPDailySummary"

and "TOTPRequest" in:

iex(1)> normalize_identifier("TOTPRequest")|>Macro.camelize()
"TOTPRequest"

But there're corner cases where uppers are at the end of the name:

iex(1)> normalize_identifier("RequestTOTP")|>Macro.camelize()
"Request_TOTP"

iex(2)> normalize_identifier("RequestTOTPCreate")|>Macro.camelize
"Request_TOTPCreate"

It's a bit funny, given the interaction with Macro.camelize()

So I don't have a real solution, except maybe to avoid the same normalization for all tokens? and use different approaches for different parts of the schema?

aj-foster commented 5 months ago

Thank you for the nice test case, and thank you for exploring some variations in the normalization. PR #43 has a possible solution, if you'd like to try it out. Let me know how it works for you.

A lot of information was being lost thanks to the String.downcase call in the normalization. Now, instead of always normalizing to lowercase snake_case, callers now have their choice of casing. The default is snake case for compatibility.

wingyplus commented 5 months ago

I forgot to reply to you. I'm sorry. 🙇

And thank you for a quick fix. Will try it in 1 or 2 days.

flaviogrossi commented 4 months ago

I had the same problem after the update, and i can confirm that the linked PR creates names compatible with the old version in my case

aj-foster commented 4 months ago

The fix from #43 is now available in version 0.1.0. ❤️