Skip to content

Commit 94b0dee

Browse files
committed
test: expand tests for file transfer
- Expanded test coverage for the file transfer feature - Refactored and improved code to address bugs identified through the new tests Signed-off-by: ArnelaL <arnela.lisic@secomind.com>
1 parent 1c47da0 commit 94b0dee

4 files changed

Lines changed: 633 additions & 67 deletions

File tree

backend/lib/edgehog/files/file/changes/handle_file_upload.ex

Lines changed: 55 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -71,16 +71,15 @@ defmodule Edgehog.Files.File.Changes.HandleFileUpload do
7171
tasks = Enum.map(uploads, fn {_encoding, task} -> task end)
7272
results = Task.yield_many(tasks, timeout)
7373

74-
{changeset, failed?} =
74+
{changeset, failed?, errors} =
7575
uploads
7676
|> Enum.zip(results)
77-
|> Enum.reduce({changeset, false}, &reduce_results/2)
77+
|> Enum.reduce({changeset, false, []}, &reduce_results/2)
7878

7979
if failed? do
80-
Ash.Changeset.add_error(changeset,
81-
field: :file,
82-
message: "One or more file uploads failed"
83-
)
80+
Enum.reduce(errors, changeset, fn error, acc ->
81+
Ash.Changeset.add_error(acc, error)
82+
end)
8483
else
8584
changeset
8685
end
@@ -161,36 +160,64 @@ defmodule Edgehog.Files.File.Changes.HandleFileUpload do
161160

162161
defp ceil_div(dividend, divisor), do: div(dividend + divisor - 1, divisor)
163162

164-
defp reduce_results({{_encoding, _task}, {_task_pid, {:ok, result}}}, {acc_changeset, failed?}) do
163+
defp reduce_results(
164+
{{_encoding, _task}, {_pid, {:ok, result}}},
165+
{acc, failed?, errors}
166+
) do
165167
case result do
166-
{:ok, changeset} -> {merge_changesets(acc_changeset, changeset), failed?}
167-
{:error, changeset} -> {merge_changesets(acc_changeset, changeset), true}
168+
{:ok, cs} ->
169+
{merge_changesets(acc, cs), failed?, errors}
170+
171+
{:error, cs} ->
172+
{merge_changesets(acc, cs), true, acc_errors(errors, cs)}
168173
end
169174
end
170175

171-
defp reduce_results({{encoding, task}, {_task_pid, nil}}, {acc_changeset, _failed?}) do
172-
Logger.error("Upload task timed out for #{encoding || "base"} encoding")
176+
defp reduce_results(
177+
{{encoding, task}, {_pid, nil}},
178+
{acc, _failed?, errors}
179+
) do
173180
_ = Task.shutdown(task, :brutal_kill)
174181

175-
{Ash.Changeset.add_error(acc_changeset,
176-
field: :file,
177-
message: "Upload process timed out"
178-
), true}
182+
cs =
183+
Ash.Changeset.add_error(acc,
184+
field: :file,
185+
message: "Upload process timed out (#{encoding || "base"})"
186+
)
187+
188+
{cs, true, acc_errors(errors, cs)}
179189
end
180190

181191
defp reduce_results(
182-
{{encoding, _task}, {_task_pid, {:exit, reason}}},
183-
{acc_changeset, _failed?}
192+
{{encoding, _task}, {_pid, {:exit, reason}}},
193+
{acc, _failed?, errors}
184194
) do
185-
Logger.error("Upload task crashed for #{encoding || "base"} encoding: #{inspect(reason)}")
195+
Logger.error("Upload task crashed for #{encoding || "base"}: #{inspect(reason)}")
196+
197+
error = %{
198+
encoding: encoding || :base,
199+
type: :crash,
200+
message: "Upload process crashed",
201+
reason: reason
202+
}
186203

187-
{Ash.Changeset.add_error(acc_changeset, field: :file, message: "Upload process failed"), true}
204+
{acc, true, [error | errors]}
188205
end
189206

190-
defp reduce_results({{encoding, _task}, {_task_result, reason}}, {acc_changeset, _failed?}) do
191-
Logger.error("Upload task failed for #{encoding || "base"} encoding: #{inspect(reason)}")
207+
defp reduce_results(
208+
{{encoding, _task}, {_result, reason}},
209+
{acc, _failed?, errors}
210+
) do
211+
Logger.error("Upload task failed for #{encoding || "base"}: #{inspect(reason)}")
212+
213+
error = %{
214+
encoding: encoding || :base,
215+
type: :error,
216+
message: "Upload process failed",
217+
reason: reason
218+
}
192219

193-
{Ash.Changeset.add_error(acc_changeset, field: :file, message: "Upload process failed"), true}
220+
{acc, true, [error | errors]}
194221
end
195222

196223
defp merge_changesets(acc_changeset, new_changeset) do
@@ -199,6 +226,10 @@ defmodule Edgehog.Files.File.Changes.HandleFileUpload do
199226
|> Map.put(:context, Map.merge(acc_changeset.context, new_changeset.context))
200227
end
201228

229+
defp acc_errors(errors, changeset) do
230+
changeset.errors ++ errors
231+
end
232+
202233
defp maybe_delete_temporary_upload(%Plug.Upload{path: original_path}, %Plug.Upload{path: path})
203234
when original_path != path do
204235
_ = File.rm(path)
@@ -210,13 +241,9 @@ defmodule Edgehog.Files.File.Changes.HandleFileUpload do
210241
# If we've uploaded the file and the transaction resulted in an error, we do our
211242
# best to clean up
212243
defp cleanup_on_error(changeset, {:error, _} = result) do
213-
case Ash.Changeset.apply_attributes(changeset) do
214-
{:ok, file_record} ->
215-
do_cleanup(file_record, changeset.context)
244+
file_record = Map.merge(changeset.data, changeset.attributes)
216245

217-
{:error, reason} ->
218-
Logger.warning("Failed to apply attributes for cleanup: #{inspect(reason)}")
219-
end
246+
do_cleanup(file_record, changeset.context)
220247

221248
result
222249
end

0 commit comments

Comments
 (0)