[Elixir] add AnyType support

[paulbalomiri]

Adds AnyType support, See also:

• Elixir generator AnyType support #14070

This work builds on the last changes made by @halostatue.

Pings

• Template creator: @niku
• Tech Com. Elixir @mrmstn

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master (6.3.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

[halostatue]

Extra whitespace.

[halostatue]

I’m not sure why this change was made. Yes, it saves two function calls, but it also means that it is harder to extract Tesla from the execution path in favour of https://github.com/wojtekmach/req.

(A different set of changes that I’m going to be looking at will be to replace the structs with validated maps, because there’s a lot of atoms generated in a sufficiently complex API.)

[paulbalomiri]

I can revert this one. It helped me with debugging the generated code.

It shows from the get-go that the pipe inputs/outputs are a Tesla.Env.
I think a good way forward with Tesla would be to just compose a middleware stack instead of piping, which cannot be altered at runtime.

I don't have experience with req, but I can look into it if you are interested in also having continued support for Tesla.

(A different set of changes that I’m going to be looking at will be to replace the structs with validated maps, because there’s a lot of atoms generated in a sufficiently complex API.)

Do you want to completely do away with structs?
I concur with you on not creating atoms to the extent that they should not be generated from input (e.g. for values supplied in function calls).

Here, on the other hand, we do have a finite number of atoms, as each struct generates only it's fields' descriptors as atoms at compile time.

So IMHO this is not a case of unbounded atom generation. Ex/Beam is good at handling lots of atoms, their number should just not be unbounded or dependent on runtime and unsafe input.

Utility module for assemling middelwares ?

If you want I can look at creating a utility Module to handle a list of middlewares instead of the current approach. The Api functions would then only use this utility class to insert the necessary middlewares to set the url, params e.t.c.

This way the utility Module would allow for runtime transformations, even outside the spec (e.g. caching, delegation to other nodes e.t.c. from client libraries depending on the generated dep).

[mrmstn]

I'm not quite sure about this discussion.

Since it's not related to the implementation of AnyType, it would be great to bring this concern into an issue. This would also allow the discussion a more retractable character.

For the change itself: (by itself, I just mean the line without the changing-possibility and debugability it could bring)

I can't see the additional value it brings for breaking the style of composing/piping, and therefore I can't just let it through.

I would appreciate it if you could extract this change into a different branch and start the discussion with an issue. You could also link the new branch in the form of a PR, so it doesn't get lost.

[paulbalomiri]

Ok, will come back with a new pr, on this
EDIT
cned here: #14070 (comment)

[halostatue]

My comments on removing atoms are completely orthogonal to this discussion, just brought up as a point (I’ve hit a very large OpenAPI spec that makes a lot more than I’m comfortable with, but yes—I want to explore how structs get handled).

[halostatue]

Given that Tesla headers (like Phoenix headers) are supposed to be all lowercase, we may want something a little more robust than just Kernel.to_string/1.

[paulbalomiri]

Agreed, also because Http header field names are case insensitive

key
|> to_string()
|> String.downcase()

Would this be sufficient, or do you mean something different by "robust" ?

[halostatue]

It’d be a start, yes, and a good one.

[mrmstn]

Funny enough, this function call is actually wrong and Tesla.put_header must not be used here anyway.

The function should look something like this:

  def add_param(request, :headers, key, value) do
    headers =
      request
      |> Map.get(:headers, [])
      |> List.keystore(key, 0, {key, value})

    Map.put(request, :headers, headers)
  end

Secondly, I wouldn't change cases as the server should/would do this to respect the case insensitivity

[paulbalomiri]

TLDR

I promised to come back, only to now postpone 2 more weeks. Shall we leave this open, or shall i create new pr's and retire this PR ?
This is still a blocker for my release & i don't want to keep fork around and release [my software], so i'll definitely come back on this.

New PR proposal

I'd like to address these topics in separate or in one PR:

• Anytype support
• Header bugfix, using camel_cased :atoms not :Atom for headers
• Supply some generated usage example in Readme.md using options EDIT: showcasing the supplied generator options for documentation.

Tesla configurability

Separately I'd like to use exposed middlewares in Tesla, because I think that is what Tesla's composability is about, instead of creating and running the client in it's api functions, but we'll have to decide whether that is a custom generator, an alternative generator in this repo or whether we come to the conclusion that req and tesla should be configurable settings of the same generator.

The Api implementation should just compose a list of middlewares, applied to a user supplied (and defaulted) %Tesla.Env{}
deserialisation e.t.c. should be implemented via middleware composition (e.g. one per field).
This way the lib user could also use non-spec ways of modifying the calls (authentication, session headers, caching e.t.c), while OFC the default is to run according to spec

[halostatue]

With respect to the Tesla configurability, I’d need to see more of what you think. We may be thinking similarly, but I have absolutely shifted my thinking on Tesla away from use Tesla with plug macros and instead using runtime client building. In my opinion, this item should absolutely be opened as a discussion / discussion ticket, because there are several modernizations to the Elixir code base that should be done beyond the work that I did earlier this year and may not have time to get to myself.

[wing328]

@halostatue can you please PM me via Slack when you've time in the coming week? Thanks.

slack: https://join.slack.com/t/openapi-generator/shared_invite/zt-12jxxd7p2-XUeQM~4pzsU9x~eGLQqX2g

[mrmstn]

Hi @paulbalomiri

Thank you very much for your time and your patience. Furthermore, a big thanks to @halostatue for the review as well.

I'm not quite sure about the approached way to handle the AnyType issue. Maybe it would make more sense to mark AnyType as a languageSpecificPrimitives (because it doesn't need a generated Module). This could also be something similarly to the handling of map()

This might could also take care of the extra noop lines, like https://github.com/OpenAPITools/openapi-generator/pull/14071/files#diff-7b5b5223df80c008d557051bc8255b5d9db965e773b52e959eee7c69ec5a6bf9R25 and would not generate unused modules (like SingleRefType in the example)

What are your thoughts on that? Did you already look into that?

[paulbalomiri]

Yes, as i commented before i need to postpone this for 2 weeks (1 week vacation, then i want to create a POC). I Intend to create a new generator regarding this, as a fork from yours, but independent. Then we'll have a talk what to do with it (custom template, same generator, new generator as PR, or a separate custom generator)   On 2022-11-29 10:13:56, Austin Ziegler ***@***.***> wrote: @halostatue commented on this pull request. In modules/openapi-generator/src/main/resources/elixir/request_builder.ex.mustache [#14071 (comment)]:

@@ -101,7 +101,7 @@ defmodule {{moduleName}}.RequestBuilder do end def add_param(request, :headers, key, value) do - Tesla.put_header(request, key, value) + Tesla.put_header(request, to_string(key), value)

With respect to the Tesla configurability, I’d need to see more of what you think. We may be thinking similarly, but I have absolutely shifted my thinking on Tesla away from use Tesla with plug macros and instead using runtime client building. In my opinion, this item should absolutely be opened as a discussion / discussion ticket, because there are several modernizations to the Elixir code base that should be done beyond the work that I did earlier this year and may not have time to get to myself. — Reply to this email directly, view it on GitHub [#14071 (comment)], or unsubscribe [https://github.com/notifications/unsubscribe-auth/AABSRCFAEQSTBYWZDBA67OTWKW3MJANCNFSM6AAAAAASFOLLLA]. You are receiving this because you were mentioned.Message ID: ***@***.***> [ef28f2f4-cebc-4a7d-ac33-6597fbca09fe]