Keep parsed requirements in the registry server

lib/hex/registry/server.ex

@@ -71,29 +71,33 @@ defmodule Hex.Registry.Server do
     end
   end
 
-  # Registries repeat the same few hundred requirements across thousands of
-  # releases, so keep the constraints parsed in this process. Unsatisfiable
-  # requirements aren't kept so they still warn for each release.
   def dependency_constraint(package, version, name, requirement) do
-    requirement = requirement || ">= 0.0.0"
-    key = {__MODULE__, :constraint, requirement}
+    constraint(package, version, name, parse_requirement(requirement))
+  end
 
-    case Process.get(key) do
-      nil ->
-        constraint = Hex.Solver.parse_constraint!(requirement)
-        Process.put(key, constraint)
-        constraint
+  defp constraint(_package, _version, _name, {:ok, constraint}), do: constraint
 
-      constraint ->
-        constraint
-    end
+  # Unsatisfiable requirements warn for each release that has them
+  defp constraint(
+         package,
+         version,
+         name,
+         {:error, %Hex.Solver.UnsatisfiableRequirementError{} = error}
+       ) do
+    warn_unsatisfiable(package, version, name, error)
+    %Hex.Solver.Constraints.Empty{}
+  end
+
+  defp constraint(_package, _version, _name, {:error, error}), do: raise(error)
+
+  defp parse_requirement(requirement) do
+    {:ok, Hex.Solver.parse_constraint!(requirement || ">= 0.0.0")}
   rescue
-    error in Hex.Solver.UnsatisfiableRequirementError ->
-      warn_unsatisfiable(package, version, name, error)
-      %Hex.Solver.Constraints.Empty{}
+    error in [Hex.Solver.UnsatisfiableRequirementError, Version.InvalidRequirementError] ->
+      {:error, error}
   end
 
-  defp solver_dependency(package, version, {_repo, name, app, _requirement, _optional} = dep) do
+  defp solver_dependency(package, version, {_repo, name, app, _parsed, _optional} = dep) do
     dependency = dependency(package, version, dep)
 
     if valid_app_name?(app) do
@@ -104,22 +108,22 @@ defmodule Hex.Registry.Server do
     end
   end
 
-  defp app_dependency(package, version, {_repo, name, app, _requirement, _optional} = dep) do
+  defp app_dependency(package, version, {_repo, name, app, _parsed, _optional} = dep) do
     unless valid_app_name?(app) do
       Mix.raise(invalid_app_name_message(package, version, name, app))
     end
 
     dependency(package, version, dep)
   end
 
-  defp dependency(package, version, {repo, name, app, requirement, optional}) do
+  defp dependency(package, version, {repo, name, app, parsed, optional}) do
     validate_name(package, version, "repository", repo, &valid_repo_name?/1)
     validate_name(package, version, "package", name, &valid_package_name?/1)
 
     %{
       repo: if(repo != "hexpm", do: repo),
       name: name,
-      constraint: dependency_constraint(package, version, name, requirement),
+      constraint: constraint(package, version, name, parsed),
       optional: optional,
       label: app
     }
@@ -238,7 +242,8 @@ defmodule Hex.Registry.Server do
       pending_fun: nil,
       pending_policies: MapSet.new(),
       fetched_policies: MapSet.new(),
-      waiting_policies: %{}
+      waiting_policies: %{},
+      constraints: %{}
     }
   end
 
@@ -351,13 +356,14 @@ defmodule Hex.Registry.Server do
   end
 
   def handle_call({:dependencies, repo, package, version}, from, state) do
-    maybe_wait({repo, package}, from, state, fn ->
+    maybe_wait({repo, package}, from, state, fn state ->
       case lookup(state.ets, {:deps, repo || "hexpm", package, to_string(version)}) do
         nil ->
-          :error
+          {:error, state}
 
         deps ->
-          {:ok, deps}
+          {deps, constraints} = Enum.map_reduce(deps, state.constraints, &parse_dependency/2)
+          {{:ok, deps}, %{state | constraints: constraints}}
       end
     end)
   end
@@ -415,9 +421,12 @@ defmodule Hex.Registry.Server do
 
     written? = write_result(result, repo, package, state)
 
-    Enum.each(replys, fn {from, fun} ->
-      GenServer.reply(from, fun.())
-    end)
+    state =
+      Enum.reduce(replys, state, fn {from, fun}, state ->
+        {reply, state} = run_waiting(fun, state)
+        GenServer.reply(from, reply)
+        state
+      end)
 
     state = %{
       state
@@ -873,7 +882,8 @@ defmodule Hex.Registry.Server do
 
     cond do
       {repo, package} in state.fetched ->
-        {:reply, fun.(), state}
+        {reply, state} = run_waiting(fun, state)
+        {:reply, reply, state}
 
       {repo, package} in state.pending ->
         tuple = {from, fun}
@@ -886,6 +896,24 @@ defmodule Hex.Registry.Server do
     end
   end
 
+  # A fun that needs the state, such as one parsing dependencies, takes it
+  # and returns it with the reply
+  defp run_waiting(fun, state) when is_function(fun, 0), do: {fun.(), state}
+  defp run_waiting(fun, state) when is_function(fun, 1), do: fun.(state)
+
+  # Registries repeat the same few hundred requirements across thousands of
+  # releases, so keep them parsed while the registry is open
+  defp parse_dependency({repo, name, app, requirement, optional}, constraints) do
+    case constraints do
+      %{^requirement => parsed} ->
+        {{repo, name, app, parsed, optional}, constraints}
+
+      %{} ->
+        parsed = parse_requirement(requirement)
+        {{repo, name, app, parsed, optional}, Map.put(constraints, requirement, parsed)}
+    end
+  end
+
   defp maybe_wait_policy({repo, name}, from, state, fun) do
     repo = repo || "hexpm"
 

test/hex/registry/server_test.exs

@@ -61,6 +61,43 @@ defmodule Hex.Registry.ServerTest do
     assert_received {:mix_shell, :error, ["\e[33mPackage other_parent 1.0.0 can't be used" <> _]}
   end
 
+  test "dependencies/3 keeps parsed requirements in the registry, not the calling process" do
+    :sys.replace_state(Registry, fn %{ets: tid, fetched: fetched} = state ->
+      :ets.insert(
+        tid,
+        {{:deps, "hexpm", "parent", "1.0.0"}, [{"hexpm", "child", "child", "~> 1.0", false}]}
+      )
+
+      %{state | fetched: MapSet.put(fetched, {"hexpm", "parent"})}
+    end)
+
+    keys = Enum.sort(Process.get_keys())
+    assert {:ok, [dependency]} = Registry.dependencies("hexpm", "parent", "1.0.0")
+    assert Enum.sort(Process.get_keys()) == keys
+
+    assert %{constraints: %{"~> 1.0" => {:ok, constraint}}} = :sys.get_state(Registry)
+    assert dependency.constraint == constraint
+  end
+
+  test "dependencies/3 raises for an invalid requirement and keeps the registry running" do
+    :sys.replace_state(Registry, fn %{ets: tid, fetched: fetched} = state ->
+      :ets.insert(
+        tid,
+        {{:deps, "hexpm", "parent", "1.0.0"}, [{"hexpm", "child", "child", "~> foo", false}]}
+      )
+
+      %{state | fetched: MapSet.put(fetched, {"hexpm", "parent"})}
+    end)
+
+    registry = Process.whereis(Registry)
+
+    assert_raise Version.InvalidRequirementError, fn ->
+      Registry.dependencies("hexpm", "parent", "1.0.0")
+    end
+
+    assert Process.whereis(Registry) == registry
+  end
+
   test "app_dependencies/3 accepts application names" do
     :sys.replace_state(Registry, fn %{ets: tid, fetched: fetched} = state ->
       :ets.insert(