more tests and logging around email

lib/towerops/accounts/user_notifier.ex

@@ -1,10 +1,13 @@
 defmodule Towerops.Accounts.UserNotifier do
   @moduledoc false
+
   import Swoosh.Email
 
   alias Towerops.Accounts.User
   alias Towerops.Mailer
 
+  require Logger
+
   # Delivers the email using the application mailer.
   defp deliver(recipient, subject, body) do
     from_address = Application.get_env(:towerops, :mailer_from, {"Towerops", "hi@towerops.net"})
@@ -16,8 +19,14 @@ defmodule Towerops.Accounts.UserNotifier do
       |> subject(subject)
       |> text_body(body)
 
-    with {:ok, _metadata} <- Mailer.deliver(email) do
-      {:ok, email}
+    case Mailer.deliver(email) do
+      {:ok, _metadata} ->
+        Logger.info("User email sent to #{recipient}: #{subject}")
+        {:ok, email}
+
+      {:error, reason} = error ->
+        Logger.error("Failed to send user email to #{recipient}: #{inspect(reason)}")
+        error
     end
   end
 

lib/towerops/alerts/alert_notifier.ex

@@ -10,6 +10,8 @@ defmodule Towerops.Alerts.AlertNotifier do
   alias Towerops.Mailer
   alias Towerops.Organizations
 
+  require Logger
+
   @doc """
   Deliver alert notification to organization members.
 
@@ -99,8 +101,14 @@ defmodule Towerops.Alerts.AlertNotifier do
       |> subject(subject)
       |> text_body(body)
 
-    with {:ok, _metadata} <- Mailer.deliver(email) do
-      {:ok, email}
+    case Mailer.deliver(email) do
+      {:ok, _metadata} ->
+        Logger.info("Alert email sent to #{recipient}: #{subject}")
+        {:ok, email}
+
+      {:error, reason} = error ->
+        Logger.error("Failed to send alert email to #{recipient}: #{inspect(reason)}")
+        error
     end
   end
 end

test/towerops/snmp/profile_behaviour_test.exs

@@ -0,0 +1,351 @@
+defmodule Towerops.Snmp.ProfileBehaviourTest do
+  @moduledoc """
+  Generic tests for SNMP profile behavior.
+  Tests that all profiles implement the expected contract without testing specific OID values.
+
+  This approach tests BEHAVIOR, not implementation details:
+  - Does the profile implement the required functions?
+  - Do they return data in the expected format?
+  - Do they handle missing data gracefully?
+
+  We don't test specific OID values or vendor-specific MIB details.
+  """
+
+  use ExUnit.Case, async: true
+
+  import Mox
+
+  alias Towerops.Snmp.Profiles.Base
+  alias Towerops.Snmp.Profiles.Cisco
+  alias Towerops.Snmp.Profiles.Mikrotik
+  alias Towerops.Snmp.Profiles.NetSnmp
+  alias Towerops.Snmp.SnmpMock
+
+  # Verify mocks are set correctly
+  setup :verify_on_exit!
+
+  @test_opts [
+    ip: "192.168.1.1",
+    community: "public",
+    version: "2c",
+    port: 161,
+    timeout: 5000
+  ]
+
+  # Test all profile modules
+  @profiles [Base, Cisco, Mikrotik, NetSnmp]
+
+  describe "profile contract compliance" do
+    for profile <- @profiles do
+      @tag profile: profile
+      test "#{inspect(profile)} implements discover_system_info/1 and returns valid data" do
+        profile = unquote(profile)
+
+        # Mock successful SNMP responses for system MIB
+        stub(SnmpMock, :get, fn _, oid, _ ->
+          case oid do
+            "1.3.6.1.2.1.1.1.0" -> {:ok, {:octet_string, "Test Device"}}
+            "1.3.6.1.2.1.1.2.0" -> {:ok, {:object_identifier, "1.3.6.1.4.1.9.1.1"}}
+            "1.3.6.1.2.1.1.3.0" -> {:ok, {:timeticks, 12_345}}
+            "1.3.6.1.2.1.1.4.0" -> {:ok, {:octet_string, "admin@example.com"}}
+            "1.3.6.1.2.1.1.5.0" -> {:ok, {:octet_string, "test-host"}}
+            "1.3.6.1.2.1.1.6.0" -> {:ok, {:octet_string, "datacenter"}}
+            _ -> {:error, :no_such_object}
+          end
+        end)
+
+        stub(SnmpMock, :walk, fn _, _, _ -> {:ok, []} end)
+        stub(SnmpMock, :get_bulk, fn _, _, _ -> {:ok, []} end)
+
+        case profile.discover_system_info(@test_opts) do
+          {:ok, info} ->
+            assert is_map(info)
+            assert map_size(info) > 0
+
+          {:error, _reason} ->
+            # Some profiles might fail if required data is missing
+            :ok
+        end
+      end
+
+      @tag profile: profile
+      test "#{inspect(profile)} implements discover_interfaces/1 and returns valid structure" do
+        profile = unquote(profile)
+
+        # Mock IF-MIB interface data
+        stub(SnmpMock, :walk, fn _, oid, _ ->
+          case oid do
+            "1.3.6.1.2.1.2.2.1.1" ->
+              {:ok,
+               [
+                 %{oid: "1.3.6.1.2.1.2.2.1.1.1", value: {:integer, 1}},
+                 %{oid: "1.3.6.1.2.1.2.2.1.1.2", value: {:integer, 2}}
+               ]}
+
+            "1.3.6.1.2.1.2.2.1.2" ->
+              {:ok,
+               [
+                 %{oid: "1.3.6.1.2.1.2.2.1.2.1", value: {:octet_string, "eth0"}},
+                 %{oid: "1.3.6.1.2.1.2.2.1.2.2", value: {:octet_string, "eth1"}}
+               ]}
+
+            _ ->
+              {:ok, []}
+          end
+        end)
+
+        stub(SnmpMock, :get, fn _, _, _ -> {:error, :no_such_object} end)
+        stub(SnmpMock, :get_bulk, fn _, _, _ -> {:ok, []} end)
+
+        assert {:ok, interfaces} = profile.discover_interfaces(@test_opts)
+        assert is_list(interfaces)
+
+        # If interfaces found, they should have required fields
+        for interface <- interfaces do
+          assert is_map(interface)
+          assert Map.has_key?(interface, :if_index)
+          assert Map.has_key?(interface, :if_descr)
+          assert is_integer(interface.if_index)
+          assert is_binary(interface.if_descr)
+        end
+      end
+
+      @tag profile: profile
+      test "#{inspect(profile)} implements discover_sensors/1 and returns valid structure" do
+        profile = unquote(profile)
+
+        # Mock generic sensor data that might be discovered by any profile
+        stub(SnmpMock, :walk, fn _, _, _ -> {:ok, []} end)
+        stub(SnmpMock, :get, fn _, _, _ -> {:error, :no_such_object} end)
+        stub(SnmpMock, :get_bulk, fn _, _, _ -> {:ok, []} end)
+
+        assert {:ok, sensors} = profile.discover_sensors(@test_opts)
+        assert is_list(sensors)
+
+        # If sensors found, they should have required fields
+        for sensor <- sensors do
+          assert is_map(sensor)
+          assert Map.has_key?(sensor, :sensor_type)
+          assert Map.has_key?(sensor, :sensor_index)
+          assert Map.has_key?(sensor, :sensor_oid)
+          assert Map.has_key?(sensor, :sensor_descr)
+          assert Map.has_key?(sensor, :sensor_unit)
+          assert Map.has_key?(sensor, :sensor_divisor)
+
+          # Validate field types
+          assert is_binary(sensor.sensor_type)
+          assert is_binary(sensor.sensor_index)
+          assert is_binary(sensor.sensor_oid)
+          assert is_binary(sensor.sensor_descr)
+          assert is_binary(sensor.sensor_unit)
+          assert is_number(sensor.sensor_divisor)
+        end
+      end
+
+      @tag profile: profile
+      test "#{inspect(profile)} implements identify_device/1 and returns device info" do
+        profile = unquote(profile)
+
+        system_info = %{
+          sys_descr: "Test Device Description",
+          sys_object_id: "1.3.6.1.4.1.9.1.1",
+          sys_name: "test-device"
+        }
+
+        result = profile.identify_device(system_info)
+
+        assert is_map(result)
+        # Should preserve original system info
+        assert Map.has_key?(result, :sys_descr)
+        # Should add device identification
+        assert Map.has_key?(result, :manufacturer)
+        assert Map.has_key?(result, :model)
+        assert is_binary(result.manufacturer)
+        assert is_binary(result.model)
+      end
+    end
+  end
+
+  describe "graceful degradation" do
+    test "profiles handle missing SNMP data gracefully" do
+      # All SNMP calls return empty/error
+      stub(SnmpMock, :get, fn _, _, _ -> {:error, :no_such_object} end)
+      stub(SnmpMock, :walk, fn _, _, _ -> {:ok, []} end)
+      stub(SnmpMock, :get_bulk, fn _, _, _ -> {:ok, []} end)
+
+      for profile <- @profiles do
+        # Should not crash
+        result = profile.discover_interfaces(@test_opts)
+        assert match?({:ok, _}, result)
+
+        result = profile.discover_sensors(@test_opts)
+        assert match?({:ok, _}, result)
+      end
+    end
+  end
+
+  describe "device identification" do
+    test "Base profile identifies generic devices" do
+      system_info = %{sys_descr: "Unknown Device", sys_object_id: "1.2.3.4"}
+      result = Base.identify_device(system_info)
+
+      assert result.manufacturer == "Unknown"
+      assert result.model == "Generic Device"
+    end
+
+    test "Cisco profile identifies Cisco from sysDescr containing 'Cisco' or 'IOS'" do
+      system_info = %{sys_descr: "Cisco IOS Software"}
+      result = Cisco.identify_device(system_info)
+
+      assert result.manufacturer == "Cisco"
+      assert is_binary(result.model)
+    end
+
+    test "NetSnmp profile identifies Linux from sysDescr containing 'Linux'" do
+      system_info = %{sys_descr: "Linux test-host 5.10.0"}
+      result = NetSnmp.identify_device(system_info)
+
+      assert result.manufacturer == "Linux"
+      assert is_binary(result.model)
+    end
+
+    test "Mikrotik profile identifies MikroTik from sysDescr containing 'RouterOS'" do
+      system_info = %{sys_descr: "RouterOS RB750"}
+      result = Mikrotik.identify_device(system_info)
+
+      assert result.manufacturer == "MikroTik"
+      assert is_binary(result.model)
+    end
+  end
+
+  describe "interface discovery" do
+    test "Base profile discovers interfaces from standard IF-MIB" do
+      # Mock the walk for ifIndex - return list of maps as snmpkit does
+      stub(SnmpMock, :walk, fn _, oid, _ ->
+        case oid do
+          "1.3.6.1.2.1.2.2.1.1" ->
+            # Return list of maps format that snmpkit returns
+            {:ok,
+             [
+               %{oid: "1.3.6.1.2.1.2.2.1.1.1", value: {:integer, 1}},
+               %{oid: "1.3.6.1.2.1.2.2.1.1.2", value: {:integer, 2}}
+             ]}
+
+          _ ->
+            {:ok, []}
+        end
+      end)
+
+      # Mock individual get calls for each interface field
+      stub(SnmpMock, :get, fn _, oid, _ ->
+        case oid do
+          # Interface 1
+          "1.3.6.1.2.1.2.2.1.2.1" -> {:ok, {:octet_string, "GigabitEthernet0/1"}}
+          "1.3.6.1.2.1.31.1.1.1.1.1" -> {:ok, {:octet_string, "Gi0/1"}}
+          "1.3.6.1.2.1.31.1.1.1.18.1" -> {:ok, {:octet_string, "Uplink"}}
+          "1.3.6.1.2.1.2.2.1.3.1" -> {:ok, {:integer, 6}}
+          "1.3.6.1.2.1.2.2.1.5.1" -> {:ok, {:gauge32, 1_000_000_000}}
+          "1.3.6.1.2.1.2.2.1.6.1" -> {:ok, {:octet_string, <<0x00, 0x1A, 0x2B, 0x3C, 0x4D, 0x5E>>}}
+          "1.3.6.1.2.1.2.2.1.7.1" -> {:ok, {:integer, 1}}
+          "1.3.6.1.2.1.2.2.1.8.1" -> {:ok, {:integer, 1}}
+          # Interface 2
+          "1.3.6.1.2.1.2.2.1.2.2" -> {:ok, {:octet_string, "GigabitEthernet0/2"}}
+          "1.3.6.1.2.1.31.1.1.1.1.2" -> {:ok, {:octet_string, "Gi0/2"}}
+          "1.3.6.1.2.1.31.1.1.1.18.2" -> {:ok, {:octet_string, "Server Port"}}
+          "1.3.6.1.2.1.2.2.1.3.2" -> {:ok, {:integer, 6}}
+          "1.3.6.1.2.1.2.2.1.5.2" -> {:ok, {:gauge32, 1_000_000_000}}
+          "1.3.6.1.2.1.2.2.1.6.2" -> {:ok, {:octet_string, <<0x00, 0x1A, 0x2B, 0x3C, 0x4D, 0x5F>>}}
+          "1.3.6.1.2.1.2.2.1.7.2" -> {:ok, {:integer, 1}}
+          "1.3.6.1.2.1.2.2.1.8.2" -> {:ok, {:integer, 1}}
+          _ -> {:error, :no_such_object}
+        end
+      end)
+
+      stub(SnmpMock, :get_bulk, fn _, _, _ -> {:ok, []} end)
+
+      assert {:ok, interfaces} = Base.discover_interfaces(@test_opts)
+      assert length(interfaces) == 2
+
+      [iface1, iface2] = Enum.sort_by(interfaces, & &1.if_index)
+
+      assert iface1.if_index == 1
+      assert iface1.if_descr == "GigabitEthernet0/1"
+
+      assert iface2.if_index == 2
+      assert iface2.if_descr == "GigabitEthernet0/2"
+    end
+
+    test "profiles return empty list when no interfaces available" do
+      stub(SnmpMock, :walk, fn _, _, _ -> {:ok, []} end)
+      stub(SnmpMock, :get, fn _, _, _ -> {:error, :no_such_object} end)
+      stub(SnmpMock, :get_bulk, fn _, _, _ -> {:ok, []} end)
+
+      for profile <- @profiles do
+        assert {:ok, interfaces} = profile.discover_interfaces(@test_opts)
+        assert interfaces == []
+      end
+    end
+  end
+
+  describe "sensor discovery" do
+    test "Base profile discovers ENTITY-SENSOR-MIB sensors when available" do
+      # Mock walk to find sensor type indices - return list of maps as snmpkit does
+      stub(SnmpMock, :walk, fn _, oid, _ ->
+        case oid do
+          "1.3.6.1.2.1.99.1.1.1.1" ->
+            # Return sensor type indices - 8 is celsius type
+            {:ok,
+             [
+               %{oid: "1.3.6.1.2.1.99.1.1.1.1.1", value: {:integer, 8}}
+             ]}
+
+          _ ->
+            {:ok, []}
+        end
+      end)
+
+      # Mock individual get calls for building sensor data
+      # Base.build_sensor_from_entity_mib calls Client.get_multiple with [type, scale, value, status]
+      stub(SnmpMock, :get, fn _, oid, _ ->
+        case oid do
+          # Sensor index 1 data
+          # type = celsius (8)
+          "1.3.6.1.2.1.99.1.1.1.1.1" -> {:ok, {:integer, 8}}
+          # scale = units (9) = divisor 1
+          "1.3.6.1.2.1.99.1.1.1.2.1" -> {:ok, {:integer, 9}}
+          # value = 45
+          "1.3.6.1.2.1.99.1.1.1.4.1" -> {:ok, {:integer, 45}}
+          # status = ok (1)
+          "1.3.6.1.2.1.99.1.1.1.5.1" -> {:ok, {:integer, 1}}
+          _ -> {:error, :no_such_object}
+        end
+      end)
+
+      stub(SnmpMock, :get_bulk, fn _, _, _ -> {:ok, []} end)
+
+      assert {:ok, sensors} = Base.discover_sensors(@test_opts)
+
+      case sensors do
+        [sensor | _] ->
+          assert sensor.sensor_type == "celsius"
+          assert sensor.sensor_descr == "Sensor 1"
+          assert is_number(sensor.last_value)
+          assert sensor.last_value == 45.0
+
+        [] ->
+          flunk("Expected at least one sensor to be discovered")
+      end
+    end
+
+    test "profiles return empty list when no sensors available" do
+      stub(SnmpMock, :walk, fn _, _, _ -> {:ok, []} end)
+      stub(SnmpMock, :get, fn _, _, _ -> {:error, :no_such_object} end)
+      stub(SnmpMock, :get_bulk, fn _, _, _ -> {:ok, []} end)
+
+      for profile <- @profiles do
+        assert {:ok, sensors} = profile.discover_sensors(@test_opts)
+        assert is_list(sensors)
+      end
+    end
+  end
+end