Nested mount points don't inherit parent declared params

[plukevdh]

Not sure if this is documented elsewhere, but I would expect the following to work:

class Another < Grape::API
  namespace :another do
    params do
      required :my_id, type: Integer
    end
    route_param :my_id do
    get do
      declared(params)
    end
  end
end

class Something < Grape::API
  format :json
  namespace :something do
    params do
       requires :id
    end
    route_param :id do
      mount Another
    end
  end
end

And for the request get /something/123/another/456

to return { id: 123, my_id: 456 }, but I only get { my_id: 456 }

I have attached a failing test case to this PR to demonstrate the issue, though I am unsure as of yet how to go about fixing.

[dblock]

If someone is going to fix this, lets carefully document what is being inherited when mounting an endpoint under some scope.

[rnubel]

So, technically this hack makes the fixed spec pass. However, I'm pretty sure it's not the right way to do this. I'm having trouble untangling how settings get passed and inherited and stacked during the API definition process. However, I can explain why both lines are necessary:

First, inheritable_setting.route on the mounted endpoint isn't updated simply because route is only populated from the parent upon the initial inherit_from call and isn't updated when its parent's value is like namespace_stackable would be. So, this hack forces the parent's new parent (step-grandfather?)'s values to be propagated downward. It would be possible to make route updates be re-propagated downward whenever a parent changes, but I'm not sure if that's desired or not.

As for the validations line, it's necessary because validations on each endpoint are only set during initialization, which isn't redone when an API is mounted. This might be solvable by having validations be re-pulled from namespace_stackable(:validations) at the time of endpoint#prepare_routes (which happens after mounting).

Anyway, maybe someone with more experience in the internals of Grape can weigh in here.

[dblock]

@rnubel Thanks! With only a cursory look it's actually possible that this is a viable solution, but I think a useful exercise would be to extend the specs to something that potentially can break.

[plukevdh]

Dunno how that path got messed up. Thanks @rnubel for the investigation!

[rnubel]

Okay, I've cleaned up the implementation a bit: master...rnubel:declared_and_nested_mount_missing_params

Neither of my suggestions from my original post actually worked out -- InheritableSetting#route is just a hash, and can't intelligently be merged with a mounting API's; for validations, we need to persist the state of the stacked validations at the time of endpoint creation.

However, the new Endpoint#inherit_settings method (in need of a better name) is fairly straightforward, and makes it pretty clear what's inherited from a mounting API. The recursion allows it to handle multiply-nested APIs (which I updated the test to prove), too. So maybe this is acceptable. I feel like there's a more elegant solution out there, but I don't know what it is.

[Arkanain]

Hi guys. Maybe it make sense to merge this pull request? Some of us really need this fix and it will be really cool to see it.

[plukevdh]

@Arkanain This PR only had a failing spec. There were some other branches that had fixes but I don't know their states. cc @rnubel

[plukevdh]

IIRC, @rnubel's fix did work for my case, but that was almost a year ago and the project that was utilizing this has not been under active development so I can't speak to this.

[Arkanain]

I will create new fresh pull request with this fixes because this issue is still available.

[Arkanain]

you can close this pull request because all needed changes is already in latest master.

[dblock]

Closed via #1422 that is.