WIP: Move Config.all_channels/0 and Config.channel_modules/1 logic into State; add State.add_loaded_module/2

The problem:

When we're going through the list of modules to send messages to based
on the channels they're a part of, it was being done so through the
config. Since the config doesn't (and shouldn't) list all of the core
modules that get included, any core modules that were loaded and running
under the ModuleSupervisor would not get included in the router's
attempt to send messages to a module.

Now, the Config.all_channels and Config.channel_modules functions live
in State, and State has a new "add_loaded_module" function where loaded
modules are registered. The aforementioned moved functions will use this
as the "source of truth" when deciding where to send messages for
modules to handle.

Signed-off-by: Alek Ratzloff <alekratz@gmail.com>

lib/config.ex

@@ -25,17 +25,6 @@ defmodule Omnibot.Config do
       |> MapSet.to_list()
   end
 
-  @doc ~S"""
-  Gets a list of all `{module, mod_cfg}` pairs from the given configuration
-  that are listening to the given channel.
-  """
-  def channel_modules(cfg, channel) do
-    cfg.modules 
-    |> Enum.filter(fn {_, cfg} ->
-      cfg[:channels] == :all or Enum.member?(cfg[:channels] || [], channel) 
-    end)
-  end
-
   def msg_prefix(cfg) do
     %Msg.Prefix {
       nick: cfg.nick,

lib/contrib/fortune.ex

@@ -20,12 +20,12 @@ defmodule Omnibot.Contrib.Fortune do
   command "!fortune", [to] do
     fortune = Enum.random(@fortunes)
     reply = "#{to}: #{fortune}"
-    Irc.send_to(channel, reply)
+    Irc.send_to(irc, channel, reply)
   end
 
   command "!fortune" do
     fortune = Enum.random(@fortunes)
     reply = "#{nick}: #{fortune}"
-    Irc.send_to(channel, reply)
+    Irc.send_to(irc, channel, reply)
   end
 end

lib/core/welcome.ex

@@ -0,0 +1,11 @@
+defmodule Omnibot.Core.Welcome do
+  use Omnibot.Module
+
+  require Logger
+
+  @impl true
+  def on_msg(irc, _) do
+    Logger.info("Syncing channels")
+    Irc.sync_channels(irc)
+  end
+end

lib/irc.ex

@@ -22,16 +22,12 @@ defmodule Omnibot.Irc do
 
   def send_msg(irc, command, param), do: send_msg(irc, command, [param])
 
-  def send_to(channel, text), do: send_to(__MODULE__, channel, text)
   def send_to(irc, channel, text), do: send_msg(irc, "PRIVMSG", [channel, text])
 
-  def join(channel), do: join(__MODULE__, channel)
   def join(irc, channel), do: send_msg(irc, "JOIN", channel)
 
-  def part(channel), do: part(__MODULE__, channel)
   def part(irc, channel), do: send_msg(irc, "PART", channel)
 
-  def sync_channels(), do: sync_channels(__MODULE__)
   def sync_channels(irc), do: GenServer.cast(irc, :sync_channels)
 
   ## Server callbacks
@@ -45,7 +41,6 @@ defmodule Omnibot.Irc do
       :gen_tcp.connect(to_charlist(cfg.server), cfg.port, [:binary, active: false, packet: :line])
 
     # Wait for first message
-    #{:ok, _} = :gen_tcp.recv(socket, 0)
     send_msg(self(), "NICK", cfg.nick)
     send_msg(self(), "USER", [cfg.user, "0", "*", cfg.real])
     :inet.setopts(socket, [active: true])

lib/module.ex

@@ -3,19 +3,19 @@ defmodule Omnibot.Module do
     defmacro __before_compile__(_env) do
       quote generated: true do
         @impl true
-        def on_channel_msg(_channel, _nick, _line), do: nil
+        def on_channel_msg(_irc, _channel, _nick, _line), do: nil
 
         @impl true
-        def on_channel_msg(_channel, _nick, _cmd, _params), do: nil
+        def on_channel_msg(_irc, _channel, _nick, _cmd, _params), do: nil
 
         @impl true
-        def on_join(_channel, _nick), do: nil
+        def on_join(_irc, _channel, _nick), do: nil
 
         @impl true
-        def on_part(_channel, _nick), do: nil
+        def on_part(_irc, _channel, _nick), do: nil
 
         @impl true
-        def on_kick(_channel, _nick), do: nil
+        def on_kick(_irc, _channel, _nick), do: nil
       end
     end
   end
@@ -38,13 +38,13 @@ defmodule Omnibot.Module do
       end
 
       @impl Module
-      def on_msg(msg) do
+      def on_msg(irc, msg) do
         # TODO - instead of using a router for modules, consider using a PubSub with a Registry:
         # https://hexdocs.pm/elixir/master/Registry.html#module-using-as-a-pubsub
-        route_msg(msg)
+        route_msg(irc, msg)
       end
 
-      defp route_msg(msg) do
+      defp route_msg(irc, msg) do
         nick = msg.prefix.nick
 
         case String.upcase(msg.command) do
@@ -53,21 +53,21 @@ defmodule Omnibot.Module do
             line = Enum.join(params, " ")
 
             case String.split(line, " ") do
-              [cmd | params] -> on_channel_msg(channel, nick, cmd, params)
+              [cmd | params] -> on_channel_msg(irc, channel, nick, cmd, params)
-              _ -> on_channel_msg(channel, nick, line)
+              _ -> on_channel_msg(irc, channel, nick, line)
             end
 
           "JOIN" ->
             [channel | _] = msg.params
-            on_join(channel, nick)
+            on_join(irc, channel, nick)
 
           "PART" ->
             [channel | _] = msg.params
-            on_part(channel, nick)
+            on_part(irc, channel, nick)
 
           "KICK" ->
             [channel | _] = msg.params
-            on_kick(channel, nick)
+            on_kick(irc, channel, nick)
 
           _ ->
             nil
@@ -80,22 +80,23 @@ defmodule Omnibot.Module do
     end
   end
 
-  @callback on_msg(msg :: %Omnibot.Irc.Msg{}) :: any
+  @callback on_msg(irc :: pid(), msg :: %Omnibot.Irc.Msg{}) :: any
-  @callback on_channel_msg(channel :: String.t(), nick :: String.t(), line :: String.t()) :: any
+  @callback on_channel_msg(irc :: pid(), channel :: String.t(), nick :: String.t(), line :: String.t()) :: any
   @callback on_channel_msg(
+              irc :: pid(),
               channel :: String.t(),
               nick :: String.t(),
               cmd :: String.t(),
               params :: [String.t()]
             ) :: any
-  @callback on_join(channel :: String.t(), nick :: String.t()) :: any
+  @callback on_join(irc :: pid(), channel :: String.t(), nick :: String.t()) :: any
-  @callback on_part(channel :: String.t(), nick :: String.t()) :: any
+  @callback on_part(irc :: pid(), channel :: String.t(), nick :: String.t()) :: any
-  @callback on_kick(channel :: String.t(), nick :: String.t()) :: any
+  @callback on_kick(irc :: pid(), channel :: String.t(), nick :: String.t()) :: any
 
   defmacro command(cmd, opts) do
     quote generated: true do
       @impl Omnibot.Module
-      def on_channel_msg(var!(channel), var!(nick), unquote(cmd), var!(params)) do
+      def on_channel_msg(var!(irc), var!(channel), var!(nick), unquote(cmd), var!(params)) do
         unquote(opts[:do])
       end
     end
@@ -115,7 +116,7 @@ defmodule Omnibot.Module do
 
     quote generated: true do
       @impl Omnibot.Module
-      def on_channel_msg(var!(channel), var!(nick), unquote(cmd), unquote(params)) do
+      def on_channel_msg(var!(irc), var!(channel), var!(nick), unquote(cmd), unquote(params)) do
         unquote(opts[:do])
       end
     end

lib/module_supervisor.ex

@@ -3,6 +3,7 @@ defmodule Omnibot.ModuleSupervisor do
 
   use Supervisor
   require Logger
+  alias Omnibot.State
 
   def start_link(opts \\ []) do
     Supervisor.start_link(__MODULE__, opts[:cfg], opts)
@@ -12,6 +13,10 @@ defmodule Omnibot.ModuleSupervisor do
   def init(cfg) do
     compile_files(cfg.module_paths || [])
 
+    # These are modules that need to be loaded for core functionality of the bot
+    #{Omnibot.Core.Welcome, cfg: [channels: :all]},
+    #{Omnibot.Core.Join, cfg: [channels: :all]},
+
     # Map the modules in the configuration to the children
     children =
       for mod <- cfg.modules do
@@ -21,6 +26,9 @@ defmodule Omnibot.ModuleSupervisor do
         end
       end
 
+    # Add each child to the "loaded modules" list in the State
+    Enum.each(children, fn module -> State.add_loaded_module(module) end)
+
     Supervisor.init(children, strategy: :one_for_one)
   end
 

lib/router.ex

@@ -1,12 +1,13 @@
 defmodule Omnibot.Router do
   require Logger
-  alias Omnibot.{Config, Irc, Irc.Msg, State}
+  alias Omnibot.{Irc.Msg, State}
   
-  def route(_irc, msg) do
+  def route(irc, msg) do
-    channel = Msg.channel(msg)
+    #channel = Msg.channel(msg)
-    State.cfg()
+    channel = IO.inspect(Msg.channel(msg))
-      |> Config.channel_modules(channel)
+    IO.inspect(State.channel_modules(channel))
-      |> Enum.each(fn {module, _} -> module.on_msg(msg) end)
+    State.channel_modules(channel)
+      |> Enum.each(fn {module, _} -> module.on_msg(irc, msg) end)
   end
 
   #def handle(_irc, :privmsg, msg) do

lib/state.ex

@@ -2,7 +2,7 @@ defmodule Omnibot.State do
   use GenServer
 
   @enforce_keys [:cfg]
-  defstruct [:cfg, channels: MapSet.new()]
+  defstruct [:cfg, channels: MapSet.new(), module_map: %{}]
 
   ## Client API
 
@@ -21,6 +21,21 @@ defmodule Omnibot.State do
     GenServer.call(state, :cfg)
   end
 
+  @doc "Adds a loaded module to the default state."
+  def add_loaded_module(module), do: add_loaded_module(__MODULE__, module)
+
+  @doc "Adds a loaded module to the given state."
+  def add_loaded_module(state, {module, cfg}), do: GenServer.cast(state, {:add_loaded_module, {module, cfg}})
+
+  @doc "Adds a loaded module to the given state."
+  def add_loaded_module(state, module), do: add_loaded_module(state, {module, []})
+
+  @doc "Gets all loaded modules from the default state."
+  def loaded_modules(), do: loaded_modules(__MODULE__)
+
+  @doc "Gets all loaded modules from the given state."
+  def loaded_modules(state), do: GenServer.call(state, :loaded_modules)
+
   @doc "Gets all channels that the bot is present in from the default State process."
   def channels(), do: channels(__MODULE__)
 
@@ -45,6 +60,34 @@ defmodule Omnibot.State do
     GenServer.cast(state, {:remove_channel, channel})
   end
 
+  def all_channels(), do: all_channels(__MODULE__)
+
+  def all_channels(state) do
+    loaded_modules(state) |> Enum.flat_map(
+      fn {_, cfg} ->
+        case cfg[:channels] do
+          :all -> []
+          nil -> []
+          channels -> channels
+        end
+      end)
+      |> MapSet.new()
+      |> MapSet.to_list()
+  end
+
+  def channel_modules(channel), do: channel_modules(__MODULE__, channel)
+
+  @doc ~S"""
+  Gets a list of all `{module, mod_cfg}` from the given State that are both
+  loaded, and listening to the given channel.
+  """
+  def channel_modules(state, channel) do
+    loaded_modules(state) |> Enum.filter(
+      fn {_, cfg} ->
+        cfg[:channels] == :all or Enum.member?(cfg[:channels] || [], channel)
+      end)
+  end
+
   ## Server API
 
   @impl true
@@ -61,7 +104,18 @@ defmodule Omnibot.State do
   def handle_call(:channels, _from, state) do
     {:reply, state.channels, state}
   end
-  
+
+  @impl true
+  def handle_call(:loaded_modules, _from, state) do
+    {:reply, state.module_map, state}
+  end
+
+  @impl true
+  def handle_cast({:add_loaded_module, {module, cfg}}, state) do
+    state = %{state | module_map: Map.put(state.module_map, module, cfg)}
+    {:noreply, state}
+  end
+
   @impl true
   def handle_cast({:add_channel, channel}, state) do
     {:noreply, %{state | channels: state.channels |> MapSet.put(channel)}}

lib/supervisor.ex

@@ -15,8 +15,8 @@ defmodule Omnibot.Supervisor do
     children = [
       {Task.Supervisor, name: Omnibot.RouterSupervisor, strategy: :one_for_one},
       {Omnibot.State, cfg: cfg, name: Omnibot.State},
+      {Omnibot.ModuleSupervisor, cfg: cfg, name: Omnibot.ModuleSupervisor},
       {Omnibot.Irc, name: Omnibot.Irc},
-      {Omnibot.ModuleSupervisor, cfg: cfg, name: Omnibot.ModuleSupervisor}
     ]
 
     # TODO : how to handle config reloading?

test/config_test.exs

@@ -1,58 +0,0 @@
-defmodule ConfigTest do
-  use ExUnit.Case
-
-  alias Omnibot.Config
-
-  test "config all_channels works correctly" do
-    cfg = %Config {
-      server: "test",
-      modules: [
-        {Test, channels: ["#foo", "#bar"]},
-        {Test, channels: ["#foo"]},
-        {Test, channels: ["#bar"]},
-        {Test, channels: ["#baz"]},
-        {Test, channels: :all},
-      ]
-    }
-
-    channels = Config.all_channels(cfg)
-
-    assert length(channels) == 3
-    assert Enum.member?(channels, "#foo")
-    assert Enum.member?(channels, "#bar")
-    assert Enum.member?(channels, "#baz")
-  end
-
-  test "config channel_modules works correctly" do
-    cfg = %Config {
-      server: "test",
-      modules: [
-        {FooBar, channels: ["#foo", "#bar"]},
-        {Foo, channels: ["#foo"]},
-        {Bar, channels: ["#bar"]},
-        {Baz, channels: ["#baz"]},
-        {All, channels: :all},
-      ]
-    }
-
-    modules = Config.channel_modules(cfg, "#foo")
-              |> Enum.map(fn {module, _} -> module end)
-    assert length(modules) == 3
-    assert Enum.member?(modules, FooBar)
-    assert Enum.member?(modules, Foo)
-    assert Enum.member?(modules, All)
-
-    modules = Config.channel_modules(cfg, "#bar")
-              |> Enum.map(fn {module, _} -> module end)
-    assert length(modules) == 3
-    assert Enum.member?(modules, FooBar)
-    assert Enum.member?(modules, Bar)
-    assert Enum.member?(modules, All)
-
-    modules = Config.channel_modules(cfg, "#baz")
-              |> Enum.map(fn {module, _} -> module end)
-    assert length(modules) == 2
-    assert Enum.member?(modules, Baz)
-    assert Enum.member?(modules, All)
-  end
-end

test/irc/msg_test.exs

@@ -1,4 +1,4 @@
-alias Omnibot.{Irc, Irc.Msg}
+alias Omnibot.Irc.Msg
 
 
 defmodule Omnibot.MsgTest do

test/state_test.exs

@@ -0,0 +1,65 @@
+defmodule StateTest do
+  use ExUnit.Case
+
+  alias Omnibot.State
+
+  setup do
+    state = start_supervised!(State)
+    {:ok, state: state}
+  end
+
+  test "state channel_modules works correctly", %{state: state} do
+    modules = [
+      {FooBar, channels: ["#foo", "#bar"]},
+      {Foo, channels: ["#foo"]},
+      {Bar, channels: ["#bar"]},
+      {Baz, channels: ["#baz"]},
+      {All, channels: :all},
+    ]
+
+    modules |> Enum.each(fn module -> State.add_loaded_module(state, module) end)
+
+    modules = State.channel_modules(state, "#foo")
+              |> Enum.map(fn {module, _} -> module end)
+    assert length(modules) == 3
+    assert Enum.member?(modules, FooBar)
+    assert Enum.member?(modules, Foo)
+    assert Enum.member?(modules, All)
+
+    modules = State.channel_modules(state, "#bar")
+              |> Enum.map(fn {module, _} -> module end)
+    assert length(modules) == 3
+    assert Enum.member?(modules, FooBar)
+    assert Enum.member?(modules, Bar)
+    assert Enum.member?(modules, All)
+
+    modules = State.channel_modules(state, "#baz")
+              |> Enum.map(fn {module, _} -> module end)
+    assert length(modules) == 2
+    assert Enum.member?(modules, Baz)
+    assert Enum.member?(modules, All)
+
+    modules = State.channel_modules(state, nil)
+              |> Enum.map(fn {module, _} -> module end)
+    assert length(modules) == 1
+    assert Enum.member?(modules, All)
+  end
+
+  test "state all_channels works correctly", %{state: state} do
+    modules = [
+      {FooBar, channels: ["#foo", "#bar"]},
+      {Foo, channels: ["#foo"]},
+      {Bar, channels: ["#bar"]},
+      {Baz, channels: ["#baz"]},
+      {All, channels: :all},
+    ]
+
+    modules |> Enum.each(fn module -> State.add_loaded_module(state, module) end)
+    channels = State.all_channels(state)
+
+    assert length(channels) == 3
+    assert Enum.member?(channels, "#foo")
+    assert Enum.member?(channels, "#bar")
+    assert Enum.member?(channels, "#baz")
+  end
+end