-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Refactor and extend coercion and type validation #1167
Conversation
3b1a461
to
ec9c6d9
Compare
I've had a go at making better use of virtus to make type validation a bit more flexible. I would say this PR needs few more things done before it should be accepted:
I think what's missing now is a |
# one String argument and returning the parsed value in its correct type. | ||
# @param type [Class] type to check | ||
# @return [Boolean] whether or not the type can be used as a custom type | ||
def self.custom_type?(type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be just custom?
to match the other names maybe. Not feeling strongly. Maybe some other methods should be private and this would not.
I really like this code and I think it's important to get some or all of it in. I would vote to even monkey-patch Virtus in Grape until there's a version out there with the PR, or implementing a work-around in Grape that explicitly does what that PR does. Either way, bring this PR to a green state and I'll merge it trusting our specs not to introduce any regressions. |
07fce88
to
6f55a61
Compare
Addresses ruby-grape#1164, ruby-grape#690, ruby-grape#689, ruby-grape#693. `Grape::ParameterTypes` is renamed `Grape::Validations::Types` to reflect that it should probably be bundled with an eventual `grape-validations` gem. It is expanded to include two new categories of types, 'special' and 'recognized' (see 'lib/grape/validations/types.rb'). `CoerceValidator` now makes use of `Virtus::Attribute::value_coerced?`, simplifying its internals. `CustomTypeCoercer` is introduced, attempting to standardize support for custom types by decoupling coercion and type-checking logic from the `type` class supplied to `Grape::Dsl::Parameters::requires`. The process for inferring which logic to use for each type and coercion method is encoded in `lib/grape/validations/types/build_coercer.rb`. `JSON`, `Array[JSON]` and `Rack::Multipart::UploadedFile (a.k.a `File`) are designated 'special' types, for which special implementations of `Virtus::Attribute` are provided. Instances of `Virtus::Attribute` built with `Virtus::Attribute.build` may now also be passed as the `type` parameter for `requires`. Depends on a monkey patch to `Virtus::Attribute::Collection`, included in `lib/grape/validations/types/virtus_collection_patch.rb`. See pull request 343 on solnic/virtus for more details.
6f55a61
to
cf6d2dd
Compare
Tidied up a bit:
Most of the inference that |
@@ -5,6 +5,7 @@ | |||
|
|||
* Your contribution here. | |||
|
|||
* [#1167](https://github.com/ruby-grape/grape/pull/1167): Refactor and extend coercion and type validation system - [@dslh](https://github.com/dslh). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've added type: File
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, doc'd and spec'd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe should be added to changelog as a line, like that we support File
now out of the box.
I'm merging this, great work. Lets iterate on top of it. |
Refactor and extend coercion and type validation
Addresses #1164, #690, #689, #693.
Depends on solnic/virtus#343
Grape::ParameterTypes
is renamedGrape::Validations::Types
to reflect that it should probably be bundled with an eventual
grape-validations
gem. It is expanded to include two newcategories of types, 'special' and 'recognized' (see
'lib/grape/validations/types.rb').
CoerceValidator
now makesuse of
Virtus::Attribute::value_coerced?
, simplifying itsinternals.
CustomTypeCoercer
is introduced, attempting to standardizesupport for custom types by decoupling coercion and type-checking
logic from the
type
class supplied toGrape::Dsl::Parameters::requires
.JSON
,Array[JSON]
andRack::Multipart::UploadedFile
(a.k.aFile
) are designated 'special' types, for which specialimplementations of
Virtus::Attribute
are provided.Instances of
Virtus::Attribute
built withVirtus::Attribute.build
may now also be passed as the
type
parameter forrequires
.A number of pre-rolled attributes are available providing coercion
for
Date
andDateTime
objects from various formats inlib/grape/validations/types/date.rb
anddate_time.rb
.