within_namespace is part on our internal api and should be exposed publicly. Also remove namespace_start and namespace_end

Author: ericproulx

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
 

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

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)

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