Adding Multipart Parser (WIP)

[pulkit4tech]

This PR contains code to add Multipart parser into reel
Progress:

• Currently using dans multipart-parser gem as parsing engine (although going through it)
• Configuring Parser into Reel (WIP)
  /cc: @kenichi @digitalextremist

[pulkit4tech]

@kenichi @digitalextremist I have added more test related to multipart, Kindly review 😃

[kenichi]

looking good so far. i have a couple notes/questions:

• should a developer just be checking for request.multipart.nil? in a handler or is #multipart? intended to be an exposed API? i immediately thought it was a way to see if this request had a multipart component to it, then realized it took a body parameter, and thought "Oh, maybe this is private?"
• once a developer has the hash returned by #multipart, should the tempfile instance in each :data key be closed? i think it should remain open, but be rewound, so it can be read. once it drops out of scope, on GC, the VM should close it - but if you want to be sure maybe we put a close after respond? what does rack do... hmm
• also, in that hash, :on_complete -> :complete 😺

i think that is it. i tried adding some more tests, refactoring some bits, but i immediately ran into the problem of rspec not being able to get at the exception. then i noticed this test pattern is copied from the server tests, which makes me wonder if those don't print meaningful messages/backtraces when they error either?

good work on your part all around though! 🎉

[pulkit4tech]

@kenichi : I have applied suggestion you mentioned, kindly have a look and review 😃 . So shall we explicitly close tempfile at response ?
/cc : @digitalextremist

[pulkit4tech]

@kenichi I have updated multipart_spec pattern here, kindly review. So shall I update/merge spec pattern as per PR ? /cc: @digitalextremist

[pulkit4tech]

@kenichi I have refactored multipart_spec as per suggestion, Kindly review 😃 /cc: @digitalextremist

[kenichi]

this is looking great, @pulkit4tech. i think we can jam on the tests a bit more to dry them up a little, i have some examples on my laptop at home. also, i noticed, when testing both sessions and multipart with angelo, the gem multipart_parser is not loaded. i think this should be added as a runtime dependency to the gemspec. i noticed you added it to the Gemfile, which is great for development, but the gemspec line in there will pick it up if it's in the reel.gemspec file. also, i see that you're using your fork of the gem, which is forked from @digitalextremist's, where he added an accessor i think. since this accessor isn't used in the implementation, i think we should add the main gem. thoughts?

[kenichi]

i'm having issues testing from curl with angelo (might be something i'm doing in angelo code):

require 'reel/request/multipart'
require 'angelo'

class Multi < Angelo::Base
  report_errors!
  post '/' do
    if request.multipart?
      puts "received mulitpart file"
    end
  end

end

Multi.run! unless $0 == 'irb'

$ curl -vv -F file=@test/test_app_root/public/what.png http://localhost:4567/
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 4567 (#0)
> POST / HTTP/1.1
> Host: localhost:4567
> User-Agent: curl/7.43.0
> Accept: */*
> Content-Length: 187831
> Expect: 100-continue
> Content-Type: multipart/form-data; boundary=------------------------704e8d134b054060
> 
< HTTP/1.1 200 OK
< Content-Type: text/html
< Set-Cookie: reel_sessions_default=7a3efee6-fd26-43de-8ca3-000000010004; Expires=Thu, 18 Aug 2016 23:24:37 -0000; Path=/; HttpOnly
< Connection: Keep-Alive
< Content-Length: 0
< 
* Connection #0 to host localhost left intact

$ be example/multipart.rb 
I, [2016-08-18T10:24:21.070091 #38886]  INFO -- : Celluloid 0.17.3 is running in BACKPORTED mode. [ http://git.io/vJf3J ]
I, [2016-08-18T10:24:21.350532 #38886]  INFO -- : Angelo 0.5.1
I, [2016-08-18T10:24:21.350580 #38886]  INFO -- : listening on 127.0.0.1:4567
received mulitpart file: 
I, [2016-08-18T10:24:37.157697 #38886]  INFO -- : 127.0.0.1 - - "POST / HTTP/1.1" 200 0
received mulitpart file: 
I, [2016-08-18T10:24:37.158530 #38886]  INFO -- : 127.0.0.1 - - "POST / HTTP/1.1" 200 0
received mulitpart file: 
I, [2016-08-18T10:24:37.159606 #38886]  INFO -- : unknown - - "POST / HTTP/1.1" 200 0
E, [2016-08-18T10:24:37.160332 #38886] ERROR -- : Actor crashed!
ArgumentError: Reel::Request::StateMachine can't change state from 'closed' to 'headers', only to: 
        /Users/knakamura/.gem/ruby/2.3.1/gems/celluloid-fsm-0.20.5/lib/celluloid/fsm.rb:114:in `validate_and_sanitize_new_state'
        /Users/knakamura/.gem/ruby/2.3.1/gems/celluloid-fsm-0.20.5/lib/celluloid/fsm.rb:90:in `transition'
        /Users/knakamura/src/ruby/reel/lib/reel/connection.rb:59:in `request'
        /Users/knakamura/src/ruby/reel/lib/reel/connection.rb:73:in `each_request'
        /Users/knakamura/src/ruby/angelo/lib/angelo/server.rb:27:in `on_connection'
        /Users/knakamura/src/ruby/reel/lib/reel/server.rb:50:in `handle_connection'
        /Users/knakamura/.gem/ruby/2.3.1/gems/celluloid-0.17.3/lib/celluloid/calls.rb:28:in `public_send'
        /Users/knakamura/.gem/ruby/2.3.1/gems/celluloid-0.17.3/lib/celluloid/calls.rb:28:in `dispatch'
        /Users/knakamura/.gem/ruby/2.3.1/gems/celluloid-0.17.3/lib/celluloid/call/async.rb:7:in `dispatch'
        /Users/knakamura/.gem/ruby/2.3.1/gems/celluloid-0.17.3/lib/celluloid/cell.rb:50:in `block in dispatch'
        /Users/knakamura/.gem/ruby/2.3.1/gems/celluloid-0.17.3/lib/celluloid/cell.rb:76:in `block in task'
        /Users/knakamura/.gem/ruby/2.3.1/gems/celluloid-0.17.3/lib/celluloid/actor.rb:339:in `block in task'
        /Users/knakamura/.gem/ruby/2.3.1/gems/celluloid-0.17.3/lib/celluloid/task.rb:44:in `block in initialize'
        /Users/knakamura/.gem/ruby/2.3.1/gems/celluloid-0.17.3/lib/celluloid/task/fibered.rb:14:in `block in create'

[pulkit4tech]

Yeah @kenichi this was the issue I am also facing as mentioned previously. Same header error occurs for POST request in basic example not involving multipart . Something like this here celluloid#229

[pulkit4tech]

@kenichi Shall I merge this PR to include multipart code into gsoc16 as now State Error is mostly cleared? 😃

[pulkit4tech]

Merging as per discussion 😃