Skip to content

Conversation

mykulyak
Copy link
Contributor

@mykulyak mykulyak commented Nov 4, 2022

Motivation and Context

I'd like to demonstrate our BE developers how generated Elixir code looks like.

Type of changes

What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Details

Code was generated out of examples/openapi-codegen-test-api.yaml schema using examples/codegen/run.sh script.

To regenerate the code by yourself, install Node >= 16.x on your machine, and then install @fresha/openapi-codegen-cli

$ npm install -g @fresha/openapi-codegen-cli

Usage

  1. Add the following code to the router.ex.
#
# ROUTER
# In your router file, add the following lines
#
scope "/api", ElixirApiWeb do
  pipe_through :api

  resources("/blocked-times", BlockedTimesController, only: [:index, :create, :update, :delete])
  resources("/blocked-times/export", BlockedTimesExportController, only: [:create, :show])
end
  1. replace in examples/codegen/elixir_api/lib/elixir_api_web/views/blocked_times_view.ex lines
def render("index.json-api", %{}) do
    %Document{
      data: [],

with

def render("index.json-api", %{blocked_times: blocked_times}) do
  %Document{
    data: Enum.map(blocked_time, &ElixirApiWeb.BlockedTimeResource.build/1,
  1. replace in examples/codegen/elixir_api/lib/elixir_api_web/controllers/blocked_times_controller.ex lines
      # TODO evaluate extra arguments, then pass them to render()
      render(conn)

with

      # TODO evaluate extra arguments, then pass them to render()
      render(conn, ElixirApi.Queries.BlockedTimesQuery.fetch_all())

Now, you should see list of blocked time resources returned.

@mykulyak mykulyak added the documentation Improvements or additions to documentation label Nov 4, 2022
@mykulyak mykulyak added this to the OpenAPI codegen improvements milestone Nov 4, 2022
@mykulyak mykulyak self-assigned this Nov 4, 2022
@mykulyak mykulyak added the wontfix This will not be worked on label Nov 4, 2022
Copy link
Contributor

@JerzyDziala JerzyDziala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic work!

alias ElixirApiWeb.EmployeesResource
alias ElixirApiWeb.LocationsResource

def build(config) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to also generate typespecs for the resources


# add aliases here

def create(conn, _params) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great start!

One obvious issue I'm seeing is that after the developer implements the missing parts they can't regenerate the thing without undoing their work.

The answer to this is ofc better modularity

We should make the thing modular enough that The controller code is actually dumb enough to not need any manual changes ever. That would require us to agree on some conventions as to what the code in the controller action should look like and what should it call into and what the error handling should look like.

same thing is obiously true about the views, the tests, the router, etc.
There are many ways to achieve this separation between generated and nongenerated code.
It's not ease to think of optimal patterns for this from the top of my head.

Another thing that comes to my mind when trying to think of the patterns is that
if we make the generator smart enough to know that If an action/test/view already exists, then It should not regenerate it then it would be much easier to make the separation natural and not requiring splitting things into many files. Wdyt? I know it's not easy to make a foolproof mechanism like that without actually parsing the code (or using mysterious nonstandard extended regexes), so it might not be worth to introduce this complexity.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to get the most value from the spec->code workflow then we have to enable painfree, nondestructive generation. I'm stating the obvious here - generation of a new endpoint should not destroy a whole controller, etc

@@ -0,0 +1,32 @@
defmodule ElixirApiWeb.BlockedTimesControllerTest do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's important to think about the 3 layers of tests that we have here to make sure we're not testing the shape of the data multiple times needlesly. The resource tests are generated. The view tests should be very dumb (are they even necessery?)

In the controller tests we could use some openApi magic to verify the responses. Also If we make the controllers dumb enough to not need any human changes then the tests could probably be fully generated too.

Then we would test logic in the core modules, and the controllers, resources data shapes would all come from openapi

@mykulyak mykulyak removed this from the OpenAPI codegen improvements milestone Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants