given and declared do not work together

[karhatsu]

I have the following params definition:

        module FooParams
          extend Grape::API::Helpers

          params :bar_params do
            requires :a, type: String
          end

          params :baz_params do
            requires :b, type: String
          end

          params :foo_params do
            requires :type, type: String
            given type: -> (val) { val == 'Bar' } do
              requires :foo, type: Hash do
                use :bar_params
              end
            end
            given type: -> (val) { val == 'Baz' } do
              requires :foo, type: Hash do
                use :baz_params
              end
            end
          end
        end

The foo_params are used in this post:

        resource :foos do
          helpers FooParams

          params do
            use :foo_params
          end

          post do
            if params[:type] == 'Bar'
              puts "original a: '#{params[:foo][:a]}'"
              puts "declared a: '#{declared(params)[:foo][:a]}'"
            end
            {}
          end
        end

Now when I call POST /foos with {type: 'Bar', {foo: {a: 'letter A'}}}, I get the following output:

original a: 'letter A'
declared a: ''

If I remove the latter given definition (val == 'Baz'), it prints:

original a: 'letter A'
declared a: 'letter A'

[karhatsu]

I am using 0.19.2.

[dblock]

Try to turn this into a spec, could be a bug.

[karhatsu]

I wrote a test for this but didn't get to the problem I mentioned above. Instead I'm getting 400 due missing second param that should not be required IMO. The failing test can be found here: https://github.com/karhatsu/grape/commit/51af2325cb29964d35e1d22414fcdae2717b1eae. Can you see what's wrong with my test?

[dblock]

Don't see anything wrong from the top of my head. Start by adding some code to see what's being parsed out of the JSON parser?

[dblock]

@darren987469 another one? :)

[kawamoto]

I tried the test karhatsu@51af232 with current master, it seems works fine as expected. it outputs

Failures:

  1) Grape::Endpoint#declared with conditional params accepts first conditional params
     Failure/Error: expect(json).to eq(type: 'A', conditional: { a: 'letter A' })

       expected: {:conditional=>{:a=>"letter A"}, :type=>"A"}
            got: {"conditional"=>{"b"=>nil}, "type"=>"A"}

       (compared using ==)

       Diff:
       @@ -1,3 +1,3 @@
       -:conditional => {:a=>"letter A"},
       -:type => "A",
       +"conditional" => {"b"=>nil},
       +"type" => "A",

     # ./spec/grape/endpoint_spec.rb:621:in `block (4 levels) in <top (required)>'

  2) Grape::Endpoint#declared with conditional params accepts second conditional params
     Failure/Error: expect(json).to eq(type: 'B', conditional: { b: 'letter B' })

       expected: {:conditional=>{:b=>"letter B"}, :type=>"B"}
            got: {"conditional"=>{"b"=>"letter B"}, "type"=>"B"}

       (compared using ==)

       Diff:
       @@ -1,3 +1,3 @@
       -:conditional => {:b=>"letter B"},
       -:type => "B",
       +"conditional" => {"b"=>"letter B"},
       +"type" => "B",

     # ./spec/grape/endpoint_spec.rb:627:in `block (4 levels) in <top (required)>'

here is my work around https://github.com/kawamoto/grape/commit/f1385783d3717e2f0986ffe359a1f7d7c3b67814 with merging declared param

it still has some problems like

1. it accepts both of params even if its condition is false
2. it will be broken when a type of param is difference between conditions

I think it's better to add checking dependent_on condition in #declared by somehow.