Skip to content

Commit

Permalink
Don't delete docs when uploading docs with shared prefix (#39)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ericmj authored May 6, 2024
1 parent 20a09a7 commit d7a408d
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
9 changes: 8 additions & 1 deletion lib/hexdocs/bucket.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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))
Expand Down
5 changes: 4 additions & 1 deletion lib/hexdocs/queue.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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} ->
Expand Down
19 changes: 19 additions & 0 deletions test/hexdocs/bucket_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit d7a408d

Please sign in to comment.