diff --git a/.travis.yml b/.travis.yml index 1626ab6f38..194f08d645 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,45 +8,41 @@ gemfile: matrix: include: - - rvm: 2.5.3 + - rvm: 2.6.5 script: - bundle exec danger - - rvm: 2.5.3 + - rvm: 2.6.5 gemfile: Gemfile - - rvm: 2.5.3 + - rvm: 2.6.5 gemfile: gemfiles/rack_edge.gemfile - - rvm: 2.5.3 + - rvm: 2.6.5 gemfile: gemfiles/rails_edge.gemfile - - rvm: 2.5.3 + - rvm: 2.6.5 gemfile: gemfiles/rails_5.gemfile - - rvm: 2.5.3 + - rvm: 2.6.5 gemfile: gemfiles/multi_json.gemfile script: - bundle exec rake - bundle exec rspec spec/integration/multi_json - - rvm: 2.5.3 + - rvm: 2.6.5 gemfile: gemfiles/multi_xml.gemfile script: - bundle exec rake - bundle exec rspec spec/integration/multi_xml - - rvm: 2.4.5 + - rvm: 2.5.7 gemfile: Gemfile - - rvm: 2.4.5 + - rvm: 2.5.7 gemfile: gemfiles/rails_5.gemfile - - rvm: 2.3.8 + - rvm: 2.4.9 gemfile: Gemfile - - rvm: 2.3.8 + - rvm: 2.4.9 gemfile: gemfiles/rails_5.gemfile - - rvm: 2.2.10 - rvm: ruby-head - rvm: jruby-head - rvm: rbx-3 allow_failures: - - rvm: 2.2.10 - rvm: ruby-head - rvm: jruby-head - rvm: rbx-3 - - rvm: 2.5.3 - gemfile: gemfiles/rack_edge.gemfile bundler_args: --without development diff --git a/CHANGELOG.md b/CHANGELOG.md index fbd8fb5485..458852b3bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,10 @@ -### 1.2.6 (Next) +### 1.3.0 (Next) #### Features * Your contribution here. * [#1938](https://github.com/ruby-grape/grape/pull/1938): Add project metadata to the gemspec - [@orien](https://github.com/orien). +* [#1920](https://github.com/ruby-grape/grape/pull/1920): Replace Virtus with dry-types - [@dnesteryuk](https://github.com/dnesteryuk). #### Fixes diff --git a/README.md b/README.md index ab6343b0c6..231ecd3d00 100644 --- a/README.md +++ b/README.md @@ -154,7 +154,7 @@ content negotiation, versioning and much more. ## Stable Release -You're reading the documentation for the next release of Grape, which should be **1.2.6**. +You're reading the documentation for the next release of Grape, which should be **1.3.0**. Please read [UPGRADING](UPGRADING.md) when upgrading from a previous version. The current stable release is [1.2.5](https://github.com/ruby-grape/grape/blob/v1.2.5/README.md). @@ -167,6 +167,8 @@ The current stable release is [1.2.5](https://github.com/ruby-grape/grape/blob/v ## Installation +Ruby 2.4 or newer is required. + Grape is available as a gem, to install it just install the gem: gem install grape diff --git a/UPGRADING.md b/UPGRADING.md index a220a468e1..ff65254e6a 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,6 +1,49 @@ Upgrading Grape =============== +### Upgrading to >= 1.3.0 + +#### Ruby + +After adding dry-types, Ruby 2.4 or newer is required. + +#### Coercion + +[Virtus](https://github.com/solnic/virtus) has been replaced by [dry-types](https://dry-rb.org/gems/dry-types/1.2/) for parameter coercion. If your project depends on Virtus, explicitly add it to your `Gemfile`. Also, if Virtus is used for defining custom types + +```ruby +class User + include Virtus.model + + attribute :id, Integer + attribute :name, String +end + +# somewhere in your API +params do + requires :user, type: User +end +``` + +Add a class-level `parse` method to the model: + +```ruby +class User + include Virtus.model + + attribute :id, Integer + attribute :name, String + + def self.parse(attrs) + new(attrs) + end +end +``` + +Custom types which don't depend on Virtus don't require any changes. + +For more information see [#1920](https://github.com/ruby-grape/grape/pull/1920). + ### Upgrading to >= 1.2.4 #### Headers in `error!` call diff --git a/grape.gemspec b/grape.gemspec index 1e5bb712d6..9062ae7eac 100644 --- a/grape.gemspec +++ b/grape.gemspec @@ -20,14 +20,15 @@ Gem::Specification.new do |s| s.add_runtime_dependency 'activesupport' s.add_runtime_dependency 'builder' + s.add_runtime_dependency 'dry-types', '~> 1.1.1' s.add_runtime_dependency 'mustermann-grape', '~> 1.0.0' s.add_runtime_dependency 'rack', '>= 1.3.0' s.add_runtime_dependency 'rack-accept' - s.add_runtime_dependency 'virtus', '>= 1.0.0' s.files = %w[CHANGELOG.md CONTRIBUTING.md README.md grape.png UPGRADING.md LICENSE] s.files += %w[grape.gemspec] s.files += Dir['lib/**/*'] s.test_files = Dir['spec/**/*'] s.require_paths = ['lib'] + s.required_ruby_version = '>= 2.4.0' end diff --git a/lib/grape.rb b/lib/grape.rb index e211e046e3..048ea2e66d 100644 --- a/lib/grape.rb +++ b/lib/grape.rb @@ -20,8 +20,6 @@ require 'i18n' require 'thread' -require 'virtus' - I18n.load_path << File.expand_path('../grape/locale/en.yml', __FILE__) module Grape diff --git a/lib/grape/dsl/helpers.rb b/lib/grape/dsl/helpers.rb index d58e2cf020..718511799e 100644 --- a/lib/grape/dsl/helpers.rb +++ b/lib/grape/dsl/helpers.rb @@ -65,7 +65,7 @@ def include_all_in_scope def define_boolean_in_mod(mod) return if defined? mod::Boolean - mod.const_set('Boolean', Virtus::Attribute::Boolean) + mod.const_set('Boolean', Grape::API::Boolean) end def inject_api_helpers_to_mod(mod, &_block) diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index 75a287c7fb..c1192e580b 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -427,8 +427,8 @@ def validate_value_coercion(coerce_type, *values_list) values_list.each do |values| next if !values || values.is_a?(Proc) value_types = values.is_a?(Range) ? [values.begin, values.end] : values - if coerce_type == Virtus::Attribute::Boolean - value_types = value_types.map { |type| Virtus::Attribute.build(type) } + if coerce_type == Grape::API::Boolean + value_types = value_types.map { |type| Grape::API::Boolean.build(type) } end unless value_types.all? { |v| v.is_a? coerce_type } raise Grape::Exceptions::IncompatibleOptionValues.new(:type, coerce_type, :values, values) diff --git a/lib/grape/validations/types.rb b/lib/grape/validations/types.rb index 9320668f05..ee2ab22ed5 100644 --- a/lib/grape/validations/types.rb +++ b/lib/grape/validations/types.rb @@ -6,10 +6,6 @@ require_relative 'types/json' require_relative 'types/file' -# Patch for Virtus::Attribute::Collection -# See the file for more details -require_relative 'types/virtus_collection_patch' - module Grape module Validations # Module for code related to grape's system for @@ -27,8 +23,7 @@ module Types # a parameter value could not be coerced. class InvalidValue; end - # Types representing a single value, which are coerced through Virtus - # or special logic in Grape. + # Types representing a single value, which are coerced. PRIMITIVES = [ # Numerical Integer, @@ -42,10 +37,12 @@ class InvalidValue; end Time, # Misc - Virtus::Attribute::Boolean, + Grape::API::Boolean, String, Symbol, - Rack::Multipart::UploadedFile + Rack::Multipart::UploadedFile, + TrueClass, + FalseClass ].freeze # Types representing data structures. @@ -86,8 +83,6 @@ def self.primitive?(type) # @param type [Class] type to check # @return [Boolean] whether or not the type is known by Grape as a valid # data structure type - # @note This method does not yet consider 'complex types', which inherit - # Virtus.model. def self.structure?(type) STRUCTURES.include?(type) end @@ -104,25 +99,6 @@ def self.multiple?(type) (type.is_a?(Array) || type.is_a?(Set)) && type.size > 1 end - # Does the given class implement a type system that Grape - # (i.e. the underlying virtus attribute system) supports - # out-of-the-box? Currently supported are +axiom-types+ - # and +virtus+. - # - # The type will be passed to +Virtus::Attribute.build+, - # and the resulting attribute object will be expected to - # respond correctly to +coerce+ and +value_coerced?+. - # - # @param type [Class] type to check - # @return [Boolean] +true+ where the type is recognized - def self.recognized?(type) - return false if type.is_a?(Array) || type.is_a?(Set) - - type.is_a?(Virtus::Attribute) || - type.ancestors.include?(Axiom::Types::Type) || - type.include?(Virtus::Model::Core) - end - # Does Grape provide special coercion and validation # routines for the given class? This does not include # automatic handling for primitives, structures and @@ -152,7 +128,6 @@ def self.custom?(type) !primitive?(type) && !structure?(type) && !multiple?(type) && - !recognized?(type) && !special?(type) && type.respond_to?(:parse) && type.method(:parse).arity == 1 diff --git a/lib/grape/validations/types/array_coercer.rb b/lib/grape/validations/types/array_coercer.rb new file mode 100644 index 0000000000..57174fa688 --- /dev/null +++ b/lib/grape/validations/types/array_coercer.rb @@ -0,0 +1,54 @@ +require_relative 'dry_type_coercer' + +module Grape + module Validations + module Types + # Coerces elements in an array. It might be an array of strings or integers or + # anything else. + # + # It could've been possible to use an +of+ + # method (https://dry-rb.org/gems/dry-types/1.2/array-with-member/) + # provided by dry-types. Unfortunately, it doesn't work for Grape because of + # behavior of Virtus which was used earlier, a `Grape::Validations::Types::PrimitiveCoercer` + # maintains Virtus behavior in coercing. + class ArrayCoercer < DryTypeCoercer + def initialize(type, strict = false) + super + + @coercer = scope::Array + @elem_coercer = PrimitiveCoercer.new(type.first, strict) + end + + def call(_val) + collection = super + + return collection if collection.is_a?(InvalidValue) + + coerce_elements collection + end + + protected + + def coerce_elements(collection) + collection.each_with_index do |elem, index| + return InvalidValue.new if reject?(elem) + + coerced_elem = @elem_coercer.call(elem) + + return coerced_elem if coerced_elem.is_a?(InvalidValue) + + collection[index] = coerced_elem + end + + collection + end + + # This method maintaine logic which was defined by Virtus for arrays. + # Virtus doesn't allow nil in arrays. + def reject?(val) + val.nil? + end + end + end + end +end diff --git a/lib/grape/validations/types/build_coercer.rb b/lib/grape/validations/types/build_coercer.rb index 2a8e9968ce..9ed1e5d6fc 100644 --- a/lib/grape/validations/types/build_coercer.rb +++ b/lib/grape/validations/types/build_coercer.rb @@ -1,78 +1,73 @@ +require_relative 'array_coercer' +require_relative 'set_coercer' +require_relative 'primitive_coercer' + module Grape module Validations module Types - # Work out the +Virtus::Attribute+ object to - # use for coercing strings to the given +type+. - # Coercion +method+ will be inferred if none is - # supplied. + # Chooses the best coercer for the given type. For example, if the type + # is Integer, it will return a coercer which will be able to coerce a value + # to the integer. + # + # There are a few very special coercers which might be returned. + # + # +Grape::Types::MultipleTypeCoercer+ is a coercer which is returned when + # the given type implies values in an array with different types. + # For example, +[Integer, String]+ allows integer and string values in + # an array. + # + # +Grape::Types::CustomTypeCoercer+ is a coercer which is returned when + # a method is specified by a user with +coerce_with+ option or the user + # specifies a custom type which implements requirments of + # +Grape::Types::CustomTypeCoercer+. # - # If a +Virtus::Attribute+ object already built - # with +Virtus::Attribute.build+ is supplied as - # the +type+ it will be returned and +method+ - # will be ignored. + # +Grape::Types::CustomTypeCollectionCoercer+ is a very similar to the + # previous one, but it expects an array or set of values having a custom + # type implemented by the user. # - # See {CustomTypeCoercer} for further details - # about coercion and type-checking inference. + # There is also a group of custom types implemented by Grape, check + # +Grape::Validations::Types::SPECIAL+ to get the full list. # # @param type [Class] the type to which input strings # should be coerced # @param method [Class,#call] the coercion method to use - # @return [Virtus::Attribute] object to be used + # @return [Object] object to be used # for coercion and type validation - def self.build_coercer(type, method = nil) - cache_instance(type, method) do - create_coercer_instance(type, method) + def self.build_coercer(type, method: nil, strict: false) + cache_instance(type, method, strict) do + create_coercer_instance(type, method, strict) end end - def self.create_coercer_instance(type, method = nil) - # Accept pre-rolled virtus attributes without interference - return type if type.is_a? Virtus::Attribute - - converter_options = { - nullify_blank: true - } - conversion_type = if method == JSON - Object - # because we want just parsed JSON content: - # if type is Array and data is `"{}"` - # result will be [] because Virtus converts hashes - # to arrays - else - type - end - + def self.create_coercer_instance(type, method, strict) # Use a special coercer for multiply-typed parameters. if Types.multiple?(type) - converter_options[:coercer] = Types::MultipleTypeCoercer.new(type, method) - conversion_type = Object + MultipleTypeCoercer.new(type, method) # Use a special coercer for custom types and coercion methods. elsif method || Types.custom?(type) - converter_options[:coercer] = Types::CustomTypeCoercer.new(type, method) + CustomTypeCoercer.new(type, method) # Special coercer for collections of types that implement a parse method. # CustomTypeCoercer (above) already handles such types when an explicit coercion # method is supplied. elsif Types.collection_of_custom?(type) - converter_options[:coercer] = Types::CustomTypeCollectionCoercer.new( + Types::CustomTypeCollectionCoercer.new( type.first, type.is_a?(Set) ) - - # Grape swaps in its own Virtus::Attribute implementations - # for certain special types that merit first-class support - # (but not if a custom coercion method has been supplied). elsif Types.special?(type) - conversion_type = Types::SPECIAL[type] + Types::SPECIAL[type].new + elsif type.is_a?(Array) + ArrayCoercer.new type, strict + elsif type.is_a?(Set) + SetCoercer.new type, strict + else + PrimitiveCoercer.new type, strict end - - # Virtus will infer coercion and validation rules - # for many common ruby types. - Virtus::Attribute.build(conversion_type, converter_options) end - def self.cache_instance(type, method, &_block) - key = cache_key(type, method) + def self.cache_instance(type, method, strict, &_block) + key = cache_key(type, method, strict) return @__cache[key] if @__cache.key?(key) @@ -85,8 +80,8 @@ def self.cache_instance(type, method, &_block) instance end - def self.cache_key(type, method) - [type, method].compact.map(&:to_s).join('_') + def self.cache_key(type, method, strict) + [type, method, strict].compact.map(&:to_s).join('_') end instance_variable_set(:@__cache, {}) diff --git a/lib/grape/validations/types/custom_type_coercer.rb b/lib/grape/validations/types/custom_type_coercer.rb index f6b26cf83f..9a6b4da184 100644 --- a/lib/grape/validations/types/custom_type_coercer.rb +++ b/lib/grape/validations/types/custom_type_coercer.rb @@ -1,21 +1,6 @@ module Grape module Validations module Types - # Instances of this class may be passed to - # +Virtus::Attribute.build+ as the +:coercer+ - # option for custom types that do not otherwise - # satisfy the requirements for +Virtus::Attribute::coerce+ - # and +Virtus::Attribute::value_coerced?+ to work - # as expected. - # - # Subclasses of +Virtus::Attribute+ or +Axiom::Types::Type+ - # (or for which an axiom type can be inferred, i.e. the - # primitives, +Date+, +Time+, etc.) do not need any such - # coercer to be passed with them. - # - # Coercion - # -------- - # # This class will detect type classes that implement # a class-level +parse+ method. The method should accept one # +String+ argument and should return the value coerced to @@ -30,14 +15,14 @@ module Types # Type Checking # ------------- # - # Calls to +value_coerced?+ will consult this class to check + # Calls to +coerced?+ will consult this class to check # that the coerced value produced above is in fact of the # expected type. By default this class performs a basic check # against the type supplied, but this behaviour will be # overridden if the class implements a class-level # +coerced?+ or +parsed?+ method. This method # will receive a single parameter that is the coerced value - # and should return +true+ iff the value meets type expectations. + # and should return +true+ if the value meets type expectations. # Arbitrary assertions may be made here but the grape validation # system should be preferred. # @@ -46,15 +31,6 @@ module Types # contract as +coerced?+, and must be supplied with a coercion # +method+. class CustomTypeCoercer - # Uses +Virtus::Attribute.build+ to build a new - # attribute that makes use of this class for - # coercion and type validation logic. - # - # @return [Virtus::Attribute] - def self.build(type, method = nil) - Virtus::Attribute.build(type, coercer: new(type, method)) - end - # A new coercer for the given type specification # and coercion method. # @@ -64,37 +40,25 @@ def self.build(type, method = nil) # optional coercion method. See class docs. def initialize(type, method = nil) coercion_method = infer_coercion_method type, method - @method = enforce_symbolized_keys type, coercion_method - @type_check = infer_type_check(type) end - # This method is called from somewhere within - # +Virtus::Attribute::coerce+ in order to coerce - # the given value. + # Coerces the given value. # # @param value [String] value to be coerced, in grape # this should always be a string. # @return [Object] the coerced result - def call(value) - @method.call value + def call(val) + return if val.nil? + + coerced_val = @method.call(val) + return InvalidValue.new unless coerced?(coerced_val) + coerced_val end - # This method is called from somewhere within - # +Virtus::Attribute::value_coerced?+ in order to - # assert that the value has been coerced successfully. - # - # @param _primitive [Axiom::Types::Type] primitive type - # for the coercion as detected by axiom-types' inference - # system. For custom types this is typically not much use - # (i.e. it is +Axiom::Types::Object+) unless special - # inference rules have been declared for the type. - # @param value [Object] a coerced result returned from {#call} - # @return [true,false] whether or not the coerced value - # satisfies type requirements. - def success?(_primitive, value) - @type_check.call value + def coerced?(val) + @type_check.call val end private @@ -160,8 +124,8 @@ def enforce_symbolized_keys(type, method) # Collections have all values processed individually if [Array, Set].include?(type) lambda do |val| - method.call(val).tap do |new_value| - new_value.map do |item| + method.call(val).tap do |new_val| + new_val.map do |item| item.is_a?(Hash) ? symbolize_keys(item) : item end end diff --git a/lib/grape/validations/types/custom_type_collection_coercer.rb b/lib/grape/validations/types/custom_type_collection_coercer.rb index 7534420fe5..ebdae4d40f 100644 --- a/lib/grape/validations/types/custom_type_collection_coercer.rb +++ b/lib/grape/validations/types/custom_type_collection_coercer.rb @@ -1,12 +1,6 @@ module Grape module Validations module Types - # Instances of this class may be passed to - # +Virtus::Attribute.build+ as the +:coercer+ - # option, to handle collections of types that - # provide their own parsing (and optionally, - # type-checking) functionality. - # # See {CustomTypeCoercer} for details on types # that will be supported by this by this coercer. # This coercer works in the same way as +CustomTypeCoercer+ @@ -38,32 +32,21 @@ def initialize(type, set = false) @set = set end - # This method is called from somewhere within - # +Virtus::Attribute::coerce+ in order to coerce - # the given value. + # Coerces the given value. # # @param value [Array] an array of values to be coerced # @return [Array,Set] the coerced result. May be an +Array+ or a # +Set+ depending on the setting given to the constructor def call(value) - coerced = value.map { |item| super(item) } + coerced = value.map do |item| + coerced_item = super(item) - @set ? Set.new(coerced) : coerced - end + return coerced_item if coerced_item.is_a?(InvalidValue) - # This method is called from somewhere within - # +Virtus::Attribute::value_coerced?+ in order to assert - # that the all of the values in the array have been coerced - # successfully. - # - # @param primitive [Axiom::Types::Type] primitive type for - # the coercion as deteced by axiom-types' inference system. - # @param value [Enumerable] a coerced result returned from {#call} - # @return [true,false] whether or not all of the coerced values in - # the collection satisfy type requirements. - def success?(primitive, value) - value.is_a?(@set ? Set : Array) && - value.all? { |item| super(primitive, item) } + coerced_item + end + + @set ? Set.new(coerced) : coerced end end end diff --git a/lib/grape/validations/types/dry_type_coercer.rb b/lib/grape/validations/types/dry_type_coercer.rb new file mode 100644 index 0000000000..a0a9841c10 --- /dev/null +++ b/lib/grape/validations/types/dry_type_coercer.rb @@ -0,0 +1,39 @@ +require 'dry-types' + +module DryTypes + # Call +Dry.Types()+ to add all registered types to +DryTypes+ which is + # a container in this case. Check documentation for more information + # https://dry-rb.org/gems/dry-types/1.2/getting-started/ + include Dry.Types() +end + +module Grape + module Validations + module Types + # A base class for classes which must identify a coercer to be used. + # If the +strict+ argument is true, it won't coerce the given value + # but check its type. More information there + # https://dry-rb.org/gems/dry-types/1.2/built-in-types/ + class DryTypeCoercer + def initialize(type, strict = false) + @type = type + @scope = strict ? DryTypes::Strict : DryTypes::Params + end + + # Coerces the given value to a type which was specified during + # initialization as a type argument. + # + # @param val [Object] + def call(val) + @coercer[val] + rescue Dry::Types::CoercionError => _e + InvalidValue.new + end + + protected + + attr_reader :scope, :type + end + end + end +end diff --git a/lib/grape/validations/types/file.rb b/lib/grape/validations/types/file.rb index 62aa3f6944..0a47bc3703 100644 --- a/lib/grape/validations/types/file.rb +++ b/lib/grape/validations/types/file.rb @@ -1,21 +1,20 @@ module Grape module Validations module Types - # +Virtus::Attribute+ implementation for parameters - # that are multipart file objects. Actual handling - # of these objects is provided by +Rack::Request+; - # this class is here only to assert that rack's - # handling has succeeded, and to prevent virtus - # from interfering. - class File < Virtus::Attribute - def coerce(input) + # Implementation for parameters that are multipart file objects. + # Actual handling of these objects is provided by +Rack::Request+; + # this class is here only to assert that rack's handling has succeeded. + class File + def call(input) + return InvalidValue.new unless coerced?(input) + # Processing of multipart file objects # is already taken care of by Rack::Request. # Nothing to do here. input end - def value_coerced?(value) + def coerced?(value) # Rack::Request creates a Hash with filename, # content type and an IO object. Do a bit of basic # duck-typing. diff --git a/lib/grape/validations/types/json.rb b/lib/grape/validations/types/json.rb index 220d1db6d7..c464477b1b 100644 --- a/lib/grape/validations/types/json.rb +++ b/lib/grape/validations/types/json.rb @@ -3,19 +3,20 @@ module Grape module Validations module Types - # +Virtus::Attribute+ implementation that handles coercion - # and type checking for parameters that are complex types - # given as JSON-encoded strings. It accepts both JSON objects + # Handles coercion and type checking for parameters that are complex + # types given as JSON-encoded strings. It accepts both JSON objects # and arrays of objects, and will coerce the input to a +Hash+ # or +Array+ object respectively. In either case the Grape # validation system will apply nested validation rules to # all returned objects. - class Json < Virtus::Attribute + class Json # Coerce the input into a JSON-like data structure. # # @param input [String] a JSON-encoded parameter value # @return [Hash,Array,nil] - def coerce(input) + def call(input) + return input if coerced?(input) + # Allow nulls and blank strings return if input.nil? || input =~ /^\s*$/ JSON.parse(input, symbolize_names: true) @@ -26,7 +27,7 @@ def coerce(input) # # @param value [Object] result of {#coerce} # @return [true,false] - def value_coerced?(value) + def coerced?(value) value.is_a?(::Hash) || coerced_collection?(value) end @@ -50,13 +51,13 @@ class JsonArray < Json # # @param input [String] JSON-encoded parameter value # @return [Array] - def coerce(input) + def call(input) json = super Array.wrap(json) unless json.nil? end # See {Json#coerced_collection?} - def value_coerced?(value) + def coerced?(value) coerced_collection? value end end diff --git a/lib/grape/validations/types/multiple_type_coercer.rb b/lib/grape/validations/types/multiple_type_coercer.rb index e78745382b..215030022e 100644 --- a/lib/grape/validations/types/multiple_type_coercer.rb +++ b/lib/grape/validations/types/multiple_type_coercer.rb @@ -22,53 +22,32 @@ def initialize(types, method = nil) @type_coercers = types.map do |type| if Types.multiple? type - VariantCollectionCoercer.new type + VariantCollectionCoercer.new type, @method else - Types.build_coercer type + Types.build_coercer type, strict: !@method.nil? end end end - # This method is called from somewhere within - # +Virtus::Attribute::coerce+ in order to coerce - # the given value. + # Coerces the given value. # - # @param value [String] value to be coerced, in grape + # @param val [String] value to be coerced, in grape # this should always be a string. # @return [Object,InvalidValue] the coerced result, or an instance # of {InvalidValue} if the value could not be coerced. - def call(value) - return @method.call(value) if @method + def call(val) + # once the value is coerced by the custom method, its type should be checked + val = @method.call(val) if @method + + coerced_val = InvalidValue.new @type_coercers.each do |coercer| - coerced = coercer.coerce(value) + coerced_val = coercer.call(val) - return coerced if coercer.value_coerced? coerced + return coerced_val unless coerced_val.is_a?(InvalidValue) end - # Declare that we couldn't coerce the value in such a way - # that Grape won't ask us again if the value is valid - InvalidValue.new - end - - # This method is called from somewhere within - # +Virtus::Attribute::value_coerced?+ in order to - # assert that the value has been coerced successfully. - # Due to Grape's design this will in fact only be called - # if a custom coercion method is being used, since {#call} - # returns an {InvalidValue} object if the value could not - # be coerced. - # - # @param _primitive [Axiom::Types::Type] primitive type - # for the coercion as detected by axiom-types' inference - # system. For custom types this is typically not much use - # (i.e. it is +Axiom::Types::Object+) unless special - # inference rules have been declared for the type. - # @param value [Object] a coerced result returned from {#call} - # @return [true,false] whether or not the coerced value - # satisfies type requirements. - def success?(_primitive, value) - @type_coercers.any? { |coercer| coercer.value_coerced? value } + coerced_val end end end diff --git a/lib/grape/validations/types/primitive_coercer.rb b/lib/grape/validations/types/primitive_coercer.rb new file mode 100644 index 0000000000..d46247bda6 --- /dev/null +++ b/lib/grape/validations/types/primitive_coercer.rb @@ -0,0 +1,56 @@ +require_relative 'dry_type_coercer' + +module Grape + module Validations + module Types + # Coerces the given value to a type defined via a +type+ argument during + # initialization. + class PrimitiveCoercer < DryTypeCoercer + MAPPING = { + Grape::API::Boolean => DryTypes::Params::Bool, + + # unfortunatelly, a +Params+ scope doesn't contain String + String => DryTypes::Coercible::String + }.freeze + + STRICT_MAPPING = { + Grape::API::Boolean => DryTypes::Strict::Bool + }.freeze + + def initialize(type, strict = false) + super + + @type = type + + @coercer = if strict + STRICT_MAPPING.fetch(type) { scope.const_get(type.name) } + else + MAPPING.fetch(type) { scope.const_get(type.name) } + end + end + + def call(val) + return InvalidValue.new if reject?(val) + return nil if val.nil? + return '' if val == '' + + super + end + + protected + + attr_reader :type + + # This method maintaine logic which was defined by Virtus. For example, + # dry-types is ok to convert an array or a hash to a string, it is supported, + # but Virtus wouldn't accept it. So, this method only exists to not introduce + # breaking changes. + def reject?(val) + (val.is_a?(Array) && type == String) || + (val.is_a?(String) && type == Hash) || + (val.is_a?(Hash) && type == String) + end + end + end + end +end diff --git a/lib/grape/validations/types/set_coercer.rb b/lib/grape/validations/types/set_coercer.rb new file mode 100644 index 0000000000..402dd60c13 --- /dev/null +++ b/lib/grape/validations/types/set_coercer.rb @@ -0,0 +1,36 @@ +require 'set' +require_relative 'dry_type_coercer' + +module Grape + module Validations + module Types + # Takes the given array and converts it to a set. Every element of the set + # is also coerced. + class SetCoercer < DryTypeCoercer + def initialize(type, strict = false) + super + + @elem_coercer = PrimitiveCoercer.new(type.first, strict) + end + + def call(value) + return InvalidValue.new unless value.is_a?(Array) + + coerce_elements(value) + end + + protected + + def coerce_elements(collection) + collection.each_with_object(Set.new) do |elem, memo| + coerced_elem = @elem_coercer.call(elem) + + return coerced_elem if coerced_elem.is_a?(InvalidValue) + + memo.add(coerced_elem) + end + end + end + end + end +end diff --git a/lib/grape/validations/types/variant_collection_coercer.rb b/lib/grape/validations/types/variant_collection_coercer.rb index f387457ee0..d841b6e552 100644 --- a/lib/grape/validations/types/variant_collection_coercer.rb +++ b/lib/grape/validations/types/variant_collection_coercer.rb @@ -3,7 +3,7 @@ module Validations module Types # This class wraps {MultipleTypeCoercer}, for use with collections # that allow members of more than one type. - class VariantCollectionCoercer < Virtus::Attribute + class VariantCollectionCoercer # Construct a new coercer that will attempt to coerce # a list of values such that all members are of one of # the given types. The container may also optionally be @@ -30,7 +30,7 @@ def initialize(types, method = nil) # @return [Array,Set,InvalidValue] # the coerced result, or an instance # of {InvalidValue} if the value could not be coerced. - def coerce(value) + def call(value) return InvalidValue.new unless value.is_a? Array value = @@ -43,16 +43,6 @@ def coerce(value) value end - - # Assert that the value has been coerced successfully. - # - # @param value [Object] a coerced result returned from {#coerce} - # @return [true,false] whether or not the coerced value - # satisfies type requirements. - def value_coerced?(value) - value.is_a?(@types.class) && - value.all? { |v| @member_coercer.success?(@types, v) } - end end end end diff --git a/lib/grape/validations/types/virtus_collection_patch.rb b/lib/grape/validations/types/virtus_collection_patch.rb deleted file mode 100644 index eab5208b27..0000000000 --- a/lib/grape/validations/types/virtus_collection_patch.rb +++ /dev/null @@ -1,16 +0,0 @@ -require 'virtus/attribute/collection' - -# See https://github.com/solnic/virtus/pull/343 -# This monkey-patch fixes type validation for collections, -# ensuring that type assertions are applied to collection -# members. -# -# This patch duplicates the code in the above pull request. -# Once the request, or equivalent functionality, has been -# published into the +virtus+ gem this file should be deleted. -Virtus::Attribute::Collection.class_eval do - # @api public - def value_coerced?(value) - super && value.all? { |item| member_type.value_coerced? item } - end -end diff --git a/lib/grape/validations/validators/coerce.rb b/lib/grape/validations/validators/coerce.rb index 4638d67f43..f21bbaadfa 100644 --- a/lib/grape/validations/validators/coerce.rb +++ b/lib/grape/validations/validators/coerce.rb @@ -1,9 +1,15 @@ module Grape class API - Boolean = Virtus::Attribute::Boolean + class Boolean + def self.build(val) + return nil if val != true && val != false + + new + end + end class Instance - Boolean = Virtus::Attribute::Boolean + Boolean = Grape::API::Boolean end end @@ -11,7 +17,12 @@ module Validations class CoerceValidator < Base def initialize(*_args) super - @converter = Types.build_coercer(type, @option[:method]) + + @converter = if type.is_a?(Grape::Validations::Types::VariantCollectionCoercer) + type + else + Types.build_coercer(type, method: @option[:method]) + end end def validate(request) @@ -19,11 +30,22 @@ def validate(request) end def validate_param!(attr_name, params) - raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:coerce) unless params.is_a? Hash - return unless requires_coercion?(params[attr_name]) + raise validation_exception(attr_name) unless params.is_a? Hash + new_value = coerce_value(params[attr_name]) - raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:coerce) unless valid_type?(new_value) - params[attr_name] = new_value + + raise validation_exception(attr_name) unless valid_type?(new_value) + + # Don't assign a value if it is identical. It fixes a problem with Hashie::Mash + # which looses wrappers for hashes and arrays after reassigning values + # + # h = Hashie::Mash.new(list: [1, 2, 3, 4]) + # => #> + # list = h.list + # h[:list] = list + # h + # => # + params[attr_name] = new_value unless params[attr_name] == new_value end private @@ -33,31 +55,25 @@ def validate_param!(attr_name, params) # # See {Types.build_coercer} # - # @return [Virtus::Attribute] + # @return [Object] attr_reader :converter def valid_type?(val) - # Special value to denote coercion failure - return false if val.instance_of?(Types::InvalidValue) - - # Allow nil, to ignore when a parameter is absent - return true if val.nil? - - converter.value_coerced? val + !val.is_a?(Types::InvalidValue) end def coerce_value(val) - # Don't coerce things other than nil to Arrays or Hashes - unless (@option[:method] && !val.nil?) || type.is_a?(Virtus::Attribute) - return val || [] if type == Array - return val || Set.new if type == Set - return val || {} if type == Hash + # define default values for structures, the dry-types lib which is used + # for coercion doesn't accept nil as a value, so it would fail + if val.nil? + return [] if type == Array || type.is_a?(Array) + return Set.new if type == Set + return {} if type == Hash end - converter.coerce(val) + converter.call(val) - # not the prettiest but some invalid coercion can currently trigger - # errors in Virtus (see coerce_spec.rb:75) + # Some custom types might fail, so it should be treated as an invalid value rescue Types::InvalidValue.new end @@ -69,9 +85,8 @@ def type @option[:type].is_a?(Hash) ? @option[:type][:value] : @option[:type] end - def requires_coercion?(value) - # JSON types do not require coercion if value is valid - !valid_type?(value) || converter.coercer.respond_to?(:method) && !converter.is_a?(Grape::Validations::Types::Json) + def validation_exception(attr_name) + Grape::Exceptions::Validation.new(params: [@scope.full_name(attr_name)], message: message(:coerce)) end end end diff --git a/spec/grape/api/defines_boolean_in_params_spec.rb b/spec/grape/api/defines_boolean_in_params_spec.rb index 166b2d0a86..d057bbbce2 100644 --- a/spec/grape/api/defines_boolean_in_params_spec.rb +++ b/spec/grape/api/defines_boolean_in_params_spec.rb @@ -21,7 +21,7 @@ def app { class: 'TrueClass', value: true }.to_s end - it 'sets Boolean as a Virtus::Attribute::Boolean' do + it 'sets Boolean as a type' do post '/echo?message=true' expect(last_response.status).to eq(201) expect(last_response.body).to eq expected_body @@ -29,8 +29,8 @@ def app context 'Params endpoint type' do subject { DefinesBooleanInstanceSpec::API.new.router.map['POST'].first.options[:params]['message'][:type] } - it 'params type is a Virtus::Attribute::Boolean' do - is_expected.to eq 'Virtus::Attribute::Boolean' + it 'params type is a boolean' do + is_expected.to eq 'Grape::API::Boolean' end end end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 63c1623bb6..8db185c757 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -1133,7 +1133,7 @@ class DummyFormatClass subject.use Rack::Chunked subject.get('/stream') { stream test_stream } - get '/stream', {}, 'HTTP_VERSION' => 'HTTP/1.1' + get '/stream', {}, 'HTTP_VERSION' => 'HTTP/1.1', 'SERVER_PROTOCOL' => 'HTTP/1.1' expect(last_response.headers['Content-Type']).to eq('text/plain') expect(last_response.headers['Content-Length']).to eq(nil) diff --git a/spec/grape/dsl/helpers_spec.rb b/spec/grape/dsl/helpers_spec.rb index 6e24cfe00d..c1ce8de700 100644 --- a/spec/grape/dsl/helpers_spec.rb +++ b/spec/grape/dsl/helpers_spec.rb @@ -75,9 +75,9 @@ def test end context 'with an external file' do - it 'sets Boolean as a Virtus::Attribute::Boolean' do + it 'sets Boolean as a Grape::API::Boolean' do subject.helpers BooleanParam - expect(subject.first_mod::Boolean).to eq Virtus::Attribute::Boolean + expect(subject.first_mod::Boolean).to eq Grape::API::Boolean end end diff --git a/spec/grape/validations/params_scope_spec.rb b/spec/grape/validations/params_scope_spec.rb index 3658efb875..6faf5680bb 100644 --- a/spec/grape/validations/params_scope_spec.rb +++ b/spec/grape/validations/params_scope_spec.rb @@ -30,7 +30,7 @@ def app context 'when the default value is false' do before do subject.params do - optional :bool, type: Virtus::Attribute::Boolean, default: false + optional :bool, type: Grape::API::Boolean, default: false end subject.get end diff --git a/spec/grape/validations/types_spec.rb b/spec/grape/validations/types_spec.rb index 353d75fda6..1380b10167 100644 --- a/spec/grape/validations/types_spec.rb +++ b/spec/grape/validations/types_spec.rb @@ -11,29 +11,10 @@ def self.parse; end end end - VirtusA = Virtus::Attribute.build(String) - - module VirtusModule - include Virtus.module - end - - class VirtusB - include VirtusModule - end - - class VirtusC - include Virtus.model - end - - MyAxiom = Axiom::Types::String.new do - minimum_length 1 - maximum_length 30 - end - describe '::primitive?' do [ Integer, Float, Numeric, BigDecimal, - Virtus::Attribute::Boolean, String, Symbol, + Grape::API::Boolean, String, Symbol, Date, DateTime, Time, Rack::Multipart::UploadedFile ].each do |type| it "recognizes #{type} as a primitive" do @@ -57,16 +38,6 @@ class VirtusC end end - describe '::recognized?' do - [ - VirtusA, VirtusB, VirtusC, MyAxiom - ].each do |type| - it "recognizes #{type}" do - expect(described_class.recognized?(type)).to be_truthy - end - end - end - describe '::special?' do [ JSON, Array[JSON], File, Rack::Multipart::UploadedFile @@ -97,14 +68,14 @@ class VirtusC expect(described_class.instance_variable_get(:@__cache_write_lock)).to be_a(Mutex) end - it 'caches the result of the Virtus::Attribute.build method' do + it 'caches the result of the build_coercer method' do original_cache = described_class.instance_variable_get(:@__cache) described_class.instance_variable_set(:@__cache, {}) - coercer = 'TestCoercer' - expect(Virtus::Attribute).to receive(:build).once.and_return(coercer) - expect(described_class.build_coercer(Array[String])).to eq(coercer) - expect(described_class.build_coercer(Array[String])).to eq(coercer) + a_coercer = described_class.build_coercer(Array[String]) + b_coercer = described_class.build_coercer(Array[String]) + + expect(a_coercer.object_id).to eq(b_coercer.object_id) described_class.instance_variable_set(:@__cache, original_cache) end diff --git a/spec/grape/validations/validators/coerce_spec.rb b/spec/grape/validations/validators/coerce_spec.rb index 94f66e873b..1c956f6fd5 100644 --- a/spec/grape/validations/validators/coerce_spec.rb +++ b/spec/grape/validations/validators/coerce_spec.rb @@ -10,11 +10,13 @@ def app end describe 'coerce' do - module CoerceValidatorSpec - class User - include Virtus.model - attribute :id, Integer - attribute :name, String + class SecureURIOnly + def self.parse(value) + URI.parse(value) + end + + def self.parsed?(value) + value.is_a? URI::HTTPS end end @@ -96,6 +98,7 @@ class User it 'respects :coerce_with' do get '/', a: 'yup' + expect(last_response.status).to eq(200) expect(last_response.body).to eq('TrueClass') end @@ -148,25 +151,6 @@ class User expect(last_response.body).to eq('array int works') end - context 'complex objects' do - it 'error on malformed input for complex objects' do - subject.params do - requires :user, type: CoerceValidatorSpec::User - end - subject.get '/user' do - 'complex works' - end - - get '/user', user: '32' - expect(last_response.status).to eq(400) - expect(last_response.body).to eq('user is invalid') - - get '/user', user: { id: 32, name: 'Bob' } - expect(last_response.status).to eq(200) - expect(last_response.body).to eq('complex works') - end - end - context 'coerces' do it 'Integer' do subject.params do @@ -181,6 +165,25 @@ class User expect(last_response.body).to eq(integer_class_name) end + it 'is a custom type' do + subject.params do + requires :uri, coerce: SecureURIOnly + end + subject.get '/secure_uri' do + params[:uri].class + end + + get 'secure_uri', uri: 'https://www.example.com' + + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('URI::HTTPS') + + get 'secure_uri', uri: 'http://www.example.com' + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('uri is invalid') + end + context 'Array' do it 'Array of Integers' do subject.params do @@ -197,7 +200,7 @@ class User it 'Array of Bools' do subject.params do - requires :arry, coerce: Array[Virtus::Attribute::Boolean] + requires :arry, coerce: Array[Grape::API::Boolean] end subject.get '/array' do params[:arry][0].class @@ -208,27 +211,6 @@ class User expect(last_response.body).to eq('TrueClass') end - it 'Array of Complex' do - subject.params do - requires :arry, coerce: Array[CoerceValidatorSpec::User] - end - subject.get '/array' do - params[:arry].size - end - - get 'array', arry: [31] - expect(last_response.status).to eq(400) - expect(last_response.body).to eq('arry is invalid') - - get 'array', arry: { id: 31, name: 'Alice' } - expect(last_response.status).to eq(400) - expect(last_response.body).to eq('arry is invalid') - - get 'array', arry: [{ id: 31, name: 'Alice' }] - expect(last_response.status).to eq(200) - expect(last_response.body).to eq('1') - end - it 'Array of type implementing parse' do subject.params do requires :uri, type: Array[URI] @@ -253,17 +235,7 @@ class User expect(last_response.body).to eq('Set,URI::HTTP,1') end - it 'Array of class implementing parse and parsed?' do - class SecureURIOnly - def self.parse(value) - URI.parse(value) - end - - def self.parsed?(value) - value.is_a? URI::HTTPS - end - end - + it 'Array of a custom type' do subject.params do requires :uri, type: Array[SecureURIOnly] end @@ -295,7 +267,7 @@ def self.parsed?(value) it 'Set of Bools' do subject.params do - requires :set, coerce: Set[Virtus::Attribute::Boolean] + requires :set, coerce: Set[Grape::API::Boolean] end subject.get '/set' do params[:set].first.class @@ -309,7 +281,7 @@ def self.parsed?(value) it 'Bool' do subject.params do - requires :bool, coerce: Virtus::Attribute::Boolean + requires :bool, coerce: Grape::API::Boolean end subject.get '/bool' do params[:bool].class @@ -443,19 +415,19 @@ def self.parsed?(value) it 'parses parameters with Array[String] type' do subject.params do - requires :values, type: Array[String], coerce_with: ->(val) { val.split(/\s+/).map(&:to_i) } + requires :values, type: Array[String], coerce_with: ->(val) { val.split(/\s+/) } end - subject.get '/ints' do + subject.get '/strings' do params[:values] end - get '/ints', values: '1 2 3 4' + get '/strings', values: '1 2 3 4' expect(last_response.status).to eq(200) expect(JSON.parse(last_response.body)).to eq(%w[1 2 3 4]) - get '/ints', values: 'a b c d' + get '/strings', values: 'a b c d' expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)).to eq(%w[0 0 0 0]) + expect(JSON.parse(last_response.body)).to eq(%w[a b c d]) end it 'parses parameters with Array[Integer] type' do @@ -914,14 +886,17 @@ def self.parsed?(value) end context 'converter' do - it 'does not build Virtus::Attribute multiple times' do + it 'does not build a coercer multiple times' do subject.params do requires :something, type: Array[String] end subject.get do end - expect(Virtus::Attribute).to receive(:build).at_most(2).times.and_call_original + expect(Grape::Validations::Types::ArrayCoercer).to( + receive(:new).at_most(:once).and_call_original + ) + 10.times { get '/' } end end diff --git a/spec/grape/validations/validators/except_values_spec.rb b/spec/grape/validations/validators/except_values_spec.rb index 7030774cc4..9e0787697f 100644 --- a/spec/grape/validations/validators/except_values_spec.rb +++ b/spec/grape/validations/validators/except_values_spec.rb @@ -110,7 +110,7 @@ def excepts optional: { type: Array[Integer], except_values: [10, 11], default: 12 }, tests: [ { value: 'invalid-type1', rc: 400, body: { error: 'type is invalid' }.to_json }, - { value: 10, rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: 10, rc: 400, body: { error: 'type is invalid' }.to_json }, { value: [10], rc: 400, body: { error: 'type has a value not allowed' }.to_json }, { value: ['3'], rc: 200, body: { type: [3] }.to_json }, { value: [3], rc: 200, body: { type: [3] }.to_json }, diff --git a/spec/grape/validations/validators/presence_spec.rb b/spec/grape/validations/validators/presence_spec.rb index d00163ead8..7de97190a3 100644 --- a/spec/grape/validations/validators/presence_spec.rb +++ b/spec/grape/validations/validators/presence_spec.rb @@ -269,4 +269,32 @@ def app expect(last_response.body).to eq('Hello optional'.to_json) end end + + context 'with a custom type' do + it 'does not validate their type when it is missing' do + class CustomType + def self.parse(value) + return if value.blank? + + new + end + end + + subject.params do + requires :custom, type: CustomType + end + subject.get '/custom' do + 'custom' + end + + get 'custom' + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('{"error":"custom is missing"}') + + get 'custom', custom: 'filled' + + expect(last_response.status).to eq(200) + end + end end