Refactor last_fm configuration

Move all configuration outside of the LastFm module, so that it's not
polluted with references to the parent music_library application.

.envrc

@@ -1 +1,2 @@
 dotenv_if_exists .secrets
+dotenv_if_exists .envrc.local

.gitignore

@@ -41,3 +41,4 @@ npm-debug.log
 
 # Environment secrets
 /.secrets
+/.envrc.local

config/config.exs

@@ -24,6 +24,13 @@ config :music_library, MusicLibraryWeb.Endpoint,
   pubsub_server: MusicLibrary.PubSub,
   live_view: [signing_salt: "g/qw4SNo"]
 
+config :music_library, LastFm,
+  # to avoid runtime errors, set the user to a valid Last.fm username
+  user: "username",
+  api: LastFm.APIImpl,
+  refresh_interval: System.convert_time_unit(60, :second, :millisecond),
+  api_key: "change me"
+
 # Configure esbuild (the version is required)
 config :esbuild,
   version: "0.17.11",

config/dev.exs

@@ -62,6 +62,9 @@ config :music_library, MusicLibraryWeb.Endpoint,
 # Enable dev routes for dashboard and mailbox
 config :music_library, dev_routes: true
 
+config :music_library, LastFm,
+  refresh_interval: System.convert_time_unit(500, :second, :millisecond)
+
 # Do not include metadata nor timestamps in development logs
 config :logger, :console, format: "[$level] $message\n"
 

config/runtime.exs

@@ -24,6 +24,10 @@ if api_key = System.get_env("LAST_FM_API_KEY") do
   config :music_library, LastFm, api_key: api_key
 end
 
+if user = System.get_env("LAST_FM_USER") do
+  config :music_library, LastFm, user: user
+end
+
 if config_env() == :prod do
   database_path =
     System.get_env("DATABASE_PATH") ||

config/test.exs

@@ -28,4 +28,4 @@ config :phoenix, :plug_init_mode, :runtime
 config :phoenix_live_view,
   enable_expensive_runtime_checks: true
 
-config :music_library, :last_fm, nil
+config :music_library, LastFm, api: nil

fly.toml

@@ -13,6 +13,7 @@ kill_signal = 'SIGTERM'
 DATABASE_PATH = '/mnt/music_library/music_library_prod.db'
 PHX_HOST = 'music-library.claudio-ortolina.org'
 PORT = '8080'
+LAST_FM_USER = 'cloud8421'
 
 [[mounts]]
 source = 'music_library'

lib/last_fm/refresh.ex

@@ -5,14 +5,14 @@ defmodule LastFm.Refresh do
 
   alias LastFm.Feed
 
-  @refresh_interval System.convert_time_unit(60, :second, :millisecond)
-
   def start_link(config) do
     GenServer.start_link(__MODULE__, config, name: __MODULE__)
   end
 
   @impl true
   def init(config) do
+    config = Map.new(config)
+
     if enabled?(config) do
       {:ok, config, {:continue, :refresh}}
     else
@@ -25,12 +25,12 @@ defmodule LastFm.Refresh do
     case config.api.get_recent_tracks(config.user, config.api_key) do
       {:ok, tracks} ->
         Feed.update(tracks)
-        Process.send_after(self(), :refresh, @refresh_interval)
+        Process.send_after(self(), :refresh, config.refresh_interval)
         {:noreply, config}
 
       {:error, _reason} ->
         # TODO: think about failure scenario - error is logged at the API level
-        Process.send_after(self(), :refresh, @refresh_interval)
+        Process.send_after(self(), :refresh, config.refresh_interval)
         {:noreply, config}
     end
   end
@@ -40,12 +40,12 @@ defmodule LastFm.Refresh do
     case config.api.get_recent_tracks(config.user, config.api_key) do
       {:ok, tracks} ->
         Feed.update(tracks)
-        Process.send_after(self(), :refresh, @refresh_interval)
+        Process.send_after(self(), :refresh, config.refresh_interval)
         {:noreply, config}
 
       {:error, _reason} ->
         # TODO: think about failure scenario - error is logged at the API level
-        Process.send_after(self(), :refresh, @refresh_interval)
+        Process.send_after(self(), :refresh, config.refresh_interval)
         {:noreply, config}
     end
   end

lib/last_fm/supervisor.ex

@@ -1,28 +1,19 @@
 defmodule LastFm.Supervisor do
   use Supervisor
 
-  def start_link(init_arg) do
+  def start_link(config) do
-    Supervisor.start_link(__MODULE__, init_arg, name: __MODULE__)
+    Supervisor.start_link(__MODULE__, config, name: __MODULE__)
   end
 
   @impl true
-  def init(_opts) do
+  def init(config) do
     :ok = LastFm.Feed.create_table!()
 
     children = [
       {Phoenix.PubSub, name: LastFm.PubSub},
-      {LastFm.Refresh, %{api: api(), user: "cloud8421", api_key: api_key()}}
+      {LastFm.Refresh, config}
     ]
 
     Supervisor.init(children, strategy: :one_for_one)
   end
-
-  defp api_key do
-    Application.get_env(:music_library, LastFm, [])
-    |> Keyword.get(:api_key)
-  end
-
-  defp api do
-    Application.get_env(:music_library, :last_fm, LastFm.APIImpl)
-  end
 end

lib/music_library/application.ex

@@ -15,7 +15,7 @@ defmodule MusicLibrary.Application do
        repos: Application.fetch_env!(:music_library, :ecto_repos), skip: skip_migrations?()},
       {DNSCluster, query: Application.get_env(:music_library, :dns_cluster_query) || :ignore},
       {Phoenix.PubSub, name: MusicLibrary.PubSub},
-      LastFm.Supervisor,
+      {LastFm.Supervisor, last_fm_config()},
       # Start a worker by calling: MusicLibrary.Worker.start_link(arg)
       # {MusicLibrary.Worker, arg},
       # Start to serve requests, typically the last entry
@@ -40,4 +40,8 @@ defmodule MusicLibrary.Application do
     # By default, sqlite migrations are run when using a release
     System.get_env("RELEASE_NAME") != nil
   end
+
+  defp last_fm_config do
+    Application.get_env(:music_library, LastFm)
+  end
 end

test/test_helper.exs

@@ -1,8 +1,5 @@
 Mox.defmock(MusicBrainz.APIBehaviourMock, for: MusicBrainz.APIBehaviour)
 Application.put_env(:music_library, :musicbrainz, MusicBrainz.APIBehaviourMock)
 
-Mox.defmock(LastFm.APIBehaviourMock, for: LastFm.APIBehaviour)
-Application.put_env(:music_library, :last_fm, LastFm.APIBehaviourMock)
-
 ExUnit.start()
 Ecto.Adapters.SQL.Sandbox.mode(MusicLibrary.Repo, :manual)