Properly track preflighted uploads on the server

Fixes phoenixframework#3115. Relates to phoenixframework#3004. Partially fixes phoenixframework#2835.
SteffenDE · Feb 22, 2024 · 0dbe35f · 0dbe35f
1 parent 6e4ddc1
commit 0dbe35f
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 19 deletions.
diff --git a/assets/js/phoenix_live_view/live_uploader.js b/assets/js/phoenix_live_view/live_uploader.js
@@ -70,7 +70,7 @@ export default class LiveUploader {
   static trackFiles(inputEl, files, dataTransfer){
     if(inputEl.getAttribute("multiple") !== null){
       let newFiles = files.filter(file => !this.activeFiles(inputEl).find(f => Object.is(f, file)))
-      DOM.putPrivate(inputEl, "files", this.activeFiles(inputEl).concat(newFiles))
+      DOM.updatePrivate(inputEl, "files", [], (existing) => existing.concat(newFiles))
       inputEl.value = null
     } else {
       // Reset inputEl files to align output with programmatic changes (i.e. drag and drop)

diff --git a/assets/js/phoenix_live_view/upload_entry.js b/assets/js/phoenix_live_view/upload_entry.js
@@ -15,10 +15,9 @@ import DOM from "./dom"
 export default class UploadEntry {
   static isActive(fileEl, file){
     let isNew = file._phxRef === undefined
-    let isPreflightInProgress = UploadEntry.isPreflightInProgress(file)
     let activeRefs = fileEl.getAttribute(PHX_ACTIVE_ENTRY_REFS).split(",")
     let isActive = activeRefs.indexOf(LiveUploader.genFileRef(file)) >= 0
-    return file.size > 0 && (isNew || isActive || !isPreflightInProgress)
+    return file.size > 0 && (isNew || isActive)
   }
 
   static isPreflighted(fileEl, file){

diff --git a/lib/phoenix_live_view/channel.ex b/lib/phoenix_live_view/channel.ex
@@ -219,7 +219,8 @@ defmodule Phoenix.LiveView.Channel do
 
         {ok_or_error, reply, %Socket{} = new_socket} =
           with {:ok, new_socket} <- Upload.put_entries(socket, conf, entries, cid) do
-            Upload.generate_preflight_response(new_socket, conf.name, cid)
+            refs = Enum.map(entries, fn %{"ref" => ref} -> ref end)
+            Upload.generate_preflight_response(new_socket, conf.name, cid, refs)
           end
 
         new_upload_names =

diff --git a/lib/phoenix_live_view/upload.ex b/lib/phoenix_live_view/upload.ex
@@ -337,16 +337,22 @@ defmodule Phoenix.LiveView.Upload do
   @doc """
   Generates a preflight response by calling the `:external` function.
   """
-  def generate_preflight_response(%Socket{} = socket, name, cid) do
+  def generate_preflight_response(%Socket{} = socket, name, cid, refs) do
     %UploadConfig{} = conf = Map.fetch!(socket.assigns.uploads, name)
 
+    # don't send more than max_entries preflight responses
+    refs = for {entry, i} <- Enum.with_index(conf.entries),
+      entry.ref in refs,
+      i < conf.max_entries && not entry.preflighted?,
+      do: entry.ref
+
     client_meta = %{
       max_file_size: conf.max_file_size,
       max_entries: conf.max_entries,
       chunk_size: conf.chunk_size
     }
 
-    {new_socket, new_conf, new_entries} = mark_preflighted(socket, conf)
+    {new_socket, new_conf, new_entries} = mark_preflighted(socket, conf, refs)
 
     case new_conf.external do
       false ->
@@ -357,8 +363,8 @@ defmodule Phoenix.LiveView.Upload do
     end
   end
 
-  defp mark_preflighted(socket, conf) do
-    {new_conf, new_entries} = UploadConfig.mark_preflighted(conf)
+  defp mark_preflighted(socket, conf, refs) do
+    {new_conf, new_entries} = UploadConfig.mark_preflighted(conf, refs)
     new_socket = update_uploads(new_conf, socket)
     {new_socket, new_conf, new_entries}
   end

diff --git a/lib/phoenix_live_view/upload_config.ex b/lib/phoenix_live_view/upload_config.ex
@@ -357,23 +357,15 @@ defmodule Phoenix.LiveView.UploadConfig do
   end
 
   @doc false
-  def mark_preflighted(%UploadConfig{} = conf) do
-    refs_awaiting = refs_awaiting_preflight(conf)
-
+  def mark_preflighted(%UploadConfig{} = conf, refs) do
     new_entries =
       for entry <- conf.entries do
-        %UploadEntry{entry | preflighted?: entry.preflighted? || entry.ref in refs_awaiting}
+        %UploadEntry{entry | preflighted?: entry.preflighted? || entry.ref in refs}
       end
 
     new_conf = %UploadConfig{conf | entries: new_entries}
 
-    {new_conf, for(ref <- refs_awaiting, do: get_entry_by_ref(new_conf, ref))}
-  end
-
-  defp refs_awaiting_preflight(%UploadConfig{} = conf) do
-    for {entry, i} <- Enum.with_index(conf.entries),
-        i < conf.max_entries && not entry.preflighted?,
-        do: entry.ref
+    {new_conf, for(ref <- refs, do: get_entry_by_ref(new_conf, ref))}
   end
 
   @doc false