api-surface #1
4 changed files with 116 additions and 15 deletions
fix(api): deterministic facade generation + returns:nil decode; address review
- [Critical] Facade delegate set is now derived from static, compile-time-stable
data (each group's __endpoints__/0 route table) instead of the group's
module_info(:exports). module_info/1 is an auto-generated function that the
Elixir compiler tracer does not record as a compile-time dependency, so no
compile-order edge was established between Dexcord.Api and the group modules;
under parallel/incremental compilation the export table was read incompletely,
silently dropping delegates (e.g. Dexcord.Api.get_gateway_bot/0 undefined after
a routine recompile). Arities are derived from segment param count + body flag
(each head's opts \\ [] yields base and base+1), with get_channel_messages/3
named explicitly as sugar. Verified: delegate always present in the compiled
beam; 8x incremental-recompile loop of facade + gateway_integration all green.
- [Important] api_facade_test.exs is now deterministic: it asserts the facade's
exported set equals the same statically-derived expected set the generator
uses (no module_info), and ensures Dexcord.Api + groups are loaded before
function_exported? probes (which do not auto-load), removing a false-negative
async load race. Historically-load-bearing arity assertions kept.
- [Minor] decode_response/2 now normalizes returns:nil endpoints to {:ok, nil}
even when the server sends a non-204 JSON body, instead of leaking the raw
decoded map through the passthrough clause. Added a unit test.
commit
9e738e95f3
|
|
@ -340,10 +340,22 @@ defmodule Dexcord.Api do
|
|||
# The typed endpoint surface lives in the group modules below (declared with
|
||||
# the `Dexcord.Api.Endpoint` DSL). Each public endpoint head is re-exported
|
||||
# here as a `defdelegate`, so `Dexcord.Api.<endpoint>` keeps working for every
|
||||
# internal and external caller. `module_info(:exports)` is read at compile
|
||||
# time, which establishes the compile-order dependency on the groups; a
|
||||
# function's default-arity heads appear as separate exports, so (e.g.) both
|
||||
# `create_message/2` and `create_message/3` receive delegates.
|
||||
# internal and external caller.
|
||||
#
|
||||
# The delegate set is derived from STATIC, compile-time-stable data — each
|
||||
# group's `__endpoints__/0` route table — NOT from `module_info(:exports)`.
|
||||
# `module_info/1` is an auto-generated function; the Elixir compiler's tracer
|
||||
# does not record calls to it as compile-time dependencies, so reading a
|
||||
# group's export table established no compile-order edge and raced under
|
||||
# parallel/incremental compilation, silently dropping delegates (an incremental
|
||||
# recompile could leave `Dexcord.Api.get_gateway_bot/0` undefined). `__endpoints__/0`
|
||||
# is an ordinary remote call: it IS tracked, forcing every group fully compiled
|
||||
# before this module, and its contents are deterministic.
|
||||
#
|
||||
# Each generated endpoint head ends in `opts \\ []`, so it exports at two
|
||||
# arities — `base` and `base + 1`, where `base = path_param_count + (body? 1 : 0)`.
|
||||
# That reproduces the old "default-arity heads are separate exports" behavior
|
||||
# (e.g. both `create_message/2` and `create_message/3`) without introspection.
|
||||
|
||||
@endpoint_groups [
|
||||
Dexcord.Api.Channels,
|
||||
|
|
@ -357,9 +369,39 @@ defmodule Dexcord.Api do
|
|||
@doc false
|
||||
def __endpoint_groups__, do: @endpoint_groups
|
||||
|
||||
for group <- @endpoint_groups,
|
||||
{name, arity} <- group.module_info(:exports),
|
||||
name not in [:module_info, :__info__, :__endpoints__] do
|
||||
# Hand-written sugar heads whose arities are NOT reproduced by the
|
||||
# `__endpoints__/0` derivation, listed explicitly as `{group, name, arity}`.
|
||||
# `Dexcord.Api.Users.create_dm/1,2` is already covered: its spec lives in the
|
||||
# `__endpoints__/0` table (body: true, 0 path params -> arities 1 and 2). Only
|
||||
# `Dexcord.Api.Messages.get_channel_messages/3` (the Nostrum-compatible locator
|
||||
# arity, a plain `def` with no `endpoint/4` spec) needs to be named here.
|
||||
@facade_sugar [
|
||||
{Dexcord.Api.Messages, :get_channel_messages, 3}
|
||||
]
|
||||
|
||||
@facade_exports (
|
||||
endpoint_exports =
|
||||
for group <- @endpoint_groups,
|
||||
spec <- group.__endpoints__(),
|
||||
params = Enum.count(spec.segments, &match?({:param, _}, &1)),
|
||||
base = params + if(spec.body, do: 1, else: 0),
|
||||
arity <- [base, base + 1] do
|
||||
{group, spec.name, arity}
|
||||
end
|
||||
|
||||
(endpoint_exports ++ @facade_sugar)
|
||||
|> Enum.uniq_by(fn {_group, name, arity} -> {name, arity} end)
|
||||
)
|
||||
|
||||
@doc false
|
||||
# The statically-derived facade delegate set as a MapSet of `{name, arity}`.
|
||||
# This is the single source of truth `api_facade_test.exs` asserts against, so
|
||||
# a dropped delegate fails the guard deterministically rather than flakily.
|
||||
def __facade_exports__ do
|
||||
MapSet.new(for {_group, name, arity} <- @facade_exports, do: {name, arity})
|
||||
end
|
||||
|
||||
for {group, name, arity} <- @facade_exports do
|
||||
args = Macro.generate_arguments(arity, __MODULE__)
|
||||
defdelegate unquote(name)(unquote_splicing(args)), to: group
|
||||
end
|
||||
|
|
|
|||
|
|
@ -307,6 +307,10 @@ defmodule Dexcord.Api.Endpoint do
|
|||
def decode_response({:ok, nil}, _returns), do: {:ok, nil}
|
||||
def decode_response({:ok, {:raw, _}} = raw, _returns), do: raw
|
||||
def decode_response(ok, :raw), do: ok
|
||||
# `returns: nil` endpoints normalize to `{:ok, nil}` even when the server sends
|
||||
# a non-204 JSON body (otherwise the raw map would leak through the passthrough
|
||||
# clause, since the atom-mod clause's `mod != nil` guard rightly skips nil).
|
||||
def decode_response({:ok, _}, nil), do: {:ok, nil}
|
||||
|
||||
def decode_response({:ok, list}, [mod]) when is_list(list),
|
||||
do: {:ok, Enum.map(list, &mod.from_map/1)}
|
||||
|
|
|
|||
|
|
@ -1,17 +1,65 @@
|
|||
defmodule Dexcord.ApiFacadeTest do
|
||||
@moduledoc """
|
||||
Guards the generated `Dexcord.Api` facade: every public endpoint head in every
|
||||
group module must be re-exported from `Dexcord.Api` at the same arity, so the
|
||||
`module_info(:exports)` delegation can never silently drop an endpoint.
|
||||
Guards the generated `Dexcord.Api` facade: every endpoint head derived from the
|
||||
group modules' static `__endpoints__/0` route tables (plus explicit sugar) must
|
||||
be re-exported from `Dexcord.Api` at the same arity, so the delegation can never
|
||||
silently drop an endpoint.
|
||||
|
||||
This guard is deterministic: it derives the expected `{name, arity}` set from
|
||||
the SAME static source of truth the facade generation uses (`__endpoints__/0`),
|
||||
never from `module_info(:exports)` (whose export table races under
|
||||
parallel/incremental compilation).
|
||||
"""
|
||||
use ExUnit.Case, async: true
|
||||
|
||||
test "every group endpoint export is re-exported from Dexcord.Api" do
|
||||
for group <- Dexcord.Api.__endpoint_groups__(),
|
||||
{name, arity} <- group.module_info(:exports),
|
||||
name not in [:module_info, :__info__, :__endpoints__] do
|
||||
# `function_exported?/3` does NOT load a module — it only inspects an
|
||||
# already-loaded one. Under `async: true` a probe-only test can otherwise run
|
||||
# before anything forces `Dexcord.Api` into the VM, yielding a false negative
|
||||
# that has nothing to do with the facade. Load every relevant module up front
|
||||
# so the export probes below reflect the compiled beam, deterministically.
|
||||
setup_all do
|
||||
Code.ensure_loaded!(Dexcord.Api)
|
||||
Enum.each(Dexcord.Api.__endpoint_groups__(), &Code.ensure_loaded!/1)
|
||||
:ok
|
||||
end
|
||||
|
||||
# Hand-written sugar arities not reproduced by the `__endpoints__/0` derivation
|
||||
# (mirrors `Dexcord.Api`'s own `@facade_sugar`).
|
||||
@sugar [get_channel_messages: 3]
|
||||
|
||||
# Independently re-derive the expected delegate set from the groups' static
|
||||
# route tables, using the identical rule the facade generation applies: each
|
||||
# generated head ends in `opts \\ []`, so it exports at `base` and `base + 1`
|
||||
# where `base = path_param_count + (body? 1 : 0)`.
|
||||
defp expected_exports do
|
||||
endpoint_exports =
|
||||
for group <- Dexcord.Api.__endpoint_groups__(),
|
||||
spec <- group.__endpoints__(),
|
||||
params = Enum.count(spec.segments, &match?({:param, _}, &1)),
|
||||
base = params + if(spec.body, do: 1, else: 0),
|
||||
arity <- [base, base + 1] do
|
||||
{spec.name, arity}
|
||||
end
|
||||
|
||||
MapSet.new(endpoint_exports ++ @sugar)
|
||||
end
|
||||
|
||||
test "the facade's export set exactly equals the statically-derived expected set" do
|
||||
# Same source of truth as the generator: if the two ever diverge, either the
|
||||
# generator dropped/added a delegate or the derivation rule drifted.
|
||||
assert Dexcord.Api.__facade_exports__() == expected_exports()
|
||||
end
|
||||
|
||||
test "every statically-derived export is actually defined on Dexcord.Api" do
|
||||
# Guards the generation step itself: the derived set must correspond to real
|
||||
# exported functions. This is what fails deterministically if a delegate is
|
||||
# silently dropped by a racy compile.
|
||||
exports = expected_exports()
|
||||
assert MapSet.size(exports) > 0
|
||||
|
||||
for {name, arity} <- exports do
|
||||
assert function_exported?(Dexcord.Api, name, arity),
|
||||
"Dexcord.Api is missing a delegate for #{inspect(group)}.#{name}/#{arity}"
|
||||
"Dexcord.Api is missing a delegate for #{name}/#{arity}"
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -163,6 +163,13 @@ defmodule Dexcord.Api.EndpointTest do
|
|||
assert Endpoint.decode_response({:ok, nil}, Dexcord.User) == {:ok, nil}
|
||||
end
|
||||
|
||||
test "returns: nil normalizes a non-204 JSON body to {:ok, nil}" do
|
||||
# A `returns: nil` endpoint must not leak the raw decoded map when the
|
||||
# server answers with a JSON body instead of an empty 204.
|
||||
assert Endpoint.decode_response({:ok, %{"unexpected" => "body"}}, nil) == {:ok, nil}
|
||||
assert Endpoint.decode_response({:ok, nil}, nil) == {:ok, nil}
|
||||
end
|
||||
|
||||
test "returns: :raw leaves the decoded map alone" do
|
||||
assert Endpoint.decode_response({:ok, %{"id" => "1"}}, :raw) == {:ok, %{"id" => "1"}}
|
||||
end
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue