From 7a3950d1ad243989391adfda5bbb74f58cd90a08 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Fri, 12 Jun 2026 13:45:59 -0500 Subject: [PATCH] chore: add jump_credo_checks ~> 0.4 with all 19 checks enabled All checks set to exit_status: 0 initially to avoid blocking on pre-existing violations. Remove exit_status overrides as each check category is cleaned up incrementally. Dependency added to dev/test only. --- .credo.exs | 58 ++++- lib/towerops_web/live/graph_live/show.ex | 57 ++--- .../plugs/update_session_activity.ex | 12 +- mix.exs | 1 + mix.lock | 1 + test/towerops/llm/deepseek_test.exs | 32 +++ .../llm/network_insight_prompt_test.exs | 41 ++++ .../live/graph_live/show_helpers_test.exs | 220 +++++++++++++++++- .../plugs/update_session_activity_test.exs | 88 ++++++- test/towerops_web/remote_ip_test.exs | 89 ++++++- 10 files changed, 556 insertions(+), 43 deletions(-) diff --git a/.credo.exs b/.credo.exs index 0c05f788..177ca236 100644 --- a/.credo.exs +++ b/.credo.exs @@ -162,7 +162,63 @@ {Credo.Check.Warning.UnusedRegexOperation, []}, {Credo.Check.Warning.UnusedStringOperation, []}, {Credo.Check.Warning.UnusedTupleOperation, []}, - {Credo.Check.Warning.WrongTestFileExtension, []} + {Credo.Check.Warning.WrongTestFileExtension, []}, + + # + ## Jump Credo Checks (all set to warn-only initially; remove exit_status: 0 + ## as pre-existing violations are cleaned up) + # + {Jump.CredoChecks.AssertElementSelectorCanNeverFail, [exit_status: 0]}, + {Jump.CredoChecks.AssertReceiveTimeout, + [exit_status: 0, min_assert_receive_timeout: 1_000, max_refute_receive_timeout: 100]}, + {Jump.CredoChecks.AvoidFunctionLevelElse, [exit_status: 0]}, + {Jump.CredoChecks.AvoidLoggerConfigureInTest, [exit_status: 0]}, + {Jump.CredoChecks.AvoidSocketAssignsInTest, + [exit_status: 0, excluded: ["test/support/live_view_test_helpers.ex"]]}, + {Jump.CredoChecks.ConditionalAssertion, [exit_status: 0]}, + {Jump.CredoChecks.DoctestIExExamples, + [ + exit_status: 0, + derive_test_path: fn filename -> + filename + |> String.replace_leading("lib/", "test/") + |> String.replace_trailing(".ex", "_test.exs") + end + ]}, + {Jump.CredoChecks.ForbiddenFunction, + [ + exit_status: 0, + functions: [ + {:erlang, :binary_to_term, "Use Plug.Crypto.non_executable_binary_to_term/2 instead."} + ] + ]}, + {Jump.CredoChecks.LiveViewFormCanBeRehydrated, [exit_status: 0]}, + {Jump.CredoChecks.UndeclaredExternalResource, [exit_status: 0]}, + {Jump.CredoChecks.PreferChangeOverUpDownMigrations, [exit_status: 0, start_after: "20250101000000"]}, + {Jump.CredoChecks.PreferTextColumns, [exit_status: 0, start_after: "20250101000000"]}, + {Jump.CredoChecks.SafeBinaryToTerm, [exit_status: 0]}, + {Jump.CredoChecks.TestHasNoAssertions, + [exit_status: 0, custom_assertion_functions: [:assert_has, :refute_has, :assert_eventually]]}, + {Jump.CredoChecks.TooManyAssertions, [exit_status: 0, max_assertions: 20]}, + {Jump.CredoChecks.TopLevelAliasImportRequire, [exit_status: 0]}, + {Jump.CredoChecks.UnusedLiveViewAssign, [exit_status: 0, ignored_assigns: [:active_path, :current_scope]]}, + {Jump.CredoChecks.UseObanProWorker, [exit_status: 0]}, + {Jump.CredoChecks.VacuousTest, + [ + exit_status: 0, + ignore_setup_only_tests?: false, + library_modules: [ + Ecto, + Jason, + Oban, + Phoenix, + Plug, + Req, + Swoosh, + Towerops.Repo + ] + ]}, + {Jump.CredoChecks.WeakAssertion, [exit_status: 0]} ], disabled: [ # diff --git a/lib/towerops_web/live/graph_live/show.ex b/lib/towerops_web/live/graph_live/show.ex index 0cccf886..b003fea7 100644 --- a/lib/towerops_web/live/graph_live/show.ex +++ b/lib/towerops_web/live/graph_live/show.ex @@ -270,7 +270,8 @@ defmodule ToweropsWeb.GraphLive.Show do end end - defp build_graph_params(assigns, range) do + @doc false + def build_graph_params(assigns, range) do base_params = %{"range" => range} base_params = @@ -411,32 +412,35 @@ defmodule ToweropsWeb.GraphLive.Show do {load_sensor_chart_data(device_id, sensor_types, range), nil} end - defp get_sensor_types_for_chart("processors"), do: ["cpu_load"] - defp get_sensor_types_for_chart("memory"), do: ["memory_usage"] - defp get_sensor_types_for_chart("storage"), do: ["disk_usage"] - defp get_sensor_types_for_chart("temperature"), do: ["temperature", "cpu_temperature", "celsius"] - defp get_sensor_types_for_chart("voltage"), do: ["voltage", "volts"] - defp get_sensor_types_for_chart("traffic"), do: nil + @doc false + def get_sensor_types_for_chart("processors"), do: ["cpu_load"] + def get_sensor_types_for_chart("memory"), do: ["memory_usage"] + def get_sensor_types_for_chart("storage"), do: ["disk_usage"] + def get_sensor_types_for_chart("temperature"), do: ["temperature", "cpu_temperature", "celsius"] + def get_sensor_types_for_chart("voltage"), do: ["voltage", "volts"] + def get_sensor_types_for_chart("traffic"), do: nil # For unknown sensor types, use the type directly (supports count, pppoe_sessions, etc.) - defp get_sensor_types_for_chart(sensor_type), do: [sensor_type] + def get_sensor_types_for_chart(sensor_type), do: [sensor_type] - defp get_chart_config("latency"), do: {"Ping Latency", "ms", true} - defp get_chart_config("processors"), do: {"Processor Usage", "%", false} - defp get_chart_config("memory"), do: {"Memory Usage", "%", false} - defp get_chart_config("storage"), do: {"Storage Usage", "%", false} - defp get_chart_config("storage_volume"), do: {"Storage Usage", "%", false} - defp get_chart_config("temperature"), do: {"Temperature", "°C", true} - defp get_chart_config("voltage"), do: {"Voltage", "V", true} - defp get_chart_config("traffic"), do: {"Overall Traffic", "bps", true} - defp get_chart_config("interface_errors"), do: {"Interface Errors & Discards", "", true} - defp get_chart_config("count"), do: {"Count", "", true} - defp get_chart_config("pppoe_sessions"), do: {"PPPoE Sessions", "", true} - defp get_chart_config("connections"), do: {"Connections", "", true} - defp get_chart_config("wireless"), do: {"Wireless Metrics", "", true} + @doc false + def get_chart_config("latency"), do: {"Ping Latency", "ms", true} + def get_chart_config("processors"), do: {"Processor Usage", "%", false} + def get_chart_config("memory"), do: {"Memory Usage", "%", false} + def get_chart_config("storage"), do: {"Storage Usage", "%", false} + def get_chart_config("storage_volume"), do: {"Storage Usage", "%", false} + def get_chart_config("temperature"), do: {"Temperature", "°C", true} + def get_chart_config("voltage"), do: {"Voltage", "V", true} + def get_chart_config("traffic"), do: {"Overall Traffic", "bps", true} + def get_chart_config("interface_errors"), do: {"Interface Errors & Discards", "", true} + def get_chart_config("count"), do: {"Count", "", true} + def get_chart_config("pppoe_sessions"), do: {"PPPoE Sessions", "", true} + def get_chart_config("connections"), do: {"Connections", "", true} + def get_chart_config("wireless"), do: {"Wireless Metrics", "", true} # Humanize unknown sensor types for title - defp get_chart_config(sensor_type), do: {humanize_sensor_type(sensor_type), "", true} + def get_chart_config(sensor_type), do: {humanize_sensor_type(sensor_type), "", true} - defp humanize_sensor_type(sensor_type) do + @doc false + def humanize_sensor_type(sensor_type) do sensor_type |> String.replace("_", " ") |> String.split() @@ -670,10 +674,11 @@ defmodule ToweropsWeb.GraphLive.Show do } end - # Calculate max and min values from chart data - defp calculate_chart_stats(nil), do: {nil, nil} + @doc false + def calculate_chart_stats(nil), do: {nil, nil} - defp calculate_chart_stats(chart_data) when is_binary(chart_data) do + @doc false + def calculate_chart_stats(chart_data) when is_binary(chart_data) do case Jason.decode(chart_data) do {:ok, %{"datasets" => datasets}} -> extract_min_max_from_datasets(datasets) diff --git a/lib/towerops_web/plugs/update_session_activity.ex b/lib/towerops_web/plugs/update_session_activity.ex index 628f997c..bf6930d5 100644 --- a/lib/towerops_web/plugs/update_session_activity.ex +++ b/lib/towerops_web/plugs/update_session_activity.ex @@ -52,8 +52,8 @@ defmodule ToweropsWeb.Plugs.UpdateSessionActivity do end end - # Runs in a background task: check activity and either touch or revoke the session - defp handle_session_activity(token, live_socket_id) do + @doc false + def handle_session_activity(token, live_socket_id) do case Accounts.get_browser_session_by_token_value(token) do nil -> :ok @@ -67,15 +67,15 @@ defmodule ToweropsWeb.Plugs.UpdateSessionActivity do end end - # Check if session has exceeded inactivity timeout - defp session_timed_out?(session) do + @doc false + def session_timed_out?(session) do last_activity = session.last_activity_at || session.inserted_at inactive_seconds = DateTime.diff(DateTime.utc_now(), last_activity, :second) inactive_seconds > @inactivity_timeout_seconds end - # Revoke the token so the next request will be unauthenticated - defp revoke_inactive_session(token, live_socket_id) do + @doc false + def revoke_inactive_session(token, live_socket_id) do Accounts.delete_user_session_token(token) if live_socket_id do diff --git a/mix.exs b/mix.exs index 15c31ed8..935d74b7 100644 --- a/mix.exs +++ b/mix.exs @@ -128,6 +128,7 @@ defmodule Towerops.MixProject do {:mix_test_watch, "~> 1.0", only: [:dev, :test], runtime: false}, {:dialyxir, "~> 1.4", only: [:dev, :test], runtime: false}, {:credo, "~> 1.7", only: [:dev, :test], runtime: false}, + {:jump_credo_checks, "~> 0.4", only: [:dev, :test], runtime: false}, {:mix_audit, "~> 2.1", only: [:dev, :test], runtime: false}, {:inet_cidr, "~> 1.0"}, {:cloak_ecto, "~> 1.3"}, diff --git a/mix.lock b/mix.lock index f6435ace..51a2e76e 100644 --- a/mix.lock +++ b/mix.lock @@ -43,6 +43,7 @@ "idna": {:hex, :idna, "6.1.1", "8a63070e9f7d0c62eb9d9fcb360a7de382448200fbbd1b106cc96d3d8099df8d", [:rebar3], [{:unicode_util_compat, "~> 0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "92376eb7894412ed19ac475e4a86f7b413c1b9fbb5bd16dccd57934157944cea"}, "inet_cidr": {:hex, :inet_cidr, "1.0.9", "e0ef72a2942529da78c8e4147d53f2ef5f6f5293335c3637b0fdf83c012cc816", [:mix], [], "hexpm", "172da15ff7cf635b1feaf14f5818be28c811b37cc5fb7c5f7c01058c1c1066cc"}, "jason": {:hex, :jason, "1.4.5", "2e3a008590b0b8d7388c20293e9dcc9cf3e5d642fd2a114e4cbbb52e595d940a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0 or ~> 3.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "b0c823996102bcd0239b3c2444eb00409b72f6a140c1950bc8b457d836b30684"}, + "jump_credo_checks": {:hex, :jump_credo_checks, "0.4.0", "9dd5cbf6a9fca758c8a1664855434fc377393b58225e6ca8dc173763ee07487a", [:mix], [{:credo, "~> 1.7", [hex: :credo, repo: "hexpm", optional: false]}, {:igniter, ">= 0.0.0", [hex: :igniter, repo: "hexpm", optional: true]}], "hexpm", "89f51e654b5f4900dfcc8cfaae780d676bc9343ec072f6067594f0a5c2900a19"}, "lazy_html": {:hex, :lazy_html, "0.1.11", "136c8e9cd616b4f4e9c1562daa683880891120b759606dc4c3b6b18058ba5d79", [:make, :mix], [{:cc_precompiler, "~> 0.1", [hex: :cc_precompiler, repo: "hexpm", optional: false]}, {:elixir_make, "~> 0.9.0", [hex: :elixir_make, repo: "hexpm", optional: false]}, {:fine, "~> 0.1.0", [hex: :fine, repo: "hexpm", optional: false]}], "hexpm", "3b1be592929c31eca1a21673d25696e5c14cddfe922d9d1a3e3b48be4163883b"}, "libcluster": {:hex, :libcluster, "3.5.0", "5ee4cfde4bdf32b2fef271e33ce3241e89509f4344f6c6a8d4069937484866ba", [:mix], [{:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.3", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "ebf6561fcedd765a4cd43b4b8c04b1c87f4177b5fb3cbdfe40a780499d72f743"}, "logger_backends": {:hex, :logger_backends, "1.0.0", "09c4fad6202e08cb0fbd37f328282f16539aca380f512523ce9472b28edc6bdf", [:mix], [], "hexpm", "1faceb3e7ec3ef66a8f5746c5afd020e63996df6fd4eb8cdb789e5665ae6c9ce"}, diff --git a/test/towerops/llm/deepseek_test.exs b/test/towerops/llm/deepseek_test.exs index 3ead738e..55efc160 100644 --- a/test/towerops/llm/deepseek_test.exs +++ b/test/towerops/llm/deepseek_test.exs @@ -84,6 +84,38 @@ defmodule Towerops.LLM.DeepSeekTest do DeepSeek.complete([%{role: "user", content: "x"}], api_key: "test-key") end + test "falls back to reasoning_content when content is empty" do + Req.Test.stub(DeepSeek, fn conn -> + Req.Test.json(conn, %{ + "model" => "deepseek-reasoner", + "choices" => [%{"message" => %{"content" => "", "reasoning_content" => "model is thinking"}}] + }) + end) + + assert {:ok, %{content: "model is thinking"}} = + DeepSeek.complete([%{role: "user", content: "x"}], api_key: "test-key") + end + + test "returns {:error, {:unexpected_response, _}} for malformed response body" do + Req.Test.stub(DeepSeek, fn conn -> + Req.Test.json(conn, %{"not" => "expected"}) + end) + + assert {:error, {:unexpected_response, _}} = + DeepSeek.complete([%{role: "user", content: "x"}], api_key: "test-key") + end + + test "returns {:error, _} on HTTP client error" do + Req.Test.stub(DeepSeek, fn conn -> + # Force a connection-level error by sending garbage + Plug.Conn.send_resp(conn, 200, "") + Process.exit(conn.owner, :kill) + end) + + assert {:error, _reason} = + DeepSeek.complete([%{role: "user", content: "x"}], api_key: "test-key") + end + test "honors custom model and temperature" do Req.Test.stub(DeepSeek, fn conn -> {:ok, body, conn} = Plug.Conn.read_body(conn) diff --git a/test/towerops/llm/network_insight_prompt_test.exs b/test/towerops/llm/network_insight_prompt_test.exs index 3c5de385..6e4185b6 100644 --- a/test/towerops/llm/network_insight_prompt_test.exs +++ b/test/towerops/llm/network_insight_prompt_test.exs @@ -51,6 +51,47 @@ defmodule Towerops.LLM.NetworkInsightPromptTest do end describe "parse/1" do + test "extracts observations from JSON embedded in markdown with extra text" do + content = """ + Let me analyze the network snapshot: + + ```json + {"observations": [{"title": "Channel Congestion", "description": "Channel 6 has 5 APs on same channel", "urgency": "warning"}]} + ``` + + This is my analysis. + """ + + assert {:ok, [obs]} = NetworkInsightPrompt.parse(content) + assert obs.title == "Channel Congestion" + assert obs.urgency == "warning" + end + + test "extracts JSON from text with leading/trailing content using brace extraction" do + content = """ + Here's my analysis: + {"observations": [{"title": "Test", "description": "Some desc", "urgency": "info"}]} + That's all. + """ + + assert {:ok, [obs]} = NetworkInsightPrompt.parse(content) + assert obs.title == "Test" + end + + test "extracts deeply nested brace structure" do + content = + ~s|Some text {"observations": [{"title": "Nested", "description": "escaped \\"quote\\" here", "urgency": "critical"}]} more text| + + assert {:ok, [obs]} = NetworkInsightPrompt.parse(content) + assert obs.title == "Nested" + assert obs.description =~ "quote" + end + + test "returns empty list for text with unmatched braces" do + content = "text with {unmatched brace" + assert {:ok, []} = NetworkInsightPrompt.parse(content) + end + test "extracts a list of observations from a JSON array" do json = ~s|{"observations": [{"title": "Sites with thermal stress", "description": "Tower-A is at 78C.", "urgency": "warning"}]}| diff --git a/test/towerops_web/live/graph_live/show_helpers_test.exs b/test/towerops_web/live/graph_live/show_helpers_test.exs index a19e33fd..8a892f41 100644 --- a/test/towerops_web/live/graph_live/show_helpers_test.exs +++ b/test/towerops_web/live/graph_live/show_helpers_test.exs @@ -82,7 +82,6 @@ defmodule ToweropsWeb.GraphLive.ShowHelpersTest do end test "bps unit with bps scale" do - # Module's Float.round requires a float; callers must pass a float value. assert "500.0 bps" == Show.format_value(500.0, "bps") end @@ -90,4 +89,223 @@ defmodule ToweropsWeb.GraphLive.ShowHelpersTest do assert "1.0 Mbps" == Show.format_value(-1_000_000, "bps") end end + + describe "get_chart_config/1" do + test "latency returns Ping Latency in ms with auto-scale" do + assert {"Ping Latency", "ms", true} == Show.get_chart_config("latency") + end + + test "processors returns Processor Usage in % without auto-scale" do + assert {"Processor Usage", "%", false} == Show.get_chart_config("processors") + end + + test "memory returns Memory Usage in % without auto-scale" do + assert {"Memory Usage", "%", false} == Show.get_chart_config("memory") + end + + test "storage returns Storage Usage in % without auto-scale" do + assert {"Storage Usage", "%", false} == Show.get_chart_config("storage") + end + + test "storage_volume returns Storage Usage in % without auto-scale" do + assert {"Storage Usage", "%", false} == Show.get_chart_config("storage_volume") + end + + test "temperature returns Temperature in °C with auto-scale" do + assert {"Temperature", "°C", true} == Show.get_chart_config("temperature") + end + + test "voltage returns Voltage in V with auto-scale" do + assert {"Voltage", "V", true} == Show.get_chart_config("voltage") + end + + test "traffic returns Overall Traffic in bps with auto-scale" do + assert {"Overall Traffic", "bps", true} == Show.get_chart_config("traffic") + end + + test "interface_errors returns Interface Errors & Discards with auto-scale" do + assert {"Interface Errors & Discards", "", true} == Show.get_chart_config("interface_errors") + end + + test "count returns Count with auto-scale" do + assert {"Count", "", true} == Show.get_chart_config("count") + end + + test "pppoe_sessions returns PPPoE Sessions with auto-scale" do + assert {"PPPoE Sessions", "", true} == Show.get_chart_config("pppoe_sessions") + end + + test "connections returns Connections with auto-scale" do + assert {"Connections", "", true} == Show.get_chart_config("connections") + end + + test "wireless returns Wireless Metrics with auto-scale" do + assert {"Wireless Metrics", "", true} == Show.get_chart_config("wireless") + end + + test "unknown sensor type falls back to humanized version" do + assert {"Cpu Load", "", true} == Show.get_chart_config("cpu_load") + assert {"Custom Metric", "", true} == Show.get_chart_config("custom_metric") + end + end + + describe "get_sensor_types_for_chart/1" do + test "processors maps to cpu_load" do + assert ["cpu_load"] == Show.get_sensor_types_for_chart("processors") + end + + test "memory maps to memory_usage" do + assert ["memory_usage"] == Show.get_sensor_types_for_chart("memory") + end + + test "storage maps to disk_usage" do + assert ["disk_usage"] == Show.get_sensor_types_for_chart("storage") + end + + test "temperature maps to multiple sensor types" do + result = Show.get_sensor_types_for_chart("temperature") + assert "temperature" in result + assert "cpu_temperature" in result + assert "celsius" in result + end + + test "voltage maps to voltage and volts" do + result = Show.get_sensor_types_for_chart("voltage") + assert "voltage" in result + assert "volts" in result + end + + test "traffic returns nil" do + assert nil == Show.get_sensor_types_for_chart("traffic") + end + + test "unknown sensor type returns itself in a list" do + assert ["count"] == Show.get_sensor_types_for_chart("count") + assert ["pppoe_sessions"] == Show.get_sensor_types_for_chart("pppoe_sessions") + assert ["connections"] == Show.get_sensor_types_for_chart("connections") + assert ["my_custom_type"] == Show.get_sensor_types_for_chart("my_custom_type") + end + end + + describe "calculate_chart_stats/1" do + test "returns {nil, nil} for nil input" do + assert {nil, nil} == Show.calculate_chart_stats(nil) + end + + test "computes max and min from chart data" do + chart_data = + Jason.encode!(%{ + datasets: [ + %{data: [%{y: 10}, %{y: 20}, %{y: 5}]} + ] + }) + + assert {20, 5} == Show.calculate_chart_stats(chart_data) + end + + test "handles multiple datasets" do + chart_data = + Jason.encode!(%{ + datasets: [ + %{data: [%{y: 100}, %{y: 200}]}, + %{data: [%{y: 50}, %{y: 300}]} + ] + }) + + assert {300, 50} == Show.calculate_chart_stats(chart_data) + end + + test "returns {nil, nil} for empty datasets" do + chart_data = Jason.encode!(%{datasets: [%{data: []}]}) + assert {nil, nil} == Show.calculate_chart_stats(chart_data) + end + + test "returns {nil, nil} for invalid JSON" do + assert {nil, nil} == Show.calculate_chart_stats("not valid json") + end + + test "returns {nil, nil} for JSON with missing datasets key" do + assert {nil, nil} == Show.calculate_chart_stats(~s({"foo": "bar"})) + end + + test "handles nil y values by skipping them" do + chart_data = + Jason.encode!(%{ + datasets: [ + %{data: [%{y: 10}, %{y: nil}, %{y: 30}]} + ] + }) + + assert {30, 10} == Show.calculate_chart_stats(chart_data) + end + + test "handles single data point" do + chart_data = + Jason.encode!(%{ + datasets: [ + %{data: [%{y: 42}]} + ] + }) + + assert {42, 42} == Show.calculate_chart_stats(chart_data) + end + end + + describe "humanize_sensor_type/1" do + test "converts snake_case to capitalized words" do + assert "Cpu Load" == Show.humanize_sensor_type("cpu_load") + assert "My Custom Type" == Show.humanize_sensor_type("my_custom_type") + end + + test "handles single word" do + assert "Temperature" == Show.humanize_sensor_type("temperature") + end + + test "handles empty string" do + assert "" == Show.humanize_sensor_type("") + end + + test "handles already uppercase" do + assert "Cpu Temperature" == Show.humanize_sensor_type("cpu_temperature") + end + end + + describe "build_graph_params/2" do + test "builds params with just range when no filters present" do + assigns = %{range: "24h", interface_id: nil, sensor_id: nil, storage_id: nil} + assert %{"range" => "24h"} == Show.build_graph_params(assigns, "24h") + end + + test "builds params with interface_id when present" do + assigns = %{range: "1h", interface_id: "iface-1", sensor_id: nil, storage_id: nil} + assert %{"range" => "1h", "interface_id" => "iface-1"} == Show.build_graph_params(assigns, "1h") + end + + test "builds params with sensor_id when present" do + assigns = %{range: "6h", interface_id: nil, sensor_id: "sensor-42", storage_id: nil} + assert %{"range" => "6h", "sensor_id" => "sensor-42"} == Show.build_graph_params(assigns, "6h") + end + + test "builds params with storage_id when present" do + assigns = %{range: "7d", interface_id: nil, sensor_id: nil, storage_id: "storage-5"} + assert %{"range" => "7d", "storage_id" => "storage-5"} == Show.build_graph_params(assigns, "7d") + end + + test "builds params with all filters when present" do + assigns = %{ + range: "30d", + interface_id: "iface-1", + sensor_id: "sensor-42", + storage_id: "storage-5" + } + + assert %{"range" => "30d", "interface_id" => "iface-1", "sensor_id" => "sensor-42", "storage_id" => "storage-5"} == + Show.build_graph_params(assigns, "30d") + end + + test "uses custom range override" do + assigns = %{range: "24h", interface_id: nil, sensor_id: nil, storage_id: nil} + assert %{"range" => "1h"} == Show.build_graph_params(assigns, "1h") + end + end end diff --git a/test/towerops_web/plugs/update_session_activity_test.exs b/test/towerops_web/plugs/update_session_activity_test.exs index 305ae778..82003a2c 100644 --- a/test/towerops_web/plugs/update_session_activity_test.exs +++ b/test/towerops_web/plugs/update_session_activity_test.exs @@ -1,6 +1,7 @@ defmodule ToweropsWeb.Plugs.UpdateSessionActivityTest do use ToweropsWeb.ConnCase, async: false + alias Towerops.Accounts.BrowserSession alias ToweropsWeb.Plugs.UpdateSessionActivity describe "init/1" do @@ -29,7 +30,6 @@ defmodule ToweropsWeb.Plugs.UpdateSessionActivityTest do end test "starts background task when user_token and live_socket_id present", %{conn: conn} do - # Exercises the get_session/2 call for :live_socket_id within call/2. conn = conn |> init_test_session(%{ @@ -41,4 +41,90 @@ defmodule ToweropsWeb.Plugs.UpdateSessionActivityTest do assert conn.state == :unset end end + + describe "session_timed_out?/1" do + test "returns true when session has been inactive for more than 30 minutes" do + old_time = DateTime.add(DateTime.utc_now(), -31, :minute) + session = %{last_activity_at: old_time} + + assert UpdateSessionActivity.session_timed_out?(session) + end + + test "returns false when session was recently active" do + recent_time = DateTime.add(DateTime.utc_now(), -5, :second) + session = %{last_activity_at: recent_time} + + refute UpdateSessionActivity.session_timed_out?(session) + end + + test "uses inserted_at when last_activity_at is nil" do + old_time = DateTime.add(DateTime.utc_now(), -31, :minute) + session = %{last_activity_at: nil, inserted_at: old_time} + + assert UpdateSessionActivity.session_timed_out?(session) + end + + test "returns false for recent inserted_at when last_activity_at is nil" do + recent_time = DateTime.add(DateTime.utc_now(), -5, :second) + session = %{last_activity_at: nil, inserted_at: recent_time} + + refute UpdateSessionActivity.session_timed_out?(session) + end + + test "returns false at exactly 30 minutes of inactivity" do + borderline_time = DateTime.add(DateTime.utc_now(), -30, :minute) + session = %{last_activity_at: borderline_time} + + refute UpdateSessionActivity.session_timed_out?(session) + end + + test "returns true just over 30 minutes of inactivity" do + old_time = DateTime.add(DateTime.utc_now(), -30, :minute) + old_time = DateTime.add(old_time, -1, :second) + session = %{last_activity_at: old_time} + + assert UpdateSessionActivity.session_timed_out?(session) + end + end + + describe "handle_session_activity/2" do + test "returns :ok when no session found for token" do + assert :ok == UpdateSessionActivity.handle_session_activity("nonexistent-token", nil) + end + + test "returns :ok when session found and not timed out" do + user = Towerops.AccountsFixtures.user_fixture() + {_token, user_token} = Towerops.Accounts.generate_user_session_token_with_record(user) + + %BrowserSession{} + |> BrowserSession.create_changeset(%{ + user_id: user.id, + user_token_id: user_token.id, + ip_address: "127.0.0.1", + last_activity_at: DateTime.utc_now(), + expires_at: DateTime.add(DateTime.utc_now(), 7, :day) + }) + |> Towerops.Repo.insert!() + + assert :ok == UpdateSessionActivity.handle_session_activity(user_token.id, nil) + end + end + + describe "revoke_inactive_session/2" do + test "returns nil when no live_socket_id" do + user = Towerops.AccountsFixtures.user_fixture() + token = Towerops.Accounts.generate_user_session_token(user) + + assert nil == UpdateSessionActivity.revoke_inactive_session(token, nil) + end + + test "broadcasts disconnect when live_socket_id present" do + user = Towerops.AccountsFixtures.user_fixture() + token = Towerops.Accounts.generate_user_session_token(user) + + ToweropsWeb.Endpoint.subscribe("users_sessions:test-session") + assert :ok == UpdateSessionActivity.revoke_inactive_session(token, "users_sessions:test-session") + assert_receive %Phoenix.Socket.Broadcast{event: "disconnect", topic: "users_sessions:test-session"} + end + end end diff --git a/test/towerops_web/remote_ip_test.exs b/test/towerops_web/remote_ip_test.exs index a6c85ce5..4643dc0d 100644 --- a/test/towerops_web/remote_ip_test.exs +++ b/test/towerops_web/remote_ip_test.exs @@ -3,17 +3,90 @@ defmodule ToweropsWeb.RemoteIpTest do alias ToweropsWeb.RemoteIp - test "extracts the first forwarded IP from conn headers", %{conn: conn} do - conn = - conn - |> put_req_header("x-forwarded-for", "203.0.113.10, 10.0.0.1") - |> put_req_header("x-real-ip", "198.51.100.1") + describe "from_conn/1" do + test "extracts the first forwarded IP when from a trusted proxy (10.x)", %{conn: conn} do + conn = + conn + |> Map.put(:remote_ip, {10, 0, 0, 1}) + |> put_req_header("x-forwarded-for", "203.0.113.10, 10.0.0.1") - assert RemoteIp.from_conn(conn) == "203.0.113.10" + assert RemoteIp.from_conn(conn) == "203.0.113.10" + end + + test "extracts the first forwarded IP when from a trusted proxy (172.16-31.x)", %{conn: conn} do + conn = + conn + |> Map.put(:remote_ip, {172, 16, 0, 1}) + |> put_req_header("x-forwarded-for", "198.51.100.1, 172.16.0.1") + + assert RemoteIp.from_conn(conn) == "198.51.100.1" + end + + test "extracts the first forwarded IP when from a trusted proxy (192.168.x)", %{conn: conn} do + conn = + conn + |> Map.put(:remote_ip, {192, 168, 1, 1}) + |> put_req_header("x-forwarded-for", "198.51.100.1") + + assert RemoteIp.from_conn(conn) == "198.51.100.1" + end + + test "uses x-real-ip when x-forwarded-for is missing and from trusted proxy", %{conn: conn} do + conn = + conn + |> Map.put(:remote_ip, {10, 0, 0, 1}) + |> put_req_header("x-real-ip", "198.51.100.1") + + assert RemoteIp.from_conn(conn) == "198.51.100.1" + end + + test "uses remote_ip directly when neither forwarded header is present and from trusted proxy", + %{conn: conn} do + conn = Map.put(conn, :remote_ip, {10, 0, 0, 1}) + + assert RemoteIp.from_conn(conn) == "10.0.0.1" + end + + test "uses remote_ip directly when NOT from a trusted proxy", %{conn: conn} do + # 8.8.8.8 is a public IP, not a trusted proxy — x-forwarded-for is ignored + conn = + conn + |> Map.put(:remote_ip, {8, 8, 8, 8}) + |> put_req_header("x-forwarded-for", "203.0.113.10") + + assert RemoteIp.from_conn(conn) == "8.8.8.8" + end + + test "trims whitespace from forwarded IP", %{conn: conn} do + conn = + conn + |> Map.put(:remote_ip, {10, 0, 0, 1}) + |> put_req_header("x-forwarded-for", " 203.0.113.10 , 10.0.0.1") + + assert RemoteIp.from_conn(conn) == "203.0.113.10" + end end - test "formats IPv6 addresses in uppercase hex" do - assert RemoteIp.format({8193, 3512, 0, 0, 0, 0, 0, 1}) == "2001:DB8:0:0:0:0:0:1" + describe "format/1" do + test "formats IPv4 tuple" do + assert RemoteIp.format({192, 168, 1, 1}) == "192.168.1.1" + assert RemoteIp.format({10, 0, 0, 1}) == "10.0.0.1" + assert RemoteIp.format({0, 0, 0, 0}) == "0.0.0.0" + end + + test "formats IPv6 addresses in uppercase hex" do + assert RemoteIp.format({8193, 3512, 0, 0, 0, 0, 0, 1}) == "2001:DB8:0:0:0:0:0:1" + end + + test "formats loopback IPv6" do + assert RemoteIp.format({0, 0, 0, 0, 0, 0, 0, 1}) == "0:0:0:0:0:0:0:1" + end + + test "returns nil for invalid tuple" do + assert RemoteIp.format(:error) == nil + assert RemoteIp.format("not-a-tuple") == nil + assert RemoteIp.format(nil) == nil + end end describe "from_socket/1" do