Skip to content

Commit 2734999

Browse files
authored
Improve Tests and Log CMC Errors (#103)
- add database table and ecto schema for cmc_errors - plumb session_id into context so it's available for audit log messages - update service to log cmc errors to database - update tests - profiled and reduced runtime by eliminating timer based tests. - set logging in test config to NONE so logs aren't polluted with messages
1 parent 78ed692 commit 2734999

File tree

14 files changed

+279
-85
lines changed

14 files changed

+279
-85
lines changed
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
defmodule Prodigy.Core.Data.CmcError do
2+
use Ecto.Schema
3+
import Ecto.Changeset
4+
5+
@moduledoc """
6+
Schema for CMC error reports (dying gasp messages) from client software
7+
"""
8+
9+
schema "cmc_error" do
10+
belongs_to(:session, Prodigy.Core.Data.Session)
11+
field(:user_id, :string)
12+
field(:system_origin, :string)
13+
field(:msg_origin, :string)
14+
field(:unit_id, :string)
15+
field(:error_code, :string)
16+
field(:severity_level, :string)
17+
field(:error_threshold, :string)
18+
field(:error_date, :string) # Original format from client
19+
field(:error_time, :string) # Original format from client
20+
field(:api_event, :string)
21+
field(:mem_to_start, :string)
22+
field(:dos_version, :string)
23+
field(:rs_version, :string)
24+
field(:window_id, :string)
25+
field(:window_last, :string)
26+
field(:selected_id, :string)
27+
field(:selected_last, :string)
28+
field(:base_id, :string)
29+
field(:base_last, :string)
30+
field(:keyword, :string)
31+
field(:raw_payload, :binary) # Store the original binary for debugging
32+
33+
timestamps(updated_at: false)
34+
end
35+
36+
def changeset(cmc_error, attrs) do
37+
cmc_error
38+
|> cast(attrs, [
39+
:session_id, :user_id, :system_origin, :msg_origin, :unit_id,
40+
:error_code, :severity_level, :error_threshold, :error_date,
41+
:error_time, :api_event, :mem_to_start, :dos_version, :rs_version,
42+
:window_id, :window_last, :selected_id, :selected_last,
43+
:base_id, :base_last, :keyword, :raw_payload
44+
])
45+
|> validate_required([:session_id, :user_id, :error_code, :severity_level])
46+
end
47+
end
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
defmodule Prodigy.Core.Data.Repo.Migrations.CreateCmcErrorTable do
2+
use Ecto.Migration
3+
4+
def change do
5+
create table(:cmc_error) do
6+
add :session_id, references(:session, on_delete: :nothing), null: false
7+
add :user_id, :string, null: false
8+
add :system_origin, :string
9+
add :msg_origin, :string
10+
add :unit_id, :string
11+
add :error_code, :string
12+
add :severity_level, :string
13+
add :error_threshold, :string
14+
add :error_date, :string
15+
add :error_time, :string
16+
add :api_event, :string
17+
add :mem_to_start, :string
18+
add :dos_version, :string
19+
add :rs_version, :string
20+
add :window_id, :string
21+
add :window_last, :string
22+
add :selected_id, :string
23+
add :selected_last, :string
24+
add :base_id, :string
25+
add :base_last, :string
26+
add :keyword, :string
27+
add :raw_payload, :binary
28+
29+
timestamps(updated_at: false)
30+
end
31+
32+
create index(:cmc_error, [:session_id])
33+
create index(:cmc_error, [:user_id])
34+
create index(:cmc_error, [:error_code])
35+
create index(:cmc_error, [:inserted_at])
36+
end
37+
end

apps/server/lib/prodigy/server/context.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ defmodule Prodigy.Server.Context do
2121
persists until the connection is terminated.
2222
"""
2323

24-
defstruct [:user, :rs_version, :auth_timeout, :messaging]
24+
defstruct [:user, :session_id, :rs_version, :auth_timeout, :messaging]
2525

2626
def set_auth_timer do
2727
Process.send_after(self(), :auth_timeout, Application.fetch_env!(:server, :auth_timeout))

apps/server/lib/prodigy/server/protocol/tcs/tcs_transmitter.ex

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ defmodule Prodigy.Server.Protocol.Tcs.Transmitter do
44

55
alias Prodigy.Server.Protocol.Tcs.Packet
66

7-
@back_pressure_size 6
87
@rs_buffer_size 8
98
@wack_threshold 4
109
# in seconds

apps/server/lib/prodigy/server/router.ex

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,11 +170,16 @@ defmodule Prodigy.Server.Router do
170170
end
171171

172172
@impl true
173-
def handle_info(:auth_timeout, state) do
173+
def handle_info(:auth_timeout, %{context: %Context{user: nil}} = state) do
174174
Logger.warning("authentication timeout")
175175
{:stop, :normal, state}
176176
end
177177

178+
def handle_info(:auth_timeout, %{context: %Context{user: _user}} = state) do
179+
# User is logged in, ignore timeout
180+
{:noreply, state}
181+
end
182+
178183
@impl GenServer
179184
def handle_info({:EXIT, _pid, _reason}, state) do
180185
{:noreply, state}

apps/server/lib/prodigy/server/service/cmc.ex

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,11 @@ defmodule Prodigy.Server.Service.Cmc do
2121

2222
require Logger
2323

24+
alias Prodigy.Core.Data.{CmcError, Repo}
2425
alias Prodigy.Server.Context
2526
alias Prodigy.Server.Protocol.Dia.Packet.{Fm0, Fm9}
2627

27-
def handle(%Fm0{fm9: %Fm9{payload: payload}} = request, %Context{} = context) do
28-
Logger.debug("cmc got #{inspect(request, base: :hex, limit: :infinity)}")
29-
30-
# TODO insert the error details into the database
31-
28+
def handle(%Fm0{fm9: %Fm9{payload: payload}}, %Context{} = context) do
3229
case payload do
3330
<<
3431
user_id::binary-size(7), # right padded with '?'
@@ -56,35 +53,47 @@ defmodule Prodigy.Server.Service.Cmc do
5653
base_last::binary-size(4), # '0104' typical
5754
keyword::binary-size(13) # 'QUOTE TRACK ' typical
5855
>> ->
59-
msg = """
60-
CMC FM9 Error reported by Reception System
56+
# Insert the CMC error into the database
57+
cmc_result = %CmcError{}
58+
|> CmcError.changeset(%{
59+
session_id: context.session_id, # Assuming session_id is in context
60+
user_id: user_id,
61+
system_origin: system_origin,
62+
msg_origin: msg_origin,
63+
unit_id: unit_id,
64+
error_code: error_code,
65+
severity_level: severity_level,
66+
error_threshold: error_threshold,
67+
error_date: date,
68+
error_time: time,
69+
api_event: api_event,
70+
mem_to_start: mem_to_start,
71+
dos_version: dos_version,
72+
rs_version: rs_version,
73+
window_id: window_id,
74+
window_last: window_last,
75+
selected_id: selected_id,
76+
selected_last: selected_last,
77+
base_id: base_id,
78+
base_last: base_last,
79+
keyword: keyword,
80+
raw_payload: payload
81+
})
82+
|> Repo.insert()
83+
84+
case cmc_result do
85+
{:ok, _} ->
86+
Logger.error("CMC error logged from #{user_id}: code=#{error_code}, severity=#{severity_level}")
87+
88+
{:error, changeset} ->
89+
Logger.error("Failed to log CMC error: #{inspect(changeset.errors)}")
90+
end
91+
92+
_ ->
93+
Logger.warning("CMC received malformed error payload: #{inspect(payload, base: :hex)}")
6194

62-
User ID: #{user_id}
63-
System Origin: #{system_origin}
64-
Message Origin: #{msg_origin}
65-
Unit ID: #{unit_id}
66-
Error Code: #{error_code}
67-
Severity Level: #{severity_level}
68-
Error Threshold: #{error_threshold}
69-
Date: #{date}
70-
Time: #{time}
71-
API Event: #{api_event}
72-
Starting RAM: #{mem_to_start}
73-
DOS Version: #{dos_version}
74-
RS Version: #{rs_version}
75-
Window ID: #{window_id}
76-
Window Last: #{inspect window_last, base: :hex}
77-
Selected ID: #{selected_id}
78-
Selected Last: #{inspect selected_last, base: :hex}
79-
Base ID: #{base_id}
80-
Base Last: #{inspect base_last, base: :hex}
81-
Keyword: #{keyword}
82-
"""
83-
Logger.warning(msg)
84-
_ ->
85-
Logger.warning("CMC received message in unknown format: #{inspect payload, base: :hex, limit: :infinity}")
86-
end
95+
end
8796

8897
{:ok, context, <<>>}
8998
end
90-
end
99+
end

apps/server/lib/prodigy/server/service/enrollment.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ defmodule Prodigy.Server.Service.Enrollment do
8585
)
8686

8787
response =
88-
Logon.make_response_payload({Logon.Status.SUCCESS, user})
88+
Logon.make_response_payload({Logon.Status.SUCCESS, user, context.session_id})
8989
|> Fm0.make_response(request)
9090
|> DiaPacket.encode()
9191

apps/server/lib/prodigy/server/service/logon.ex

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ defmodule Prodigy.Server.Service.Logon do
6565
end
6666
end
6767

68-
def make_response_payload({Status.SUCCESS, user}) do
68+
def make_response_payload({Status.SUCCESS, user, _session_id}) do
6969
now = Calendar.DateTime.now_utc()
7070
# TODO refactor to use Timex
7171
date = now |> Calendar.strftime("%m%d%y")
@@ -146,7 +146,7 @@ defmodule Prodigy.Server.Service.Logon do
146146
res
147147
end
148148

149-
def make_response_payload({status, _}) do
149+
def make_response_payload({status, _, _}) do
150150
now = Calendar.DateTime.now_utc()
151151
date = now |> Calendar.strftime("%m%d%y")
152152
time = now |> Calendar.strftime("%H%M%S")
@@ -268,23 +268,23 @@ defmodule Prodigy.Server.Service.Logon do
268268
end
269269

270270
case SessionManager.create_session(user, status, version) do
271-
{:ok, _db_session} ->
271+
{:ok, db_session} ->
272272
case enrollment_status do
273-
true -> {Status.SUCCESS, user}
274-
{:enroll_subscriber, user} -> {Status.ENROLL_SUBSCRIBER, user}
275-
{:enroll_other, user} -> {Status.ENROLL_OTHER, user}
273+
true -> {Status.SUCCESS, user, db_session.id}
274+
{:enroll_subscriber, user} -> {Status.ENROLL_SUBSCRIBER, user, db_session.id}
275+
{:enroll_other, user} -> {Status.ENROLL_OTHER, user, db_session.id}
276276
end
277277
{:error, :concurrency_exceeded} ->
278278
Logger.warning("User #{user.id} attempted logon, but exceeded concurrency limit")
279-
{Status.ID_IN_USE, nil}
279+
{Status.ID_IN_USE, nil, nil}
280280
end
281281
else
282-
:bad_version -> {Status.BAD_VERSION, nil}
283-
:bad_password -> {Status.BAD_PASSWORD, nil}
284-
{:enroll_subscriber, user} -> {Status.ENROLL_SUBSCRIBER, user}
285-
{:enroll_other, user} -> {Status.ENROLL_OTHER, user}
286-
:id_in_use -> {Status.ID_IN_USE, nil}
287-
_ -> {Status.ACCOUNT_PROBLEM, nil}
282+
:bad_version -> {Status.BAD_VERSION, nil, nil}
283+
:bad_password -> {Status.BAD_PASSWORD, nil, nil}
284+
# {:enroll_subscriber, user} -> {Status.ENROLL_SUBSCRIBER, user}
285+
# {:enroll_other, user} -> {Status.ENROLL_OTHER, user}
286+
:id_in_use -> {Status.ID_IN_USE, nil, nil}
287+
_ -> {Status.ACCOUNT_PROBLEM, nil, nil}
288288
end
289289

290290
response =
@@ -293,20 +293,20 @@ defmodule Prodigy.Server.Service.Logon do
293293
|> Packet.encode()
294294

295295
case result do
296-
{Status.SUCCESS, user} ->
296+
{Status.SUCCESS, user, session_id} ->
297297
Context.cancel_auth_timer(auth_timeout)
298298
Logger.info("User #{user_id} logged on (Normal)")
299-
{:ok, %Context{user: user, rs_version: version}, response}
299+
{:ok, %Context{user: user, session_id: session_id, rs_version: version}, response}
300300

301-
{Status.ENROLL_SUBSCRIBER, user} ->
301+
{Status.ENROLL_SUBSCRIBER, user, session_id} ->
302302
Context.cancel_auth_timer(auth_timeout)
303303
Logger.info("User #{user_id} logged on (Enroll Subscriber)")
304-
{:ok, %Context{user: user, rs_version: version}, response}
304+
{:ok, %Context{user: user, session_id: session_id, rs_version: version}, response}
305305

306-
{Status.ENROLL_OTHER, user} ->
306+
{Status.ENROLL_OTHER, user, session_id} ->
307307
Context.cancel_auth_timer(auth_timeout)
308308
Logger.info("User #{user_id} logged on (Enroll Other)")
309-
{:ok, %Context{user: user, rs_version: version}, response}
309+
{:ok, %Context{user: user, session_id: session_id, rs_version: version}, response}
310310

311311
_ ->
312312
{:error, context, response}

apps/server/test/prodigy/server/protocol/tcs/tcs_transmitter_test.exs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ defmodule Prodigy.Server.Protocol.Tcs.Transmitter.Test do
44
import Cachex.Spec
55

66
alias Prodigy.Server.Protocol.Tcs.Transmitter
7-
alias Prodigy.Server.Protocol.Tcs.Packet
87

98
setup do
109
Cachex.start_link(:transmit, [
@@ -64,7 +63,7 @@ defmodule Prodigy.Server.Protocol.Tcs.Transmitter.Test do
6463

6564
test "sends wackpk after interval", %{transmitter: pid} do
6665
Transmitter.transmit_packet(pid, "test_packet", 1)
67-
:timer.sleep(11_000) # Wait longer than @wack_interval
66+
# :timer.sleep(11_000) # Wait longer than @wack_interval
6867
# assert_receive {:send, _wackpk}
6968
end
7069
end

apps/server/test/prodigy/server/service/cmc_test.exs

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,54 @@
1616
defmodule Prodigy.Server.Service.Cmc.Test do
1717
@moduledoc false
1818
use Prodigy.Server.RepoCase
19-
20-
import ExUnit.CaptureLog
19+
import Server
20+
import Ecto.Changeset
2121

2222
require Logger
2323

24+
alias Prodigy.Core.Data.{CmcError, Household, Repo, Session, User}
25+
alias Prodigy.Server.Protocol.Dia.Packet, as: DiaPacket
2426
alias Prodigy.Server.Protocol.Dia.Packet.{Fm0, Fm9}
2527
alias Prodigy.Server.Protocol.Dia.Packet.Fm9.{Flags, Function, Reason}
2628
alias Prodigy.Server.Router
29+
alias Prodigy.Server.Service.Logon.Status
2730

2831
# @moduletag :capture_log
2932

33+
@today DateTime.to_date(DateTime.utc_now())
34+
3035
setup do
3136
{:ok, router_pid} = GenServer.start_link(Router, nil)
3237

3338
[router_pid: router_pid]
3439
end
3540

3641
test "error report", context do
37-
fm0 = %Fm0{
42+
cmc_error_count =
43+
CmcError
44+
|> Repo.aggregate(:count)
45+
46+
assert cmc_error_count == 0
47+
48+
sessions =
49+
Session
50+
|> Repo.all()
51+
52+
assert(length(sessions) == 0)
53+
54+
%Household{id: "AAAA99", enabled_date: @today}
55+
|> change
56+
|> put_assoc(:users, [
57+
%User{id: "AAAA99A", gender: "M", date_enrolled: @today}
58+
|> User.changeset(%{password: "test"})
59+
])
60+
|> Repo.insert()
61+
62+
{:ok, response} = logon(context.router_pid, "AAAA99A", "test", "06.03.10")
63+
{:ok, %Fm0{payload: <<status, _rest::binary>>}} = DiaPacket.decode(response)
64+
assert status == Status.SUCCESS.value()
65+
66+
Router.handle_packet(context.router_pid, %Fm0{
3867
concatenated: true,
3968
src: 0x0,
4069
dest: 0x020200,
@@ -89,12 +118,12 @@ defmodule Prodigy.Server.Service.Cmc.Test do
89118
"QUOTE TRACK "
90119
>>
91120
}
92-
}
121+
})
93122

94-
assert capture_log([level: :warning], fn ->
95-
Router.handle_packet(context.router_pid, fm0)
96-
end) =~ "CMC FM9"
123+
cmc_error_count =
124+
CmcError
125+
|> Repo.aggregate(:count)
97126

98-
# TODO assert the message was inserted into the database
127+
assert cmc_error_count == 1
99128
end
100129
end

0 commit comments

Comments
 (0)