From 7e896f80f7c03fe9ff8a30602afa546376ea16dc Mon Sep 17 00:00:00 2001
From: Alex Banna <alexanderbanna@gmail.com>
Date: Thu, 26 Mar 2026 00:04:10 -0500
Subject: [PATCH] feat(config): add PORT env var support with validation and
 tests

- Extract port resolution into testable resolve_port/0 function
- Validate PORT is a positive integer within valid range (1-65535)
- Log warning and fall back to default (4000) for invalid PORT values
- Add 10 tests covering: default port, custom port, edge cases
  (non-numeric, empty, zero, overflow, trailing chars, negative)

Implements: HAR-571
---
 lib/items_api/application.ex        |  39 +++++++++-
 test/items_api/port_config_test.exs | 110 ++++++++++++++++++++++++++++
 2 files changed, 148 insertions(+), 1 deletion(-)
 create mode 100644 test/items_api/port_config_test.exs

diff --git a/lib/items_api/application.ex b/lib/items_api/application.ex
index d42d6d7..eba2749 100644
--- a/lib/items_api/application.ex
+++ b/lib/items_api/application.ex
@@ -1,9 +1,11 @@
 defmodule ItemsApi.Application do
   use Application
 
+  @default_port 4000
+
   @impl true
   def start(_type, _args) do
-    port = String.to_integer(System.get_env("PORT") || "4000")
+    port = resolve_port()
 
     children = [
       {ItemsApi.Repo, []},
@@ -18,6 +20,41 @@ defmodule ItemsApi.Application do
     {:ok, pid}
   end
 
+  @doc """
+  Resolves the port from the PORT environment variable.
+
+  Returns the integer value of PORT if set and valid,
+  otherwise returns the default port (4000).
+
+  Invalid PORT values (non-numeric strings) are ignored
+  and the default port is used instead, with a warning logged.
+  """
+  @spec resolve_port() :: non_neg_integer()
+  def resolve_port do
+    case System.get_env("PORT") do
+      nil ->
+        @default_port
+
+      port_string ->
+        parse_port(port_string)
+    end
+  end
+
+  @doc false
+  def default_port, do: @default_port
+
+  defp parse_port(port_string) do
+    case Integer.parse(port_string) do
+      {port, ""} when port > 0 and port <= 65_535 ->
+        port
+
+      _ ->
+        require Logger
+        Logger.warning("Invalid PORT value #{inspect(port_string)}, falling back to #{@default_port}")
+        @default_port
+    end
+  end
+
   defp run_migrations do
     migrations_path = Application.app_dir(:items_api, "priv/repo/migrations")
     Ecto.Migrator.run(ItemsApi.Repo, migrations_path, :up, all: true)
diff --git a/test/items_api/port_config_test.exs b/test/items_api/port_config_test.exs
new file mode 100644
index 0000000..9f34d7b
--- /dev/null
+++ b/test/items_api/port_config_test.exs
@@ -0,0 +1,110 @@
+defmodule ItemsApi.PortConfigTest do
+  use ExUnit.Case, async: false
+
+  describe "resolve_port/0" do
+    test "returns default port 4000 when PORT env var is not set" do
+      System.delete_env("PORT")
+      assert ItemsApi.Application.resolve_port() == 4000
+    end
+
+    test "returns custom port when PORT env var is set" do
+      System.put_env("PORT", "5001")
+
+      try do
+        assert ItemsApi.Application.resolve_port() == 5001
+      after
+        System.delete_env("PORT")
+      end
+    end
+
+    test "returns custom port for various valid values" do
+      for {input, expected} <- [{"8080", 8080}, {"3000", 3000}, {"1", 1}, {"65535", 65535}] do
+        System.put_env("PORT", input)
+
+        try do
+          assert ItemsApi.Application.resolve_port() == expected,
+                 "Expected PORT=#{input} to resolve to #{expected}"
+        after
+          System.delete_env("PORT")
+        end
+      end
+    end
+
+    test "falls back to default for non-numeric PORT value" do
+      System.put_env("PORT", "abc")
+
+      try do
+        assert ItemsApi.Application.resolve_port() == 4000
+      after
+        System.delete_env("PORT")
+      end
+    end
+
+    test "falls back to default for empty string PORT" do
+      System.put_env("PORT", "")
+
+      try do
+        assert ItemsApi.Application.resolve_port() == 4000
+      after
+        System.delete_env("PORT")
+      end
+    end
+
+    test "falls back to default for PORT value of 0" do
+      System.put_env("PORT", "0")
+
+      try do
+        assert ItemsApi.Application.resolve_port() == 4000
+      after
+        System.delete_env("PORT")
+      end
+    end
+
+    test "falls back to default for PORT value exceeding 65535" do
+      System.put_env("PORT", "70000")
+
+      try do
+        assert ItemsApi.Application.resolve_port() == 4000
+      after
+        System.delete_env("PORT")
+      end
+    end
+
+    test "falls back to default for PORT with trailing characters" do
+      System.put_env("PORT", "4000abc")
+
+      try do
+        assert ItemsApi.Application.resolve_port() == 4000
+      after
+        System.delete_env("PORT")
+      end
+    end
+
+    test "falls back to default for negative PORT value" do
+      System.put_env("PORT", "-1")
+
+      try do
+        assert ItemsApi.Application.resolve_port() == 4000
+      after
+        System.delete_env("PORT")
+      end
+    end
+  end
+
+  describe "default_port/0" do
+    test "returns 4000" do
+      assert ItemsApi.Application.default_port() == 4000
+    end
+  end
+
+  describe "application startup" do
+    test "Bandit is running and accepting connections" do
+      # The app is already started by mix test. Verify Bandit is listening.
+      assert Process.whereis(ItemsApi.Supervisor) != nil
+
+      children = Supervisor.which_children(ItemsApi.Supervisor)
+      bandit_running = Enum.any?(children, fn {_id, pid, _type, _modules} -> is_pid(pid) end)
+      assert bandit_running, "Expected Bandit to be running under supervisor"
+    end
+  end
+end