Mock adapter does not decode JSON about tesla HOT 14 CLOSED

@amatalai I see... I agree and off the top of my head can't think of a good way to support all cases without a lot of edge case conditions. So yes, perhaps raw body parameter should just go away for the mock adapter?

from tesla.

I'd go with

def json(data) do
  %Tesla.Env{status: 200, body: Posion.encode!(data), headers: %{"content-type" => "application/json"}}
end

Tesla.Mock.mock fn
  env -> json(["some", "data"])
end

As @amatalai wrote, I don't think we can find a bullet-proof solution to handle any data structure as body since adapters always return a very specific ones.

from tesla.

from tesla.

already decoded body*

from tesla.

I see! Great :) Note that I did try to do that prior to opening this issue and ran into an issue:

...
 setup do
    Tesla.Mock.mock fn
      %{method: :get, url: "http://example.com/json"} ->
        %Tesla.Env{status: 200, body: [%{}]}
    end
...

Result:

  1) test mock adapter calls middleware to decode json (MyAppTest)
     test/my_app_test.exs:13
  ** (ArgumentError) argument error
     code: assert %Tesla.Env{} = env = MyApp.get("/json")
     stacktrace:
       :erlang.iolist_to_binary([%{}])
       (tesla) lib/tesla/middleware/core.ex:31: Tesla.Middleware.Normalize.normalize_body/1
       (elixir) lib/map.ex:727: Map.update!/3
       (tesla) lib/tesla/middleware/json.ex:50: Tesla.Middleware.JSON.call/3
       test/my_app_test.exs:14: (test)

I think something like this in middleware/core.ex would solve the issue:

iex(16)> defmodule Test do
...(16)> def foo(data) when is_map(hd(data)), do: IO.puts "it's a list of maps"
...(16)> def foo(data) when is_list(data), do: IO.puts "default to io list"
...(16)> end
{:module, Test,
 <<70, 79, 82, 49, 0, 0, 4, 100, 66, 69, 65, 77, 65, 116, 85, 56, 0, 0, 0, 99,
   0, 0, 0, 11, 11, 69, 108, 105, 120, 105, 114, 46, 84, 101, 115, 116, 8, 95,
   95, 105, 110, 102, 111, 95, 95, 9, 102, ...>>, {:foo, 1}}
iex(17)> list_of_maps = [%{}, %{}, %{}]
[%{}, %{}, %{}]
iex(18)> list_of_str = ["Hello", ", ", "world"]
["Hello", ", ", "world"]
iex(19)> Test.foo
foo/1
iex(19)> Test.foo(list_of_str)
default to io list
:ok
iex(20)> Test.foo(list_of_maps)
it's a list of maps
:ok
iex(21)>

The above definitely could be better and should have more sanity checks before hitting the is_map() guard.

from tesla.

Better...

iex(8)> defmodule Test do
...(8)>    def foo(data) when is_list(data) do
...(8)>      case is_map(hd(data)) do
...(8)>        true -> "is map"
...(8)>        false -> "is not a map"
...(8)>      end
...(8)>    end
...(8)> end

{:module, Test,
 <<70, 79, 82, 49, 0, 0, 4, 20, 66, 69, 65, 77, 65, 116, 85, 56, 0, 0, 0, 84, 0,
   0, 0, 9, 11, 69, 108, 105, 120, 105, 114, 46, 84, 101, 115, 116, 8, 95, 95,
   105, 110, 102, 111, 95, 95, 9, 102, ...>>, {:foo, 1}}
iex(9)> Test.foo([""])
"is not a map"
iex(10)> Test.foo([%{}])
"is map"
iex(11)>

Unless of course, there's a good reason the code doesn't already handle this case. If not, then I'll submit a PR.

from tesla.

See #125

from tesla.

Hi

iex(8)> defmodule Test do
...(8)>    def foo(data) when is_list(data) do
...(8)>      case is_map(hd(data)) do
...(8)>        true -> "is map"
...(8)>        false -> "is not a map"
...(8)>      end
...(8)>    end
...(8)> end

This approach is invalid for ["foo", %{"bar" => "baz"}], which has valid json representation
"[\"foo\", {\"bar\": \"baz\"}]"

It could be done as case Enum.any?(data, &is_map/1) do, but there's one more edge case
[97, 115, 100, 102]
is this a four integers list after decoding or 'asdf' before charlist => binary conversion?

Personally I would prefer this approach

Tesla.Mock.mock fn
  %{method: :get, url: "http://example.com/json"} ->
    %Tesla.Env{status: 200, body: '[{}]', headers: %{"content-type" => "application/json"}}
  end

as it doesn't need any change in codebase to make mock work and is closer to how it works in reality

from tesla.

@amatalai See #125 , all tests passing.

from tesla.

@starbelly yes, for one specific case. I meant that it should work for all possible cases.

Use those and it will fail:

• ["foo", %{"bar" => "baz"}]
• [97, 115, 100, 102]

Both are valid decoded jsons. In second example it's not even possible to distinguish if it's charlist from adapter or decoded json passed to mock

from tesla.

@amatalai Unless you know of a particular resolution to this problem, I'm gonna go ahead and close my PR in favor of doing away with this feature for the mock adapter.

from tesla.

At the moment I don't see any solution that will be universal.
As stated before I would prefer to use something like this:

json = [...] |> Poison.encode!
Tesla.Mock.mock fn
  %{method: :get, url: "http://example.com/json"} ->
    %Tesla.Env{status: 200, body: json, headers: %{"content-type" => "application/json"}}
  end

from tesla.

Closing PR #125

from tesla.

Will close this issue in favor of new issue.

from tesla.