After 1.5.0 , 'route_setting :custom , key: :value' is broken with 'mount'.

[neocoin]

env

Ruby 3.0.0
Grape 1.5.3
Rails 6.1

      def route_setting(key, value = nil)
        get_or_set :route, key, value
      end

'value' work like frozen hash key. You can reproduce below code.

module V1
  class Status < Grape::API
    desc 'time'
    route_setting :custom, key: :value
    get '/status/time' do
      { time: Time.zone.now }
    end

    desc 'time2'
    route_setting :custom, key: :value
    get '/status/time2' do
      { time: Time.zone.now }
    end
  end
  
  class Mount < Grape::API
    mount ::V1::Status
  end
end

Second , 'route_setting' method calling doesn't work.

[1] pry(main)> ap V1::Status.routes.map{ [_1.path, _1.settings]}
[
    [0] [
        [0] "/:version/status/time(.json)",
        [1] {
            :description => {
                :description => "time"
            },
                 :custom => {
                :key => :value
            }
        }
    ],
    [1] [
        [0] "/:version/status/time2(.json)",
        [1] {
            :description => {
                :description => "time2"
            }
        }
    ]
]
=> nil

Anybody, can explain this situation?

route_setting work fine at ~> 1.4.0 on ruby 2.7

[dblock]

Probably a bug. Turn it into a failing spec and PR that?

[neocoin]

I write spec and pr on #2174 .
But I don't know a solution yet.

[neocoin]

@dblock

I find problem point and solve with rollback that line. #2174

Fix retaining setup blocks when remounting APIs 416a7e1 Sep 18, 2020

416a7e1#diff-74e08e6432db25a22b629a2f8031d24d01f8c9435bda532d36e22971d0b1c9a0R33

I give up performance during init.
check plz @jylamont

[stephannv]

I was about to open an issue about it, I even created a file to test it.

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "grape", "1.5.3"
end

require "grape"

class BaseAPI < Grape::API
end

class SomeAPI < Grape::API
  route_setting :auth, public: true
  get '/a' do
  end

  route_setting :auth, public: true
  get '/b' do
  end
end

raise 'ERROR ON /a' if SomeAPI.routes.first.settings[:auth] != { public: true }
raise 'ERROR ON /b' if SomeAPI.routes.last.settings[:auth] != { public: true }

BaseAPI.mount SomeAPI

raise 'ERROR ON /a after mount' if SomeAPI.routes.first.settings[:auth] != { public: true }
raise 'ERROR ON /b after mount' if SomeAPI.routes.last.settings[:auth] != { public: true }

[madejejej]

I was about to open an issue about this as well...

I looked around the Grape code and I'm not sure how to actually fix the issue. However, I noticed that adding an empty namespace block actually fixes the issue. If someone wants to create a potential fix then perhaps answering why it works within a namespace might be a good starting point.

Slightly modified reproduction of @stephannv, with grape 1.6.2 and an empty namespace block works:

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "grape", "1.6.2"
end

require "grape"

class BaseAPI < Grape::API
end

class SomeAPI < Grape::API
  route_setting :auth, public: true
  namespace do
    get '/a' do
    end

    route_setting :auth, public: true
    get '/b' do
    end
  end
end

raise 'ERROR ON /a' if SomeAPI.routes.first.settings[:auth] != { public: true }
raise 'ERROR ON /b' if SomeAPI.routes.last.settings[:auth] != { public: true }

BaseAPI.mount SomeAPI

raise 'ERROR ON /a after mount' if SomeAPI.routes.first.settings[:auth] != { public: true }
raise 'ERROR ON /b after mount' if SomeAPI.routes.last.settings[:auth] != { public: true }

[Haerezis]

The reason wrapping the DSL in an namespace (even blank) is working is because the DSL calls are only saved as step when they are at the root level of an Grape::API class.

Once you put them in a namespace, they are not at root level, but in a block, and its this block that is "replayed"/re-called : the "setup steps" mechanism is skipped (except for the namespace call which is saved as the only "setup step" of the class, with a block to be re-called).

[Haerezis]

I really don't understand why a Set is used for @setup. I could understand the wish for performance optimization, but this is not the answer.

A Set is by definition NOT ordered. So there's a fundamental issue with using it : when setup steps are replayed, they are done so with a specific order in mind (an route description need to be called before route or get for example).
Of course the Set implementation doesn't shuffle things when items are added to it, so we still get an "ordered" list of items when we enumerate on it.

But you can't rely on this behavior, because Set explicitly says that it doesn't guaranty ordering.

We should rollback to an Array. I can put together a more fresh PR if there's any appetite for that.

[dblock]

@Haerezis you're onto something - probably Set was used because every setup was deemed unique and possibly overwritten every time it was re-declared. If you can implement a test that fails with a Set but passes with an Array without breaking existing tests, we would definitely merge that.

[Haerezis]

@dblock here's the PR : #2529

[dblock]

Closed via #2529.