From 4e57eeef58b59f20c670790f50e1082b41e32a97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 22 Nov 2022 13:00:13 +0100 Subject: [PATCH] Use persistent term form precomputed static configuration --- lib/phoenix/config.ex | 2 +- lib/phoenix/endpoint.ex | 51 ++--- lib/phoenix/endpoint/supervisor.ex | 215 +++++++++------------- test/phoenix/config_test.exs | 2 +- test/phoenix/endpoint/supervisor_test.exs | 71 +++---- 5 files changed, 143 insertions(+), 198 deletions(-) diff --git a/lib/phoenix/config.ex b/lib/phoenix/config.ex index 78269066c8..fc9ed22ed9 100644 --- a/lib/phoenix/config.ex +++ b/lib/phoenix/config.ex @@ -52,7 +52,7 @@ defmodule Phoenix.Config do e -> case :ets.info(module) do :undefined -> - raise "could not find ets table for endpoint #{inspect(module)}. Make sure your endpoint is started and note you cannot access endpoint functions at compile-time." + raise "could not find ets table for endpoint #{inspect(module)}. Make sure your endpoint is started and note you cannot access endpoint functions at compile-time" _ -> reraise e, __STACKTRACE__ diff --git a/lib/phoenix/endpoint.ex b/lib/phoenix/endpoint.ex index 7a3be9366c..300e3c13bd 100644 --- a/lib/phoenix/endpoint.ex +++ b/lib/phoenix/endpoint.ex @@ -524,16 +524,17 @@ defmodule Phoenix.Endpoint do Phoenix.Endpoint.Supervisor.config_change(__MODULE__, changed, removed) end + defp persistent!() do + :persistent_term.get({Phoenix.Endpoint, __MODULE__}, nil) || + raise "could not find persistern term for endpoint #{inspect(__MODULE__)}. Make sure your endpoint is started and note you cannot access endpoint functions at compile-time" + end + @doc """ Generates the endpoint base URL without any path information. It uses the configuration under `:url` to generate such. """ - def url do - Phoenix.Config.cache(__MODULE__, - :__phoenix_url__, - &Phoenix.Endpoint.Supervisor.url/1) - end + def url, do: persistent!().url @doc """ Generates the static URL without any path information. @@ -541,11 +542,7 @@ defmodule Phoenix.Endpoint do It uses the configuration under `:static_url` to generate such. It falls back to `:url` if `:static_url` is not set. """ - def static_url do - Phoenix.Config.cache(__MODULE__, - :__phoenix_static_url__, - &Phoenix.Endpoint.Supervisor.static_url/1) - end + def static_url, do: persistent!().static_url @doc """ Generates the endpoint base URL but as a `URI` struct. @@ -554,55 +551,33 @@ defmodule Phoenix.Endpoint do Useful for manipulating the URL data and passing it to URL helpers. """ - def struct_url do - Phoenix.Config.cache(__MODULE__, - :__phoenix_struct_url__, - &Phoenix.Endpoint.Supervisor.struct_url/1) - end + def struct_url, do: persistent!().struct_url @doc """ Returns the host for the given endpoint. """ - def host do - Phoenix.Config.cache(__MODULE__, - :__phoenix_host__, - &Phoenix.Endpoint.Supervisor.host/1) - end + def host, do: persistent!().host @doc """ Generates the path information when routing to this endpoint. """ - def path(path) do - Phoenix.Config.cache(__MODULE__, - :__phoenix_path__, - &Phoenix.Endpoint.Supervisor.path/1) <> path - end + def path(path), do: persistent!().path <> path @doc """ Generates the script name. """ - def script_name do - Phoenix.Config.cache(__MODULE__, - :__phoenix_script_name__, - &Phoenix.Endpoint.Supervisor.script_name/1) - end + def script_name, do: persistent!().script_name @doc """ Generates a route to a static file in `priv/static`. """ - def static_path(path) do - Phoenix.Config.cache(__MODULE__, :__phoenix_static__, - &Phoenix.Endpoint.Supervisor.static_path/1) <> - elem(static_lookup(path), 0) - end + def static_path(path), do: persistent!().static_path <> elem(static_lookup(path), 0) @doc """ Generates a base64-encoded cryptographic hash (sha512) to a static file in `priv/static`. Meant to be used for Subresource Integrity with CDNs. """ - def static_integrity(path) do - elem(static_lookup(path), 1) - end + def static_integrity(path), do: elem(static_lookup(path), 1) @doc """ Returns a two item tuple with the first item being the `static_path` diff --git a/lib/phoenix/endpoint/supervisor.ex b/lib/phoenix/endpoint/supervisor.ex index 2fa5aac99b..49149704ad 100644 --- a/lib/phoenix/endpoint/supervisor.ex +++ b/lib/phoenix/endpoint/supervisor.ex @@ -34,13 +34,16 @@ defmodule Phoenix.Endpoint.Supervisor do case mod.init(:supervisor, env_conf) do {:ok, init_conf} -> if is_nil(Application.get_env(otp_app, mod)) and init_conf == env_conf do - Logger.warning("no configuration found for otp_app #{inspect(otp_app)} and module #{inspect(mod)}") + Logger.warning( + "no configuration found for otp_app #{inspect(otp_app)} and module #{inspect(mod)}" + ) end init_conf other -> - raise ArgumentError, "expected init/2 callback to return {:ok, config}, got: #{inspect other}" + raise ArgumentError, + "expected init/2 callback to return {:ok, config}, got: #{inspect(other)}" end extra_conf = [ @@ -57,7 +60,9 @@ defmodule Phoenix.Endpoint.Supervisor do server? = server?(conf) if conf[:instrumenters] do - Logger.warning(":instrumenters configuration for #{inspect(mod)} is deprecated and has no effect") + Logger.warning( + ":instrumenters configuration for #{inspect(mod)} is deprecated and has no effect" + ) end if server? and conf[:code_reloader] do @@ -72,10 +77,10 @@ defmodule Phoenix.Endpoint.Supervisor do children = config_children(mod, secret_conf, default_conf) ++ - pubsub_children(mod, conf) ++ - socket_children(mod) ++ - server_children(mod, conf, server?) ++ - watcher_children(mod, conf, server?) + pubsub_children(mod, conf) ++ + socket_children(mod) ++ + server_children(mod, conf, server?) ++ + watcher_children(mod, conf, server?) Supervisor.init(children, strategy: :one_for_one) end @@ -84,19 +89,19 @@ defmodule Phoenix.Endpoint.Supervisor do pub_conf = conf[:pubsub] if pub_conf do - Logger.warning """ - The :pubsub key in your #{inspect mod} is deprecated. + Logger.warning(""" + The :pubsub key in your #{inspect(mod)} is deprecated. You must now start the pubsub in your application supervision tree. Go to lib/my_app/application.ex and add the following: - {Phoenix.PubSub, #{inspect pub_conf}} + {Phoenix.PubSub, #{inspect(pub_conf)}} Now, back in your config files in config/*, you can remove the :pubsub key and add the :pubsub_server key, with the PubSub name: - pubsub_server: #{inspect pub_conf[:name]} - """ + pubsub_server: #{inspect(pub_conf[:name])} + """) end if pub_conf[:adapter] do @@ -184,12 +189,12 @@ defmodule Phoenix.Endpoint.Supervisor do # Supervisor config watchers: [], force_watchers: false - ] + ] end defp render_errors(module) do module - |> Module.split + |> Module.split() |> Enum.at(0) |> Module.concat("ErrorView") end @@ -203,95 +208,6 @@ defmodule Phoenix.Endpoint.Supervisor do res end - @doc """ - Builds the endpoint url from its configuration. - - The result is wrapped in a `{:cache, value}` tuple so - the `Phoenix.Config` layer knows how to cache it. - """ - def url(endpoint) do - {:cache, build_url(endpoint, endpoint.config(:url)) |> String.Chars.URI.to_string()} - end - - @doc """ - Builds the host for caching. - """ - def host(endpoint) do - {:cache, host_to_binary(endpoint.config(:url)[:host] || "localhost")} - end - - @doc """ - Builds the path for caching. - """ - def path(endpoint) do - {:cache, empty_string_if_root(endpoint.config(:url)[:path] || "/")} - end - - @doc """ - Builds the script_name for caching. - """ - def script_name(endpoint) do - {:cache, String.split(endpoint.config(:url)[:path] || "/", "/", trim: true)} - end - - @doc """ - Builds the static url from its configuration. - - The result is wrapped in a `{:cache, value}` tuple so - the `Phoenix.Config` layer knows how to cache it. - """ - def static_url(endpoint) do - url = endpoint.config(:static_url) || endpoint.config(:url) - {:cache, build_url(endpoint, url) |> String.Chars.URI.to_string()} - end - - @doc """ - Builds a struct url for user processing. - - The result is wrapped in a `{:cache, value}` tuple so - the `Phoenix.Config` layer knows how to cache it. - """ - def struct_url(endpoint) do - url = endpoint.config(:url) - {:cache, build_url(endpoint, url)} - end - - defp build_url(endpoint, url) do - https = endpoint.config(:https) - http = endpoint.config(:http) - - {scheme, port} = - cond do - https -> - {"https", https[:port]} - http -> - {"http", http[:port]} - true -> - {"http", 80} - end - - scheme = url[:scheme] || scheme - host = host_to_binary(url[:host] || "localhost") - port = port_to_integer(url[:port] || port) - - if host =~ ~r"[^:]:\d" do - Logger.warning("url: [host: ...] configuration value #{inspect(host)} for #{inspect(endpoint)} is invalid") - end - - %URI{scheme: scheme, port: port, host: host} - end - - @doc """ - Returns the script path root. - """ - def static_path(endpoint) do - script_path = (endpoint.config(:static_url) || endpoint.config(:url))[:path] || "/" - {:cache, empty_string_if_root(script_path)} - end - - defp empty_string_if_root("/"), do: "" - defp empty_string_if_root(other), do: other - @doc """ Returns a two item tuple with the first element containing the static path of a file in the static root directory @@ -311,7 +227,7 @@ defmodule Phoenix.Endpoint.Supervisor do def static_lookup(_endpoint, "/" <> _ = path) do if String.contains?(path, @invalid_local_url_chars) do - raise ArgumentError, "unsafe characters detected for path #{inspect path}" + raise ArgumentError, "unsafe characters detected for path #{inspect(path)}" else {:nocache, {path, nil}} end @@ -322,7 +238,7 @@ defmodule Phoenix.Endpoint.Supervisor do end defp raise_invalid_path(path) do - raise ArgumentError, "expected a path starting with a single / but got #{inspect path}" + raise ArgumentError, "expected a path starting with a single / but got #{inspect(path)}" end # TODO: Remove the first function clause once {:system, env_var} tuples are removed @@ -339,9 +255,12 @@ defmodule Phoenix.Endpoint.Supervisor do if Enum.any?(deprecated_configs) do deprecated_config_lines = for {k, v} <- deprecated_configs, do: "#{k}: #{inspect(v)}" - runtime_exs_config_lines = for {key, {:system, env_var}} <- deprecated_configs, do: ~s|#{key}: System.get_env("#{env_var}")| - Logger.warning """ + runtime_exs_config_lines = + for {key, {:system, env_var}} <- deprecated_configs, + do: ~s|#{key}: System.get_env("#{env_var}")| + + Logger.warning(""" #{inspect(key)} configuration containing {:system, env_var} tuples for #{inspect(mod)} is deprecated. Configuration with deprecated values: @@ -358,7 +277,7 @@ defmodule Phoenix.Endpoint.Supervisor do #{key}: [ #{runtime_exs_config_lines |> Enum.join(",\r\n ")} ] - """ + """) end end @@ -366,25 +285,66 @@ defmodule Phoenix.Endpoint.Supervisor do Invoked to warm up caches on start and config change. """ def warmup(endpoint) do - endpoint.host() - endpoint.script_name() - endpoint.path("/") - warmup_url(endpoint) - warmup_static(endpoint) - :ok + warmup_persistent(endpoint) + warmup_static(endpoint, cache_static_manifest(endpoint)) + true rescue - _ -> :ok + _ -> false end - defp warmup_url(endpoint) do - endpoint.url() - endpoint.static_url() - endpoint.struct_url() + defp warmup_persistent(endpoint) do + url_config = endpoint.config(:url) + static_url_config = endpoint.config(:static_url) || url_config + + struct_url = build_url(endpoint, url_config) + host = host_to_binary(url_config[:host] || "localhost") + path = empty_string_if_root(url_config[:path] || "/") + script_name = String.split(path, "/", trim: true) + + static_url = build_url(endpoint, static_url_config) |> String.Chars.URI.to_string() + static_path = empty_string_if_root(static_url_config[:path] || "/") + + :persistent_term.put({Phoenix.Endpoint, endpoint}, %{ + struct_url: struct_url, + url: String.Chars.URI.to_string(struct_url), + host: host, + path: path, + script_name: script_name, + static_path: static_path, + static_url: static_url + }) end - defp warmup_static(endpoint) do - warmup_static(endpoint, cache_static_manifest(endpoint)) - endpoint.static_path("/") + defp empty_string_if_root("/"), do: "" + defp empty_string_if_root(other), do: other + + defp build_url(endpoint, url) do + https = endpoint.config(:https) + http = endpoint.config(:http) + + {scheme, port} = + cond do + https -> + {"https", https[:port]} + + http -> + {"http", http[:port]} + + true -> + {"http", 80} + end + + scheme = url[:scheme] || scheme + host = host_to_binary(url[:host] || "localhost") + port = port_to_integer(url[:port] || port) + + if host =~ ~r"[^:]:\d" do + Logger.warning( + "url: [host: ...] configuration value #{inspect(host)} for #{inspect(endpoint)} is invalid" + ) + end + + %URI{scheme: scheme, port: port, host: host} end defp warmup_static(endpoint, %{"latest" => latest, "digests" => digests}) do @@ -399,7 +359,8 @@ defmodule Phoenix.Endpoint.Supervisor do end defp warmup_static(_endpoint, _manifest) do - raise ArgumentError, "expected warmup_static/2 to include 'latest' and 'digests' keys in manifest" + raise ArgumentError, + "expected warmup_static/2 to include 'latest' and 'digests' keys in manifest" end defp static_cache(digests, value, true) do @@ -427,9 +388,11 @@ defmodule Phoenix.Endpoint.Supervisor do if File.exists?(outer) do outer |> File.read!() |> Phoenix.json_library().decode!() else - Logger.error "Could not find static manifest at #{inspect outer}. " <> - "Run \"mix phx.digest\" after building your static files " <> - "or remove the \"cache_static_manifest\" configuration from your config files." + Logger.error( + "Could not find static manifest at #{inspect(outer)}. " <> + "Run \"mix phx.digest\" after building your static files " <> + "or remove the \"cache_static_manifest\" configuration from your config files." + ) end else %{} diff --git a/test/phoenix/config_test.exs b/test/phoenix/config_test.exs index 6c0156d496..8eaccc7122 100644 --- a/test/phoenix/config_test.exs +++ b/test/phoenix/config_test.exs @@ -24,7 +24,7 @@ defmodule Phoenix.ConfigTest do test "raises with warning about compile time when table not started" do assert_raise RuntimeError, - "could not find ets table for endpoint Fooz. Make sure your endpoint is started and note you cannot access endpoint functions at compile-time.", + "could not find ets table for endpoint Fooz. Make sure your endpoint is started and note you cannot access endpoint functions at compile-time", fn -> cache(Fooz, :foo, fn _ -> {:nocache, :bar} end) end end diff --git a/test/phoenix/endpoint/supervisor_test.exs b/test/phoenix/endpoint/supervisor_test.exs index ede199d1ad..959b216602 100644 --- a/test/phoenix/endpoint/supervisor_test.exs +++ b/test/phoenix/endpoint/supervisor_test.exs @@ -2,43 +2,28 @@ defmodule Phoenix.Endpoint.SupervisorTest do use ExUnit.Case, async: true alias Phoenix.Endpoint.Supervisor - setup do - Application.put_env(:phoenix, SupervisorApp.Endpoint, custom: true) - System.put_env("PHOENIX_PORT", "8080") - System.put_env("PHOENIX_HOST", "example.org") - :ok - end - - test "loads router configuration" do - config = Supervisor.config(:phoenix, SupervisorApp.Endpoint) - assert config[:otp_app] == :phoenix - assert config[:custom] == true - - assert config[:render_errors] == - [view: SupervisorApp.ErrorView, accepts: ~w(html), layout: false] - end - defmodule HTTPSEndpoint do - def path(path), do: path - def config(:http), do: false + def config(:otp_app), do: :phoenix def config(:https), do: [port: 443] + def config(:http), do: false def config(:url), do: [host: "example.com"] - def config(:otp_app), do: :phoenix + def config(:static_url), do: nil end defmodule HTTPEndpoint do - def path(path), do: path + def config(:otp_app), do: :phoenix def config(:https), do: false def config(:http), do: [port: 80] def config(:url), do: [host: "example.com"] - def config(:otp_app), do: :phoenix + def config(:static_url), do: nil end defmodule HTTPEnvVarEndpoint do + def config(:otp_app), do: :phoenix def config(:https), do: false def config(:http), do: [port: {:system, "PHOENIX_PORT"}] def config(:url), do: [host: {:system, "PHOENIX_HOST"}] - def config(:otp_app), do: :phoenix + def config(:static_url), do: nil end defmodule URLEndpoint do @@ -51,28 +36,45 @@ defmodule Phoenix.Endpoint.SupervisorTest do defmodule StaticURLEndpoint do def config(:https), do: false def config(:http), do: false + def config(:url), do: [] def config(:static_url), do: [host: "static.example.com"] end - defmodule WatchersEndpoint do - def init(:supervisor, config), do: {:ok, config} - def __sockets__(), do: [] + setup_all do + Application.put_env(:phoenix, SupervisorApp.Endpoint, custom: true) + System.put_env("PHOENIX_PORT", "8080") + System.put_env("PHOENIX_HOST", "example.org") + + [HTTPSEndpoint, HTTPEndpoint, HTTPEnvVarEndpoint, URLEndpoint, StaticURLEndpoint] + |> Enum.each(&Supervisor.warmup/1) + + :ok + end + + defp persistent!(endpoint), do: :persistent_term.get({Phoenix.Endpoint, endpoint}) + + test "loads router configuration" do + config = Supervisor.config(:phoenix, SupervisorApp.Endpoint) + assert config[:otp_app] == :phoenix + assert config[:custom] == true + + assert config[:render_errors] == + [view: SupervisorApp.ErrorView, accepts: ~w(html), layout: false] end test "generates the static url based on the static host configuration" do - static_host = {:cache, "http://static.example.com"} - assert Supervisor.static_url(StaticURLEndpoint) == static_host + assert persistent!(StaticURLEndpoint).static_url == "http://static.example.com" end test "static url fallbacks to url when there is no configuration for static_url" do - assert Supervisor.static_url(URLEndpoint) == {:cache, "random://example.com:678"} + assert persistent!(URLEndpoint).static_url == "random://example.com:678" end test "generates url" do - assert Supervisor.url(URLEndpoint) == {:cache, "random://example.com:678"} - assert Supervisor.url(HTTPEndpoint) == {:cache, "http://example.com"} - assert Supervisor.url(HTTPSEndpoint) == {:cache, "https://example.com"} - assert Supervisor.url(HTTPEnvVarEndpoint) == {:cache, "http://example.org:8080"} + assert persistent!(URLEndpoint).url == "random://example.com:678" + assert persistent!(HTTPEndpoint).url == "http://example.com" + assert persistent!(HTTPSEndpoint).url == "https://example.com" + assert persistent!(HTTPEnvVarEndpoint).url == "http://example.org:8080" end test "static_path/2 returns file's path with lookup cache" do @@ -84,6 +86,11 @@ defmodule Phoenix.Endpoint.SupervisorTest do end describe "watchers" do + defmodule WatchersEndpoint do + def init(:supervisor, config), do: {:ok, config} + def __sockets__(), do: [] + end + @watchers [esbuild: {Esbuild, :install_and_run, [:default, ~w(--sourcemap=inline --watch)]}] test "init/1 starts watcher children when `:server` config is true" do