From d7a408dc94ea5389481d007baff03e385329638b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eric=20Meadows-J=C3=B6nsson?= Date: Mon, 6 May 2024 15:21:19 +0200 Subject: [PATCH] Don't delete docs when uploading docs with shared prefix (#39) 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 | 9 ++++++++- lib/hexdocs/queue.ex | 5 ++++- test/hexdocs/bucket_test.exs | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/hexdocs/bucket.ex b/lib/hexdocs/bucket.ex index ff7b7d8..73b82dd 100644 --- a/lib/hexdocs/bucket.ex +++ b/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)) diff --git a/lib/hexdocs/queue.ex b/lib/hexdocs/queue.ex index 1e0299b..fd85fbb 100644 --- a/lib/hexdocs/queue.ex +++ b/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} -> diff --git a/test/hexdocs/bucket_test.exs b/test/hexdocs/bucket_test.exs index 0b2b67f..b02c034 100644 --- a/test/hexdocs/bucket_test.exs +++ b/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")