Use tarball outer checksum to check cache freshness (hexpm#812)

Use tarball outer checksum to check if cache is fresh instead of using
etag to make conditional HTTP requests.

lib/hex/dev.ex

@@ -41,7 +41,6 @@ if Mix.env() == :dev do
               copy(original_ets, new_ets, {:inner_checksum, @repo, package, version})
               copy(original_ets, new_ets, {:outer_checksum, @repo, package, version})
               copy(original_ets, new_ets, {:retired, @repo, package, version})
-              copy(original_ets, new_ets, {:tarball_etag, @repo, package, version})
               copy(original_ets, new_ets, {:timestamp, @repo, package, version})
 
               [{_, dep_tuples}] = :ets.lookup(original_ets, {:deps, @repo, package, version})

lib/hex/registry/server.ex

@@ -50,14 +50,6 @@ defmodule Hex.Registry.Server do
     GenServer.call(@name, {:retired, repo, package, version}, @timeout)
   end
 
-  def tarball_etag(repo, package, version) do
-    GenServer.call(@name, {:tarball_etag, repo, package, version}, @timeout)
-  end
-
-  def tarball_etag(repo, package, version, etag) do
-    GenServer.call(@name, {:tarball_etag, repo, package, version, etag}, @timeout)
-  end
-
   def last_update() do
     GenServer.call(@name, :last_update, @timeout)
   end
@@ -173,16 +165,6 @@ defmodule Hex.Registry.Server do
     end)
   end
 
-  def handle_call({:tarball_etag, repo, package, version}, _from, state) do
-    etag = lookup(state.ets, {:tarball_etag, repo, package, version})
-    {:reply, etag, state}
-  end
-
-  def handle_call({:tarball_etag, repo, package, version, etag}, _from, state) do
-    :ets.insert(state.ets, {{:tarball_etag, repo, package, version}, etag})
-    {:reply, :ok, state}
-  end
-
   def handle_call(:last_update, _from, state) do
     time = lookup(state.ets, :last_update)
     {:reply, time, state}
@@ -272,7 +254,6 @@ defmodule Hex.Registry.Server do
   #   {{:inner_checksum, ^repo, _package, _version}, _} -> true
   #   {{:outer_checksum, ^repo, _package, _version}, _} -> true
   #   {{:retired, ^repo, _package, _version}, _} -> true
-  #   {{:tarball_etag, ^repo, _package, _version}, _} -> true
   #   {{:registry_etag, ^repo, _package}, _} -> true
   #   {{:timestamp, ^repo, _package}, _} -> true
   #   {{:timestamp, ^repo, _package, _version}, _} -> true
@@ -286,7 +267,6 @@ defmodule Hex.Registry.Server do
       {{{:inner_checksum, :"$1", :"$2", :"$3"}, :_}, [{:"=:=", {:const, repo}, :"$1"}], [true]},
       {{{:outer_checksum, :"$1", :"$2", :"$3"}, :_}, [{:"=:=", {:const, repo}, :"$1"}], [true]},
       {{{:retired, :"$1", :"$2", :"$3"}, :_}, [{:"=:=", {:const, repo}, :"$1"}], [true]},
-      {{{:tarball_etag, :"$1", :"$2", :"$3"}, :_}, [{:"=:=", {:const, repo}, :"$1"}], [true]},
       {{{:registry_etag, :"$1", :"$2"}, :_}, [{:"=:=", {:const, repo}, :"$1"}], [true]},
       {{{:timetamp, :"$1", :"$2"}, :_}, [{:"=:=", {:const, repo}, :"$1"}], [true]},
       {{{:timetamp, :"$1", :"$2", :"$3"}, :_}, [{:"=:=", {:const, repo}, :"$1"}], [true]},

lib/hex/repo.ex

@@ -166,8 +166,8 @@ defmodule Hex.Repo do
     HTTP.request(:get, docs_url(repo, package, version), headers, nil)
   end
 
-  def get_tarball(repo, package, version, etag) do
-    headers = Map.merge(etag_headers(etag), auth_headers(repo))
+  def get_tarball(repo, package, version) do
+    headers = auth_headers(repo)
     HTTP.request(:get, tarball_url(repo, package, version), headers, nil)
   end
 

lib/hex/scm.ex

@@ -123,9 +123,7 @@ defmodule Hex.SCM do
     name = opts[:hex]
     dest = opts[:dest]
     repo = opts[:repo]
-    filename = "#{name}-#{lock.version}.tar"
-    path = cache_path(repo, filename)
-    url = Hex.Repo.get_repo(repo).url <> "/tarballs/#{filename}"
+    path = cache_path(repo, name, lock.version)
 
     case Hex.Parallel.await(:hex_fetcher, {:tarball, repo, name, lock.version}, @fetch_timeout) do
       {:ok, :cached} ->
@@ -134,34 +132,29 @@ defmodule Hex.SCM do
       {:ok, :offline} ->
         Hex.Shell.debug("  [OFFLINE] Using locally cached package (#{path})")
 
-      {:ok, :new, etag} ->
-        Registry.tarball_etag(repo, name, lock.version, etag)
-
+      {:ok, :new} ->
         if Version.compare(System.version(), "1.4.0") == :lt do
           Registry.persist()
         end
 
-        safe_url = prune_uri_userinfo(url)
-        Hex.Shell.debug("  Fetched package (#{safe_url})")
+        tarball_url = tarball_url(repo, name, lock.version)
+        Hex.Shell.debug("  Fetched package (#{tarball_url})")
 
       {:error, reason} ->
         Hex.Shell.error(reason)
 
         unless File.exists?(path) do
-          safe_url = prune_uri_userinfo(url)
-          Mix.raise("Package fetch failed and no cached copy available (#{safe_url})")
+          tarball_url = tarball_url(repo, name, lock.version)
+          Mix.raise("Package fetch failed and no cached copy available (#{tarball_url})")
         end
 
         Hex.Shell.info("  Fetch failed. Using locally cached package (#{path})")
     end
 
     File.rm_rf!(dest)
 
-    registry_inner_checksum =
-      Hex.Registry.Server.inner_checksum(repo, to_string(name), lock.version)
-
-    registry_outer_checksum =
-      Hex.Registry.Server.outer_checksum(repo, to_string(name), lock.version)
+    registry_inner_checksum = Registry.inner_checksum(repo, to_string(name), lock.version)
+    registry_outer_checksum = Registry.outer_checksum(repo, to_string(name), lock.version)
 
     %{
       inner_checksum: tarball_inner_checksum,
@@ -218,13 +211,6 @@ defmodule Hex.SCM do
      lock.repo, lock.outer_checksum}
   end
 
-  defp prune_uri_userinfo(url) do
-    case URI.parse(url) do
-      %URI{userinfo: nil} -> url
-      uri -> URI.to_string(%{uri | userinfo: "******"})
-    end
-  end
-
   def checkout(opts) do
     # For checkout (deps.get) it's important the lock is not modified in the parts of
     # the lock entries that actually locks the dependency. Those entries are the package repo,
@@ -356,21 +342,12 @@ defmodule Hex.SCM do
     :erlang.term_to_binary({{:hex, 2, 0}, map})
   end
 
-  defp cache_path(repo, name) do
-    repo = Hex.Utils.windows_repo_path_fix(repo)
-    Path.join([Hex.State.fetch!(:cache_home), @packages_dir, repo, name])
-  end
-
   def prefetch(lock) do
     fetch = fetch_from_lock(lock)
 
     Enum.each(fetch, fn {repo, package, version} ->
-      filename = "#{package}-#{version}.tar"
-      path = cache_path(repo, filename)
-      etag = if File.exists?(path), do: Registry.tarball_etag(repo, package, version), else: nil
-
       Hex.Parallel.run(:hex_fetcher, {:tarball, repo, package, version}, fn ->
-        fetch(repo, package, version, path, etag)
+        fetch(repo, package, version)
       end)
     end)
   end
@@ -395,44 +372,61 @@ defmodule Hex.SCM do
     end)
   end
 
-  @doc false
-  def fetch(repo, package, version, path, etag) do
+  defp tarball_url(repo, package, version) do
+    filename = "#{package}-#{version}.tar"
+    prune_uri_userinfo(Hex.Repo.get_repo(repo).url <> "/tarballs/#{filename}")
+  end
+
+  defp prune_uri_userinfo(url) do
+    case URI.parse(url) do
+      %URI{userinfo: nil} -> url
+      uri -> URI.to_string(%{uri | userinfo: "******"})
+    end
+  end
+
+  def cache_path(repo, package, version) do
+    repo = Hex.Utils.windows_repo_path_fix(repo)
+    filename = "#{package}-#{version}.tar"
+    Path.join([Hex.State.fetch!(:cache_home), @packages_dir, repo, filename])
+  end
+
+  def fetch(repo, package, version) do
     if Hex.State.fetch!(:offline) do
       {:ok, :offline}
     else
-      case Hex.Repo.get_tarball(repo, package, version, etag) do
-        {:ok, {200, body, headers}} ->
-          etag = headers['etag']
-          etag = if etag, do: List.to_string(etag)
+      outer_checksum = Registry.outer_checksum(repo, package, version)
+      path = cache_path(repo, package, version)
 
-          case path do
-            :memory ->
-              {:ok, :new, body, etag}
+      case Hex.Tar.outer_checksum(path) do
+        {:ok, ^outer_checksum} ->
+          {:ok, :cached}
 
-            _ when is_binary(path) ->
+        {:error, _reason} ->
+          case Hex.Repo.get_tarball(repo, package, version) do
+            {:ok, {200, body, _headers}} ->
               File.mkdir_p!(Path.dirname(path))
               File.write!(path, body)
-              {:ok, :new, etag}
-          end
+              {:ok, :new}
 
-        {:ok, {304, _body, _headers}} ->
-          {:ok, :cached}
+            {:ok, {304, _body, _headers}} ->
+              {:ok, :cached}
 
-        {:ok, {code, _body, _headers}} ->
-          {:error, "Request failed (#{code})"}
+            {:ok, {code, _body, _headers}} ->
+              {:error, "Request failed (#{code})"}
 
-        {:error, :timeout} ->
-          reason = [
-            "Request failed (:timeout)",
-            :reset,
-            "\nIf this happens consistently, adjust your concurrency and timeout settings:",
-            "\n\n    HEX_HTTP_CONCURRENCY=1 HEX_HTTP_TIMEOUT=120 mix deps.get"
-          ]
+            {:error, :timeout} ->
+              reason = [
+                "Request failed (:timeout)",
+                :reset,
+                "\nIf this happens consistently, adjust your concurrency and timeout settings:",
+                "\n\n    HEX_HTTP_CONCURRENCY=1 HEX_HTTP_TIMEOUT=120 mix deps.get"
+              ]
 
-          {:error, reason}
+              {:error, reason}
 
-        {:error, reason} ->
-          {:error, "Request failed (#{inspect(reason)})"}
+            {:error, reason} ->
+              {:error, "Request failed (#{inspect(reason)})"}
+          end
       end
     end
   end

lib/hex/tar.ex

@@ -41,4 +41,12 @@ defmodule Hex.Tar do
         Mix.raise("Unpacking tarball failed: #{:mix_hex_tarball.format_error(reason)}")
     end
   end
+
+  # TODO: Add this function to
+  def outer_checksum(path) do
+    case File.read(path) do
+      {:ok, tarball} -> {:ok, :crypto.hash(:sha256, tarball)}
+      {:error, reason} -> {:error, reason}
+    end
+  end
 end

lib/mix/tasks/hex.package.ex

@@ -169,17 +169,21 @@ defmodule Mix.Tasks.Hex.Package do
   end
 
   defp fetch_tarball!(repo, package, version) do
-    etag = nil
+    path = Hex.SCM.cache_path(repo, package, version)
 
-    case Hex.SCM.fetch(repo, package, version, :memory, etag) do
-      {:ok, :new, tarball, _etag} ->
-        tarball
+    case Hex.SCM.fetch(repo, package, version) do
+      {:ok, _} ->
+        File.read!(path)
 
       {:error, reason} ->
-        Mix.raise(
-          "Downloading " <>
-            Hex.Repo.tarball_url(repo, package, version) <> " failed:\n\n" <> reason
-        )
+        if File.exists?(path) do
+          File.read!(path)
+        else
+          Mix.raise(
+            "Downloading " <>
+              Hex.Repo.tarball_url(repo, package, version) <> " failed:\n\n" <> reason
+          )
+        end
     end
   end