Don't delete docs when uploading docs with shared prefix

Private packages with a shared prefix, such that ecto is a prefix of
ecto_sql, would delete the docs of ecto_sql when ecto was uploaded.
This is because we list bucket files by prefix so that ecto/ would
list all files under the ecto directory. The trailing / was removed
by Path.join so we would also list the files of ecto_sql. This was
only happened for private packages.

Closes hexpm/hexpm#1252.

lib/hexdocs/bucket.ex

@@ -19,6 +19,8 @@ defmodule Hexdocs.Bucket do
       meta: [{"surrogate-key", key}]
     ]
 
+    Logger.info("Uploading docs_public_bucket #{path}")
+
     case Hexdocs.Store.put(:docs_public_bucket, path, sitemap, opts) do
       {:ok, 200, _headers, _body} ->
         :ok
@@ -205,6 +207,10 @@ defmodule Hexdocs.Bucket do
         &delete_key?(&1, paths, repository, package, versions, upload_type)
       )
 
+    Enum.each(keys_to_delete, fn key ->
+      Logger.info("Deleting #{bucket} #{key}")
+    end)
+
     Hexdocs.Store.delete_many(bucket, keys_to_delete)
   end
 
@@ -253,7 +259,8 @@ defmodule Hexdocs.Bucket do
   defp cache_control(false = _public?), do: "private, max-age=3600"
 
   defp repository_path("hexpm", path), do: path
-  defp repository_path(repository, path), do: Path.join(repository, path)
+  # Don't use Path.join as it removes trailing / which is needed for bucket listing
+  defp repository_path(repository, path), do: Enum.join([repository, "/", path])
 
   defp purge_hexdocs_cache("hexpm", package, versions, :both) do
     keys = Enum.map(versions, &docspage_versioned_cdn_key("hexpm", package, &1))

lib/hexdocs/queue.ex

@@ -139,6 +139,7 @@ defmodule Hexdocs.Queue do
 
   defp handle_record(%{"eventName" => "ObjectRemoved:" <> _, "s3" => s3}) do
     key = s3["object"]["key"]
+    start = System.os_time(:millisecond)
     Logger.info("OBJECT DELETED #{key}")
 
     case key_components(key) do
@@ -147,7 +148,9 @@ defmodule Hexdocs.Queue do
         all_versions = all_versions(repository, package)
         Hexdocs.Bucket.delete(repository, package, version, all_versions)
         update_index_sitemap(repository, key)
-        Logger.info("FINISHED DELETING DOCS #{key}")
+
+        elapsed = System.os_time(:millisecond) - start
+        Logger.info("FINISHED DELETING DOCS #{key} #{elapsed}ms")
         :ok
 
       {:ok, _repository, _package, _version} ->

test/hexdocs/bucket_test.exs

@@ -52,6 +52,25 @@ defmodule Hexdocs.BucketTest do
     refute Store.get(@bucket, "buckettest/#{test}/remove.html")
   end
 
+  test "dont overwrite package which same prefix name", %{test: test} do
+    version = Version.parse!("0.0.1")
+    test = Atom.to_string(test)
+    prefix_name = String.slice(test, -1000, String.length(test) - 1)
+
+    Bucket.upload("buckettest", "#{test}", version, [], [
+      {"file2", ""}
+    ])
+
+    Bucket.upload("buckettest", "#{prefix_name}", version, [], [
+      {"file1", ""}
+    ])
+
+    assert Store.get(@bucket, "buckettest/#{prefix_name}/file1")
+    assert Store.get(@bucket, "buckettest/#{prefix_name}/#{version}/file1")
+    assert Store.get(@bucket, "buckettest/#{test}/file2")
+    assert Store.get(@bucket, "buckettest/#{test}/#{version}/file2")
+  end
+
   test "newer beta docs do not overwrite stable main docs", %{test: test} do
     first = Version.parse!("0.5.0")
     second = Version.parse!("1.0.0-beta")