Add support for multipart requests. #87

ijcd · 2017-06-18T11:18:53Z

I needed multipart support so I took a stab at it. Feedback welcome.

amatalai · 2017-06-22T19:03:40Z

README.md

+You can pass a Multipart struct as the body.
+
+Example:
+```ex


I think this example is missing alias Tesla.Multipart. It could be misleading for someone who read only a readme.

Thanks, I'll make the change.

Done. Thanks!

teamon · 2017-06-23T18:46:18Z

Whoo this is really cool!
I will give it a try after weekend, but the API looks pretty well-thought 👍

ijcd · 2017-07-06T15:39:15Z

I updated the docs with the suggestion. Are there any other changes you would like to see?

ijcd · 2017-08-17T21:18:03Z

Do you require any changes or can we get this merged?

amatalai · 2017-08-24T09:31:32Z

I'm aware that it's been a long time since the PR was created, but it's big one and therefore it requires a lot of free time to review it properly. I'll try over the weekend if other duties won't prevent me.

chingor13

This PR is great!

We are looking at improving Swagger Codegen for elixir clients and want to be able to support multipart file uploads with autogenerated clients.

One other issue is that this doesn't play nicely with the Tesla.Middleware.JSON middleware. Currently, it will try to serialize the Tesla.Multipart struct and fail on a {file, "name"} tuple if you added a file to your request.

I think we can tell the JSON middleware that a body that is a Multipart struct should not be encodable?.

chingor13 · 2017-09-01T16:49:01Z

lib/tesla/adapter/hackney.ex

@@ -19,11 +21,16 @@ if Code.ensure_loaded?(:hackney) do
    end
    defp request(method, url, headers, %Stream{} = body, opts), do: request_stream(method, url, headers, body, opts)
    defp request(method, url, headers, body, opts) when is_function(body), do: request_stream(method, url, headers, body, opts)
+    defp request(method, url, headers, %Multipart{} = mp, opts) do
+      headers = Keyword.merge(headers, Multipart.headers(mp))


headers may not always be a keyword list. When creating an API client, you can add a middleware that provides headers like:

plug Tesla.Middleware.Headers, %{"Authorization" => "token xyz"}

This is translated to a list of 2-tuples where the first element is a string (so it's not a keyword list).

This example yields:

(ArgumentError) expected a keyword list as the first argument, got: [{'authorization', 'token xyz'}]

chingor13 · 2017-09-01T16:54:22Z

lib/tesla/adapter/httpc.ex

@@ -40,6 +41,15 @@ defmodule Tesla.Adapter.Httpc do
    :httpc.request(method, {url, headers}, http_opts, opts)
  end

+  defp request(method, url, headers, _content_type, %Multipart{} = mp, opts) do
+    headers = Keyword.merge(headers, Multipart.headers(mp))


Same here about headers necessarily being a keyword list.

chingor13 · 2017-09-01T16:54:30Z

lib/tesla/adapter/ibrowse.ex

@@ -21,6 +22,13 @@ if Code.ensure_loaded?(:ibrowse) do
      )
    end

+    defp request(url, headers, method, %Multipart{} = mp, opts) do
+      headers = Keyword.merge(headers, Multipart.headers(mp))


Same here about headers necessarily being a keyword list.

Multipart fixes

amatalai · 2017-09-10T20:59:04Z

I think that list of possible opts should be added to docs for add_field and add_file.

Also found 2 issues:

it doesn't work with DebugLogger middleware due to missing pattern match for %Multipart{}
when passed invalid path to add_file and then I tried to send request everything hangs, maybe it should check if file exist first

Anyway big 👍 from me

@teamon what do you think about merging this and working on further improvements in separate PRs?

amatalai · 2017-09-10T21:36:33Z

got green light for merge 💚
I'll do it tomorrow. Also, I will copy thing from my previous post to issues to not forget about them

teamon · 2017-09-11T14:04:12Z

There is a to :mimerl which is not added to dependencies.

warning: function :mimerl.filename/1 is undefined (module :mimerl is not available)
  lib/tesla/multipart.ex:92

@ijcd Have you tried https://hex.pm/packages/mime ?

ijcd · 2017-09-12T04:00:37Z

I don't recall if I tried that one. It looks like :mimerl came in with hackney for me, possibly, since benoitc works on both. It's probably better to go with the Elixir one as it looks more recently updated and maintained. It's only used for guessing the mime-type on upload.

fdbeirao · 2018-01-24T17:03:03Z

I reached this post because I am trying to use elixir-google-api and I was messing around with it in the repl/iex.

This is what happened to me:

when passed invalid path to add_file and then I tried to send request everything hangs, maybe it should check if file exist first

How is it possible to completely halt the repl? Why isn't it "just crashing"?

I now see what I am doing wrong: I am setting "Hello from Elixir!" as the "path".. but it blows my mind that the repl can hang like that, as if it were stuck in some "low-level" receive() method.. :( if I was running this code inside an Elixir process, would the process also just hang (in there)?

I am running in Windows and these were the commands I run:

{:ok, token} = Goth.Token.for_scope("https://www.googleapis.com/auth/cloud-platform")
conn = GoogleApi.Storage.V1.Connection.new(token.token)
GoogleApi.Storage.V1.Api.Objects.storage_objects_insert_simple(conn, "--my--bucket--id--", "multipart", %GoogleApi.Storage.V1.Model.Object{:"contentType" => "plain/text", :"name" => "upload.txt"}, "Hello from Elixir!", [])

My intention was to "upload" a file called upload.txt, with content Hello from Elixir!.. I will now try to find a way to do that. Preferably one that doesn't require writing to temporary files :)

Cheers.

teamon · 2018-01-24T17:27:37Z

Thanks for letting me know elixir-google-api uses tesla! 🎉

You don't need to create a tmp file - you can use add_file_content

About the hanging - can you check with raw tesla? Does it hang at add_file or when perfrming the request?

fdbeirao · 2018-01-25T09:58:56Z

@teamon your suggestion was spot on! I had to locally create a function that uses add_file_content behind the scenes, because the original elixir-google-api doesn't support that function.

About the freezing thing, it must be something wrong with my setup (I'm looking at you Windows!).

If I run iex -S mix followed by all the Elixir commands that lead to a freeze, and then I Ctrl+C to kill it and do it all over again, the second time it works fine 🤦‍♂️

I have access to a linux VM, so I'll try too see if it also hangs in there (in there).

Massive kudos for your hint 👍

teamon · 2018-01-25T10:03:55Z

Happy to help! 🕺

In case you still have some problems please open a separate issue, good luck!

henry-hz · 2018-04-18T14:15:32Z

@fdbeirao Fabio, pls, how can you uplaod a file into a bucket folder (in case you need to upload to /my-bucket/my-folder/file.txt) ?

fdbeirao · 2018-04-18T20:57:43Z

@henry-hz Henry, in order to place the file in a "folder" I specify the file_name as the full path with folder, separated by /. Do keep in mind that folders are a lie.

This file_name gets added to the query params:

defp storage_objects_insert_from_payload(
         connection,
         bucket,
         content_type,
         file_name,
         file_payload
       ),
       do:
         %{}
         |> method(:post)
         |> url("/upload/storage/v1/b/#{bucket}/o")
         |> add_param(:headers, "content-type", content_type)
         |> add_param(:query, :uploadType, "media")
         |> add_param(:query, :name, file_name)
         |> add_param(:body, :body, file_payload)
         |> Enum.into([])
         |> (&Connection.request(connection, &1)).()
         |> decode(%GoogleApi.Storage.V1.Model.Object{})

Let me know if this helped you. I haven't touched that code for a while.

Add support for multipart requests.

6ebfdb9

amatalai reviewed Jun 22, 2017

View reviewed changes

teamon added the feature ✨ label Jun 23, 2017

Update docs with alias.

875049f

chingor13 reviewed Sep 1, 2017

View reviewed changes

chingor13 added 3 commits September 1, 2017 12:31

Skip json or form url encoding for multipart body payloads

04e3a61

Append headers to list as headers may not be a keyword list

efe4788

Add test for merging headers and skipping encoding multipart bodies

ec3849b

This was referenced Sep 1, 2017

Multipart fixes ijcd/tesla#1

Merged

File uploads not functional googleapis/elixir-google-api#1

Closed

Merge pull request #1 from chingor13/multipart-fixes

dc74aa5

Multipart fixes

teamon merged commit be52124 into elixir-tesla:master Sep 11, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for multipart requests. #87

Add support for multipart requests. #87

ijcd commented Jun 18, 2017

amatalai Jun 22, 2017

ijcd Jun 22, 2017

ijcd Jul 6, 2017

teamon commented Jun 23, 2017

ijcd commented Jul 6, 2017

ijcd commented Aug 17, 2017

amatalai commented Aug 24, 2017

chingor13 left a comment

chingor13 Sep 1, 2017

chingor13 Sep 1, 2017

chingor13 Sep 1, 2017

amatalai commented Sep 10, 2017

amatalai commented Sep 10, 2017

teamon commented Sep 11, 2017

ijcd commented Sep 12, 2017 •

edited

Loading

fdbeirao commented Jan 24, 2018

teamon commented Jan 24, 2018

fdbeirao commented Jan 25, 2018

teamon commented Jan 25, 2018

henry-hz commented Apr 18, 2018

fdbeirao commented Apr 18, 2018

Add support for multipart requests. #87

Add support for multipart requests. #87

Conversation

ijcd commented Jun 18, 2017

amatalai Jun 22, 2017

Choose a reason for hiding this comment

ijcd Jun 22, 2017

Choose a reason for hiding this comment

ijcd Jul 6, 2017

Choose a reason for hiding this comment

teamon commented Jun 23, 2017

ijcd commented Jul 6, 2017

ijcd commented Aug 17, 2017

amatalai commented Aug 24, 2017

chingor13 left a comment

Choose a reason for hiding this comment

chingor13 Sep 1, 2017

Choose a reason for hiding this comment

chingor13 Sep 1, 2017

Choose a reason for hiding this comment

chingor13 Sep 1, 2017

Choose a reason for hiding this comment

amatalai commented Sep 10, 2017

amatalai commented Sep 10, 2017

teamon commented Sep 11, 2017

ijcd commented Sep 12, 2017 • edited Loading

fdbeirao commented Jan 24, 2018

teamon commented Jan 24, 2018

fdbeirao commented Jan 25, 2018

teamon commented Jan 25, 2018

henry-hz commented Apr 18, 2018

fdbeirao commented Apr 18, 2018

ijcd commented Sep 12, 2017 •

edited

Loading