Merge pull request #2738 from dmitrivereshchagin/empty-if-none-match

Do not send empty If-None-Match in tarball request
erlang · Aug 22, 2022 · c0c5a0e · c0c5a0e
2 parents 0d95d08 + 2828bdb
commit c0c5a0e
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 6 deletions.
diff --git a/apps/rebar/src/rebar_pkg_resource.erl b/apps/rebar/src/rebar_pkg_resource.erl
@@ -138,7 +138,7 @@ format_error({bad_registry_checksum, Name, Vsn, Expected, Found}) ->
 %% {ok, Contents, NewEtag}, otherwise if some error occurred return error.
 %% @end
 %%------------------------------------------------------------------------------
--spec request(rebar_hex_repos:repo(), binary(), binary(), false | binary())
+-spec request(rebar_hex_repos:repo(), binary(), binary(), binary() | undefined)
              -> {ok, cached} | {ok, binary(), binary()} | error.
 request(Config, Name, Version, ETag) ->
     Config1 = Config#{http_etag => ETag},
@@ -169,7 +169,7 @@ request(Config, Name, Version, ETag) ->
 -spec etag(PackagePath, ETagPath) -> Res when
       PackagePath :: file:name(),
       ETagPath :: file:name(),
-      Res :: binary().
+      Res :: binary() | undefined.
 etag(PackagePath, ETagPath) ->
     case file:read_file(ETagPath) of
         {ok, Bin} ->
@@ -179,10 +179,10 @@ etag(PackagePath, ETagPath) ->
                 true ->
                     Bin;
                 false ->
-                    <<>>
+                    undefined
             end;
         {error, _} ->
-            <<>>
+            undefined
     end.
 
 %%------------------------------------------------------------------------------
@@ -205,7 +205,7 @@ store_etag_in_cache(Path, ETag) ->
       CachePath :: file:name(),
       Pkg :: package(),
       State :: rebar_state:t(),
-      ETag :: binary(),
+      ETag :: binary() | undefined,
       ETagPath :: file:name(),
       UpdateETag :: boolean(),
       Res :: ok | {unexpected_hash, integer(), integer()} | {fetch_fail, binary(), binary()}
@@ -222,7 +222,7 @@ cached_download(TmpDir, CachePath, Pkg={pkg, Name, Vsn, _OldHash, _Hash, RepoCon
             ?DEBUG("Downloaded package ~ts, caching at ~ts", [Name, CachePath]),
             maybe_store_etag_in_cache(UpdateETag, ETagPath, NewETag),
             serve_from_download(TmpDir, CachePath, Pkg, Body);
-        error when ETag =/= <<>> ->
+        error when ETag =/= undefined ->
             store_etag_in_cache(ETagPath, ETag),
             ?INFO("Download error, using cached file at ~ts", [CachePath]),
             serve_from_cache(TmpDir, CachePath, Pkg);

diff --git a/apps/rebar/test/rebar_pkg_SUITE.erl b/apps/rebar/test/rebar_pkg_SUITE.erl
@@ -117,6 +117,8 @@ good_uncached(Config) ->
     State = ?config(state, Config),
     ?assertEqual(ok,
                  rebar_pkg_resource:download(Tmp, {pkg, Pkg, Vsn, ?good_checksum, ?good_checksum, #{}}, State, #{}, true)),
+    ?assert(meck:called(r3_hex_repo, get_tarball,
+                        [meck:is(fun(#{http_etag := T}) -> T =:= undefined end), '_', '_'])),
     Cache = ?config(cache_dir, Config),
     ?assert(filelib:is_regular(filename:join(Cache, <<Pkg/binary, "-", Vsn/binary, ".tar">>))).
 
@@ -127,9 +129,13 @@ good_cached(Config) ->
     Cache = ?config(cache_dir, Config),
     CachedFile = filename:join(Cache, <<Pkg/binary, "-", Vsn/binary, ".tar">>),
     ?assert(filelib:is_regular(CachedFile)),
+    ETagPath = filename:join(Cache, <<Pkg/binary, "-", Vsn/binary, ".etag">>),
+    rebar_pkg_resource:store_etag_in_cache(ETagPath, ?good_etag),
     {ok, Content} = file:read_file(CachedFile),
     ?assertEqual(ok,
                  rebar_pkg_resource:download(Tmp, {pkg, Pkg, Vsn, ?good_checksum, ?good_checksum, #{}}, State, #{}, true)),
+    ?assert(meck:called(r3_hex_repo, get_tarball,
+                        [meck:is(fun(#{http_etag := T}) -> T =:= ?good_etag end), '_', '_'])),
     {ok, Content} = file:read_file(CachedFile).