🐛 Reserve S3 download path without creating the file

The S3 file download opens the temp file with CREATE_NEW, so it
fails when the path already exists. tempfile creates the empty
file on reserve, which made every large download depend on the
SDK deleting the placeholder between its own retries.

Add tmp/temp-path, which reserves a unique path and registers
cleanup without creating the file, and use it for S3 downloads.
The old retry on FileAlreadyExists stays as a safety net for a
name clash.

AI-assisted-by: muse-spark-1.3-contributor
This commit is contained in:
Andrey Antukh 2026-10-05 15:15:41 +00:00
parent ab58922f2c
commit 4b70c9aa21
3 changed files with 60 additions and 1 deletions

View File

@ -316,7 +316,7 @@
;; to the filesystem and then read with buffered inputstream; if
;; not, read the contento into memory using bytearrays.
(if (> ^long size (* 1024 1024 2))
(let [path (tmp/tempfile :prefix "penpot.storage.s3." :min-age "6h")
(let [path (tmp/temp-path :prefix "penpot.storage.s3." :min-age "6h")
rxf (AsyncResponseTransformer/toFile ^Path path)]
(->> (.getObject ^S3AsyncClient client
^GetObjectRequest gor

View File

@ -93,6 +93,25 @@
(sp/offer! queue [path (some-> min-age ct/duration)])
path))
(defn temp-path
"Reserve a unique temp path WITHOUT creating the file.
Use it when an external writer needs to create the file itself and
fails if it already exists (like the S3 SDK file download, which
opens with CREATE_NEW). The path is still registered for cleanup,
so a crash between reserve and write does not leak."
[& {:keys [suffix prefix dir min-age]
:or {prefix "penpot."
suffix ".tmp"
dir default-tmp-dir}}]
(fs/create-dir dir)
(loop [path (fs/join dir (str prefix (uuid/next) suffix))]
(if (fs/exists? path)
(recur (fs/join dir (str prefix (uuid/next) suffix)))
(do
(sp/offer! queue [path (some-> min-age ct/duration)])
path))))
(defn tempfile-from
"Create a new tempfile from from consuming the stream"
[input & {:as options}]

View File

@ -0,0 +1,40 @@
;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS SUBSIDIARY SL
(ns backend-tests.storage-tmp-test
(:require
[app.storage.tmp :as tmp]
[clojure.test :as t]
[datoteka.fs :as fs])
(:import
java.nio.file.Files
java.nio.file.Path))
(t/deftest temp-path-does-not-create-the-file
(let [path (tmp/temp-path :prefix "penpot.test." :min-age "6h")]
(t/testing "the reserved path does not exist yet"
(t/is (not (fs/exists? path))))
(t/testing "an external writer can create it with CREATE_NEW"
(Files/createFile ^Path path (into-array java.nio.file.attribute.FileAttribute []))
(t/is (fs/exists? path)))
(fs/delete path)))
(t/deftest temp-path-is-unique
(let [a (tmp/temp-path :prefix "penpot.test.")
b (tmp/temp-path :prefix "penpot.test.")]
(t/is (not= a b))
(t/is (not (fs/exists? a)))
(t/is (not (fs/exists? b)))))
(t/deftest tempfile-creates-the-file-so-create-new-fails
(t/testing "documents why s3 downloads cannot use tempfile: the SDK opens with CREATE_NEW"
(let [path (tmp/tempfile :prefix "penpot.test.")]
(try
(t/is (fs/exists? path))
(t/is (thrown? java.nio.file.FileAlreadyExistsException
(Files/createFile ^Path path (into-array java.nio.file.attribute.FileAttribute []))))
(finally
(fs/delete path))))))