Author: ericproulx

CHANGELOG.md

@@ -3,6 +3,7 @@
 #### Features
 
 * [#2629](https://github.com/ruby-grape/grape/pull/2629): Refactor Router Architecture - [@ericproulx](https://github.com/ericproulx).
+* [#2633](https://github.com/ruby-grape/grape/pull/2633): Refactor API::Instance and reorganize DSL modules - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes

lib/grape/api.rb

@@ -5,7 +5,7 @@ module Grape
   # should subclass this class in order to build an API.
   class API
     # Class methods that we want to call on the API rather than on the API object
-    NON_OVERRIDABLE = %i[call call! configuration compile! inherited recognize_path routes].freeze
+    NON_OVERRIDABLE = %i[base= base_instance? call change! configuration compile! inherit_settings recognize_path reset! routes top_level_setting= top_level_setting].freeze
 
     Helpers = Grape::DSL::Helpers::BaseHelper
 
@@ -29,7 +29,7 @@ class << self
       # the headers, and the body. See [the rack specification]
       # (https://github.com/rack/rack/blob/main/SPEC.rdoc) for more.
       # NOTE: This will only be called on an API directly mounted on RACK
-      def_delegators :base_instance, :new, :configuration, :call, :compile!
+      def_delegators :base_instance, :new, :configuration, :call, :change!, :compile!, :recognize_path, :routes
 
       # Initialize the instance variables on the remountable class, and the base_instance
       # an instance that will be used to create the set up but will not be mounted

lib/grape/api/instance.rb

@@ -21,28 +21,17 @@ class Instance
       class << self
         extend Forwardable
 
-        attr_reader :instance, :base
         attr_accessor :configuration
 
-        def_delegators :base, :to_s
-
-        def given(conditional_option, &block)
-          return unless conditional_option
-
-          mounted(&block)
-        end
-
-        def mounted(&block)
-          evaluate_as_instance_with_configuration(block, lazy: true)
-        end
+        def_delegators :@base, :to_s
 
         def base=(grape_api)
           @base = grape_api
           grape_api.instances << self
         end
 
         def base_instance?
-          self == base.base_instance
+          self == @base.base_instance
         end
 
         # A class-level lock to ensure the API is not compiled by multiple
@@ -56,43 +45,30 @@ def reset!
           reset_validations!
         end
 
-        # Parses the API's definition and compiles it into an instance of
-        # Grape::API.
-        def compile
-          @instance ||= new # rubocop:disable Naming/MemoizedInstanceVariableName
-        end
-
         # This is the interface point between Rack and Grape; it accepts a request
         # from Rack and ultimately returns an array of three values: the status,
         # the headers, and the body. See [the rack specification]
         # (http://www.rubydoc.info/github/rack/rack/master/file/SPEC) for more.
         def call(env)
           compile!
-          call!(env)
-        end
-
-        # A non-synchronized version of ::call.
-        def call!(env)
-          instance.call(env)
-        end
-
-        # (see #cascade?)
-        def cascade(value = nil)
-          return inheritable_setting.namespace_inheritable.key?(:cascade) ? !inheritable_setting.namespace_inheritable(:cascade).nil? : true if value.nil?
-
-          inheritable_setting.namespace_inheritable[:cascade] = value
+          @instance.call(env)
         end
 
         def compile!
-          return if instance
+          return if @instance
 
-          LOCK.synchronize { compile unless instance }
+          LOCK.synchronize { @instance ||= new }
         end
 
         # see Grape::Router#recognize_path
         def recognize_path(path)
           compile!
-          instance.router.recognize_path(path)
+          @instance.router.recognize_path(path)
+        end
+
+        # Wipe the compiled API so we can recompile after changes were made.
+        def change!
+          @instance = nil
         end
 
         protected
@@ -110,11 +86,6 @@ def inherit_settings(other_settings)
           reset_routes!
         end
 
-        # Wipe the compiled API so we can recompile after changes were made.
-        def change!
-          @instance = nil
-        end
-
         private
 
         def inherited(subclass)

lib/grape/dsl/routing.rb

@@ -5,6 +5,22 @@ module DSL
     module Routing
       attr_reader :endpoints
 
+      def given(conditional_option, &block)
+        return unless conditional_option
+
+        mounted(&block)
+      end
+
+      def mounted(&block)
+        evaluate_as_instance_with_configuration(block, lazy: true)
+      end
+
+      def cascade(value = nil)
+        return inheritable_setting.namespace_inheritable.key?(:cascade) ? !inheritable_setting.namespace_inheritable(:cascade).nil? : true if value.nil?
+
+        inheritable_setting.namespace_inheritable[:cascade] = value
+      end
+
       # Specify an API version.
       #
       # @example API with legacy support.
@@ -202,16 +218,6 @@ def routes
         @routes ||= endpoints.map(&:routes).flatten
       end
 
-      # Remove all defined routes.
-      def reset_routes!
-        endpoints.each(&:reset_routes!)
-        @routes = nil
-      end
-
-      def reset_endpoints!
-        @endpoints = []
-      end
-
       # This method allows you to quickly define a parameter route segment
       # in your API.
       #
@@ -238,6 +244,16 @@ def versions
 
       private
 
+      # Remove all defined routes.
+      def reset_routes!
+        endpoints.each(&:reset_routes!)
+        @routes = nil
+      end
+
+      def reset_endpoints!
+        @endpoints = []
+      end
+
       def refresh_mounted_api(mounts, *opts)
         opts << { refresh_already_mounted: true }
         mount(mounts, *opts)
@@ -265,7 +281,7 @@ def evaluate_as_instance_with_configuration(block, lazy: false)
           self.configuration = value_for_configuration
           response
         end
-        if base && base_instance? && lazy
+        if @base && base_instance? && lazy
           lazy_block
         else
           lazy_block.evaluate_from(configuration)

lib/grape/dsl/settings.rb

@@ -7,7 +7,7 @@ module DSL
     # matter where they're defined, and inheritable settings which apply only
     # in the current scope and scopes nested under it.
     module Settings
-      attr_writer :inheritable_setting, :top_level_setting
+      attr_writer :inheritable_setting
 
       # Fetch our top-level settings, which apply to all endpoints in the API.
       def top_level_setting

lib/grape/dsl/validations.rb

@@ -3,6 +3,28 @@
 module Grape
   module DSL
     module Validations
+      # Opens a root-level ParamsScope, defining parameter coercions and
+      # validations for the endpoint.
+      # @yield instance context of the new scope
+      def params(&block)
+        Grape::Validations::ParamsScope.new(api: self, type: Hash, &block)
+      end
+
+      # Declare the contract to be used for the endpoint's parameters.
+      # @param contract [Class<Dry::Validation::Contract> | Dry::Schema::Processor]
+      #   The contract or schema to be used for validation. Optional.
+      # @yield a block yielding a new instance of Dry::Schema::Params
+      #   subclass, allowing to define the schema inline. When the
+      #   +contract+ parameter is a schema, it will be used as a parent. Optional.
+      def contract(contract = nil, &block)
+        raise ArgumentError, 'Either contract or block must be provided' unless contract || block
+        raise ArgumentError, 'Cannot inherit from contract, only schema' if block && contract.respond_to?(:schema)
+
+        Grape::Validations::ContractScope.new(self, contract, &block)
+      end
+
+      private
+
       # Clears all defined parameters and validations. The main purpose of it is to clean up
       # settings, so next endpoint won't interfere with previous one.
       #
@@ -24,26 +46,6 @@ module Validations
       def reset_validations!
         inheritable_setting.namespace_stackable.delete(:declared_params, :params, :validations)
       end
-
-      # Opens a root-level ParamsScope, defining parameter coercions and
-      # validations for the endpoint.
-      # @yield instance context of the new scope
-      def params(&block)
-        Grape::Validations::ParamsScope.new(api: self, type: Hash, &block)
-      end
-
-      # Declare the contract to be used for the endpoint's parameters.
-      # @param contract [Class<Dry::Validation::Contract> | Dry::Schema::Processor]
-      #   The contract or schema to be used for validation. Optional.
-      # @yield a block yielding a new instance of Dry::Schema::Params
-      #   subclass, allowing to define the schema inline. When the
-      #   +contract+ parameter is a schema, it will be used as a parent. Optional.
-      def contract(contract = nil, &block)
-        raise ArgumentError, 'Either contract or block must be provided' unless contract || block
-        raise ArgumentError, 'Cannot inherit from contract, only schema' if block && contract.respond_to?(:schema)
-
-        Grape::Validations::ContractScope.new(self, contract, &block)
-      end
     end
   end
 end

spec/grape/api_spec.rb

@@ -3938,19 +3938,12 @@ def my_method
     end
   end
 
-  describe '.compile' do
-    it 'sets the instance' do
-      expect(subject.instance).to be_nil
-      subject.compile
-      expect(subject.instance).to be_a(subject.base_instance)
-    end
-  end
-
   describe '.change!' do
     it 'invalidates any compiled instance' do
-      subject.compile
+      expect(Grape::API::Instance.singleton_class::LOCK).to receive(:synchronize).and_call_original.twice
+      subject.compile!
       subject.change!
-      expect(subject.instance).to be_nil
+      subject.compile!
     end
   end
 

spec/grape/dsl/routing_spec.rb

@@ -72,30 +72,53 @@ class << self
   describe '.mount' do
     it 'mounts on a nested path' do
       subject = Class.new(Grape::API)
-      app1 = Class.new(Grape::API)
-      app2 = Class.new(Grape::API)
-      app2.get '/nice' do
-        'play'
+      app1 = Class.new(Grape::API) do
+        get '/' do
+          return_no_content
+        end
+      end
+      app2 = Class.new(Grape::API) do
+        get '/' do
+          return_no_content
+        end
       end
-
       subject.mount app1 => '/app1'
       app1.mount app2 => '/app2'
 
-      expect(subject.inheritable_setting.to_hash[:namespace]).to eq({})
-      expect(subject.inheritable_setting.to_hash[:namespace_inheritable]).to eq({})
-      expect(app1.inheritable_setting.to_hash[:namespace_stackable]).to eq(mount_path: ['/app1'])
+      env = Rack::MockRequest.env_for('/app1', method: Rack::GET)
+      response = Rack::MockResponse[*subject.call(env)]
 
-      expect(app2.inheritable_setting.to_hash[:namespace_stackable]).to eq(mount_path: ['/app1', '/app2'])
+      expect(response).to be_no_content
+
+      env = Rack::MockRequest.env_for('/app1/app2', method: Rack::GET)
+      response = Rack::MockResponse[*subject.call(env)]
+
+      expect(response).to be_no_content
     end
 
     it 'mounts multiple routes at once' do
       base_app = Class.new(Grape::API)
-      app1     = Class.new(Grape::API)
-      app2     = Class.new(Grape::API)
+      app1 = Class.new(Grape::API) do
+        get '/' do
+          return_no_content
+        end
+      end
+      app2 = Class.new(Grape::API) do
+        get '/' do
+          return_no_content
+        end
+      end
       base_app.mount(app1 => '/app1', app2 => '/app2')
 
-      expect(app1.inheritable_setting.to_hash[:namespace_stackable]).to eq(mount_path: ['/app1'])
-      expect(app2.inheritable_setting.to_hash[:namespace_stackable]).to eq(mount_path: ['/app2'])
+      env = Rack::MockRequest.env_for('/app1', method: Rack::GET)
+      response = Rack::MockResponse[*base_app.call(env)]
+
+      expect(response).to be_no_content
+
+      env = Rack::MockRequest.env_for('/app2', method: Rack::GET)
+      response = Rack::MockResponse[*base_app.call(env)]
+
+      expect(response).to be_no_content
     end
   end
 

spec/grape/dsl/validations_spec.rb

@@ -10,21 +10,6 @@
     end
   end
 
-  describe '.reset_validations' do
-    subject { dummy_class.reset_validations! }
-
-    before do
-      %i[declared_params params validations other].each do |key|
-        dummy_class.inheritable_setting.namespace_stackable[key] = key
-      end
-    end
-
-    it 'calls unset_namespace_stackable properly' do
-      subject
-      expect(dummy_class.inheritable_setting.namespace_stackable.to_hash).to eq(other: [:other])
-    end
-  end
-
   describe '.params' do
     subject { dummy_class.params { :my_block } }