From 79e1da722a2aa4c97698dafe898512f988fcef01 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Thu, 21 Aug 2025 15:28:37 +0200 Subject: [PATCH] `within_namespace` is part on our internal api and should be exposed publicly. Also remove `namespace_start` and `namespace_end` --- CHANGELOG.md | 1 + lib/grape/dsl/routing.rb | 2 +- lib/grape/dsl/settings.rb | 29 +++----- spec/grape/dsl/routing_spec.rb | 78 +++++----------------- spec/grape/dsl/settings_spec.rb | 114 +++++++++++--------------------- 5 files changed, 67 insertions(+), 157 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ffe12c87..ffb3467c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ * [#2588](https://github.com/ruby-grape/grape/pull/2588): Fix defaut format regression on */* - [@ericproulx](https://github.com/ericproulx). * [#2593](https://github.com/ruby-grape/grape/pull/2593): Fix warning message when overriding global registry key - [@ericproulx](https://github.com/ericproulx). * [#2594](https://github.com/ruby-grape/grape/pull/2594): Fix routes memoization - [@ericproulx](https://github.com/ericproulx). +* [#2595](https://github.com/ruby-grape/grape/pull/2595): Keep `within_namespace` as part of our internal api - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 2.4.0 (2025-06-18) diff --git a/lib/grape/dsl/routing.rb b/lib/grape/dsl/routing.rb index 1bf839e25..4e6839223 100644 --- a/lib/grape/dsl/routing.rb +++ b/lib/grape/dsl/routing.rb @@ -154,7 +154,7 @@ def route(methods, paths = ['/'], route_options = {}, &block) new_endpoint = Grape::Endpoint.new(inheritable_setting, endpoint_options, &block) endpoints << new_endpoint unless endpoints.any? { |e| e.equals?(new_endpoint) } - route_end + inheritable_setting.route_end reset_validations! end diff --git a/lib/grape/dsl/settings.rb b/lib/grape/dsl/settings.rb index 50f055933..438876c77 100644 --- a/lib/grape/dsl/settings.rb +++ b/lib/grape/dsl/settings.rb @@ -7,12 +7,8 @@ module DSL # matter where they're defined, and inheritable settings which apply only # in the current scope and scopes nested under it. module Settings - extend Forwardable - attr_writer :inheritable_setting, :top_level_setting - def_delegators :inheritable_setting, :route_end - # Fetch our top-level settings, which apply to all endpoints in the API. def top_level_setting @top_level_setting ||= Grape::Util::InheritableSetting.new.tap do |setting| @@ -102,27 +98,20 @@ def namespace_reverse_stackable_with_hash(key) end end - # Fork our inheritable settings to a new instance, copied from our - # parent's, but separate so we won't modify it. Every call to this - # method should have an answering call to #namespace_end. - def namespace_start - @inheritable_setting = Grape::Util::InheritableSetting.new.tap { |new_settings| new_settings.inherit_from inheritable_setting } - end - - # Set the inheritable settings pointer back up by one level. - def namespace_end - route_end - @inheritable_setting = inheritable_setting.parent - end + private # Execute the block within a context where our inheritable settings are forked # to a new copy (see #namespace_start). - def within_namespace(&block) - namespace_start + def within_namespace + new_inheritable_settings = Grape::Util::InheritableSetting.new + new_inheritable_settings.inherit_from inheritable_setting - result = yield if block + @inheritable_setting = new_inheritable_settings - namespace_end + result = yield + + inheritable_setting.route_end + @inheritable_setting = inheritable_setting.parent reset_validations! result diff --git a/spec/grape/dsl/routing_spec.rb b/spec/grape/dsl/routing_spec.rb index dcebb66e2..d3682cec8 100644 --- a/spec/grape/dsl/routing_spec.rb +++ b/spec/grape/dsl/routing_spec.rb @@ -6,62 +6,8 @@ let(:dummy_class) do Class.new do extend Grape::DSL::Routing - - def self.namespace_stackable(key, value = nil) - if value - namespace_stackable_hash[key] << value - else - namespace_stackable_hash[key] - end - end - - def self.namespace_stackable_with_hash(key, value = nil) - if value - namespace_stackable_with_hash_hash[key] = value - else - namespace_stackable_with_hash_hash[key] - end - end - - def self.namespace_inheritable(key, value = nil) - if value - namespace_inheritable_hash[key] = value - else - namespace_inheritable_hash[key] - end - end - - def self.route_setting(key, value = nil) - if value - route_setting_hash[key] = value - else - route_setting_hash[key] - end - end - - def self.inheritable_setting - @inheritable_setting ||= Grape::Util::InheritableSetting.new - end - - def self.namespace_stackable_hash - @namespace_stackable_hash ||= Hash.new do |hash, key| - hash[key] = [] - end - end - - def self.namespace_stackable_with_hash_hash - @namespace_stackable_with_hash_hash ||= Hash.new do |hash, key| - hash[key] = [] - end - end - - def self.namespace_inheritable_hash - @namespace_inheritable_hash ||= {} - end - - def self.route_setting_hash - @route_setting_hash ||= {} - end + extend Grape::DSL::Settings + extend Grape::DSL::Validations end end @@ -87,9 +33,20 @@ def self.route_setting_hash end describe '.scope' do + let(:root_app) do + Class.new(Grape::API) do + scope :my_scope do + get :my_endpoint do + return_no_content + end + end + end + end + it 'create a scope without affecting the URL' do - expect(subject).to receive(:within_namespace) - subject.scope {} + env = Rack::MockRequest.env_for('/my_endpoint', method: Rack::GET) + response = Rack::MockResponse[*root_app.call(env)] + expect(response).to be_no_content end end @@ -140,12 +97,12 @@ def self.route_setting_hash describe '.route' do before do allow(subject).to receive(:endpoints).and_return([]) - allow(subject).to receive(:route_end) + allow(subject.inheritable_setting).to receive(:route_end) allow(subject).to receive(:reset_validations!) end it 'marks end of the route' do - expect(subject).to receive(:route_end) + expect(subject.inheritable_setting).to receive(:route_end) subject.route(:any) end @@ -233,7 +190,6 @@ def self.route_setting_hash let(:new_namespace) { Object.new } it 'creates a new namespace with given name and options' do - expect(subject).to receive(:within_namespace).and_yield expect(subject).to receive(:nest).and_yield expect(Grape::Namespace).to receive(:new).with(:foo, { foo: 'bar' }).and_return(new_namespace) expect(subject).to receive(:namespace_stackable).with(:namespace, new_namespace) diff --git a/spec/grape/dsl/settings_spec.rb b/spec/grape/dsl/settings_spec.rb index 550195472..1a475a764 100644 --- a/spec/grape/dsl/settings_spec.rb +++ b/spec/grape/dsl/settings_spec.rb @@ -7,6 +7,10 @@ Class.new do include Grape::DSL::Settings + def with_namespace(&block) + within_namespace(&block) + end + def reset_validations!; end end end @@ -50,7 +54,7 @@ def reset_validations!; end subject.route_setting :some_thing, :foo_bar expect(subject.route_setting(:some_thing)).to eq :foo_bar - subject.route_end + subject.inheritable_setting.route_end expect(subject.route_setting(:some_thing)).to be_nil end @@ -63,31 +67,22 @@ def reset_validations!; end end it 'sets a value until the end of a namespace' do - subject.namespace_start - - subject.namespace_setting :some_thing, :foo_bar - expect(subject.namespace_setting(:some_thing)).to eq :foo_bar - - subject.namespace_end - + subject.with_namespace do + subject.namespace_setting :some_thing, :foo_bar + expect(subject.namespace_setting(:some_thing)).to eq :foo_bar + end expect(subject.namespace_setting(:some_thing)).to be_nil end it 'resets values after leaving nested namespaces' do - subject.namespace_start - - subject.namespace_setting :some_thing, :foo_bar - expect(subject.namespace_setting(:some_thing)).to eq :foo_bar - - subject.namespace_start - - expect(subject.namespace_setting(:some_thing)).to be_nil - - subject.namespace_end - expect(subject.namespace_setting(:some_thing)).to eq :foo_bar - - subject.namespace_end - + subject.with_namespace do + subject.namespace_setting :some_thing, :foo_bar + expect(subject.namespace_setting(:some_thing)).to eq :foo_bar + subject.with_namespace do + expect(subject.namespace_setting(:some_thing)).to be_nil + end + expect(subject.namespace_setting(:some_thing)).to eq :foo_bar + end expect(subject.namespace_setting(:some_thing)).to be_nil end end @@ -99,22 +94,16 @@ def reset_validations!; end end it 'inherits values from surrounding namespace' do - subject.namespace_start - - subject.namespace_inheritable(:some_thing, :foo_bar) - expect(subject.namespace_inheritable(:some_thing)).to eq :foo_bar - - subject.namespace_start - - expect(subject.namespace_inheritable(:some_thing)).to eq :foo_bar - - subject.namespace_inheritable(:some_thing, :foo_bar_2) - - expect(subject.namespace_inheritable(:some_thing)).to eq :foo_bar_2 - - subject.namespace_end - expect(subject.namespace_inheritable(:some_thing)).to eq :foo_bar - subject.namespace_end + subject.with_namespace do + subject.namespace_inheritable(:some_thing, :foo_bar) + expect(subject.namespace_inheritable(:some_thing)).to eq :foo_bar + subject.with_namespace do + expect(subject.namespace_inheritable(:some_thing)).to eq :foo_bar + subject.namespace_inheritable(:some_thing, :foo_bar_2) + expect(subject.namespace_inheritable(:some_thing)).to eq :foo_bar_2 + end + expect(subject.namespace_inheritable(:some_thing)).to eq :foo_bar + end end end @@ -125,22 +114,15 @@ def reset_validations!; end end it 'stacks values from surrounding namespace' do - subject.namespace_start - - subject.namespace_stackable(:some_thing, :foo_bar) - expect(subject.namespace_stackable(:some_thing)).to eq [:foo_bar] - - subject.namespace_start - - expect(subject.namespace_stackable(:some_thing)).to eq [:foo_bar] - - subject.namespace_stackable(:some_thing, :foo_bar_2) - - expect(subject.namespace_stackable(:some_thing)).to eq %i[foo_bar foo_bar_2] - - subject.namespace_end - expect(subject.namespace_stackable(:some_thing)).to eq [:foo_bar] - subject.namespace_end + subject.with_namespace do + subject.namespace_stackable(:some_thing, :foo_bar) + expect(subject.namespace_stackable(:some_thing)).to eq [:foo_bar] + subject.with_namespace do + subject.namespace_stackable(:some_thing, :foo_bar_2) + expect(subject.namespace_stackable(:some_thing)).to eq %i[foo_bar foo_bar_2] + end + expect(subject.namespace_stackable(:some_thing)).to eq [:foo_bar] + end end end @@ -151,24 +133,6 @@ def reset_validations!; end end end - describe '#within_namespace' do - it 'calls start and end for a namespace' do - expect(subject).to receive :namespace_start - expect(subject).to receive :namespace_end - - subject.within_namespace do - end - end - - it 'returns the last result' do - result = subject.within_namespace do - 1 - end - - expect(result).to eq 1 - end - end - describe 'complex scenario' do it 'plays well' do obj1 = dummy_class.new @@ -179,7 +143,7 @@ def reset_validations!; end obj2_copy = nil obj3_copy = nil - obj1.within_namespace do + obj1.with_namespace do obj1.namespace_stackable(:some_thing, :obj1) expect(obj1.namespace_stackable(:some_thing)).to eq [:obj1] obj1_copy = obj1.inheritable_setting.point_in_time_copy @@ -188,7 +152,7 @@ def reset_validations!; end expect(obj1.namespace_stackable(:some_thing)).to eq [] expect(obj1_copy.namespace_stackable[:some_thing]).to eq [:obj1] - obj2.within_namespace do + obj2.with_namespace do obj2.namespace_stackable(:some_thing, :obj2) expect(obj2.namespace_stackable(:some_thing)).to eq [:obj2] obj2_copy = obj2.inheritable_setting.point_in_time_copy @@ -197,7 +161,7 @@ def reset_validations!; end expect(obj2.namespace_stackable(:some_thing)).to eq [] expect(obj2_copy.namespace_stackable[:some_thing]).to eq [:obj2] - obj3.within_namespace do + obj3.with_namespace do obj3.namespace_stackable(:some_thing, :obj3) expect(obj3.namespace_stackable(:some_thing)).to eq [:obj3] obj3_copy = obj3.inheritable_setting.point_in_time_copy