fix discovery issue

lib/towerops/snmp/discovery.ex

@@ -31,6 +31,7 @@ defmodule Towerops.Snmp.Discovery do
   alias Towerops.Snmp.Profiles.NetSnmp
   alias Towerops.Snmp.Profiles.Ubiquiti
   alias Towerops.Snmp.Sensor
+  alias Towerops.Snmp.Vlan
 
   require Logger
 
@@ -130,7 +131,8 @@ defmodule Towerops.Snmp.Discovery do
          {:ok, device_info} <- build_device_info(client_opts, system_info, profile),
          {:ok, interfaces} <- discover_interfaces_with_timeout(client_opts, profile, timeouts),
          {:ok, sensors} <- discover_sensors_with_timeout(client_opts, profile, timeouts),
-         {:ok, discovered_device} <- save_discovery_results(device, device_info, interfaces, sensors),
+         {:ok, vlans} <- discover_vlans_with_timeout(client_opts, profile, timeouts),
+         {:ok, discovered_device} <- save_discovery_results(device, device_info, interfaces, sensors, vlans),
          {:ok, neighbors} <- discover_neighbors_with_timeout(client_opts, discovered_device.interfaces, timeouts),
          :ok <- save_neighbors(discovered_device.device_id, neighbors) do
       _ = update_device_discovery_time(device)
@@ -189,6 +191,16 @@ defmodule Towerops.Snmp.Discovery do
     )
   end
 
+  # VLAN discovery with timeout - falls back to empty list on timeout
+  defp discover_vlans_with_timeout(client_opts, profile, timeouts) do
+    DeferredDiscovery.slow_check(
+      client_opts,
+      fn -> discover_vlans(client_opts, profile) end,
+      timeout: timeouts[:slow],
+      default: []
+    )
+  end
+
   @doc """
   Runs discovery for all SNMP-enabled devices in an organization.
   Returns a summary of successful and failed discoveries.
@@ -411,20 +423,43 @@ defmodule Towerops.Snmp.Discovery do
     end
   end
 
+  @spec discover_vlans(Client.connection_opts(), profile()) :: {:ok, [map()]}
+  defp discover_vlans(client_opts, {:dynamic, _profile}) do
+    # Dynamic profiles use Base VLAN discovery
+    {:ok, vlans} = Base.discover_vlans(client_opts)
+    Logger.debug("Discovered #{length(vlans)} VLANs")
+    {:ok, vlans}
+  end
+
+  defp discover_vlans(client_opts, profile) when is_atom(profile) do
+    if function_exported?(profile, :discover_vlans, 1) do
+      {:ok, vlans} = profile.discover_vlans(client_opts)
+      Logger.debug("Discovered #{length(vlans)} VLANs")
+      {:ok, vlans}
+    else
+      # Fall back to Base profile for VLAN discovery
+      {:ok, vlans} = Base.discover_vlans(client_opts)
+      Logger.debug("Discovered #{length(vlans)} VLANs using Base profile")
+      {:ok, vlans}
+    end
+  end
+
   @spec save_discovery_results(
           DeviceSchema.t(),
           device_info(),
           [interface_data()],
-          [sensor_data()]
+          [sensor_data()],
+          [map()]
         ) :: {:ok, Device.t()} | {:error, term()}
-  defp save_discovery_results(device, device_info, interfaces, sensors) do
+  defp save_discovery_results(device, device_info, interfaces, sensors, vlans) do
     Repo.transaction(fn ->
       # Upsert Device
       snmp_device = upsert_device(device, device_info)
 
-      # Sync interfaces and sensors (preserving historical data)
+      # Sync interfaces, sensors, and VLANs (preserving historical data)
       synced_interfaces = sync_interfaces(snmp_device, interfaces)
       _ = sync_sensors(snmp_device, sensors)
+      _ = sync_vlans(snmp_device, vlans)
 
       # Return device with interfaces loaded and device_id added
       # Use snmp_device.device_id which references the Equipment table
@@ -437,7 +472,12 @@ defmodule Towerops.Snmp.Discovery do
     Logger.info("upsert_device called with device_info keys: #{inspect(Map.keys(device_info))}")
 
     # Extract raw discovery data if present (using underscore prefix to distinguish from regular fields)
-    raw_discovery_data = Map.get(device_info, :_raw_discovery_data)
+    # Sanitize to ensure all binary data is JSON-encodable
+    raw_discovery_data =
+      device_info
+      |> Map.get(:_raw_discovery_data)
+      |> sanitize_for_json()
+
     last_discovery_at = Map.get(device_info, :_last_discovery_at, DateTime.utc_now())
 
     Logger.info("Extracted raw_discovery_data: #{inspect(raw_discovery_data != nil)}")
@@ -558,6 +598,50 @@ defmodule Towerops.Snmp.Discovery do
     end)
   end
 
+  @spec sync_vlans(Device.t(), [map()]) :: [Vlan.t()]
+  defp sync_vlans(device, discovered_vlans) do
+    # Get existing VLANs for this device, indexed by vlan_id
+    existing_vlans =
+      from(v in Vlan, where: v.snmp_device_id == ^device.id)
+      |> Repo.all()
+      |> Map.new(&{&1.vlan_id, &1})
+
+    # Get the set of discovered vlan_ids
+    discovered_vlan_ids = MapSet.new(discovered_vlans, & &1.vlan_id)
+
+    # Delete VLANs that no longer exist on the device
+    existing_vlan_ids = MapSet.new(Map.keys(existing_vlans))
+    removed_vlan_ids = MapSet.difference(existing_vlan_ids, discovered_vlan_ids)
+
+    if MapSet.size(removed_vlan_ids) != 0 do
+      removed_list = MapSet.to_list(removed_vlan_ids)
+
+      Repo.delete_all(
+        from v in Vlan,
+          where: v.snmp_device_id == ^device.id and v.vlan_id in ^removed_list
+      )
+
+      Logger.debug("Deleted #{MapSet.size(removed_vlan_ids)} removed VLANs")
+    end
+
+    # Upsert each discovered VLAN
+    Enum.map(discovered_vlans, fn vlan_data ->
+      case Map.get(existing_vlans, vlan_data.vlan_id) do
+        nil ->
+          # New VLAN - insert
+          %Vlan{}
+          |> Vlan.changeset(Map.put(vlan_data, :snmp_device_id, device.id))
+          |> Repo.insert!()
+
+        existing ->
+          # Existing VLAN - update
+          existing
+          |> Vlan.changeset(vlan_data)
+          |> Repo.update!()
+      end
+    end)
+  end
+
   @spec update_device_discovery_time(DeviceSchema.t()) ::
           {:ok, DeviceSchema.t()} | {:error, Ecto.Changeset.t()}
   defp update_device_discovery_time(device) do
@@ -641,4 +725,48 @@ defmodule Towerops.Snmp.Discovery do
       {:device_event, event_attrs}
     )
   end
+
+  # Sanitizes data structures to ensure they are JSON-encodable
+  # Converts non-UTF8 binary strings to base64 representation
+  @spec sanitize_for_json(term()) :: term()
+  defp sanitize_for_json(nil), do: nil
+
+  # Handle DateTime and other structs before generic map clause
+  defp sanitize_for_json(%DateTime{} = dt), do: DateTime.to_iso8601(dt)
+  defp sanitize_for_json(%NaiveDateTime{} = dt), do: NaiveDateTime.to_iso8601(dt)
+  defp sanitize_for_json(%Date{} = d), do: Date.to_iso8601(d)
+  defp sanitize_for_json(%Time{} = t), do: Time.to_iso8601(t)
+
+  # Generic struct handling - convert to map first, then sanitize
+  defp sanitize_for_json(%{__struct__: _} = struct) do
+    struct
+    |> Map.from_struct()
+    |> sanitize_for_json()
+  end
+
+  defp sanitize_for_json(map) when is_map(map) do
+    Map.new(map, fn {k, v} -> {sanitize_for_json(k), sanitize_for_json(v)} end)
+  end
+
+  defp sanitize_for_json(list) when is_list(list) do
+    Enum.map(list, &sanitize_for_json/1)
+  end
+
+  defp sanitize_for_json(binary) when is_binary(binary) do
+    if String.printable?(binary) do
+      binary
+    else
+      # Convert non-printable binary to base64 with marker
+      # Use Elixir.Base to avoid conflict with Towerops.Snmp.Profiles.Base alias
+      %{"_binary_base64" => Elixir.Base.encode64(binary)}
+    end
+  end
+
+  defp sanitize_for_json(tuple) when is_tuple(tuple) do
+    tuple
+    |> Tuple.to_list()
+    |> Enum.map(&sanitize_for_json/1)
+  end
+
+  defp sanitize_for_json(other), do: other
 end