diff --git a/lib/phoenix_kit/cache/cache.ex b/lib/phoenix_kit/cache/cache.ex index 9dccdea1f..e4717cd8b 100644 --- a/lib/phoenix_kit/cache/cache.ex +++ b/lib/phoenix_kit/cache/cache.ex @@ -323,12 +323,24 @@ defmodule PhoenixKit.Cache do def init(opts) do name = Keyword.fetch!(opts, :name) warmer = Keyword.get(opts, :warmer) + critical_warmer = Keyword.get(opts, :critical_warmer) + sync_init = Keyword.get(opts, :sync_init, false) ttl = Keyword.get(opts, :ttl) max_size = Keyword.get(opts, :max_size) table = :ets.new(:"cache_#{name}", [:set, :protected, :named_table, {:read_concurrency, true}]) + # Support legacy cache_settings table for backwards compatibility + if name == :settings do + try do + :ets.new(:cache_settings, [:set, :protected, :named_table, {:read_concurrency, true}]) + rescue + # Table already exists + ArgumentError -> :ok + end + end + state = %__MODULE__{ name: name, table: table, @@ -337,13 +349,19 @@ defmodule PhoenixKit.Cache do max_size: max_size } - # Warm cache if warmer function is provided - if warmer do - send(self(), :warm_cache) + Logger.info( + "Started cache #{name} with table #{table}#{if sync_init, do: " (sync_init enabled)", else: ""}" + ) + + # Use handle_continue to warm cache after init returns + # This prevents blocking supervisor initialization + if sync_init and critical_warmer do + {:ok, state, {:continue, {:warm_critical, critical_warmer, warmer}}} + else + # Standard async warming + if warmer, do: send(self(), :warm_cache) + {:ok, state} end - - Logger.info("Started cache #{name} with table #{table}") - {:ok, state} end @impl GenServer @@ -481,6 +499,41 @@ defmodule PhoenixKit.Cache do {:noreply, state} end + @impl GenServer + def handle_continue({:warm_critical, _critical_warmer, warmer}, %{name: name} = state) do + # Load ALL data synchronously in handle_continue when sync_init is enabled + # This runs after init returns, so supervisor can continue starting other processes + # We load all data instead of just critical to avoid race conditions + + # Retry warming if data is empty (repo might not be ready yet) + case warm_with_retry(warmer, name, 3, 100) do + {:ok, data} when is_map(data) and map_size(data) > 0 -> + warm_critical_data(state, data) + + Logger.info( + "Synchronously warmed cache #{name} with #{map_size(data)} entries (sync_init mode)" + ) + + {:ok, empty_data} when is_map(empty_data) -> + Logger.warning( + "Cache #{name} warmed but no data loaded - repository may not be ready yet. Will retry asynchronously." + ) + + # Schedule async retry + Process.send_after(self(), :warm_cache, 1000) + + {:error, error} -> + Logger.error("Failed to synchronously warm cache #{name}: #{inspect(error)}") + # Schedule async retry + Process.send_after(self(), :warm_cache, 1000) + + _ -> + Logger.warning("Warmer for cache #{name} returned invalid data") + end + + {:noreply, state} + end + @impl GenServer def handle_info(:warm_cache, state) do handle_cast(:warm, state) @@ -492,12 +545,42 @@ defmodule PhoenixKit.Cache do PhoenixKit.Cache.Registry.via_tuple(name) end + defp warm_critical_data(%{name: name, table: table, ttl: ttl}, data) do + Enum.each(data, fn {key, value} -> + expires_at = if ttl, do: System.monotonic_time(:millisecond) + ttl, else: nil + :ets.insert(table, {key, value, expires_at}) + + # Also write to legacy cache_settings table if this is settings cache + if name == :settings do + :ets.insert(:cache_settings, {key, value, expires_at}) + end + end) + end + defp safe_warm(warmer) when is_function(warmer, 0) do {:ok, warmer.()} rescue error -> {:error, error} end + defp warm_with_retry(warmer, name, retries, delay) do + case safe_warm(warmer) do + {:ok, data} when is_map(data) and map_size(data) > 0 -> + {:ok, data} + + {:ok, empty_data} when is_map(empty_data) and retries > 0 -> + Logger.debug( + "Cache #{name} warming returned empty data, retrying in #{delay}ms (#{retries} retries left)" + ) + + Process.sleep(delay) + warm_with_retry(warmer, name, retries - 1, delay * 2) + + result -> + result + end + end + defp maybe_evict(%{max_size: nil} = state), do: state defp maybe_evict(%{table: table, max_size: max_size} = state) do diff --git a/lib/phoenix_kit/config/ueber_auth.ex b/lib/phoenix_kit/config/ueber_auth.ex index b4bd286e4..a91d7ef75 100644 --- a/lib/phoenix_kit/config/ueber_auth.ex +++ b/lib/phoenix_kit/config/ueber_auth.ex @@ -52,7 +52,7 @@ defmodule PhoenixKit.Config.UeberAuth do """ @spec get_all() :: Keyword.t() def get_all do - Config.get_list(:ueberauth, []) + Application.get_env(:ueberauth, Ueberauth, []) end @doc """ @@ -156,7 +156,7 @@ defmodule PhoenixKit.Config.UeberAuth do def set_all(options) when is_list(options) do current_config = get_all() new_config = Keyword.merge(current_config, options) - Config.set(:ueberauth, new_config) + Application.put_env(:ueberauth, Ueberauth, new_config) :ok end diff --git a/lib/phoenix_kit/settings/settings.ex b/lib/phoenix_kit/settings/settings.ex index 25483d92d..29fe3909b 100644 --- a/lib/phoenix_kit/settings/settings.ex +++ b/lib/phoenix_kit/settings/settings.ex @@ -1328,6 +1328,67 @@ defmodule PhoenixKit.Settings do %{} end + @doc """ + Warm cache with critical OAuth settings only. + + Returns map of critical OAuth settings for synchronous cache warming. + This is used during startup to ensure OAuth configuration is available + immediately, preventing race conditions with OAuthConfigLoader. + + Only loads OAuth-related settings that are required for provider configuration. + """ + def warm_critical_cache do + # Critical OAuth keys that must be loaded synchronously at startup + critical_keys = [ + # Google OAuth + "oauth_google_client_id", + "oauth_google_client_secret", + # GitHub OAuth + "oauth_github_client_id", + "oauth_github_client_secret", + # Apple OAuth + "oauth_apple_client_id", + "oauth_apple_team_id", + "oauth_apple_key_id", + "oauth_apple_private_key_path", + # Facebook OAuth + "oauth_facebook_app_id", + "oauth_facebook_app_secret", + # OAuth general settings + "oauth_enabled" + ] + + # Check if repository is available + if repo_available?() do + settings = + Setting + |> where([s], s.key in ^critical_keys) + |> repo().all() + + settings + |> Enum.map(fn setting -> + # Prioritize JSON value over string value for cache storage + value = + if setting.value_json do + setting.value_json + else + setting.value + end + + {setting.key, value} + end) + |> Map.new() + else + # Repo not available - return empty map + # This should rarely happen as critical cache is loaded at startup + %{} + end + rescue + error -> + Logger.error("Failed to warm critical cache: #{inspect(error)}") + %{} + end + ## Private Batch Query Functions # Batch query multiple string settings from database in a single operation @@ -1440,8 +1501,13 @@ defmodule PhoenixKit.Settings do _ -> true end - # Check if the repository is available and ready to accept queries - defp repo_available? do + @doc """ + Check if the repository is available and ready to accept queries. + + Returns true if the repo is configured and running, false otherwise. + Used to prevent errors during Mix tasks when repo might not be started. + """ + def repo_available? do # First check if repo is configured case PhoenixKit.Config.get(:repo, nil) do nil -> diff --git a/lib/phoenix_kit/supervisor.ex b/lib/phoenix_kit/supervisor.ex index ba0447aba..59e14dc50 100644 --- a/lib/phoenix_kit/supervisor.ex +++ b/lib/phoenix_kit/supervisor.ex @@ -14,11 +14,16 @@ defmodule PhoenixKit.Supervisor do PhoenixKit.PubSub.Manager, PhoenixKit.Admin.SimplePresence, {PhoenixKit.Cache.Registry, []}, - {PhoenixKit.Cache, name: :settings, warmer: &PhoenixKit.Settings.warm_cache_data/0}, + # Settings cache with synchronous initialization + # Loads all settings in handle_continue (after init returns) + # This ensures OAuth configuration is available before OAuthConfigLoader starts + # while not blocking supervisor initialization + {PhoenixKit.Cache, + name: :settings, sync_init: true, warmer: &PhoenixKit.Settings.warm_cache_data/0}, # Rate limiter backend MUST be started before any authentication requests PhoenixKit.Users.RateLimiter.Backend, - # OAuth config loader MUST be first to ensure configuration - # is available before any OAuth requests are processed + # OAuth config loader - now guaranteed to have critical settings in cache + # No longer needs retry logic as cache is pre-warmed with OAuth settings PhoenixKit.Workers.OAuthConfigLoader, PhoenixKit.Entities.Presence, # Email tracking supervisor - handles SQS Worker for automatic bounce event processing diff --git a/lib/phoenix_kit/workers/oauth_config_loader.ex b/lib/phoenix_kit/workers/oauth_config_loader.ex index 3efb1849a..2d8a35c06 100644 --- a/lib/phoenix_kit/workers/oauth_config_loader.ex +++ b/lib/phoenix_kit/workers/oauth_config_loader.ex @@ -3,14 +3,14 @@ defmodule PhoenixKit.Workers.OAuthConfigLoader do GenServer worker that ensures OAuth configuration is loaded from database before any OAuth requests are processed. - This worker runs synchronously during application startup to prevent timing - issues where Ueberauth plug is initialized before OAuth providers are configured. + This worker runs synchronously during application startup to configure + OAuth providers from database settings. ## Startup Sequence - 1. Worker starts as first child in PhoenixKit.Supervisor - 2. Waits for Settings cache to be ready (with timeout) - 3. Loads OAuth configuration from database + 1. PhoenixKit.Cache starts with sync_init, loading critical OAuth settings + 2. OAuthConfigLoader starts, OAuth settings already in cache + 3. Loads OAuth configuration from cache 4. Configures Ueberauth with available providers 5. Returns :ok when complete @@ -22,8 +22,8 @@ defmodule PhoenixKit.Workers.OAuthConfigLoader do - Parent Endpoint (router compiles, Ueberauth.init() runs) - PhoenixKit.Supervisor (OAuth config should load here) - If PhoenixKit.Supervisor starts AFTER Parent Endpoint, the OAuth configuration - arrives too late and Ueberauth fails with MatchError. + With sync_init enabled in the Cache, critical OAuth settings are loaded + synchronously before this worker starts, eliminating race conditions. This worker ensures OAuth configuration is available as early as possible during PhoenixKit.Supervisor initialization. @@ -32,15 +32,12 @@ defmodule PhoenixKit.Workers.OAuthConfigLoader do use GenServer require Logger - @max_retries 10 - @retry_delay 100 - ## Client API @doc """ Starts the OAuth configuration loader. - Blocks until OAuth configuration is successfully loaded or max retries exceeded. + Loads OAuth configuration from cache which is pre-warmed with critical settings. """ def start_link(_opts) do GenServer.start_link(__MODULE__, [], name: __MODULE__) @@ -103,26 +100,24 @@ defmodule PhoenixKit.Workers.OAuthConfigLoader do @impl true def init(_) do # Load OAuth configuration synchronously during initialization - # This ensures configuration is ready before supervisor completes startup - case load_oauth_config_with_retry() do + # With sync_init in the Cache, critical OAuth settings are already loaded + case load_oauth_config() do :ok -> Logger.info("OAuth config loaded successfully during startup") {:ok, %{status: :loaded}} - {:error, :cache_not_ready} -> - # During Mix tasks (like phoenix_kit.update --status), the cache may not be ready - # This is expected and not an error condition - log as debug instead of warning - Logger.debug( - "OAuth config loading skipped: Settings cache not ready (likely during Mix task execution)" - ) - - # Don't fail supervisor startup if cache is not ready - # The fallback plug will handle this case - {:ok, %{status: :not_loaded, reason: :cache_not_ready}} - {:error, :modules_not_loaded} -> Logger.info("OAuth modules not loaded, OAuth features will be unavailable") {:ok, %{status: :not_loaded, reason: :modules_not_loaded}} + + {:error, :repo_not_available} -> + # During Mix tasks, the repo may not be available + # This is expected and not an error condition + Logger.debug( + "OAuth config loading skipped: Repository not available (likely during Mix task execution)" + ) + + {:ok, %{status: :not_loaded, reason: :repo_not_available}} end rescue # Catch unexpected errors during initialization @@ -158,27 +153,6 @@ defmodule PhoenixKit.Workers.OAuthConfigLoader do ## Private Helpers - defp load_oauth_config_with_retry(attempt \\ 1) do - case load_oauth_config() do - :ok -> - :ok - - {:error, :cache_not_ready} when attempt < @max_retries -> - # Only log every 3rd attempt to reduce noise during Mix tasks - if rem(attempt, 3) == 0 do - Logger.debug( - "Settings cache not ready, retrying... (attempt #{attempt}/#{@max_retries})" - ) - end - - Process.sleep(@retry_delay) - load_oauth_config_with_retry(attempt + 1) - - {:error, reason} -> - {:error, reason} - end - end - defp load_oauth_config do # Check if required modules are loaded if Code.ensure_loaded?(PhoenixKit.Users.OAuthConfig) and @@ -191,25 +165,15 @@ defmodule PhoenixKit.Workers.OAuthConfigLoader do end defp do_load_oauth_config do - # CRITICAL: Verify Settings cache is FULLY warmed before configuring OAuth - # The cache warming is asynchronous (via handle_info(:warm_cache)) - # We must wait until ALL settings are loaded, not just one setting - - # Strategy 1: Check cache size (most reliable) - # Based on production data: cache should have ~50+ entries when fully warmed - cache_size = get_cache_size() - - if cache_size < 40 do - # Don't log every attempt to reduce noise during Mix tasks - # Cache will be empty during Mix tasks when repo is not available - {:error, :cache_not_ready} - else - # Strategy 2: Verify OAuth-specific settings are present - # This ensures we have actual OAuth data, not just general settings + # With sync_init enabled, critical OAuth settings are already in cache + # No need to check cache size or wait for warming + + # Check if repository is available (for Mix tasks) + if PhoenixKit.Settings.repo_available?() do + # OAuth settings are already loaded via sync_init oauth_enabled = PhoenixKit.Settings.get_setting("oauth_enabled", "false") # Check that at least one provider's credentials are accessible - # This confirms the cache contains OAuth-related data has_any_oauth_data = PhoenixKit.Settings.has_oauth_credentials?(:google) or PhoenixKit.Settings.has_oauth_credentials?(:apple) or @@ -217,34 +181,24 @@ defmodule PhoenixKit.Workers.OAuthConfigLoader do PhoenixKit.Settings.has_oauth_credentials?(:facebook) Logger.debug( - "Settings cache ready: size=#{cache_size}, oauth_enabled=#{oauth_enabled}, has_oauth_data=#{has_any_oauth_data}" + "OAuth configuration: enabled=#{oauth_enabled}, has_oauth_data=#{has_any_oauth_data}" ) - # Configure OAuth providers from database - # At this point, ALL providers' credentials should be in cache + # Configure OAuth providers from settings alias PhoenixKit.Users.OAuthConfig OAuthConfig.configure_providers() :ok + else + {:error, :repo_not_available} end rescue - # Specific error types that indicate cache not ready (retriable) - error in [RuntimeError, UndefinedFunctionError] -> - # These are expected during startup - Settings cache may not be ready - Logger.debug("Settings cache not ready: #{Exception.message(error)}") - {:error, :cache_not_ready} - - # Database connection errors (retriable) + # Database connection errors error in [DBConnection.ConnectionError, Postgrex.Error] -> Logger.warning("Database connection error: #{Exception.message(error)}") - {:error, :cache_not_ready} - - # ArgumentError when ETS table doesn't exist yet - error in [ArgumentError] -> - Logger.debug("Settings cache table not created yet: #{Exception.message(error)}") - {:error, :cache_not_ready} + {:error, :repo_not_available} - # Any other unexpected error (non-retriable) + # Any other unexpected error error -> # Log full error with stacktrace for debugging Logger.error(""" @@ -255,14 +209,4 @@ defmodule PhoenixKit.Workers.OAuthConfigLoader do # Re-raise to fail fast on unexpected errors reraise error, __STACKTRACE__ end - - # Get the size of Settings cache ETS table - # Returns 0 if table doesn't exist yet - defp get_cache_size do - :ets.info(:cache_settings, :size) - rescue - ArgumentError -> - # Table doesn't exist yet - 0 - end end