Skip to content

Commit b0cc5ba

Browse files
authored
fix: Compilation with no Ecto support (#79)
Phoenix.Sync should work over HTTP for a bare Plug app
1 parent 97c0e50 commit b0cc5ba

File tree

9 files changed

+1819
-1752
lines changed

9 files changed

+1819
-1752
lines changed

.github/workflows/elixir_tests.yml

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ jobs:
7777
- name: Run tests
7878
run: mix test --trace
7979

80-
test-as-dep:
81-
name: Test installation as a dependency
80+
test-as-dep-standalone:
81+
name: Test installation as a dependency without Electric or Ecto
8282
runs-on: ubuntu-latest
8383
env:
8484
MIX_ENV: test
@@ -94,7 +94,26 @@ jobs:
9494
run: mix deps.get && mix deps.compile
9595

9696
- name: Test installation as a dependency
97-
run: mix test.as_a_dep
97+
run: mix test.as_a_dep.standalone
98+
99+
test-as-dep-embedded:
100+
name: Test installation as a dependency with embedded Electric
101+
runs-on: ubuntu-latest
102+
env:
103+
MIX_ENV: test
104+
steps:
105+
- uses: actions/checkout@v4
106+
107+
- uses: erlef/setup-beam@v1
108+
with:
109+
version-type: strict
110+
version-file: ".tool-versions"
111+
112+
- name: Install dependencies
113+
run: mix deps.get && mix deps.compile
114+
115+
- name: Test installation as a dependency
116+
run: mix test.as_a_dep.embedded
98117

99118
test-apps:
100119
name: Test integration apps

lib/phoenix/sync/gateway.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ defmodule Phoenix.Sync.Gateway do
3737
Map.merge(%{"url" => url, "headers" => auth_headers}, shape_params)
3838
end
3939

40-
def configuration(%Ecto.Query{} = query, %Client{} = client) do
40+
def configuration(query, %Client{} = client) when is_struct(query, Ecto.Query) do
4141
query
4242
|> Electric.Client.shape!()
4343
|> configuration(client)

lib/phoenix/sync/plug.ex

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,6 @@ defmodule Phoenix.Sync.Plug do
146146
alias Electric.Client.ShapeDefinition
147147
alias Phoenix.Sync.Gateway
148148

149-
require Ecto.Query
150-
151149
@valid_ops [:==, :!=, :>, :<, :>=, :<=]
152150

153151
@type table_column :: atom()
@@ -174,7 +172,7 @@ defmodule Phoenix.Sync.Plug do
174172
%ShapeDefinition{} = shape ->
175173
%{shape: shape}
176174

177-
%Ecto.Query{} = query ->
175+
query when is_struct(query, Ecto.Query) ->
178176
%{shape: Electric.Client.shape!(query)}
179177

180178
schema when is_atom(schema) ->
@@ -357,16 +355,24 @@ defmodule Phoenix.Sync.Plug do
357355
Plug.Conn.assign(conn, :shape, shape)
358356
end
359357

360-
for op <- @valid_ops do
361-
where =
362-
{op, [],
363-
[
364-
{:field, [], [Macro.var(:q, nil), {:^, [], [Macro.var(:column, nil)]}]},
365-
{:^, [], [Macro.var(:value, nil)]}
366-
]}
367-
368-
defp add_filter(query, var!(column), unquote(op), var!(value)) do
369-
Ecto.Query.where(query, [q], unquote(where))
358+
if Code.ensure_loaded?(Ecto.Query) do
359+
for op <- @valid_ops do
360+
where =
361+
{op, [],
362+
[
363+
{:field, [], [Macro.var(:q, nil), {:^, [], [Macro.var(:column, nil)]}]},
364+
{:^, [], [Macro.var(:value, nil)]}
365+
]}
366+
367+
defp add_filter(query, var!(column), unquote(op), var!(value)) do
368+
require Ecto.Query
369+
Ecto.Query.where(query, [q], unquote(where))
370+
end
371+
end
372+
else
373+
defp add_filter(_query, _, _, _) do
374+
raise ArgumentError,
375+
message: "Ecto.Query is required for dynamic shapes, please add it to your dependencies."
370376
end
371377
end
372378

lib/phoenix/sync/predefined_shape.ex

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -165,15 +165,17 @@ defmodule Phoenix.Sync.PredefinedShape do
165165
ShapeDefinition.new!(shape_config)
166166
end
167167

168-
# we resolve the query at runtime to avoid compile-time dependencies in
169-
# router modules
170-
defp to_shape_definition(%__MODULE__{query: queryable, shape_config: shape_config}) do
171-
try do
172-
Electric.Client.EctoAdapter.shape!(queryable, shape_config)
173-
rescue
174-
e in Protocol.UndefinedError ->
175-
raise ArgumentError,
176-
message: "Invalid query `#{inspect(queryable)}`: #{e.description}"
168+
if Code.ensure_loaded?(Ecto) do
169+
# we resolve the query at runtime to avoid compile-time dependencies in
170+
# router modules
171+
defp to_shape_definition(%__MODULE__{query: queryable, shape_config: shape_config}) do
172+
try do
173+
Electric.Client.EctoAdapter.shape!(queryable, shape_config)
174+
rescue
175+
e in Protocol.UndefinedError ->
176+
raise ArgumentError,
177+
message: "Invalid query `#{inspect(queryable)}`: #{e.description}"
178+
end
177179
end
178180
end
179181
end

lib/phoenix/sync/sandbox.ex

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -186,11 +186,7 @@ if Code.ensure_loaded?(Ecto.Adapters.SQL.Sandbox) do
186186
use Supervisor
187187

188188
alias __MODULE__
189-
190-
defmodule Error do
191-
@moduledoc false
192-
defexception [:message]
193-
end
189+
alias __MODULE__.Error
194190

195191
@type start_opts() :: [{:shared, boolean()}]
196192

lib/phoenix/sync/sandbox/error.ex

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
defmodule Phoenix.Sync.Sandbox.Error do
2+
@moduledoc false
3+
defexception [:message]
4+
end

lib/phoenix/sync/sandbox/fetch.ex

Lines changed: 51 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,66 +1,68 @@
1-
defmodule Phoenix.Sync.Sandbox.Fetch do
2-
@moduledoc false
1+
if Code.ensure_loaded?(Ecto.Adapters.SQL.Sandbox) do
2+
defmodule Phoenix.Sync.Sandbox.Fetch do
3+
@moduledoc false
34

4-
alias Electric.Client
5-
alias Electric.Client.Fetch
5+
alias Electric.Client
6+
alias Electric.Client.Fetch
67

7-
require Logger
8+
require Logger
89

9-
@callback request(Client.t(), Fetch.Request.t(), opts :: Keyword.t()) ::
10-
Fetch.Response.t() | {:error, Fetch.Response.t() | term()}
10+
@callback request(Client.t(), Fetch.Request.t(), opts :: Keyword.t()) ::
11+
Fetch.Response.t() | {:error, Fetch.Response.t() | term()}
1112

12-
@behaviour Electric.Client.Fetch.Pool
13+
@behaviour Electric.Client.Fetch.Pool
1314

14-
def name(stack_id) do
15-
Phoenix.Sync.Sandbox.name({__MODULE__, stack_id})
16-
end
15+
def name(stack_id) do
16+
Phoenix.Sync.Sandbox.name({__MODULE__, stack_id})
17+
end
1718

18-
@impl Electric.Client.Fetch.Pool
19-
def request(%Client{} = client, %Fetch.Request{} = request, opts) do
20-
{:ok, stack_id} = Keyword.fetch(opts, :stack_id)
19+
@impl Electric.Client.Fetch.Pool
20+
def request(%Client{} = client, %Fetch.Request{} = request, opts) do
21+
{:ok, stack_id} = Keyword.fetch(opts, :stack_id)
2122

22-
request_id = request_id(client, request, stack_id)
23+
request_id = request_id(client, request, stack_id)
2324

24-
# The monitor process is unique to the request and launches the actual
25-
# request as a linked process.
26-
#
27-
# This coalesces requests, so no matter how many simultaneous
28-
# clients we have, we only ever make one request to the backend.
29-
{:ok, monitor_pid} = start_monitor(stack_id, request_id, request, client)
25+
# The monitor process is unique to the request and launches the actual
26+
# request as a linked process.
27+
#
28+
# This coalesces requests, so no matter how many simultaneous
29+
# clients we have, we only ever make one request to the backend.
30+
{:ok, monitor_pid} = start_monitor(stack_id, request_id, request, client)
3031

31-
try do
32-
ref = Fetch.Monitor.register(monitor_pid, self())
32+
try do
33+
ref = Fetch.Monitor.register(monitor_pid, self())
3334

34-
Fetch.Monitor.wait(ref)
35-
catch
36-
:exit, {reason, _} ->
37-
Logger.debug(fn ->
38-
"Request process ended with reason #{inspect(reason)} before we could register. Re-attempting."
39-
end)
35+
Fetch.Monitor.wait(ref)
36+
catch
37+
:exit, {reason, _} ->
38+
Logger.debug(fn ->
39+
"Request process ended with reason #{inspect(reason)} before we could register. Re-attempting."
40+
end)
4041

41-
request(client, request, opts)
42+
request(client, request, opts)
43+
end
4244
end
43-
end
4445

45-
defp start_monitor(stack_id, request_id, request, client) do
46-
DynamicSupervisor.start_child(
47-
name(stack_id),
48-
{Electric.Client.Fetch.Monitor, {request_id, request, client}}
49-
)
50-
|> return_existing()
51-
end
46+
defp start_monitor(stack_id, request_id, request, client) do
47+
DynamicSupervisor.start_child(
48+
name(stack_id),
49+
{Electric.Client.Fetch.Monitor, {request_id, request, client}}
50+
)
51+
|> return_existing()
52+
end
5253

53-
defp return_existing({:ok, pid}), do: {:ok, pid}
54-
defp return_existing({:error, {:already_started, pid}}), do: {:ok, pid}
55-
defp return_existing(error), do: error
54+
defp return_existing({:ok, pid}), do: {:ok, pid}
55+
defp return_existing({:error, {:already_started, pid}}), do: {:ok, pid}
56+
defp return_existing(error), do: error
5657

57-
defp request_id(%Client{fetch: {fetch_impl, _}}, %Fetch.Request{} = request, stack_id) do
58-
{
59-
fetch_impl,
60-
stack_id,
61-
URI.to_string(request.endpoint),
62-
request.headers,
63-
Fetch.Request.params(request)
64-
}
58+
defp request_id(%Client{fetch: {fetch_impl, _}}, %Fetch.Request{} = request, stack_id) do
59+
{
60+
fetch_impl,
61+
stack_id,
62+
URI.to_string(request.endpoint),
63+
request.headers,
64+
Fetch.Request.params(request)
65+
}
66+
end
6567
end
6668
end

0 commit comments

Comments
 (0)