(#1743): store the etags in cache files

2 files changed, 38 insertions, 15 deletions

diff --git a/src/rebar_pkg_resource.erl b/src/rebar_pkg_resource.erl
index 7d42ccd..f77a81a 100644
--- a/src/rebar_pkg_resource.erl
+++ b/src/rebar_pkg_resource.erl
@@ -6,6 +6,7 @@
 
 -export([lock/2
         ,download/3
+        ,download/4
         ,needs_update/2
         ,make_vsn/1]).
 
@@ -13,6 +14,8 @@
         ,etag/1
         ,ssl_opts/1]).
 
+-export([store_etag_in_cache/2]).
+
 -include("rebar.hrl").
 -include_lib("public_key/include/OTP-PUB-KEY.hrl").
 
@@ -28,27 +31,34 @@ needs_update(Dir, {pkg, _Name, Vsn, _Hash}) ->
             true
     end.
 
-download(TmpDir, Pkg={pkg, Name, Vsn, _Hash}, State) ->
+download(TmpDir, Pkg, State) ->
+    download(TmpDir, Pkg, State, true).
+
+download(TmpDir, Pkg={pkg, Name, Vsn, _Hash}, State, UpdateETag) ->
     CDN = rebar_state:get(State, rebar_packages_cdn, ?DEFAULT_CDN),
     {ok, PackageDir} = rebar_packages:package_dir(State),
     Package = binary_to_list(<<Name/binary, "-", Vsn/binary, ".tar">>),
+    ETagFile = binary_to_list(<<Name/binary, "-", Vsn/binary, ".etag">>),
     CachePath = filename:join(PackageDir, Package),
+    ETagPath = filename:join(PackageDir, ETagFile),
     case rebar_utils:url_append_path(CDN, filename:join(?REMOTE_PACKAGE_DIR, Package)) of
         {ok, Url} ->
-            cached_download(TmpDir, CachePath, Pkg, Url, etag(CachePath), State);
+            cached_download(TmpDir, CachePath, Pkg, Url, etag(ETagPath), State, ETagPath, UpdateETag);
         _ ->
             {fetch_fail, Name, Vsn}
     end.
 
-cached_download(TmpDir, CachePath, Pkg={pkg, Name, Vsn, _Hash}, Url, ETag, State) ->
+cached_download(TmpDir, CachePath, Pkg={pkg, Name, Vsn, _Hash}, Url, ETag, State, ETagPath, UpdateETag) ->
     case request(Url, ETag) of
         {ok, cached} ->
             ?INFO("Version cached at ~ts is up to date, reusing it", [CachePath]),
             serve_from_cache(TmpDir, CachePath, Pkg, State);
         {ok, Body, NewETag} ->
             ?INFO("Downloaded package, caching at ~ts", [CachePath]),
-            serve_from_download(TmpDir, CachePath, Pkg, NewETag, Body, State);
+            maybe_store_etag_in_cache(UpdateETag, ETagPath, NewETag),
+            serve_from_download(TmpDir, CachePath, Pkg, NewETag, Body, State, ETagPath);
         error when ETag =/= false ->
+            store_etag_in_cache(ETagPath, ETag),
             ?INFO("Download error, using cached file at ~ts", [CachePath]),
             serve_from_cache(TmpDir, CachePath, Pkg, State);
         error ->
@@ -75,10 +85,10 @@ serve_from_cache(TmpDir, CachePath, Pkg, State) ->
             {bad_checksum, CachePath}
     end.
 
-serve_from_download(TmpDir, CachePath, Package, ETag, Binary, State) ->
+serve_from_download(TmpDir, CachePath, Package, ETag, Binary, State, ETagPath) ->
     ?DEBUG("Writing ~p to cache at ~ts", [Package, CachePath]),
     file:write_file(CachePath, Binary),
-    case etag(CachePath) of
+    case etag(ETagPath) of
         ETag ->
             serve_from_cache(TmpDir, CachePath, Package, State);
         FileETag ->
@@ -86,7 +96,6 @@ serve_from_download(TmpDir, CachePath, Package, ETag, Binary, State) ->
             {bad_download, CachePath}
     end.
 
-
 extract(TmpDir, CachePath) ->
     ec_file:mkdir_p(TmpDir),
     {ok, Files} = erl_tar:extract(CachePath, [memory]),
@@ -131,9 +140,8 @@ request(Url, ETag) ->
 
 etag(Path) ->
     case file:read_file(Path) of
-        {ok, Binary} ->
-            <<X:128/big-unsigned-integer>> = crypto:hash(md5, Binary),
-            rebar_string:lowercase(lists:flatten(io_lib:format("~32.16.0b", [X])));
+        {ok, Bin} ->
+            binary_to_list(Bin);
         {error, _} ->
             false
     end.
@@ -216,3 +224,11 @@ version_pad([Major, Minor, Patch]) ->
     {list_to_integer(Major), list_to_integer(Minor), list_to_integer(Patch)};
 version_pad([Major, Minor, Patch | _]) ->
     {list_to_integer(Major), list_to_integer(Minor), list_to_integer(Patch)}.
+
+maybe_store_etag_in_cache(false = _UpdateETag, _Path, _ETag) ->
+    ok;
+maybe_store_etag_in_cache(true = _UpdateETag, Path, ETag) ->
+    store_etag_in_cache(Path, ETag).
+
+store_etag_in_cache(Path, ETag) ->
+    ok = file:write_file(Path, ETag).
diff --git a/test/rebar_pkg_SUITE.erl b/test/rebar_pkg_SUITE.erl
index 8ddf58f..9ddd704 100644
--- a/test/rebar_pkg_SUITE.erl
+++ b/test/rebar_pkg_SUITE.erl
@@ -9,6 +9,7 @@
 -define(good_etag, "22e1d7387c9085a462340088a2a8ba67").
 -define(bad_checksum, <<"D576B442A68C7B92BACDE1EFE9C6E54D8D6C74BDB71D8175B9D3C6EC8C7B62A7">>).
 -define(good_checksum, <<"1C6CE379D191FBAB41B7905075E0BF87CBBE23C77CECE775C5A0B786B2244C35">>).
+-define(BADPKG_ETAG, <<"BADETAG">>).
 
 all() -> [good_uncached, good_cached, badindexchk, badpkg,
           badhash_nocache, badhash_cache,
@@ -83,7 +84,7 @@ init_per_testcase(bad_to_good=Name, Config0) ->
     Config;
 init_per_testcase(good_disconnect=Name, Config0) ->
     Pkg = {<<"goodpkg">>, <<"1.0.0">>},
-    Config1 = [{good_cache, true},
+    Config1 = [{good_cache, false},
                {pkg, Pkg}
               | Config0],
     Config = mock_config(Name, Config1),
@@ -147,11 +148,15 @@ badpkg(Config) ->
     Tmp = ?config(tmp_dir, Config),
     {Pkg,Vsn} = ?config(pkg, Config),
     State = ?config(state, Config),
-    ?assertMatch({bad_download, _Path},
-                 rebar_pkg_resource:download(Tmp, {pkg, Pkg, Vsn, ?good_checksum}, State)),
-    %% The cached file is there for forensic purposes
     Cache = ?config(cache_dir, Config),
-    ?assert(filelib:is_regular(filename:join(Cache, <<Pkg/binary, "-", Vsn/binary, ".tar">>))).
+    CachePath = filename:join(Cache, <<Pkg/binary, "-", Vsn/binary, ".tar">>),
+    ETagPath = filename:join(Cache, <<Pkg/binary, "-", Vsn/binary, ".etag">>),
+    rebar_pkg_resource:store_etag_in_cache(ETagPath, ?BADPKG_ETAG),
+    ?assertMatch({bad_download, _Path},
+                 rebar_pkg_resource:download(Tmp, {pkg, Pkg, Vsn, ?good_checksum}, State, false)),
+    %% The cached/etag files are there for forensic purposes
+    ?assert(filelib:is_regular(ETagPath)),
+    ?assert(filelib:is_regular(CachePath)).
 
 badhash_nocache(Config) ->
     Tmp = ?config(tmp_dir, Config),
@@ -196,8 +201,10 @@ good_disconnect(Config) ->
     State = ?config(state, Config),
     Cache = ?config(cache_dir, Config),
     CachedFile = filename:join(Cache, <<Pkg/binary, "-", Vsn/binary, ".tar">>),
+    ETagFile = filename:join(Cache, <<Pkg/binary, "-", Vsn/binary, ".etag">>),
     ?assert(filelib:is_regular(CachedFile)),
     {ok, Content} = file:read_file(CachedFile),
+    rebar_pkg_resource:store_etag_in_cache(ETagFile, ?BADPKG_ETAG),
     ?assertEqual({ok, true},
                  rebar_pkg_resource:download(Tmp, {pkg, Pkg, Vsn, ?good_checksum}, State)),
     {ok, Content} = file:read_file(CachedFile).