feat: keep the API and its OpenAPI document in sync continuously

.formatter.exs

@@ -1,5 +1,5 @@
 [
-  import_deps: [:ecto, :ecto_sql, :phoenix],
+  import_deps: [:ecto, :ecto_sql, :phoenix, :open_api_spex],
   subdirectories: ["priv/*/migrations"],
   plugins: [Phoenix.LiveView.HTMLFormatter],
   inputs: ["*.{heex,ex,exs}", "{config,lib,test}/**/*.{heex,ex,exs}", "priv/*/seeds.exs"]

AGENTS.md

@@ -7,6 +7,7 @@ This is a web application written using the Phoenix web framework.
 - Use Conventional Commits style for pull request titles, such as `feat: add workspace membership controls`
 - When Jujutsu is available for the workspace, use `jj` to record changes with detailed explanations; use Git commits only when `jj` is unavailable
 - Add concise comments or module/function docs that explain intent, domain boundaries, security decisions, or non-obvious tradeoffs. Avoid comments that merely restate what the code does
+- The API contract lives next to the code: each API controller action declares its OpenAPI operation with `operation` (helpers in `TextbinWeb.ApiDocs`, schemas in `TextbinWeb.ApiSpec.Schemas`). Requests that do not match it are rejected, every API response in the test suite is checked against it, and a full `mix test` fails if a documented response is never produced. After changing an operation, run `mix openapi.export` and commit `priv/openapi.json`
 - Use the already included and available `:req` (`Req`) library for HTTP requests, **avoid** `:httpoison`, `:tesla`, and `:httpc`. Req is included by default and is the preferred HTTP client for Phoenix apps
 
 ### Phoenix v1.8 guidelines

config/test.exs

@@ -40,6 +40,10 @@ config :textbin, Textbin.Mailer, adapter: Swoosh.Adapters.Test
 
 config :textbin, Textbin.Storage, adapter: Textbin.Storage.Memory
 
+# Every API response a test produces is checked against the operation its
+# action declares (see TextbinWeb.ApiResponseCheck).
+config :textbin, :api_response_check, TextbinWeb.ApiContract
+
 # Cleanup is exercised explicitly so the global worker does not race SQL sandbox tests.
 config :textbin, expiration_cleanup_enabled: false
 

lib/textbin_web/api_docs.ex

@@ -0,0 +1,89 @@
+defmodule TextbinWeb.ApiDocs do
+  @moduledoc """
+  Building blocks for the `operation` specs that API controllers declare
+  next to their actions with `OpenApiSpex.ControllerSpecs`.
+
+  Specs are evaluated when the controller compiles, so they cannot call
+  functions of the controller itself; the shared pieces live here instead.
+  Request and response schemas live in `TextbinWeb.ApiSpec.Schemas` and are
+  referenced by title.
+  """
+
+  alias OpenApiSpex.{MediaType, RequestBody, Response, Schema}
+  alias TextbinWeb.ApiSpec.Schemas
+
+  @doc "A JSON response whose body is the named schema."
+  def response(description, title), do: {description, "application/json", Schemas.ref(title)}
+
+  def error(description), do: response(description, "Error")
+
+  def unauthorized, do: error("Missing, invalid or revoked API token")
+
+  @doc "The 422 every operation with parameters or a body can return."
+  def invalid_request,
+    do: response("The parameters or body do not match this operation", "ValidationError")
+
+  def no_content(description), do: %Response{description: description}
+
+  def uuid_path(description) do
+    [
+      in: :path,
+      required: true,
+      description: description,
+      schema: %Schema{type: :string, format: :uuid}
+    ]
+  end
+
+  def query(schema, description),
+    do: [in: :query, required: false, description: description || "", schema: schema]
+
+  def json_body(title) do
+    %RequestBody{
+      required: true,
+      content: %{"application/json" => %MediaType{schema: Schemas.ref(title)}}
+    }
+  end
+
+  @doc """
+  Adds the responses every authenticated, validated operation shares: a
+  missing token, and a request that does not match the operation. An
+  operation's own wording for a status wins.
+  """
+  def with_common_errors(responses) do
+    Map.merge(%{401 => unauthorized(), 422 => invalid_request()}, responses)
+  end
+
+  # Paste operations share these. The personal workspace always exists, so
+  # the "Workspace not found" 404 is added by in_workspace/1 only.
+  def with_scope_errors(responses), do: with_common_errors(responses)
+
+  @doc "The optional paste fields as query parameters, for raw-body uploads."
+  def paste_field_parameters do
+    for {name, schema} <- Enum.sort(Schemas.create_paste_fields()),
+        do: {name, query(schema, schema.description)}
+  end
+
+  def create_paste_body do
+    %RequestBody{
+      required: true,
+      content: %{
+        "application/json" => %MediaType{schema: Schemas.ref("CreatePasteRequest")},
+        "*/*" => %MediaType{schema: %Schema{type: :string, format: :binary}}
+      }
+    }
+  end
+
+  @doc """
+  The workspace-scoped variant of a personal-workspace paste operation: the
+  same behaviour under /workspaces/{workspace_id}, with its own operation id.
+  """
+  def in_workspace(spec) do
+    workspace_id = {:workspace_id, uuid_path("Workspace id")}
+
+    spec
+    |> Keyword.update(:parameters, [workspace_id], &[workspace_id | &1])
+    |> Keyword.update!(:responses, &Map.put_new(&1, 404, error("Workspace not found")))
+    |> Keyword.update!(:operation_id, &(&1 <> "InWorkspace"))
+    |> Keyword.update!(:summary, &String.replace(&1, "your personal workspace", "a workspace"))
+  end
+end

lib/textbin_web/api_request_validation.ex

@@ -0,0 +1,135 @@
+defmodule TextbinWeb.ApiRequestValidation do
+  @moduledoc """
+  Rejects API requests that do not match the operation the action declares
+  with `operation`, before the action runs.
+
+  The operation is the contract: path and query parameters, and a JSON body,
+  are validated against it, and anything it does not describe is rejected
+  with a 422 in the `ValidationError` shape. That keeps the document and the
+  accepted input from drifting apart, because an undocumented parameter
+  simply does not work.
+
+  Raw uploads are the one exception to body validation. Paste creation
+  accepts the content itself with any content type (the type is stored with
+  the paste), and open_api_spex only matches request bodies by exact content
+  type, so a body whose type the operation lists only as `*/*` skips body
+  validation; its query parameters are still checked.
+
+  Validation leaves `conn.params` as the strings the action already expects.
+  """
+
+  @behaviour Plug
+
+  import Plug.Conn
+
+  alias OpenApiSpex.Cast.Error
+  alias OpenApiSpex.Operation
+  alias OpenApiSpex.Plug.PutApiSpec
+  alias OpenApiSpex.RequestBody
+
+  @impl Plug
+  def init(opts), do: opts
+
+  @impl Plug
+  def call(%{private: %{phoenix_controller: controller, phoenix_action: action}} = conn, _opts) do
+    {spec, _operation_lookup} = PutApiSpec.get_spec_and_operation_lookup(conn)
+    content_type = request_content_type(conn)
+    operation = controller.open_api_operation(action) |> for_content_type(content_type)
+
+    {conn, body_errors} = body_boundary(conn, operation)
+
+    errors =
+      case OpenApiSpex.cast_and_validate(spec, operation, conn, content_type,
+             replace_params: false
+           ) do
+        {:ok, _conn} -> []
+        {:error, errors} -> errors
+      end
+
+    case body_errors ++ errors ++ undeclared_query_errors(conn, operation) do
+      [] -> conn
+      errors -> reject(conn, errors)
+    end
+  end
+
+  defp request_content_type(conn) do
+    case get_req_header(conn, "content-type") do
+      [value | _rest] -> value |> String.split(";") |> hd() |> String.trim() |> String.downcase()
+      [] -> nil
+    end
+  end
+
+  defp for_content_type(
+         %Operation{requestBody: %RequestBody{content: content}} = operation,
+         content_type
+       )
+       when is_binary(content_type) do
+    if Map.has_key?(content, content_type) or not Map.has_key?(content, "*/*"),
+      do: operation,
+      else: %{operation | requestBody: nil}
+  end
+
+  defp for_content_type(operation, _content_type), do: operation
+
+  # Operation2 unwraps _json even when it was supplied as a real object key.
+  # Our JSON contracts require flat objects, never Plug's scalar wrapper.
+  defp body_boundary(%{body_params: %{"_json" => _}} = conn, %Operation{
+         requestBody: %RequestBody{}
+       }),
+       do: {conn, [unexpected_body("_json")]}
+
+  defp body_boundary(conn, %Operation{requestBody: nil}) do
+    cond do
+      conn.private[:raw_paste_upload] ->
+        {conn, []}
+
+      map_size(conn.body_params) > 0 ->
+        {conn, [unexpected_body("body")]}
+
+      true ->
+        # Content-Type alone does not imply a body. Probe only one byte when
+        # parsers left it unread, rejecting chunked/raw bodies without buffering.
+        case read_body(conn, length: 1, read_length: 1) do
+          {:ok, "", conn} -> {conn, []}
+          {_state, _body, conn} -> {conn, [unexpected_body("body")]}
+          {:error, _reason} -> {conn, [unexpected_body("body")]}
+        end
+    end
+  end
+
+  defp body_boundary(conn, _operation), do: {conn, []}
+
+  defp unexpected_body(name), do: %Error{reason: :unexpected_field, name: name, path: [name]}
+
+  # open_api_spex only checks query parameters for operations that declare
+  # some; an operation that declares none should accept none either.
+  defp undeclared_query_errors(conn, %Operation{parameters: parameters}) do
+    if Enum.any?(parameters, &(&1.in == :query)) do
+      []
+    else
+      conn = fetch_query_params(conn)
+
+      for name <- Map.keys(conn.query_params) do
+        %Error{reason: :unexpected_field, name: name, path: [name]}
+      end
+    end
+  end
+
+  defp reject(conn, errors) do
+    body = %{
+      errors:
+        Enum.group_by(errors, &field/1, fn error ->
+          error |> Error.message() |> String.trim()
+        end)
+    }
+
+    conn
+    |> put_status(:unprocessable_entity)
+    |> Phoenix.Controller.json(body)
+    |> halt()
+  end
+
+  defp field(%Error{path: [_ | _] = path}), do: Enum.map_join(path, ".", &to_string/1)
+  defp field(%Error{name: name}) when not is_nil(name), do: to_string(name)
+  defp field(%Error{}), do: "request"
+end

lib/textbin_web/api_response_check.ex

@@ -0,0 +1,31 @@
+defmodule TextbinWeb.ApiResponseCheck do
+  @moduledoc """
+  Hands every API response to a checker just before it is sent, so the test
+  suite can hold each one to the operation its action declares.
+
+  It does nothing unless `config :textbin, :api_response_check` names a
+  checker module, which only the test environment does; the value is read at
+  compile time, so production carries no cost. The checker gets the conn in a
+  `register_before_send/2` callback and raises on a mismatch, which fails
+  whichever test made the request.
+
+  It sits in the `:api` pipeline rather than the controllers so responses
+  from the pipeline itself, such as a missing token's 401, are checked too.
+  """
+
+  @behaviour Plug
+
+  @checker Application.compile_env(:textbin, :api_response_check)
+
+  @impl Plug
+  def init(opts), do: opts
+
+  if @checker do
+    @impl Plug
+    def call(conn, _opts),
+      do: Plug.Conn.register_before_send(conn, fn conn -> @checker.check_response!(conn) end)
+  else
+    @impl Plug
+    def call(conn, _opts), do: conn
+  end
+end