Skip to content

Commit

Permalink
fix: Removing mocks and adding rest of edge cases with empty files. #69
Browse files Browse the repository at this point in the history
  • Loading branch information
LuchoTurtle committed Jul 12, 2023
1 parent 4f97807 commit d43169a
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 33 deletions.
4 changes: 3 additions & 1 deletion lib/app/upload.ex
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,15 @@ defmodule App.Upload do
case File.read(image.path) do
# Create `CID` from file contents so filenames are unique
{:ok, file_binary} ->
file_cid = Cid.cid(file_binary)
contents = if byte_size(file_binary) == 0, do: [], else: file_binary
file_cid = Cid.cid(contents)

file_extension =
image.content_type
|> MIME.extensions()
|> List.first()


# Return the file's content CID and its MIME extension if valid.
# Otherwise, return error.
case {file_cid, file_extension} do
Expand Down
5 changes: 0 additions & 5 deletions lib/app_web/controllers/api_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ defmodule AppWeb.ApiController do
body: "Error uploading file. Failure creating the CID filename."
})

{:error, :invalid_extension} ->
render(conn |> put_status(400), %{
body: "Error uploading file. Failure parsing the file extension."
})

{:error, :invalid_extension_and_cid} ->
render(conn |> put_status(400), %{
body: "Error uploading file. The file extension and contents are invalid."
Expand Down
Binary file modified priv/static/images/empty.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added priv/static/images/phoenix.xyz
Binary file not shown.
18 changes: 12 additions & 6 deletions test/app/upload_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,8 @@ defmodule App.UploadTest do
filename: "empty.jpg",
path: [:code.priv_dir(:app), "static", "images", "empty.jpg"] |> Path.join()
}
# Even though the jpeg is *deliberately* empty the upload & CID still works!!
expected_response = %{
compressed_url: "https://s3.eu-west-3.amazonaws.com/imgup-compressed/zb2rhngHXWi8mR5YHX3Go4xDYpZqqcAtGefn8sktQMM7YzKEz.jpg",
url: "https://s3.eu-west-3.amazonaws.com/imgup-original/zb2rhngHXWi8mR5YHX3Go4xDYpZqqcAtGefn8sktQMM7YzKEz.jpg"
}
assert App.Upload.upload(image) == {:ok, expected_response}

assert App.Upload.upload(image) == {:error, :invalid_cid}
end

test "upload/1 an EMPTY file with no extension to test failure" do
Expand All @@ -72,6 +68,16 @@ defmodule App.UploadTest do
path: [:code.priv_dir(:app), "static", "images", "empty"] |> Path.join()
}

assert App.Upload.upload(image) == {:error, :invalid_extension_and_cid}
end

test "upload/1 a file with an invalid extension to test failure" do
image = %Plug.Upload{
content_type: "xyz",
filename: "phoenix.xyz",
path: [:code.priv_dir(:app), "static", "images", "phoenix.xyz"] |> Path.join()
}

assert App.Upload.upload(image) == {:error, :invalid_extension}
end
end
37 changes: 16 additions & 21 deletions test/app_web/api_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,21 @@ defmodule AppWeb.APITest do
# empty_file
@empty_file %{
"" => %Plug.Upload{
content_type: "image/something",
content_type: "image_something",
filename: "empty",
path: [:code.priv_dir(:app), "static", "images", "empty"] |> Path.join()
}
}

# empty image
@empty_image %{
"" => %Plug.Upload{
content_type: "image/jpeg",
filename: "empty.jpg",
path: [:code.priv_dir(:app), "static", "images", "empty.jpg"] |> Path.join()
}
}

test "upload succeeds (happy path)", %{conn: conn} do
conn = post(conn, ~p"/api/images", @create_attrs)

Expand Down Expand Up @@ -111,30 +120,16 @@ defmodule AppWeb.APITest do
conn = post(conn, ~p"/api/images", @empty_file)

assert Map.get(Jason.decode!(response(conn, 400)), "errors") == %{
"detail" => "Error uploading file. Failure parsing the file extension."
"detail" => "Error uploading file. The file extension and contents are invalid."
}
end

test "file with invalid binary data type and extension should return error.", %{conn: conn} do

with_mock Cid, [cid: fn(_input) -> "invalid data type" end] do
conn = post(conn, ~p"/api/images", @empty_file)

assert Map.get(Jason.decode!(response(conn, 400)), "errors") == %{
"detail" => "Error uploading file. The file extension and contents are invalid."
}
end
end

test "file with invalid binary data (cid) but valid content type should return error", %{conn: conn} do
test "empty image (meaning it has a valid content type) should return an error", %{conn: conn} do
conn = post(conn, ~p"/api/images", @empty_image)

with_mock Cid, [cid: fn(_input) -> "invalid data type" end] do
conn = post(conn, ~p"/api/images", @valid_image_attrs)

assert Map.get(Jason.decode!(response(conn, 400)), "errors") == %{
"detail" => "Error uploading file. Failure creating the CID filename."
}
end
assert Map.get(Jason.decode!(response(conn, 400)), "errors") == %{
"detail" => "Error uploading file. Failure creating the CID filename."
}
end

test "valid file but the upload to S3 failed. It should return an error.", %{conn: conn} do
Expand Down

0 comments on commit d43169a

Please sign in to comment.