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

Virtus::Attribute::Collection.value_coerced? override. #343

Merged
merged 1 commit into from
Feb 2, 2016

Conversation

dslh
Copy link
Contributor

@dslh dslh commented Sep 27, 2015

Added override of Virtus::Attribute.value_coerced? to
Virtus::Attribute::Collection, which checks collection
members against the expected member type. Prevents e.g.
Virtus::Attribute.build(Array[Integer]).value_coerced? %w{1 2 3}
from returning true.

@dslh dslh force-pushed the collection_value_coerced branch from 676af93 to 07cb939 Compare September 27, 2015 17:01
dslh added a commit to dslh/grape that referenced this pull request Sep 28, 2015
Addresses ruby-grape#1164, ruby-grape#690, ruby-grape#689, ruby-grape#693.
Depends on solnic/virtus#343

`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`.

`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`.
A number of pre-rolled attributes are available providing coercion
for `Date` and `DateTime` objects from various formats in
`lib/grape/validations/types/date.rb` and `date_time.rb`.
dslh added a commit to dslh/grape that referenced this pull request Sep 28, 2015
Addresses ruby-grape#1164, ruby-grape#690, ruby-grape#689, ruby-grape#693.
Depends on solnic/virtus#343

`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`.

`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`.
A number of pre-rolled attributes are available providing coercion
for `Date` and `DateTime` objects from various formats in
`lib/grape/validations/types/date.rb` and `date_time.rb`.
dslh added a commit to dslh/grape that referenced this pull request Sep 28, 2015
Addresses ruby-grape#1164, ruby-grape#690, ruby-grape#689, ruby-grape#693.
Depends on solnic/virtus#343

`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`.

`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`.
A number of pre-rolled attributes are available providing coercion
for `Date` and `DateTime` objects from various formats in
`lib/grape/validations/types/date.rb` and `date_time.rb`.
dslh added a commit to dslh/grape that referenced this pull request Sep 30, 2015
Addresses ruby-grape#1164, ruby-grape#690, ruby-grape#689, ruby-grape#693.
Depends on solnic/virtus#343

`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`.
A number of pre-rolled attributes are available providing coercion
for `Date` and `DateTime` objects from various formats in
`lib/grape/validations/formats/date.rb` and `date_time.rb`.
@dblock
Copy link

dblock commented Sep 30, 2015

👍 we (Grape) would love this to be merged

let(:object) { described_class.build(Array[Integer]) }

context 'when input has correctly typed members' do
let(:input) { [1,2,3] }
Copy link

Choose a reason for hiding this comment

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

it's just a style thing but put spaces after comas source

let(:input) { [1, 2, 3] }

Copy link

Choose a reason for hiding this comment

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

Related, Would you take a PR that enforces rubocop?

Copy link

Choose a reason for hiding this comment

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

How can you enforce rubocop? Do you mean connecting it to https://houndci.com/ ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't personally use Rubocop but you don't need Hound CI, this repo uses a public CI somewhere and all CI's can be adjusted to run a custom command that does both and fails a test. That said I don't know if we need Rubocop.

Copy link

Choose a reason for hiding this comment

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

@Bartuz By making it part of the build, like here: https://github.com/dblock/ruby-enum or in larger projects, https://github.com/ruby-grape/grape

Copy link

Choose a reason for hiding this comment

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

Like so: #349

@dblock
Copy link

dblock commented Nov 17, 2015

@dslh Will you finish this PR, I am happy to reopen a new one after addressing the issues by the maintainers.

@dslh dslh force-pushed the collection_value_coerced branch from 07cb939 to 63ef092 Compare November 17, 2015 20:37
Added override of `Virtus::Attribute.value_coerced?` to
`Virtus::Attribute::Collection`, which checks collection
members against the expected member type. Prevents e.g.
`Virtus::Attribute.build(Array[Integer]).value_coerced? %w{1 2 3}`
from returning `true`.
@dslh dslh force-pushed the collection_value_coerced branch from 63ef092 to de9224b Compare November 17, 2015 20:39
@dslh
Copy link
Contributor Author

dslh commented Nov 17, 2015

Commas all sorted, really I should know better :)

Let me know if I've missed anything else.

@u2
Copy link

u2 commented Jan 28, 2016

@booch

booch added a commit that referenced this pull request Feb 2, 2016
Virtus::Attribute::Collection.value_coerced? override.
@booch booch merged commit 1db0d28 into solnic:master Feb 2, 2016
@dslh dslh deleted the collection_value_coerced branch February 3, 2016 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants