Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Virtus with dry-types #1920

Merged
merged 1 commit into from
Dec 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 11 additions & 15 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand All @@ -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
Expand Down
43 changes: 43 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -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
```

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to explain how to rewrite a custom type with dry-types here, first. If I am upgrading I'd rather not use Virtus unless I can't.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dry-types cannot be used in place of Virtus for custom types, dry-schema might be. So, if users need something like Virtus they need to keep it or migrate to dry-schema. I guess it is up to them to decide, the goal is to avoid breaking changes, so if they use Virtus, they can still use it until they have time to migrate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds like you're saying that if you want a custom type you have to use Virtus? Or are you saying you can use Virtus? Either way let's make this very clear in the upgrading/readme doc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, Virtus isn't required for custom types. As it was before, any class implementing the parse method might be used. 🙂 I've added an additional sentence to the upgrading doc.

Custom types which don't depend on Virtus don't require any changes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a "For more information see ..." and link to this PR.


For more information see [#1920](https://github.com/ruby-grape/grape/pull/1920).

### Upgrading to >= 1.2.4

#### Headers in `error!` call
Expand Down
3 changes: 2 additions & 1 deletion grape.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 0 additions & 2 deletions lib/grape.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
require 'i18n'
require 'thread'

require 'virtus'

I18n.load_path << File.expand_path('../grape/locale/en.yml', __FILE__)

module Grape
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/dsl/helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions lib/grape/validations/params_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
35 changes: 5 additions & 30 deletions lib/grape/validations/types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
54 changes: 54 additions & 0 deletions lib/grape/validations/types/array_coercer.rb
Original file line number Diff line number Diff line change
@@ -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
Loading