Merge pull request #851 from dblock/fix-801

dblock · dblock · commit a35eca647533 · 2014-12-16T09:21:44.000-05:00
Fix #801: Evaluate values proc lazily.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@
 * [#813](https://github.com/intridea/grape/pull/813): Routing methods dsl refactored to get rid of explicit `paths` parameter - [@AlexYankee](https://github.com/AlexYankee).
 * [#826](https://github.com/intridea/grape/pull/826): Find `coerce_type` for `Array` when not specified - [@manovotn](https://github.com/manovotn).
 * [#645](https://github.com/intridea/grape/issues/645): Invoking `body false` will return `204 No Content` - [@dblock](https://github.com/dblock).
+* [#801](https://github.com/intridea/grape/issues/801): Evaluate permitted parameter `values` lazily on each request when declared as a proc - [@dblock](https://github.com/dblock).
 * Your contribution here.
 
 0.9.0 (8/27/2014)
diff --git a/README.md b/README.md
@@ -747,7 +747,7 @@ params do
 end
 ```
 
-The `:values` option can also be supplied with a `Proc` to be evalutated at runtime. For example, given a status
+The `:values` option can also be supplied with a `Proc` to be evalutated at runtime for each request. For example, given a status
 model you may want to restrict by hashtags that you have previously defined in the `HashTag` model.
 
 ```ruby
diff --git a/UPGRADING.md b/UPGRADING.md
@@ -189,6 +189,26 @@ end
 
 See the [the updated API Formats documentation](https://github.com/intridea/grape#api-formats) and [#809](https://github.com/intridea/grape/pull/809) for more info.
 
+#### Changes to Evaluation of Permitted Parameter Values
+
+Permitted parameter values are now evaluated lazily for each request when declared as a proc. The following code would produce the same allowed value in 0.9.0 for each request and a new value since 0.10.0.
+
+```ruby
+params do
+  optional :v, values: -> { [SecureRandom.uuid] }
+end
+```
+
+Remove the proc to get the previous behavior.
+
+```ruby
+params do
+  optional :v, values: [SecureRandom.uuid]
+end
+```
+
+See [#801](https://github.com/intridea/grape/issues/801) for more information.
+
 ### Upgrading to >= 0.9.0
 
 #### Changes in Authentication
diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb
@@ -109,17 +109,17 @@ def validates(attrs, validations)
         values = validations[:values]
         doc_attrs[:values] = values if values
 
-        values = values.call if values.is_a?(Proc)
+        evaluated_values = values.is_a?(Proc) ? values.call : values
 
-        coerce_type = values.first.class if values && coerce_type == Array && !values.empty?
+        coerce_type = evaluated_values.first.class if evaluated_values && coerce_type == Array && !evaluated_values.empty?
 
         # default value should be present in values array, if both exist
-        if default && values && !values.include?(default)
-          fail Grape::Exceptions::IncompatibleOptionValues.new(:default, default, :values, values)
+        if default && evaluated_values && !evaluated_values.include?(default)
+          fail Grape::Exceptions::IncompatibleOptionValues.new(:default, default, :values, evaluated_values)
         end
 
         # type should be compatible with values array, if both exist
-        validate_value_coercion(coerce_type, values) if coerce_type && values
+        validate_value_coercion(coerce_type, evaluated_values) if coerce_type && evaluated_values
 
         doc_attrs[:documentation] = validations.delete(:documentation) if validations.key?(:documentation)
 
diff --git a/spec/grape/validations/validators/values_spec.rb b/spec/grape/validations/validators/values_spec.rb
@@ -76,6 +76,11 @@ class API < Grape::API
           end
         end
         get '/optional_with_required_values'
+
+        params do
+          optional :type, type: String, values: -> { [SecureRandom.uuid] }
+        end
+        get '/random_values'
       end
     end
   end
@@ -199,4 +204,10 @@ def app
       subject.params { requires :type, values: [10.5, 11], type: Integer }
     end.to raise_error Grape::Exceptions::IncompatibleOptionValues
   end
+
+  it 'evaluates values dynamically with each request' do
+    allow(SecureRandom).to receive(:uuid).and_return('foo')
+    get '/random_values', type: 'foo'
+    expect(last_response.status).to eq 200
+  end
 end
