Test all S3 modules; move is_file to is_regular

As it turns out, S3 does not allow is_regular-like usage and calling
HEAD on a directory isn't guaranteed to return a result. That's alright
though because the use that was being made by the tracker behind it all
only actually cares about regular files in the first place, so change
the whole working set for is_regular.

All the other significant calls are mocked out based on manually
verified S3 headers and bodies being returned, so we can get ongoing
tests of s3 operations without doing the full integration suite that
costs money.

apps/revault/src/revault_dirmon_tracker.erl

@@ -317,7 +317,7 @@ apply_operation({deleted, {FileName, Hash}}, SetMap, ITC, Dir) ->
             %% scanner moved and reaped files at once.
             %% As such, updating the conflict files if it's gone overwrites the
             %% user's change.
-            case revault_file:is_file(conflict_marker(Dir, BaseFile)) of
+            case revault_file:is_regular(conflict_marker(Dir, BaseFile)) of
                 true ->
                     write_conflict_marker(Dir, BaseFile, {Ct, {conflict, Hashes, WorkingHash}});
                 false ->

apps/revault/src/revault_file.erl

@@ -5,7 +5,7 @@
          tmp/0, tmp/1, extension/2,
          find_hashes/2,
          %% wrappers to file module
-         delete/1, consult/1, read_file/1, ensure_dir/1, is_file/1,
+         delete/1, consult/1, read_file/1, ensure_dir/1, is_regular/1,
          write_file/2, write_file/3, rename/2]).
 
 -type hash() :: binary().
@@ -121,11 +121,10 @@ read_file(Path) ->
 ensure_dir(Path) ->
     (mod()):ensure_dir(Path).
 
-%% @doc Returns true if the path refers to a file or a directory,
-%% otherwise false.
--spec is_file(file:filename_all()) -> boolean().
-is_file(Path) ->
-    (mod()):is_file(Path).
+%% @doc Returns true if the path refers to a file, otherwise false.
+-spec is_regular(file:filename_all()) -> boolean().
+is_regular(Path) ->
+    (mod()):is_regular(Path).
 
 %% @doc Writes the content to the file mentioned.  The file is created if it
 %% does not exist. If it exists, the previous contents are overwritten.

apps/revault/src/revault_file_disk.erl

@@ -4,7 +4,7 @@
          tmp/0, tmp/1,
          find_hashes/2,
          %% wrappers to file module
-         delete/1, consult/1, read_file/1, ensure_dir/1, is_file/1,
+         delete/1, consult/1, read_file/1, ensure_dir/1, is_regular/1,
          write_file/2, write_file/3, rename/2]).
 
 -type hash() :: binary().
@@ -110,11 +110,10 @@ read_file(Path) ->
 ensure_dir(Path) ->
     filelib:ensure_dir(Path).
 
-%% @doc Returns true if the path refers to a file or a directory,
-%% otherwise false.
--spec is_file(file:filename_all()) -> boolean().
-is_file(Path) ->
-    filelib:is_file(Path).
+%% @doc Returns true if the path refers to a file, otherwise false.
+-spec is_regular(file:filename_all()) -> boolean().
+is_regular(Path) ->
+    filelib:is_regular(Path).
 
 %% @doc Writes the content to the file mentioned.  The file is created if it
 %% does not exist. If it exists, the previous contents are overwritten.

apps/revault/src/revault_fsm.erl

@@ -844,7 +844,6 @@ server_sync_files(info, {revault, Marker, {peer, Peer, _Attrs}},
 server_sync_files(_, _, Data) ->
     {keep_state, Data, [postpone]}.
 
-%% TODO: test
 format_status(Status) ->
     maps:map(
       fun(Messages, Queue) when Messages == queue; Messages == postponed ->

apps/revault/src/revault_s3.erl

@@ -5,7 +5,7 @@
          find_hashes/2, find_hashes_uncached/2,
          %%% wrappers to file module; can never use a cache if we
          %%% want the cache module to safely use these.
-         delete/1, consult/1, read_file/1, ensure_dir/1, is_file/1,
+         delete/1, consult/1, read_file/1, ensure_dir/1, is_regular/1,
          write_file/2, write_file/3, rename/2
         ]).
 
@@ -93,16 +93,21 @@ read_file(Path) ->
         {ok, Contents}
     end.
 
-%% @doc Returns true if the path refers to a file or a directory,
-%% otherwise false.
--spec is_file(file:filename_all()) -> boolean().
-is_file(Path) ->
-    %% TODO: test this
+%% @doc Returns true if the path refers to a file, otherwise false.
+-spec is_regular(file:filename_all()) -> boolean().
+is_regular(Path) ->
     %% S3 returns a valid object for directories, but with a
     %% different content-type. We don't care here, whatever's good
     %% is good for us.
     Res = aws_s3:head_object(client(), bucket(), Path, #{}),
-    handle_result(Res) == ok.
+    case Res of
+        {ok, #{<<"ContentType">> := <<"application/x-directory;", _/binary>>}, _} ->
+            false;
+        {ok, _, _} ->
+            true;
+        _ ->
+            false
+    end.
 
 %% @doc This operation isn't required on S3 and is a no-op.
 ensure_dir(_Path) ->
@@ -197,7 +202,13 @@ parse_all(L) ->
     end.
 
 s3_tmpdir() ->
-    application:get_env(revault, s3_tmpdir, ".tmp").
+    Dir = application:get_env(revault, s3_tmpdir, ".tmp"),
+    %% S3 really does not like an absolute path here as it will mess
+    %% with copy paths, so bail out if we find that happening.
+    case string:prefix(Dir, "/") of
+        nomatch -> Dir;
+        _ -> error(absolute_s3_tmpdir)
+    end.
 
 randname() ->
     float_to_list(rand:uniform()).
@@ -226,7 +237,6 @@ list_all_files(Dir, Continuation) ->
                     Files
             end;
         {ok, #{<<"ListBucketResult">> := #{<<"KeyCount">> := <<"0">>}}, _} ->
-            %% TODO: test
             []
     end.
 

apps/revault/src/revault_s3_cache.erl

@@ -27,6 +27,7 @@ ensure_loaded(Name) ->
 
 %% since we are mostly caching for cost ($) and not speed, it's okay to go simple
 %% and serialize all reads and writes.
+-spec hash(term(), term()) -> {ok, term()} | undefined.
 hash(Name, Key) ->
     gen_server:call(?VIA_GPROC(Name), {get, Key}, timer:minutes(1)).