Skip to content

Commit e22e13c

Browse files
committed
fix: Correct Registry.select match specification syntax
Fix incorrect string syntax for match variables in Registry.select calls. Changed from string atoms (":$1", ":$2") to proper atom syntax (:"$1", :"$2") throughout the codebase. Changes: - Fix list_session/0 Registry.select syntax and add guard to filter session entries - Fix get_session_id/0 in both :process and :process_link macros - Refactor list_sessions_by_module/1 to use Registry.lookup for cleaner implementation - Add @impl annotations to handle_info/2 and terminate/2 in :process_link macro - Update list_session/0 typespec to allow empty lists (fix Dialyzer warning) Tests: - Add comprehensive unit tests for list_session/0, list_sessions_by_module/1 - Add tests for session_info/0 and get_session_id/0 functionality - Create TestProcessLink module for :process_link testing All tests pass (75/75) with no warnings or linting errors.
1 parent ff2aae0 commit e22e13c

File tree

4 files changed

+188
-9
lines changed

4 files changed

+188
-9
lines changed

lib/phoenix/session_process.ex

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -351,10 +351,10 @@ defmodule Phoenix.SessionProcess do
351351
352352
# Returns list of {session_id, pid} tuples, or empty list if no sessions exist
353353
"""
354-
@spec list_session :: [{binary(), pid()}, ...]
354+
@spec list_session :: [{binary(), pid()}]
355355
def list_session do
356356
Registry.select(SessionRegistry, [
357-
{{:":$1", :":$2", :_}, [], [{{:":$1", :":$2"}}]}
357+
{{:"$1", :"$2", :_}, [{:is_binary, :"$1"}], [{{:"$1", :"$2"}}]}
358358
])
359359
end
360360

@@ -416,11 +416,14 @@ defmodule Phoenix.SessionProcess do
416416
"""
417417
@spec list_sessions_by_module(module()) :: [binary()]
418418
def list_sessions_by_module(module) do
419-
Registry.select(SessionRegistry, [
420-
{{:"$1", :"$2", :"$_"}, [], [{{:"$1", :"$2", :"$_"}}]}
421-
])
422-
|> Enum.filter(fn {_session_id, _pid, mod} -> mod == module end)
423-
|> Enum.map(fn {session_id, _pid, _mod} -> session_id end)
419+
list_session()
420+
|> Enum.filter(fn {_session_id, pid} ->
421+
case Registry.lookup(SessionRegistry, pid) do
422+
[{_, ^module}] -> true
423+
_ -> false
424+
end
425+
end)
426+
|> Enum.map(fn {session_id, _pid} -> session_id end)
424427
end
425428

426429
@doc """
@@ -529,7 +532,7 @@ defmodule Phoenix.SessionProcess do
529532
current_pid = self()
530533

531534
Registry.select(unquote(SessionRegistry), [
532-
{{:":$1", :":$2", :_}, [{:==, :":$2", current_pid}], [{{:":$1", :":$2"}}]}
535+
{{:"$1", :"$2", :_}, [{:==, :"$2", current_pid}], [{{:"$1", :"$2"}}]}
533536
])
534537
|> Enum.at(0)
535538
|> elem(0)
@@ -551,7 +554,7 @@ defmodule Phoenix.SessionProcess do
551554
current_pid = self()
552555

553556
Registry.select(unquote(SessionRegistry), [
554-
{{:":$1", :":$2", :_}, [{:==, :":$2", current_pid}], [{{:":$1", :":$2"}}]}
557+
{{:"$1", :"$2", :_}, [{:==, :"$2", current_pid}], [{{:"$1", :"$2"}}]}
555558
])
556559
|> Enum.at(0)
557560
|> elem(0)
@@ -565,6 +568,7 @@ defmodule Phoenix.SessionProcess do
565568
{:noreply, new_state}
566569
end
567570

571+
@impl true
568572
def handle_info({:DOWN, _ref, :process, pid, _reason}, state) do
569573
new_state =
570574
state
@@ -573,6 +577,7 @@ defmodule Phoenix.SessionProcess do
573577
{:noreply, new_state}
574578
end
575579

580+
@impl true
576581
def terminate(_reason, state) do
577582
state
578583
|> Map.get(:__live_view__, [])

test/phoenix/session_process_test.exs

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,138 @@ defmodule Phoenix.SessionProcessTest do
5454
SessionProcess.cast(session_id, :add_one)
5555
assert SessionProcess.call(session_id, :get_value) == 1
5656
end
57+
58+
describe "list_session/0" do
59+
test "returns list type" do
60+
assert is_list(SessionProcess.list_session())
61+
end
62+
63+
test "returns list of session_id and pid tuples" do
64+
session_id1 = SessionId.generate_unique_session_id()
65+
session_id2 = SessionId.generate_unique_session_id()
66+
67+
{:ok, pid1} = SessionProcess.start(session_id1, TestProcess, %{value: 1})
68+
{:ok, pid2} = SessionProcess.start(session_id2, TestProcess, %{value: 2})
69+
70+
sessions = SessionProcess.list_session()
71+
# Check that our created sessions are in the list
72+
assert {session_id1, pid1} in sessions
73+
assert {session_id2, pid2} in sessions
74+
75+
# Cleanup
76+
SessionProcess.terminate(session_id1)
77+
SessionProcess.terminate(session_id2)
78+
end
79+
80+
test "removes session when terminated" do
81+
session_id = SessionId.generate_unique_session_id()
82+
{:ok, pid} = SessionProcess.start(session_id, TestProcess, %{value: 0})
83+
84+
# Check session exists in list
85+
sessions_before = SessionProcess.list_session()
86+
assert {session_id, pid} in sessions_before
87+
88+
# Terminate and verify it's removed
89+
SessionProcess.terminate(session_id)
90+
sessions_after = SessionProcess.list_session()
91+
assert {session_id, pid} not in sessions_after
92+
end
93+
end
94+
95+
describe "list_sessions_by_module/1" do
96+
test "returns list type" do
97+
assert is_list(SessionProcess.list_sessions_by_module(TestProcess))
98+
end
99+
100+
test "returns session IDs for specific module" do
101+
session_id1 = SessionId.generate_unique_session_id()
102+
session_id2 = SessionId.generate_unique_session_id()
103+
104+
{:ok, _pid1} = SessionProcess.start(session_id1, TestProcess, %{value: 1})
105+
{:ok, _pid2} = SessionProcess.start(session_id2, TestProcess, %{value: 2})
106+
107+
sessions = SessionProcess.list_sessions_by_module(TestProcess)
108+
# Check that our created sessions are in the list
109+
assert session_id1 in sessions
110+
assert session_id2 in sessions
111+
112+
# Cleanup
113+
SessionProcess.terminate(session_id1)
114+
SessionProcess.terminate(session_id2)
115+
end
116+
117+
test "filters sessions by module correctly" do
118+
session_id1 = SessionId.generate_unique_session_id()
119+
session_id2 = SessionId.generate_unique_session_id()
120+
121+
{:ok, _pid1} = SessionProcess.start(session_id1, TestProcess, %{value: 1})
122+
{:ok, _pid2} =
123+
SessionProcess.start(session_id2, TestProcessLink, %{value: 2})
124+
125+
test_sessions = SessionProcess.list_sessions_by_module(TestProcess)
126+
link_sessions = SessionProcess.list_sessions_by_module(TestProcessLink)
127+
128+
# Check that each session appears in the correct module list
129+
assert session_id1 in test_sessions
130+
assert session_id1 not in link_sessions
131+
assert session_id2 in link_sessions
132+
assert session_id2 not in test_sessions
133+
134+
# Cleanup
135+
SessionProcess.terminate(session_id1)
136+
SessionProcess.terminate(session_id2)
137+
end
138+
end
139+
140+
describe "session_info/0" do
141+
test "returns map with count and modules keys" do
142+
info = SessionProcess.session_info()
143+
assert is_map(info)
144+
assert Map.has_key?(info, :count)
145+
assert Map.has_key?(info, :modules)
146+
assert is_integer(info.count)
147+
assert is_list(info.modules)
148+
end
149+
150+
test "includes modules of active sessions" do
151+
session_id1 = SessionId.generate_unique_session_id()
152+
session_id2 = SessionId.generate_unique_session_id()
153+
154+
{:ok, _pid1} = SessionProcess.start(session_id1, TestProcess, %{value: 1})
155+
{:ok, _pid2} =
156+
SessionProcess.start(session_id2, TestProcessLink, %{value: 2})
157+
158+
info = SessionProcess.session_info()
159+
# Check that both modules are in the list
160+
assert TestProcess in info.modules
161+
assert TestProcessLink in info.modules
162+
# Check that count is at least 2 (could be more from other tests)
163+
assert info.count >= 2
164+
165+
# Cleanup
166+
SessionProcess.terminate(session_id1)
167+
SessionProcess.terminate(session_id2)
168+
end
169+
end
170+
171+
describe "get_session_id/0 in :process macro" do
172+
test "returns correct session_id from within session process" do
173+
session_id = SessionId.generate_unique_session_id()
174+
{:ok, _pid} = SessionProcess.start(session_id, TestProcess, %{value: 0})
175+
176+
# Call a function that uses get_session_id internally
177+
result = SessionProcess.call(session_id, :get_my_session_id)
178+
assert result == session_id
179+
end
180+
end
181+
182+
describe "get_session_id/0 in :process_link macro" do
183+
test "returns correct session_id from within session process" do
184+
session_id = SessionId.generate_unique_session_id()
185+
{:ok, _pid} = SessionProcess.start(session_id, TestProcessLink, %{value: 0})
186+
187+
result = SessionProcess.call(session_id, :get_my_session_id)
188+
assert result == session_id
189+
end
190+
end
57191
end

test/support/test_process.ex

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ defmodule TestProcess do
2121
{:reply, State.get(state.agent, :value), state}
2222
end
2323

24+
@impl true
25+
def handle_call(:get_my_session_id, _from, state) do
26+
{:reply, get_session_id(), state}
27+
end
28+
2429
@impl true
2530
def handle_cast(:add_one, state) do
2631
value = State.get(state.agent, :value)

test/support/test_process_link.ex

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
defmodule TestProcessLink do
2+
@moduledoc """
3+
Test helper module for session process with LiveView link testing.
4+
5+
This module provides a session process implementation using the :process_link
6+
option to verify LiveView monitoring functionality and get_session_id behavior.
7+
"""
8+
9+
use Phoenix.SessionProcess, :process_link
10+
11+
alias Phoenix.SessionProcess.State
12+
13+
@impl true
14+
def init(init_arg \\ %{}) do
15+
{:ok, agent} = State.start_link(init_arg)
16+
{:ok, %{agent: agent}}
17+
end
18+
19+
@impl true
20+
def handle_call(:get_value, _from, state) do
21+
{:reply, State.get(state.agent, :value), state}
22+
end
23+
24+
@impl true
25+
def handle_call(:get_my_session_id, _from, state) do
26+
{:reply, get_session_id(), state}
27+
end
28+
29+
@impl true
30+
def handle_cast(:add_one, state) do
31+
value = State.get(state.agent, :value)
32+
State.put(state.agent, :value, value + 1)
33+
{:noreply, state}
34+
end
35+
end

0 commit comments

Comments
 (0)