summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHeinrich Lee Yu <hleeyu@gmail.com>2018-10-25 22:20:06 +0800
committerHeinrich Lee Yu <hleeyu@gmail.com>2018-10-26 10:32:14 +0800
commitbf1ed85a9d6a932a99d0a5fdf70e72ea36c2600c (patch)
tree66fabf664fc6e3f29a19322dbf3b7c6c11f1dc06
parentd4e26636e72ef150a6f4446558c4d24d3bf21e69 (diff)
downloadgitlab-ce-bf1ed85a9d6a932a99d0a5fdf70e72ea36c2600c.tar.gz
Refactor api validator to separate class
-rw-r--r--lib/api/helpers/custom_validators.rb13
-rw-r--r--lib/api/issues.rb5
-rw-r--r--lib/api/merge_requests.rb5
-rw-r--r--spec/lib/api/helpers/custom_validators_spec.rb62
4 files changed, 77 insertions, 8 deletions
diff --git a/lib/api/helpers/custom_validators.rb b/lib/api/helpers/custom_validators.rb
index 23b1cd1ad45..e4af75f1971 100644
--- a/lib/api/helpers/custom_validators.rb
+++ b/lib/api/helpers/custom_validators.rb
@@ -10,8 +10,21 @@ module API
raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:absence)
end
end
+
+ class IntegerNoneAny < Grape::Validations::Base
+ def validate_param!(attr_name, params)
+ value = params[attr_name]
+
+ return if value.is_a?(Integer) ||
+ [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(value)
+
+ raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)],
+ message: "should be an integer, 'None' or 'Any'"
+ end
+ end
end
end
end
Grape::Validations.register_validator(:absence, ::API::Helpers::CustomValidators::Absence)
+Grape::Validations.register_validator(:integer_none_any, ::API::Helpers::CustomValidators::IntegerNoneAny)
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index d863a37238c..405fc30a2ed 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -40,10 +40,7 @@ module API
optional :updated_after, type: DateTime, desc: 'Return issues updated after the specified time'
optional :updated_before, type: DateTime, desc: 'Return issues updated before the specified time'
optional :author_id, type: Integer, desc: 'Return issues which are authored by the user with the given ID'
- optional :assignee_id, types: [Integer, String],
- values: -> (v) {
- v.is_a?(Integer) || [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(v)
- },
+ optional :assignee_id, types: [Integer, String], integer_none_any: true,
desc: 'Return issues which are assigned to the user with the given ID'
optional :scope, type: String, values: %w[created-by-me assigned-to-me created_by_me assigned_to_me all],
desc: 'Return issues for the given scope: `created_by_me`, `assigned_to_me` or `all`'
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 5f209228105..a617efaaa4c 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -89,10 +89,7 @@ module API
optional :updated_before, type: DateTime, desc: 'Return merge requests updated before the specified time'
optional :view, type: String, values: %w[simple], desc: 'If simple, returns the `iid`, URL, title, description, and basic state of merge request'
optional :author_id, type: Integer, desc: 'Return merge requests which are authored by the user with the given ID'
- optional :assignee_id, types: [Integer, String],
- values: -> (v) {
- v.is_a?(Integer) || [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(v)
- },
+ optional :assignee_id, types: [Integer, String], integer_none_any: true,
desc: 'Return merge requests which are assigned to the user with the given ID'
optional :scope, type: String, values: %w[created-by-me assigned-to-me created_by_me assigned_to_me all],
desc: 'Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`'
diff --git a/spec/lib/api/helpers/custom_validators_spec.rb b/spec/lib/api/helpers/custom_validators_spec.rb
new file mode 100644
index 00000000000..810d05c479c
--- /dev/null
+++ b/spec/lib/api/helpers/custom_validators_spec.rb
@@ -0,0 +1,62 @@
+require 'spec_helper'
+
+describe API::Helpers::CustomValidators do
+ let(:scope) do
+ Struct.new(:opts) do
+ def full_name(attr_name)
+ attr_name
+ end
+ end
+ end
+
+ describe API::Helpers::CustomValidators::Absence do
+ subject do
+ described_class.new(['test'], {}, false, scope.new)
+ end
+
+ context 'empty param' do
+ it 'does not raise a validation error' do
+ expect_no_validation_error({})
+ end
+ end
+
+ context 'invalid parameters' do
+ it 'should raise a validation error' do
+ expect_validation_error({ 'test' => 'some_value' })
+ end
+ end
+ end
+
+ describe API::Helpers::CustomValidators::IntegerNoneAny do
+ subject do
+ described_class.new(['test'], {}, false, scope.new)
+ end
+
+ context 'valid parameters' do
+ it 'does not raise a validation error' do
+ expect_no_validation_error({ 'test' => 2 })
+ expect_no_validation_error({ 'test' => 100 })
+ expect_no_validation_error({ 'test' => 'None' })
+ expect_no_validation_error({ 'test' => 'Any' })
+ end
+ end
+
+ context 'invalid parameters' do
+ it 'should raise a validation error' do
+ expect_validation_error({ 'test' => 'some_other_string' })
+ end
+ end
+ end
+
+ def expect_no_validation_error(params)
+ expect { validate_test_param!(params) }.not_to raise_error
+ end
+
+ def expect_validation_error(params)
+ expect { validate_test_param!(params) }.to raise_error(Grape::Exceptions::Validation)
+ end
+
+ def validate_test_param!(params)
+ subject.validate_param!('test', params)
+ end
+end