@doc attributes stripped by mix releases

[mbuhot]

By default, mix release will stream beams, which removes the documentation data.
This affects open_api_spex in two places:

1. PutApiSpec: We can't construct the %OpenApi{} struct at runtime, as the @doc attributes no longer exist.
2. CastAndValidate: We can't resolve the operation_id from the controller / action tuple.

PutApiSpec

The workaround for this one is to

1. Use a mix task to write the API spec to a JSON file during the build
2. In the prod.exs config file, add an app config for the path to the file, `config :my_app, :open_api_path, "priv/static/swagger.json"
3. In the Spec module, conditionally load the spec from file if the app config is present:

def spec do
  case Application.get_env(:my_app, : open_api_path) do
    nil -> build_spec()
    file_path -> load_spec(file_path)
  end
end

Updating the example phoenix app and README with the above approach should be sufficient.

CastAndValidate

This problem is a bit trickier. When a request arrives in the CastAndValidate plug, we know the phoenix controller and action that will eventually handle the request, but not the operationId that needs to be used to cast_and_validate the params.
The existing code calls into the controller to get the operationId at runtime:

operationId = controller.open_api_operation(action).operationId

To get this working again, we'd either need to assume a naming convention for operationId, eg "#{controller}.#{action}" or somehow make the operation info available even after the @doc attributes are stripped.

I think we can adjust the OpenApiSpex.Controller.__using__ macro to store the %Operation{} structs in a module attribute, making it available at runtime.

[moxley]

This issue of not having documentation attributes in a release seems like a serious impact to the user experience of OpenApiSpex. @mbuhot: I appreciate you putting together a proposal to fix it.

I ran an experiment to see what it would take to construct the specs at compile time (versus runtime). It was looking promising at first, but then a problem prevented further progress: During compilation of a clean project, let's say half way through compiling the project, some modules are compiled and some others aren't. Let's say a controller module with OpenApiSpex ExDoc-based operation specs is compiled, and another module that wants to read them is currently being compiled. That second module cannot read the documentation attributes because the compiled controller module is not yet written to a BEAM file. Normally, reading documentation attributes requires a BEAM file to read them from. The documentation attributes are stored in ETS until they're written to the BEAM file (as with any kind of module attribute). However, there doesn't seem to be a way for module A to access the attributes of module B during compilation because they're being compiled by different processes, with their own ETS process. ETS is process-local, meaning ETS data is not shared between multiple processes.

The solution of building the specs as a separate step doesn't sit well with me. Maybe it's fine as a temporary stop-gap measure, but longer term, it degrades the user experience. I would rather see a solution that works for Elixir releases that doesn't degrade the user experience. Elixir release are now the canonical way to do releases rather than an exception. What do you think about replacing ExDoc-based specs with something that is just as easy to work with but doesn't have the described problem? Would using custom module attributes instead of @doc attributes work?

At the same time, I would like to remove the need for putting API specs into the Conn. It adds complexity to the debugging process, and it might have a performance penalty.

[noozo]

Currently being affected by this (commented out all my cast and validates for now), i think the simplest solution with the least amount of typing for the users is always the best. Custom module attributes look like a nice solution (could even "just work" if you generated the same functions as OpenApiSpex already supports, right?).

[mbuhot]

With a combination of on_definition and before_compile callbacks I was able to capture the doc and doc metadata, and make them available through a function. The downside is I couldn't find a public API to access the doc meta, so it is reaching into a compiler module to get it...

  def on_def(env, :def, name, _args, _guards, _body) do
    docs = Module.get_attribute(env.module, :doc)
    {tab, _} = :elixir_module.data_tables(env.module)
    meta = :ets.lookup(tab, {:doc, :meta})

    Module.put_attribute(env.module, :docs, Macro.escape({name, docs, meta}))
    nil
  end
  def on_def(_env, _kind, _name, _args, _guards, _body), do: nil

  defmacro __before_compile__(env) do
    mod = env.module
    docs = Module.get_attribute(mod, :docs)
    quote do
      def __docs__, do: unquote(docs)
    end
  end

  defmacro __using__(_opts) do
    quote do
      Module.register_attribute(__MODULE__, :docs, accumulate: true)
      @before_compile OpenApiSpex.Controller
      @on_definition {OpenApiSpex.Controller, :on_def}

      @doc false
      @spec open_api_operation(atom()) :: OpenApiSpex.Operation.t()
      def open_api_operation(name),
        do: unquote(__MODULE__).__api_operation__(__MODULE__, name)

        defoverridable open_api_operation: 1
    end
  end

iex(1)> PhoenixAppWeb.UserController.__docs__()
[
  {:action, nil, []},
  {:delete, nil,
   [{{:doc, :meta}, %{responses: [no_content: "User deleted"]}, 81}]},
  {:show, {58, "Show user.\n\nShow a user by ID.\n"},
   [
     {{:doc, :meta},
      %{
        parameters: [
          id: [
            in: :path,
            type: %OpenApiSpex.Schema%{minimum: 1, type: :integer},
            description: "User ID",
            example: 123,
            required: true
          ]
        ],
        responses: [
          ok: {"User", "application/json", PhoenixAppWeb.Schemas.UserResponse}
        ]
      }, 63}
   ]},

[moxley]

Nice find @mbuhot! So the next question is: should we rely on this undocumented API, and move forward with this solution for the long-term, or should we assume the API is unstable and not rely on it. And if not, come up with a different way for defining operation specs that doesn't have this limitation (e.g., custom module attributes)?

Maybe @josevalim could provide guidance here. For context, OpenApiSpex provides a way to define web API specs using @doc tags in a Phoenix controller, like this: https://github.com/open-api-spex/open_api_spex#operations, but the docs data is not available in a release generated with mix release.

[josevalim]

This is definitely private API and it can change at any time, so I don’t recommend relying on it. For users of this library, maybe you can ask them to not strip the docs chunk from prod when building a release?

[mbuhot]

Thanks @josevalim - to confirm: there's no way to access the @doc metadata in an @on_definition callback?

In the short term it feels like pointing users to the strip_beams option in the mix release docs will be the fastest way to get things working.

Would using custom module attributes instead of @doc attributes work?

Yes that should work 👍

At the same time, I would like to remove the need for putting API specs into the Conn. It adds complexity to the debugging process, and it might have a performance penalty.

I think we can limit the data we put on the Conn to only the name of the spec module.
The full OpenApi struct can be retrieved from the cache when required.

[josevalim]

We want to support so but unfortinately it would be a breaking change, so it is marked for 2.0: elixir-lang/elixir#8095

It is actually the only breaking change we have planned for the language for an eventual 2.0.

[noozo]

This is definitely private API and it can change at any time, so I don’t recommend relying on it. For users of this library, maybe you can ask them to not strip the docs chunk from prod when building a release?

@josevalim, How much would that add to the size of the app when releasing, say with Docker? Everyone wants their containers small :)

Is there, or will there be a way to cherry pick what to strip (say leave only docs)?

[josevalim]

It is unlikely this would be more than 1MB or 2MB even for a relatively large app.

[josevalim]

But yes, you can also cherry pick IIRC

[noozo]

FYI not stripping beams ends up upping my container size by about 25MB, from 71MB to 96Mb. Not a deal breaker for me, but maybe for someone else. Also could not figure out a way to cherry pick docs only. The only boolean param in the release process seems to be "strip_beams (true/false)" which does all or nothing :(

[josevalim]

@noozo that's bad. Please open up an issue on the Elixir repo so we can provide more granular control over what to strip.

[stefanluptak]

Thanks @noozo for your blog with the solution to this. It was the only result when googling for open_api_spex "{:error, :chunk_not_found}". Hopefully this issue will now be discoverable too thank to this comment.