Merge pull request #1047 from rnubel/given_parameter

Validate params only if another param is present

Author: dblock

CHANGELOG.md

@@ -4,6 +4,7 @@ Next Release
 #### Features
 
 * [#1039](https://github.com/intridea/grape/pull/1039): Added support for custom parameter types - [@rnubel](https://github.com/rnubel).
+* [#1047](https://github.com/intridea/grape/pull/1047): Adds `given` to DSL::Parameters, allowing for dependent params - [@rnubel](https://github.com/rnubel).
 * Your contribution here!
 
 #### Fixes

README.md

@@ -31,6 +31,7 @@
 - [Parameter Validation and Coercion](#parameter-validation-and-coercion)
   - [Supported Parameter Types](#supported-parameter-types)
   - [Custom Types](#custom-types)
+  - [Dependent Parameters](#dependent-parameters)
   - [Built-in Validators](#built-in-validators)
   - [Namespace Validation and Coercion](#namespace-validation-and-coercion)
   - [Custom Validators](#custom-validators)
@@ -803,6 +804,21 @@ params do
 end
 ```
 
+#### Dependent Parameters
+
+Suppose some of your parameters are only relevant if another parameter is given;
+Grape allows you to express this relationship through the `given` method in your
+parameters block, like so:
+
+```ruby
+params do
+  optional :shelf_id, type: Integer
+  given :shelf_id do
+    requires :bin_id, type: Integer
+  end
+end
+```
+
 ### Built-in Validators
 
 #### `allow_blank`

lib/grape.rb

@@ -66,6 +66,7 @@ module Exceptions
     autoload :InvalidVersionerOption
     autoload :UnknownValidator
     autoload :UnknownOptions
+    autoload :UnknownParameter
     autoload :InvalidWithOptionForRepresent
     autoload :IncompatibleOptionValues
     autoload :MissingGroupTypeError,          'grape/exceptions/missing_group_type'

lib/grape/dsl/parameters.rb

@@ -149,6 +149,27 @@ def all_or_none_of(*attrs)
         validates(attrs, all_or_none_of: true)
       end
 
+      # Define a block of validations which should be applied if and only if
+      # the given parameter is present. The parameters are not nested.
+      # @param attr [Symbol] the parameter which, if present, triggers the
+      #   validations
+      # @throws Grape::Exceptions::UnknownParameter if `attr` has not been
+      #   defined in this scope yet
+      # @yield a parameter definition DSL
+      def given(attr, &block)
+        fail Grape::Exceptions::UnknownParameter.new(attr) unless declared_param?(attr)
+        new_lateral_scope(dependent_on: attr, &block)
+      end
+
+      # Test for whether a certain parameter has been defined in this params
+      # block yet.
+      # @returns [Boolean] whether the parameter has been defined
+      def declared_param?(param)
+        # @declared_params also includes hashes of options and such, but those
+        # won't be flattened out.
+        @declared_params.flatten.include?(param)
+      end
+
       alias_method :group, :requires
 
       # @param params [Hash] initial hash of parameters

lib/grape/exceptions/unknown_parameter.rb

@@ -0,0 +1,10 @@
+# encoding: utf-8
+module Grape
+  module Exceptions
+    class UnknownParameter < Base
+      def initialize(param)
+        super(message: compose_message('unknown_parameter', param: param))
+      end
+    end
+  end
+end

lib/grape/locale/en.yml

@@ -28,6 +28,7 @@ en:
           resolution: 'available strategy for :using is :path, :header, :param'
         unknown_validator: 'unknown validator: %{validator_type}'
         unknown_options: 'unknown options: %{options}'
+        unknown_parameter: 'unknown parameter: %{param}'
         incompatible_option_values: '%{option1}: %{value1} is incompatible with %{option2}: %{value2}'
         mutual_exclusion: 'are mutually exclusive'
         at_least_one: 'are missing, at least one parameter must be provided'

lib/grape/validations/params_scope.rb

@@ -15,13 +15,16 @@ class ParamsScope
       # @option opts :optional [Boolean] whether or not this scope needs to have
       #   any parameters set or not
       # @option opts :type [Class] a type meant to govern this scope (deprecated)
+      # @option opts :dependent_on [Symbol] if present, this scope should only
+      #   validate if this param is present in the parent scope
       # @yield the instance context, open for parameter definitions
       def initialize(opts, &block)
-        @element  = opts[:element]
-        @parent   = opts[:parent]
-        @api      = opts[:api]
-        @optional = opts[:optional] || false
-        @type     = opts[:type]
+        @element      = opts[:element]
+        @parent       = opts[:parent]
+        @api          = opts[:api]
+        @optional     = opts[:optional] || false
+        @type         = opts[:type]
+        @dependent_on = opts[:dependent_on]
         @declared_params = []
 
         instance_eval(&block) if block_given?
@@ -33,21 +36,45 @@ def initialize(opts, &block)
       #   validated
       def should_validate?(parameters)
         return false if @optional && params(parameters).respond_to?(:all?) && params(parameters).all?(&:blank?)
+        return false if @dependent_on && params(parameters).try(:[], @dependent_on).blank?
         return true if parent.nil?
         parent.should_validate?(parameters)
       end
 
       # @return [String] the proper attribute name, with nesting considered.
       def full_name(name)
-        return "#{@parent.full_name(@element)}[#{name}]" if @parent
-        name.to_s
+        case
+        when nested?
+          # Find our containing element's name, and append ours.
+          "#{@parent.full_name(@element)}[#{name}]"
+        when lateral?
+          # Find the name of the element as if it was at the
+          # same nesting level as our parent.
+          @parent.full_name(name)
+        else
+          # We must be the root scope, so no prefix needed.
+          name.to_s
+        end
       end
 
       # @return [Boolean] whether or not this scope is the root-level scope
       def root?
         !@parent
       end
 
+      # A nested scope is contained in one of its parent's elements.
+      # @return [Boolean] whether or not this scope is nested
+      def nested?
+        @parent && @element
+      end
+
+      # A lateral scope is subordinate to its parent, but its keys are at the
+      # same level as its parent and thus is not contained within an element.
+      # @return [Boolean] whether or not this scope is lateral
+      def lateral?
+        @parent && !@element
+      end
+
       # @return [Boolean] whether or not this scope needs to be present, or can
       #   be blank
       def required?
@@ -57,7 +84,7 @@ def required?
       protected
 
       # Adds a parameter declaration to our list of validations.
-      # @param attrs [Array] (see Grape::DSL::Parameters#required)
+      # @param attrs [Array] (see Grape::DSL::Parameters#requires)
       def push_declared_params(attrs)
         @declared_params.concat attrs
       end
@@ -98,6 +125,13 @@ def validate_attributes(attrs, opts, &block)
         validates(attrs, validations)
       end
 
+      # Returns a new parameter scope, subordinate to the current one and nested
+      # under the parameter corresponding to `attrs.first`.
+      # @param attrs [Array] the attributes passed to the `requires` or
+      #   `optional` invocation that opened this scope.
+      # @param optional [Boolean] whether the parameter this are nested under
+      #   is optional or not (and hence, whether this block's params will be).
+      # @yield parameter scope
       def new_scope(attrs, optional = false, &block)
         # if required params are grouped and no type or unsupported type is provided, raise an error
         type = attrs[1] ? attrs[1][:type] : nil
@@ -107,12 +141,30 @@ def new_scope(attrs, optional = false, &block)
         end
 
         opts = attrs[1] || { type: Array }
-        ParamsScope.new(api: @api, element: attrs.first, parent: self, optional: optional, type: opts[:type], &block)
+        self.class.new(api: @api, element: attrs.first, parent: self, optional: optional, type: opts[:type], &block)
+      end
+
+      # Returns a new parameter scope, not nested under any current-level param
+      # but instead at the same level as the current scope.
+      # @param options [Hash] options to control how this new scope behaves
+      # @option options :dependent_on [Symbol] if given, specifies that this
+      #   scope should only validate if this parameter from the above scope is
+      #   present
+      # @yield parameter scope
+      def new_lateral_scope(options, &block)
+        self.class.new(
+          api:          @api,
+          element:      nil,
+          parent:       self,
+          options:      @optional,
+          type:         Hash,
+          dependent_on: options[:dependent_on],
+          &block)
       end
 
       # Pushes declared params to parent or settings
       def configure_declared_params
-        if @parent
+        if nested?
           @parent.push_declared_params [element => @declared_params]
         else
           @api.namespace_stackable(:declared_params, @declared_params)

spec/grape/validations/params_scope_spec.rb

@@ -271,4 +271,59 @@ def initialize(value)
       end.to raise_error Grape::Exceptions::UnsupportedGroupTypeError
     end
   end
+
+  context 'when validations are dependent on a parameter' do
+    before do
+      subject.params do
+        optional :a
+        given :a do
+          requires :b
+        end
+      end
+      subject.get('/test') { declared(params).to_json }
+    end
+
+    it 'applies the validations only if the parameter is present' do
+      get '/test'
+      expect(last_response.status).to eq(200)
+
+      get '/test', a: true
+      expect(last_response.status).to eq(400)
+      expect(last_response.body).to eq('b is missing')
+
+      get '/test', a: true, b: true
+      expect(last_response.status).to eq(200)
+    end
+
+    it 'raises an error if the dependent parameter was never specified' do
+      expect do
+        subject.params do
+          given :c do
+          end
+        end
+      end.to raise_error(Grape::Exceptions::UnknownParameter)
+    end
+
+    it 'includes the parameter within #declared(params)' do
+      get '/test', a: true, b: true
+
+      expect(JSON.parse(last_response.body)).to eq('a' => 'true', 'b' => 'true')
+    end
+
+    it 'returns a sensible error message within a nested context' do
+      subject.params do
+        requires :bar, type: Hash do
+          optional :a
+          given :a do
+            requires :b
+          end
+        end
+      end
+      subject.get('/nested') { 'worked' }
+
+      get '/nested', bar: { a: true }
+      expect(last_response.status).to eq(400)
+      expect(last_response.body).to eq('bar[b] is missing')
+    end
+  end
 end