Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Regression: PATCH behavior along with Header version matching #1355

Closed
dblock opened this issue Apr 7, 2016 · 3 comments
Closed

Regression: PATCH behavior along with Header version matching #1355

dblock opened this issue Apr 7, 2016 · 3 comments
Assignees
@namusyaka
Labels
confirmed bug

Comments

@dblock
Copy link
Member

dblock commented Apr 7, 2016 

require 'spec_helper'

describe Grape::API::Helpers do
  module PatchHelpersSpec
    class Patch_Public < Grape::API
      format :json
      version 'public-v1', using: :header, vendor: 'grape'

      get do
        { ok: 'public' }
      end
    end

    module AuthMethods
      def authenticate!

      end
    end

    class Patch_Private < Grape::API
      format :json
      version 'private-v1', using: :header, vendor: 'grape'

      helpers AuthMethods

      before do
        authenticate!
      end

      get do
        { ok: 'private' }
      end
    end

    class Main < Grape::API
      mount Patch_Public
      mount Patch_Private
    end
  end

  def app
    PatchHelpersSpec::Main
  end

  context 'default' do
    it 'public' do
      get '/', {}, 'HTTP_ACCEPT' => 'application/vnd.grape-public-v1+json'
      expect(last_response.status).to eq 200
      expect(last_response.body).to eq({ ok: 'public' }.to_json)
    end

    it 'private' do
      get '/', {}, 'HTTP_ACCEPT' => 'application/vnd.grape-private-v1+json'
      expect(last_response.status).to eq 200
      expect(last_response.body).to eq({ ok: 'private' }.to_json)
    end

    it 'default' do
      get '/'
      expect(last_response.status).to eq 200
      expect(last_response.body).to eq({ ok: 'public' }.to_json)
    end
  end

  context 'patch' do
    it 'public' do
      patch '/', {}, 'HTTP_ACCEPT' => 'application/vnd.grape-public-v1+json'
      expect(last_response.status).to eq 405
    end

    it 'private' do
      patch '/', {}, 'HTTP_ACCEPT' => 'application/vnd.grape-private-v1+json'
      expect(last_response.status).to eq 405
    end

    it 'default' do
      patch '/'
      expect(last_response.status).to eq 405
    end
  end
end

This passes on 0.15.0.

In 0.16.1 this fails in interesting ways.

~/source/grape/grape/dblock (patch-method-helpers)$ rspec spec/grape/api/patch_method_helpers_spec.rb 

Grape::API::Helpers
  default
    public
    private
    default
  patch
    public (FAILED - 1)
    private
    default

Failures:

  1) Grape::API::Helpers patch public
     Failure/Error: expect(last_response.status).to eq 405

       expected: 405
            got: 406

       (compared using ==)
     # ./spec/grape/api/patch_method_helpers_spec.rb:68:in `block (3 levels) in <top (required)>'

Finished in 0.05966 seconds (files took 0.34104 seconds to load)
6 examples, 1 failure

Failed examples:

rspec ./spec/grape/api/patch_method_helpers_spec.rb:66 # Grape::API::Helpers patch public

And

~/source/grape/grape/dblock (patch-method-helpers)$ rspec spec/grape/api/patch_method_helpers_spec.rb:65
Run options: include {:locations=>{"./spec/grape/api/patch_method_helpers_spec.rb"=>[65]}}

Grape::API::Helpers
  patch
    public (FAILED - 1)
    private (FAILED - 2)
    default (FAILED - 3)

Failures:

  1) Grape::API::Helpers patch public
     Failure/Error: authenticate!

     NoMethodError:
       undefined method `authenticate!' for #<#<Class:0x007fe599f99c38>:0x007fe59997fed8>
     # ./spec/grape/api/patch_method_helpers_spec.rb:27:in `block in <class:Patch_Private>'
     # ./lib/grape/endpoint.rb:334:in `instance_eval'
     # ./lib/grape/endpoint.rb:334:in `block (2 levels) in run_filters'
     # ./lib/grape/endpoint.rb:334:in `each'
     # ./lib/grape/endpoint.rb:334:in `block in run_filters'
     # /Users/dblock/.rvm/gems/ruby-2.3.0/gems/activesupport-4.2.6/lib/active_support/notifications.rb:166:in `instrument'
     # ./lib/grape/endpoint.rb:333:in `run_filters'
     # ./lib/grape/router.rb:127:in `block in method_not_allowed'
     # ./lib/grape/router.rb:126:in `instance_eval'
     # ./lib/grape/router.rb:126:in `method_not_allowed'
     # ./lib/grape/router.rb:87:in `transaction'
     # ./lib/grape/router.rb:57:in `identity'
     # ./lib/grape/router.rb:44:in `block in call'
     # ./lib/grape/router.rb:104:in `with_optimization'
     # ./lib/grape/router.rb:43:in `call'
     # ./lib/grape/api.rb:112:in `call'
     # ./lib/grape/api.rb:45:in `call!'
     # ./lib/grape/api.rb:40:in `call'
     # /Users/dblock/.rvm/gems/ruby-2.3.0/gems/rack-test-0.6.3/lib/rack/mock_session.rb:30:in `request'
     # /Users/dblock/.rvm/gems/ruby-2.3.0/gems/rack-test-0.6.3/lib/rack/test.rb:244:in `process_request'
     # /Users/dblock/.rvm/gems/ruby-2.3.0/gems/rack-test-0.6.3/lib/rack/test.rb:85:in `patch'
     # ./spec/grape/api/patch_method_helpers_spec.rb:67:in `block (3 levels) in <top (required)>'

  2) Grape::API::Helpers patch private
     Failure/Error: authenticate!

     NoMethodError:
       undefined method `authenticate!' for #<#<Class:0x007fe599f99c38>:0x007fe59d09a030>
     # ./spec/grape/api/patch_method_helpers_spec.rb:27:in `block in <class:Patch_Private>'
     # ./lib/grape/endpoint.rb:334:in `instance_eval'
     # ./lib/grape/endpoint.rb:334:in `block (2 levels) in run_filters'
     # ./lib/grape/endpoint.rb:334:in `each'
     # ./lib/grape/endpoint.rb:334:in `block in run_filters'
     # /Users/dblock/.rvm/gems/ruby-2.3.0/gems/activesupport-4.2.6/lib/active_support/notifications.rb:166:in `instrument'
     # ./lib/grape/endpoint.rb:333:in `run_filters'
     # ./lib/grape/router.rb:127:in `block in method_not_allowed'
     # ./lib/grape/router.rb:126:in `instance_eval'
     # ./lib/grape/router.rb:126:in `method_not_allowed'
     # ./lib/grape/router.rb:87:in `transaction'
     # ./lib/grape/router.rb:57:in `identity'
     # ./lib/grape/router.rb:44:in `block in call'
     # ./lib/grape/router.rb:104:in `with_optimization'
     # ./lib/grape/router.rb:43:in `call'
     # ./lib/grape/api.rb:112:in `call'
     # ./lib/grape/api.rb:45:in `call!'
     # ./lib/grape/api.rb:40:in `call'
     # /Users/dblock/.rvm/gems/ruby-2.3.0/gems/rack-test-0.6.3/lib/rack/mock_session.rb:30:in `request'
     # /Users/dblock/.rvm/gems/ruby-2.3.0/gems/rack-test-0.6.3/lib/rack/test.rb:244:in `process_request'
     # /Users/dblock/.rvm/gems/ruby-2.3.0/gems/rack-test-0.6.3/lib/rack/test.rb:85:in `patch'
     # ./spec/grape/api/patch_method_helpers_spec.rb:72:in `block (3 levels) in <top (required)>'

  3) Grape::API::Helpers patch default
     Failure/Error: authenticate!

     NoMethodError:
       undefined method `authenticate!' for #<#<Class:0x007fe599f99c38>:0x007fe59d0a2eb0>
     # ./spec/grape/api/patch_method_helpers_spec.rb:27:in `block in <class:Patch_Private>'
     # ./lib/grape/endpoint.rb:334:in `instance_eval'
     # ./lib/grape/endpoint.rb:334:in `block (2 levels) in run_filters'
     # ./lib/grape/endpoint.rb:334:in `each'
     # ./lib/grape/endpoint.rb:334:in `block in run_filters'
     # /Users/dblock/.rvm/gems/ruby-2.3.0/gems/activesupport-4.2.6/lib/active_support/notifications.rb:166:in `instrument'
     # ./lib/grape/endpoint.rb:333:in `run_filters'
     # ./lib/grape/router.rb:127:in `block in method_not_allowed'
     # ./lib/grape/router.rb:126:in `instance_eval'
     # ./lib/grape/router.rb:126:in `method_not_allowed'
     # ./lib/grape/router.rb:87:in `transaction'
     # ./lib/grape/router.rb:57:in `identity'
     # ./lib/grape/router.rb:44:in `block in call'
     # ./lib/grape/router.rb:104:in `with_optimization'
     # ./lib/grape/router.rb:43:in `call'
     # ./lib/grape/api.rb:112:in `call'
     # ./lib/grape/api.rb:45:in `call!'
     # ./lib/grape/api.rb:40:in `call'
     # /Users/dblock/.rvm/gems/ruby-2.3.0/gems/rack-test-0.6.3/lib/rack/mock_session.rb:30:in `request'
     # /Users/dblock/.rvm/gems/ruby-2.3.0/gems/rack-test-0.6.3/lib/rack/test.rb:244:in `process_request'
     # /Users/dblock/.rvm/gems/ruby-2.3.0/gems/rack-test-0.6.3/lib/rack/test.rb:85:in `patch'
     # ./spec/grape/api/patch_method_helpers_spec.rb:77:in `block (3 levels) in <top (required)>'

Finished in 0.0118 seconds (files took 0.33298 seconds to load)
3 examples, 3 failures

Failed examples:

rspec ./spec/grape/api/patch_method_helpers_spec.rb:66 # Grape::API::Helpers patch public
rspec ./spec/grape/api/patch_method_helpers_spec.rb:71 # Grape::API::Helpers patch private
rspec ./spec/grape/api/patch_method_helpers_spec.rb:76 # Grape::API::Helpers patch default
dblock added a commit to dblock/grape that referenced this issue Apr 7, 2016
@dblock
Repro for ruby-grape#1355.
fe02395
@dblock
Copy link
Member Author

dblock commented Apr 7, 2016

I could use some help from @namusyaka here ;(

@dblock dblock changed the title Regression: PATCH behavior along with Header version matching Regression: PATCH (?) behavior along with Header version matching Apr 7, 2016
@namusyaka namusyaka self-assigned this Apr 7, 2016
@dblock dblock mentioned this issue Apr 11, 2016
Closed
namusyaka added a commit that referenced this issue Apr 11, 2016
@namusyaka
Return 405 even if version is using as header
548c112 
Fixes #1355
namusyaka added a commit that referenced this issue Apr 11, 2016
@namusyaka
Return 405 even if version is using as header
19d785a 
Fixes #1355
namusyaka added a commit that referenced this issue Apr 11, 2016
@namusyaka
Return 405 even if version is using as header
c1baeb9 
Fixes #1355
@namusyaka namusyaka mentioned this issue Apr 11, 2016
Merged
@dblock dblock closed this as completed in a0166e7 Apr 11, 2016
@dblock dblock reopened this Apr 11, 2016
@dblock
Copy link
Member Author

dblock commented Apr 11, 2016

I see what you mean @namusyaka, the undefined method problem hasn't been fixed.

Run https://github.com/dblock/grape/blob/patch-method-helpers/spec/grape/api/patch_method_helpers_spec.rb.

~/source/grape/grape/dblock (patch-method-helpers)$ rspec spec/grape/api/patch_method_helpers_spec.rb
Run options: include {:locations=>{"./spec/grape/api/patch_method_helpers_spec.rb"=>[65]}}

Grape::API::Helpers
  patch
    public (FAILED - 1)

Failures:

  1) Grape::API::Helpers patch public
     Failure/Error: authenticate!

     NoMethodError:
       undefined method `authenticate!' for #<#<Class:0x007f80bdd3e3a8>:0x007f80bd25f450>
     # ./spec/grape/api/patch_method_helpers_spec.rb:26:in `block in <class:PatchPrivate>'
     # ./lib/grape/endpoint.rb:334:in `instance_eval'
     # ./lib/grape/endpoint.rb:334:in `block (2 levels) in run_filters'
     # ./lib/grape/endpoint.rb:334:in `each'
     # ./lib/grape/endpoint.rb:334:in `block in run_filters'
     # /Users/dblock/.rvm/gems/ruby-2.2.1/gems/activesupport-4.2.6/lib/active_support/notifications.rb:166:in `instrument'
     # ./lib/grape/endpoint.rb:333:in `run_filters'
     # ./lib/grape/router.rb:132:in `block in method_not_allowed'
     # ./lib/grape/router.rb:129:in `instance_eval'
     # ./lib/grape/router.rb:129:in `method_not_allowed'
     # ./lib/grape/router.rb:90:in `transaction'
     # ./lib/grape/router.rb:59:in `identity'
     # ./lib/grape/router.rb:44:in `block in call'
     # ./lib/grape/router.rb:107:in `with_optimization'
     # ./lib/grape/router.rb:43:in `call'
     # ./lib/grape/api.rb:112:in `call'
     # ./lib/grape/api.rb:45:in `call!'
     # ./lib/grape/api.rb:40:in `call'
     # /Users/dblock/.rvm/gems/ruby-2.2.1/gems/rack-test-0.6.3/lib/rack/mock_session.rb:30:in `request'
     # /Users/dblock/.rvm/gems/ruby-2.2.1/gems/rack-test-0.6.3/lib/rack/test.rb:244:in `process_request'
     # /Users/dblock/.rvm/gems/ruby-2.2.1/gems/rack-test-0.6.3/lib/rack/test.rb:85:in `patch'
     # ./spec/grape/api/patch_method_helpers_spec.rb:66:in `block (3 levels) in <top (required)>'

Finished in 0.01835 seconds (files took 0.35786 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/grape/api/patch_method_helpers_spec.rb:65 # Grape::API::Helpers patch public

dblock added a commit to dblock/grape that referenced this issue Apr 11, 2016
@dblock
Repro for ruby-grape#1355.
e8143ab
dblock added a commit to dblock/grape that referenced this issue Apr 11, 2016
@dblock
Repro for ruby-grape#1355.
c1d0464
@dblock dblock mentioned this issue Apr 11, 2016
Closed
@dblock dblock changed the title Regression: PATCH (?) behavior along with Header version matching Regression: PATCH behavior along with Header version matching Apr 11, 2016
namusyaka pushed a commit that referenced this issue Apr 11, 2016
@dblock @namusyaka
Repro for #1355.
4a8dbbc
@namusyaka
Copy link
Contributor

Closed via #1362

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@namusyaka namusyaka

Labels
confirmed bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@namusyaka @dblock